User Agent: Mozilla/5.0 (Windows NT 6.1; rv:16.0) Gecko/20100101 Firefox/16.0 Build ID: 20121024073032 Steps to reproduce: canvas.fillRect() taking 64 times longer (or more) in FF17 than in FF16 Try the test case attached in both versions. Check your console for the draw times Actual results: Tried to run same code in FF17 Expected results: Same render time as FF16

I am not able to reproduce the slowness on Win 7. With FF16 or FF17, the canvas takes the same time to render (~430 ms after 10 runs).

Replaced console.time() with alert()

Sorry, Does not let me save the attachment in other than plain. Am testing on two separate systems, both Win7, no FF extensions. I am getting 10~20ms on FF16 FF17: 1200~1300ms and 2000~2050ms (slower CPU)

(In reply to Loic from comment #2) > I am not able to reproduce the slowness on Win 7. With FF16 or FF17, the > canvas takes the same time to render (~430 ms after 10 runs). Same over here. I get slower Results (Factor 8-9) only when disabling HWA...

(In reply to XtC4UaLL [:xtc4uall] from comment #8) > only when disabling HWA... What is HWA?

GPU Hardware Acceleration.

I am tracking the draw times for the users of my web site. All are getting the slowness except one, he is still on FF16!

I've sent the above attachment out to several users. They are reporting back with the same discrepancy in times between those on FF16 vs FF17.

(In reply to XtC4UaLL [:xtc4uall] from comment #8) > (In reply to Loic from comment #2) > > I am not able to reproduce the slowness on Win 7. With FF16 or FF17, the > > canvas takes the same time to render (~430 ms after 10 runs). > > Same over here. I get slower Results (Factor 8-9) only when disabling HWA... Confirmed, only with HWA off. With HWA on: ~1600-1700 ms With HWA off: ~23 ms m-c good=2012-08-03 bad=2012-08-03 http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=89dcadd42ec4&tochange=20fc34efd733 My guess goes to: Nicholas Cameron — Bug 773460. Pref on Azure/Cairo for Windows. r=roc

Sorry, read: bad=2012-08-04

in condition of HWA off,setting gfx.canvas.azure.backends = direct2d helps Regression window with HWA off(m-c) 12ms: http://hg.mozilla.org/mozilla-central/rev/73b3b3f828b0 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:17.0) Gecko/17.0 Firefox/17.0 ID:20120803042624 1470ms: http://hg.mozilla.org/mozilla-central/rev/62d4f0efe485 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:17.0) Gecko/17.0 Firefox/17.0 ID:20120803073024 Pushlog: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=73b3b3f828b0&tochange=62d4f0efe485 Regression window with HWA off(m-i) 11ms: http://hg.mozilla.org/integration/mozilla-inbound/rev/3a17236e9084 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:17.0) Gecko/17.0 Firefox/17.0 ID:20120802142137 1407ms: http://hg.mozilla.org/integration/mozilla-inbound/rev/032ba64ab1f1 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:17.0) Gecko/17.0 Firefox/17.0 ID:20120802150336 Pushlog: http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=3a17236e9084&tochange=032ba64ab1f1 Suspected: Bug 773460

I would guess that Thebes canvas was using the D2D Cairo backend, but Azure Canvas uses some slower backend for Cairo. I'll try to confirm this.

Confirmed that Azure canvas ends up using win32 surface backend for Cairo. I assume that with Thebes canvas, we end up with the D2D surface backend, but there seems no point in confirming this because it will be very system dependent (I've previously seen this happen).

(In reply to Nick Cameron [:nrc] from comment #17) > Confirmed that Azure canvas ends up using win32 surface backend for Cairo. I > assume that with Thebes canvas, we end up with the D2D surface backend, but > there seems no point in confirming this because it will be very system > dependent (I've previously seen this happen). Or not, actually unlikely that Cairo is using D2D behind our back. I'll try and check what's going on at some point...

Since this is a regression, please try to figure it out as soon as possible. If you can reproduce the bug and get a profile of the slow case, that'll probably be enough.

(In reply to Loic from comment #13) > (In reply to XtC4UaLL [:xtc4uall] from comment #8) > > (In reply to Loic from comment #2) > > > I am not able to reproduce the slowness on Win 7. With FF16 or FF17, the > > > canvas takes the same time to render (~430 ms after 10 runs). > > > > Same over here. I get slower Results (Factor 8-9) only when disabling HWA... > > Confirmed, only with HWA off. > With HWA on: ~1600-1700 ms > With HWA off: ~23 ms > > m-c > good=2012-08-03 > bad=2012-08-03 > http://hg.mozilla.org/mozilla-central/ > pushloghtml?fromchange=89dcadd42ec4&tochange=20fc34efd733 > > My guess goes to: > Nicholas Cameron — Bug 773460. Pref on Azure/Cairo for Windows. r=roc IS that the right way round for HWA on/off? I get ~50ms with HWA on and ~550ms with HWA and D2D off. With latest nightly, which is kind of expected. With FF16, with HWA on I get 100ms and HWA off I get 5ms. So with HWA we get twice as fast and without we get 110 times slower! Huh, what? And non-accelerated was 20 times faster than accelerated?

Bas, Jeff: do you know of any Cairo optimisation we might be missing out on? Or is this likely just due to overhead of Azure?

So, the slowdown is due to the test case using ctx.globalCompositeOperation = 'destination-out', which causes us to take a slow path in DrawTargetCairo::DrawPattern. In theory we shouldn't need to affect uncovered areas with destination out composition, but I think I remember needing to do it for some reason. I'm making a build without this which should be fast, but I'll need to check it is correct.

Updating the summary -- if this is due to dest-out and regular over fills are still just as fast/faster, then it's somewhat less urgent.

(In reply to Vladimir Vukicevic [:vlad] [:vladv] from comment #23) > Updating the summary -- if this is due to dest-out and regular over fills > are still just as fast/faster, then it's somewhat less urgent. Yep, regular fills are just as fast.

Looks good on try: https://tbpl.mozilla.org/?tree=Try&rev=c98b927e9fce

Comment on attachment 685459 [details] [diff] [review] patch Review of attachment 685459 [details] [diff] [review]: ----------------------------------------------------------------- This is true.

Comment on attachment 685459 [details] [diff] [review] patch Review of attachment 685459 [details] [diff] [review]: ----------------------------------------------------------------- Now that I think about it.. how about we just use the function already in Tools.h?

new patch: https://tbpl.mozilla.org/?tree=Try&rev=b896791c678a

Comment on attachment 687653 [details] [diff] [review] pacth [Approval Request Comment] Bug caused by (feature/regressing bug #): User impact if declined: poor performance for some canvas operations Testing completed (on m-c, etc.): m-c a few days Risk to taking this patch (and alternatives if risky): low, it's an optimisation only, and previous behaviour was technically incorrect. String or UUID changes made by this patch: none

Comment on attachment 687653 [details] [diff] [review] pacth Approving for Aurora/Beta given the magnitude of this canvas perf regression, this fix is considered low risk, and the fact that we're not sure what the volume of affected sites would be.

https://hg.mozilla.org/releases/mozilla-aurora/rev/bad0552d0442 https://hg.mozilla.org/releases/mozilla-beta/rev/23c595cd1ec4

Comment on attachment 687653 [details] [diff] [review] pacth [Approval Request Comment] If this is not a sec:{high,crit} bug, please state case for ESR consideration: optimisation leading to vastly better performance in some cases, addresses regression, simple patch User impact if declined: slow canvas rendering in some cases Fix Landed on Version: 18,19,20 Risk to taking this patch (and alternatives if risky): low String or UUID changes made by this patch: none See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.

Comment on attachment 687653 [details] [diff] [review] pacth Low risk canvas perf fix, we can take this for our next ESR.

Nick, this seems to be causing test failures on ESR17. Can you take a look at what's going on please? https://tbpl.mozilla.org/php/getParsedLog.php?id=17802670&tree=Mozilla-Esr17

(In reply to Ryan VanderMeulen from comment #39) > Nick, this seems to be causing test failures on ESR17. Can you take a look > at what's going on please? > https://tbpl.mozilla.org/php/getParsedLog.php?id=17802670&tree=Mozilla-Esr17 Hmm, I can't see what is going wrong, the errors are with the xor operator and we shouldn't touch paths that are xor. I can only assume that something changed between 17 and 18 to make this patch not work properly. Given that this is just a perf issue and is a relatively rare case, I don't think it is worth spending a lot of time investigating. I think we should just back it out and not land on esr17. Would you mind doing that? I don't have an esr17 tree, sorry.

Thanks for looking into it. Backed out. https://hg.mozilla.org/releases/mozilla-esr17/rev/e3ba5fa73f73

(In reply to Ryan VanderMeulen from comment #41) > Thanks for looking into it. Backed out. > https://hg.mozilla.org/releases/mozilla-esr17/rev/e3ba5fa73f73 Thanks for the backout!

FF 16 <10ms FF 17 >1000 ms FF 18b4 <10ms Verified fixed.

FF 19b3 < 10ms. Verified fixed Win 7, Ubuntu 12.04, Mac OS X 10.8.2

FF 20b7 < 10ms. Verified fixed.