Closed
Bug 769993
Opened 12 years ago
Closed 10 years ago
20% performance regression on IE Maze If gfx.content.azure.enabled=true
Categories
(Core :: Graphics, defect)
Tracking
()
People
(Reporter: alice0775, Assigned: bas.schouten)
References
(Blocks 1 open bug, )
Details
(Keywords: perf, Whiteboard: ietestdrive)
Attachments
(1 file)
(deleted),
patch
|
jrmuizel
:
review+
lsblakk
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
Build Identifier:
http://hg.mozilla.org/mozilla-central/rev/f08d285b63b0
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:16.0) Gecko/16.0 Firefox/16.0 ID:20120630030532
20% performance regression on IE Maze If gfx.content.azure.enabled=true
gfx.content.azure.enabled=true : 136sec
gfx.content.azure.enabled=false : 111sec
Steps to Reproduce:
1. Prepare a new profile and make sure setting of gfx.content.azure.enabled
2. Open http://ie.microsoft.com/testdrive/Performance/MazeSolver/Default.html
3. Click "Start"
Actual Results:
20% slow, If gfx.content.azure.enabled=true
Expected Results:
Not so.
Reporter | ||
Comment 1•12 years ago
|
||
Graphics
Adapter Description : ATI Radeon HD 4300/4500 Series
Vendor ID : 0x1002
Device ID : 0x954f
Adapter RAM : 512
Adapter Driver : saticfx64 aticfx64 aticfx32 aticfx32 atiumd64 atidxx64 atiumdag atidxx32 atiumdva atiumd6a atitmm64
Driver Version : 8.961.0.0
Driver Date : 4-5-2012
Direct2D Enabled : true
DirectWrite Enabled : true (6.1.7601.17789)
ClearType Parameters : Gamma: 2200 Pixel Structure: RGB ClearType Level: 50 Enhanced Contrast: 50
WebGL Renderer : Google Inc. -- ANGLE (ATI Radeon HD 4300/4500 Series) -- OpenGL ES 2.0 (ANGLE 1.0.0.1041)
GPU Accelerated Windows : 1/1 Direct3D 10
AzureBackend : direct2d
Assignee | ||
Comment 2•12 years ago
|
||
Although I've found a fix to this specific regression. It's revealed a couple of more serious issues with the Azure-thebes wrapper. They're not too hard to fix, however for now I suggest we switch off the wrapper for 15. We've gotten enough value from its testing and right now it doesn't offer any big wins (as expected).
Assignee | ||
Comment 3•12 years ago
|
||
This is turning off a new feature to fix some regressions we're still working on fixing.
Assignee: nobody → bas.schouten
Status: NEW → ASSIGNED
Attachment #647400 -
Flags: review?(jmuizelaar)
Attachment #647400 -
Flags: approval-mozilla-beta?
Updated•12 years ago
|
Attachment #647400 -
Flags: review?(jmuizelaar) → review+
Comment 4•12 years ago
|
||
Comment on attachment 647400 [details] [diff] [review]
Switch off Azure content in Beta
Approving for disabling this on beta, please set status flags to reflect this status once it lands.
Attachment #647400 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Assignee | ||
Comment 5•12 years ago
|
||
https://hg.mozilla.org/releases/mozilla-beta/rev/5f2f7027b65b
Accidentally put the wrong person for approval in the commit message.
Updated•12 years ago
|
Bas, what's happening here?
Assignee | ||
Comment 7•12 years ago
|
||
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #6)
> Bas, what's happening here?
So, the main reason Azure-content is slower here is that Layout is saving. pushing a clip, and then restoring, without actually doing any drawing. In Cairo this would result in nothing actually happening, in Azure-content the clip is naively pushed/popped causing a significant performance penalty. I've got a patch that 'fixes' this problem, it really just works around it from layout land (i.e. it doesn't clip if it can easily determine it won't be drawing anything).
Additionally, after the benchmark finishes we suffer considerably with Azure-content due to a very small clip being applied and than a PushGroup/PopGroup occurring numerous times. With cairo the intermediate blend would be limited to the bounds of the clip, with current Azure-content it's not. I've uploaded a patch for this in another bug (See bug 778367).
(In reply to Bas Schouten (:bas) from comment #7)
> So, the main reason Azure-content is slower here is that Layout is saving.
> pushing a clip, and then restoring, without actually doing any drawing. In
> Cairo this would result in nothing actually happening, in Azure-content the
> clip is naively pushed/popped causing a significant performance penalty.
> I've got a patch that 'fixes' this problem, it really just works around it
> from layout land (i.e. it doesn't clip if it can easily determine it won't
> be drawing anything).
Avoiding it in layout is OK, but this could also be triggered by scripts using canvas, right?
> Additionally, after the benchmark finishes we suffer considerably with
> Azure-content due to a very small clip being applied and than a
> PushGroup/PopGroup occurring numerous times. With cairo the intermediate
> blend would be limited to the bounds of the clip, with current Azure-content
> it's not. I've uploaded a patch for this in another bug (See bug 778367).
Great, thanks.
Comment 9•12 years ago
|
||
Are we planning to perform the same disable in FF16 that we did in FF15? Or are we still trying to uplift bug 778367.
Assignee | ||
Comment 10•12 years ago
|
||
(In reply to Alex Keybl [:akeybl] from comment #9)
> Are we planning to perform the same disable in FF16 that we did in FF15? Or
> are we still trying to uplift bug 778367.
I would like to uplift bug 778367, but I'm not 100% sure we should. Unless we think this bug is a big issue on beta I'd like to pref off later in the cycle so we get some more test coverage for the next cycle, where we'll actually start seeing real improvements from it being preffed on.
Comment 11•12 years ago
|
||
(In reply to Bas Schouten (:bas) from comment #10)
> I would like to uplift bug 778367, but I'm not 100% sure we should. Unless
> we think this bug is a big issue on beta I'd like to pref off later in the
> cycle so we get some more test coverage for the next cycle, where we'll
> actually start seeing real improvements from it being preffed on.
Will wait to hear your risk vs reward of bug 778367. If that doesn't pan out, we should land the same preference change 9/18 (b4's go to build).
Comment 12•12 years ago
|
||
Spoke with Bas - we agreed to again disable azure for FF16 to prevent new perf regressions. Let's land this before Tuesday.
Comment 13•12 years ago
|
||
Comment on attachment 647400 [details] [diff] [review]
Switch off Azure content in Beta
Approving for beta again. a=akeybl
Updated•12 years ago
|
Whiteboard: ietestdrive
Comment 14•12 years ago
|
||
Landed in Beta: http://hg.mozilla.org/releases/mozilla-beta/rev/d1bdd265730b
status-firefox16:
--- → fixed
Updated•12 years ago
|
tracking-firefox17:
--- → ?
tracking-firefox18:
--- → ?
Updated•12 years ago
|
Assignee | ||
Comment 15•12 years ago
|
||
I discussed this with Lukas a little on IRC. There's a fairly simple, safe 'hack' we can do to 'fix' this regression. But I personally don't think it's worth it. This is a 10-20% regression in a benchmark where we're already 400% slower than the competition. If we value this benchmark we should probably focus on improving the actual layout algorithm here which is eating all of the time, that will automatically take care of the regression.
Updated•12 years ago
|
Comment 16•12 years ago
|
||
If we value this benchmark, then probably because it's a good proxy for real applications, where a 10-20% regression might be significant regardless of how much worse we are than our competitors. Can we rule this out? Would the hack you mention only affect this benchmark or real-world use cases too?
Assignee | ||
Comment 17•12 years ago
|
||
(In reply to Dão Gottwald [:dao] from comment #16)
> If we value this benchmark, then probably because it's a good proxy for real
> applications, where a 10-20% regression might be significant regardless of
> how much worse we are than our competitors. Can we rule this out? Would the
> hack you mention only affect this benchmark or real-world use cases too?
I don't imagine that this would ever happen in the real world. But the layout people would know better than I do. If something like this would happen in the real world performance would be pretty bad anyway and it would probably be somewhere on our radar. I'm happy to being convinced otherwise, if we can find a real-world example.
Comment 18•12 years ago
|
||
Jeff had a real world example at one point.
Comment 19•12 years ago
|
||
Beta 4 is going out later this week with all of Bas's recent work in it, we'll be meeting next Monday (Nov 5) to assess if we'll need to do the disabling again in 17 based on feedback and testing over the weekend.
status-firefox17:
--- → affected
status-firefox18:
--- → affected
Comment 20•12 years ago
|
||
The fixes have landed, and are looking good with no regressions. However, for the perf hit - as Bas mentioned in comment 15, we're looking at a slight perf hit on something we're already not performing well on. It's not worth pushing further on this right now for release tracking purposes.
Updated•12 years ago
|
Updated•11 years ago
|
Blocks: ietestdrive
Comment 21•10 years ago
|
||
Tha patches landed. Is there some more work to do here or we can close is as RESOLVED?
Flags: needinfo?(bas)
Assignee | ||
Comment 22•10 years ago
|
||
Don't think so.
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Flags: needinfo?(bas)
Resolution: --- → FIXED
Updated•10 years ago
|
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•