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)

x86
Windows 7
defect
Not set
normal

Tracking

()

VERIFIED FIXED
Tracking Status
firefox15 + fixed
firefox16 + fixed
firefox17 + wontfix
firefox18 - affected

People

(Reporter: alice0775, Assigned: bas.schouten)

References

(Blocks 1 open bug, )

Details

(Keywords: perf, Whiteboard: ietestdrive)

Attachments

(1 file)

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.
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
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).
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?
Attachment #647400 - Flags: review?(jmuizelaar) → review+
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+
https://hg.mozilla.org/releases/mozilla-beta/rev/5f2f7027b65b Accidentally put the wrong person for approval in the commit message.
(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.
Are we planning to perform the same disable in FF16 that we did in FF15? Or are we still trying to uplift bug 778367.
Depends on: 778367
Depends on: 789061
(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.
(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).
Spoke with Bas - we agreed to again disable azure for FF16 to prevent new perf regressions. Let's land this before Tuesday.
Comment on attachment 647400 [details] [diff] [review] Switch off Azure content in Beta Approving for beta again. a=akeybl
Whiteboard: ietestdrive
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.
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?
(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.
Jeff had a real world example at one point.
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.
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.
Tha patches landed. Is there some more work to do here or we can close is as RESOLVED?
Flags: needinfo?(bas)
Don't think so.
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Flags: needinfo?(bas)
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: