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

Status: RESOLVED FIXED

(Core :: Graphics: Canvas2D, defect)

Description

[Chris Peterson [:cpeterson]]

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

Comment 1

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

(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.

Comment 2

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Note: if I disable Chrome's accelerated canvas support, I get 2343.  So I guess raw software-only skia gets 2343; SkiaGL gets ~8000.

Comment 3

[Matt Woodrow (:mattwoodrow)]

Attachment: Use gfxGradientCache for canvas

I haven't tested this at all, should help though.

Comment 4

[Robert O'Callahan (:roc) (email my personal email if necessary)]

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.

Comment 5

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

(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.

Comment 6

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

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)

Comment 7

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

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.

Comment 8

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(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).

Comment 9

[Olli Pettay [:smaug][[e-mail]]]

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.)

Comment 10

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

It's coming from deleting tons of stuff, during GC/CC.

Comment 11

[Olli Pettay [:smaug][[e-mail]]]

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.)

Comment 12

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

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.

Comment 13

[Chris Peterson [:cpeterson]]

(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/

Comment 14

[Olli Pettay [:smaug][[e-mail]]]

thanks.
On linux I don't see jank, but the just is rather slow.
(and all the time is spent in 2d canvas / cairo)

Comment 15

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

(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).

Comment 16

[Chris Peterson [:cpeterson]]

Vlad: do you want to land Matt's gradient cache patch? It is still waiting for f+ from you.

Comment 17

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

The gradient cache helps, but doesn't solve the problem fully (helps us match IE though).  Let's see what Jeff thinks.

Comment 18

[Jeff Muizelaar [:jrmuizel]]

(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.

Comment 19

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

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...

Comment 20

[Jeff Muizelaar [:jrmuizel]]

(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.

Comment 21

[Bas Schouten (:bas.schouten)]

Attachment: Gradient cache test

This test case shows that IE uses a similar sort of gradient cache.

Comment 22

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

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.

Comment 23

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

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.

Comment 24

[Chris Peterson [:cpeterson]]

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.

Comment 25

[Olli Pettay [:smaug][[e-mail]]]

Whatever is done on branches, please land to m-i or m-c asap so that this gets testing.

Comment 26

[Matt Woodrow (:mattwoodrow)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4330724d9e05

Comment 27

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

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.

Comment 28

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Ugh, I think all of us fixed that typo locally, but noone uploaded a fixed version.  Whoops.

Comment 29

[bhavana bajaj [:bajaj]]

(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.

Comment 30

[bhavana bajaj [:bajaj]]

(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.

Comment 31

[Chris Peterson [:cpeterson]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a61b0a3f2d60

Comment 32

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/a61b0a3f2d60

Comment 33

[Chris Peterson [:cpeterson]]

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

Comment 34

[Lukas Blakk [:lsblakk] use ?needinfo]

Comment on attachment 8335058 [details] [diff] [review]
Use gfxGradientCache for canvas

wouldn't block release on this, but definitely can take just not tracking.

Comment 35

[Chris Peterson [:cpeterson]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/a018a8a8e9e5

Comment 36

[Chris Peterson [:cpeterson]]

Vlad: does this bug need any more work besides the gradient cache?

Comment 37

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

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.

Comment 38

[Chris Peterson [:cpeterson]]

Sounds good. I'll follow up with Naveed and Rightware.