Closed Bug 940811 Opened 6 years ago Closed 6 years ago

Browsermark "Graphics HTML5 Canvas" test is 8x slower than Chrome

Categories

(Core :: Canvas: 2D, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla29
Tracking Status
firefox27 --- wontfix
firefox28 - fixed
firefox29 --- fixed

People

(Reporter: cpeterson, Assigned: mattwoodrow)

References

(Blocks 1 open bug)

Details

(Whiteboard: [benchmark_regression][qa-])

Attachments

(2 files)

From Naveed's Browsermark "Graphics HTML5 Canvas" test results (bigger is better):

* Chrome 31 = 10,991
* Chrome 27 = 9,808
* Firefox 25 = 1,356
* Firefox 22 = 1,339
* IE11 = 5,023
(Note: this is one of the biggest benchmark failures in the Tom's hardware shootout; if we can dramatically raise our score here, we will be doing much better overall.)

We are absolutely awful here; it feels like we have some huge GC pauses as well.  The test, however, seems to basically be a test of how fast you can create and draw one-off gradients, both linear and radial.

(The test code is on their site at http://browsermark.rightware.com/tests/benchmarks/graphics/html5_canvas/test.js -- not minified or anything, just whitespace stripped; paste it into jsbeautifier)

1) The test loop is

    timerDraw = setTimeout("draw()", 1);

This is crappy for all sorts of reasons, including that we're at the mercy of whatever the minimum is for each browser.  Using a string here is also clownshoes.  If I bump our frame rate to 500 (from 60), we go from ~1350 -> ~1830 on the test.

2) The test is basically a benchmark of gradient creation and drawing.  It is a dumb test.  The majority of the time is spent in Ball.prototype.drawTrajectories, likely because we don't hit gradient cache for these because the coordinates are different all the time.  It does the following, for each trajectory line:

 ctx.beginPath();
 ctx.moveTo(px, py);
 grd = ctx.createLinearGradient(px, py, tx, ty);
 grd.addColorStop(0, ...);
 grd.addColorStop(endP, ...);
 ctx.strokeStyle = grd;
 ctx.lineTo(tx, ty);
 ctx.stroke();

Each gradient is created and used only once.  The lines are 3 pixels thick.  Gradients are used for the balls and for the background grid, but those are more constant coordinates so I suspect we hit the cache.  But the trajectory lines thrash it.

Some numbers on my machine:
 -- latest nightly (64-bit, but shouldn't matter):  1350
 -- chrome 31: 6902  (I don't have chrome 30 any more to test; I think they have a regression since 30 though)
 -- bumping up layout.frame_rate to 500:  1830   (everything else is with standard -1 [60])
 -- using cairo as canvas backend instead of d2d:  2657 (see point #2 below)
 -- cairo, no-op drawTrajectories:  3734
 -- d2d, no-op drawTrajectories:  4242
 -- d2d, no gradients; only solid colors: 9725
 -- chrome 30, no gradients; only solid colors:  10120
 -- chrome 31, no gradients; only solid colors:  7920  (note: they regressed)

I can't test what Skia's perf is like, but I would assume similar to chrome's perf.
Note: if I disable Chrome's accelerated canvas support, I get 2343.  So I guess raw software-only skia gets 2343; SkiaGL gets ~8000.
I haven't tested this at all, should help though.
Attachment #8335058 - Flags: feedback?(vladimir)
Chrome is presumably getting some benefit from pushing the actual drawing off the main thread. That's bug 857895.

CanvasTest.prototype.draw starts by filling the entire canvas with a gradient, so under some conditions you could benefit from recognizing that and squelching draw operations that you know are about to be overwritten.
(In reply to Matt Woodrow (:mattwoodrow) from comment #3)
> Created attachment 8335058 [details] [diff] [review]
> Use gfxGradientCache for canvas
> 
> I haven't tested this at all, should help though.

I'll give it a try tomorrow.  Didn't realize we weren't caching gradients in canvas.

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #4)
> Chrome is presumably getting some benefit from pushing the actual drawing
> off the main thread. That's bug 857895.
> 
> CanvasTest.prototype.draw starts by filling the entire canvas with a
> gradient, so under some conditions you could benefit from recognizing that
> and squelching draw operations that you know are about to be overwritten.

I don't think it would help in this case -- that's the first thing that's rendered for a frame, and the first part of the setTimeout loop.  So there isn't any incomplete rendering that you could abort when you see this frame being rendered, because you have to present the previous frame.
Jeff and I have been looking at this:

- The gradient cache does make a big difference.  Adding it in gives us performance comparable (just a tad slower) than IE.

- The biggest difference in performance has to do with the linear gradients in drawTrajectories.  Removing all the radial gradients etc. makes very little difference in perf.

- The horrible jank is caused by cycle collection -> GC -> freeing a whole bunch of D2D gradient resources.  We create a bunch of d2d gradients (which is a 1xN texture) and only use them once, and they don't get deleted until we need to GC/CC.  When that happens we end up marking a whole bunch of D3D resources for deletion, which all seem to happen when we call OMSetRenderTargets in the compositor.

- Over the entire benchmark run, we spend 50% of the time in D2D::CreateLinearGradientBrush

- 25% of it is in D2D::Flush (called from FinalizeRTFromOperation)
Note: with the gradient cache, the benchmark goes from 2291 -> 3417, matching IE10.  The jank is significantly reduced.  IE10 on this machine is 3334.  Chrome 31 is 8000.
(In reply to Vladimir Vukicevic [:vlad] [:vladv] from comment #6)
> - The horrible jank is caused by cycle collection -> GC -> freeing a whole
> bunch of D2D gradient resources.  We create a bunch of d2d gradients (which
> is a 1xN texture) and only use them once, and they don't get deleted until
> we need to GC/CC.  When that happens we end up marking a whole bunch of D3D
> resources for deletion, which all seem to happen when we call
> OMSetRenderTargets in the compositor.

Incremental cycle collection might help here (bug IncrementalCC).
Is the jank coming from cycle collector? I didn't read vlad's comment that way.
I thought the jank is from deleting tons of stuff.

(I guess I need to figure out how to run the test and profile.)
It's coming from deleting tons of stuff, during GC/CC.
Is SnowWhite in the stack at that point? If so, we could probably reduce the jank by scheduling
deletion to happen at different times. A new bug for that would be good.
(I still don't know how to run that particular test.)
Whiteboard: [benchmark_regression]
One interesting thing about this particular case is that we *could* delete/release the D3D resource on a different thread.  I have no idea if that would actually help; not sure if there's under-the-hood synchronization that would bite us.
(In reply to Olli Pettay [:smaug] from comment #11)
> (I still don't know how to run that particular test.)

smaug: if you VPN into the Mozilla corporate network, you can run these tests from Vlad's server:

http://vladimir5.corp.tor1.mozilla.com/b/browsermark/browser/graphics-html5-canvas.html

The other tests are in:

http://vladimir5.corp.tor1.mozilla.com/b/browsermark/browser/
thanks.
On linux I don't see jank, but the just is rather slow.
(and all the time is spent in 2d canvas / cairo)
(In reply to Olli Pettay [:smaug] from comment #14)
> thanks.
> On linux I don't see jank, but the just is rather slow.
> (and all the time is spent in 2d canvas / cairo)

Right, the jank is fairly windows specific; see comment #6 (it's due to freeing d2d objects).
Vlad: do you want to land Matt's gradient cache patch? It is still waiting for f+ from you.
Flags: needinfo?(vladimir)
The gradient cache helps, but doesn't solve the problem fully (helps us match IE though).  Let's see what Jeff thinks.
Flags: needinfo?(vladimir) → needinfo?(jmuizelaar)
(In reply to Vladimir Vukicevic [:vlad] [:vladv] from comment #17)
> The gradient cache helps, but doesn't solve the problem fully (helps us
> match IE though).  Let's see what Jeff thinks.

If we can show that IE is caching the gradients I'd feel better about taking this. I feel better about joining a sadness arms race than starting one.
Flags: needinfo?(jmuizelaar)
How do we show that?  I mean given that adding a cache makes us basically match IE's score sounds like decent evidence to me...
(In reply to Vladimir Vukicevic [:vlad] [:vladv] from comment #19)
> How do we show that?  I mean given that adding a cache makes us basically
> match IE's score sounds like decent evidence to me...

A specifically crafted test case should show this.
Attached file Gradient cache test
This test case shows that IE uses a similar sort of gradient cache.
Comment on attachment 8335058 [details] [diff] [review]
Use gfxGradientCache for canvas

r+, let's land this.  Bas wrote a testcase that showed that IE is definitely caching gradients in exactly this way.
Attachment #8335058 - Flags: feedback?(vladimir) → feedback+
Chris, Naveed -- this is simple enough that if you think there's a pressing need for benchmark wins, we could land this on aurora/beta.
I think we should uplift this fix to Aurora 28 and Beta 27, if Rel Man will let us. :) This benchmark is one of our biggest benchmark failures in Tom's Hardware's Grand Prix.

We are still early in a release cycle that is conveniently extended +2 weeks for the holidays, so we should have plenty of test time, even on Beta. The next merge date is February 3.
Whatever is done on branches, please land to m-i or m-c asap so that this gets testing.
Assignee: nobody → matt.woodrow
Backed out in http://hg.mozilla.org/integration/mozilla-inbound/rev/332bfa1847e0 for breaking lots of builds like this: https://tbpl.mozilla.org/php/getParsedLog.php?id=32257040&tree=Mozilla-Inbound


In file included from /builds/slave/m-in-l64-asan-0000000000000000/build/obj-firefox/dom/bindings/UnionTypes.cpp:2:
../../dist/include/mozilla/dom/CanvasGradient.h:48:55: error: unexpected namespace name 'gfx': expected expression
                                                      gfx;:EXTEND_CLAMP);
                                                      ^
../../dist/include/mozilla/dom/CanvasGradient.h:48:59: error: expected expression
                                                      gfx;:EXTEND_CLAMP);
                                                          ^
2 errors generated.
Ugh, I think all of us fixed that typo locally, but noone uploaded a fixed version.  Whoops.
(In reply to Chris Peterson (:cpeterson) from comment #24)
> I think we should uplift this fix to Aurora 28 and Beta 27, if Rel Man will
> let us. :) This benchmark is one of our biggest benchmark failures in Tom's
> Hardware's Grand Prix.
> 
> We are still early in a release cycle that is conveniently extended +2 weeks
> for the holidays, so we should have plenty of test time, even on Beta. The
> next merge date is February 3.

Also a nice win, this is too late to get landed on Beta cycle, given this is not on m-c yet so not tracking for Fx27. Might consider for Fx28(current aurora) given the risk and stabilization on nightly.
(In reply to bhavana bajaj [:bajaj] from comment #29)
> (In reply to Chris Peterson (:cpeterson) from comment #24)
> > I think we should uplift this fix to Aurora 28 and Beta 27, if Rel Man will
> > let us. :) This benchmark is one of our biggest benchmark failures in Tom's
> > Hardware's Grand Prix.
> > 
> > We are still early in a release cycle that is conveniently extended +2 weeks
> > for the holidays, so we should have plenty of test time, even on Beta. The
> > next merge date is February 3.
> 
> Also a nice win, this is too late to get landed on Beta cycle, given this is
Also** , meant Although above
> not on m-c yet so not tracking for Fx27. Might consider for Fx28(current
> aurora) given the risk and stabilization on nightly.
https://hg.mozilla.org/mozilla-central/rev/a61b0a3f2d60
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
Comment on attachment 8335058 [details] [diff] [review]
Use gfxGradientCache for canvas

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
User impact if declined: This bug is one of the biggest benchmark failures in the Tom's Hardware "Web Browser Grand Prix". If we can dramatically raise our score here, we will be doing much better overall.
Testing completed (on m-c, etc.): m-c
Risk to taking this patch (and alternatives if risky): This is a small code change to use a gfx cache (that is already used in other Gecko code).
String or IDL/UUID changes made by this patch: None
Attachment #8335058 - Flags: approval-mozilla-aurora?
Comment on attachment 8335058 [details] [diff] [review]
Use gfxGradientCache for canvas

wouldn't block release on this, but definitely can take just not tracking.
Attachment #8335058 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Vlad: does this bug need any more work besides the gradient cache?
Flags: needinfo?(vladimir)
I don't think so.  We're not going to match Chrome's speed on Windows with this, but we will match IE.  The next steps here are to provide feedback on this benchmark to Rightware.
Flags: needinfo?(vladimir)
Sounds good. I'll follow up with Naveed and Rightware.
Whiteboard: [benchmark_regression] → [benchmark_regression][qa-]
You need to log in before you can comment on or make changes to this bug.