522900 - Canvas Planetarium is slower than in Safari and Chrome

Status: NEW

(Core :: General, defect)

Description

[Jeff Muizelaar [:jrmuizel]]

The demo at http://tonightsky.free.fr/all-sky-map/ is noticeably slower in Firefox on Mac than Safari and Chrome.

Looking at the profile we spend about 32% in canvas.fill(). Safari spends about 34% in fill(). However we spend 14.9% in CGContextFillPath and Safari spends 27.5% in CGContextFillPath. So we're doing a lot more stuff in fill() other than actually filling. 6.5% of this extra stuff is in _cairo_quartz_setup_source which seems avoidable. 2% is in CGContextRestoreGState() and 1.6% is in _cairo_quartz_cairo_path_to_quartz_context

As for js: we spend 12.6% in js_NativeSet() and 6.0% in js_MonitorLoopEdge()

We also spend 4.8% in canvas.arc() vs Safari's 5.5%

Looking from the bottom-up:

Firefox:
14.5% js_Interpret
2.8% aa_render
2.6% szone_free
1.9% aa_cubeto
1.8% __spin_lock
1.3% tiny_malloc_from_free_list
1.2% cos$fenv_access_off

Safari:
5.4% aa_render
4.1% JSC:JSObject::mark()
4.0% aa_cubeto
3.8% cos$fenv_access_off
2.8% sin

Comment 1

[Jeff Muizelaar [:jrmuizel]]

In summary, none of these percentages are directly comparable, but it looks like we're doing worse in both gfx and js.

Some other notes: We currently use cairo's arc-to-spline implementation which doesn't necessarily generate the same number of segments as CoreGraphics. Moving the arc-to-spline implementation into js should make the amount of time we spend in aa_render about the same in both browsers which would help when comparing the other percentages.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Relevant aborts:

   5 Abort recording of tree http://tonightsky.free.fr/all-sky-map/:291@452 at http://tonightsky.free.fr/all-sky-map/:295@565: No compatible inner tree.
   5 Abort recording of tree http://tonightsky.free.fr/all-sky-map/:262@74 at http://tonightsky.free.fr/all-sky-map/:35@102: Inner tree is trying to grow, abort outer recording.
   4 Abort recording of tree http://tonightsky.free.fr/all-sky-map/:291@452 at http://tonightsky.free.fr/all-sky-map/:35@102: Inner tree is trying to grow, abort outer recording.
   4 Abort recording of tree http://tonightsky.free.fr/all-sky-map/:207@60 at http://tonightsky.free.fr/all-sky-map/:35@102: Inner tree is trying to grow, abort outer recording.
   4 Abort recording of tree http://tonightsky.free.fr/all-sky-map/:207@60 at http://tonightsky.free.fr/all-sky-map/:209@80: No compatible inner tree.
   4 Abort recording of tree http://tonightsky.free.fr/all-sky-map/:176@23 at http://tonightsky.free.fr/all-sky-map/:181@125: SetPropHit.
   1 Abort recording of tree http://tonightsky.free.fr/all-sky-map/:176@23 at http://tonightsky.free.fr/all-sky-map/:13@54: Inner tree is trying to grow, abort outer recording.

What was that about inner trees trying to grow being benign?  ;)

Comment 3

[Boris Zbarsky [:bzbarsky]]

The SetPropHit abort comes here:

  ctx.fillStyle = colorFromB_V(s.bv);

with this message:

trace stopped: 11124: can't trace JavaScript function setter
Abort recording of tree http://tonightsky.free.fr/all-sky-map/:176@23 at http://tonightsky.free.fr/all-sky-map/:181@125: SetPropHit.

This is the main star-drawing loop it's aborting.  fillStyle doesn't seem to be quickstubbed, but I don't understand why we think it has a scripted setter...

Comment 4

[Andreas Gal :gal]

If its deeply nested such an abort can propagate outward and cause the outer trees to fail compiling.

Comment 5

[Boris Zbarsky [:bzbarsky]]

For what it's worth, I commented out that one line.  That made the SetPropHit abort go away, but the others were still there, as was some blacklisting, etc.  A few seconds of dragging about gave me several tens of thousands of trace triggers/exits.

Comment 6

[Andreas Gal :gal]

Are we running to the tree branch limit again?

Comment 7

[Luke Wagner [:luke]]

(In reply to comment #2)
> What was that about inner trees trying to grow being benign?  ;)

Its not that we don't think they happen (there are even more than you show here in v8); the question is: how much time is wasted from these aborts?  It would be a major bummer if, after getting it to work (and indeed its harder than I originally thought) the speedup was imperceptible.  That's why we're going after the visualization in bug 521038 first.

Comment 8

[Boris Zbarsky [:bzbarsky]]

Ah, Andreas has kept claiming they're benign for a while now.  ;)

Note that if they're leading to blacklisting then the speedup is likely to be perceptible.  Still a good thing to measure, of course.

Comment 9

[Luke Wagner [:luke]]

(In reply to comment #8)
> Note that if they're leading to blacklisting then the speedup is likely to be
> perceptible.  Still a good thing to measure, of course.

But they only stay blacklisted if the inner tree never finishes recording (because if it does, it un-blacklists its outer recorder).  Nested recording won't fix that situation, since what we need is a profitability heuristic for deciding whether its worth it or not to record a trace that definitely bails (because its often much more expensive than just interpreting).  That could also be a worthwhile venture, though.

Comment 10

[Boris Zbarsky [:bzbarsky]]

Ah, ok.  On this testcase, I get blacklisted(1), unblacklisted(0) when I comment out the fillStyle.

Can someone file a separate bug on the fillStyle thing, blocking this one?