919992 - Peacekeeper ripple tests slower than in Chrome

Status: NEW

(Core :: Canvas: 2D, defect)

Description

[Jan de Mooij [:jandem]]

Attachment: Testcase

I'm attaching a standalone, reduced version of http://www.peacekeeper.therichins.net/test12.html

I get 18-19 fps on OS X. With Chrome 31 it's 30-31 fps.

On the JS side, we seem to spend a lot of time in js_math_round; we should find out why we're not inlining that. However, JS seems to be faster than Chrome already (see also bug 606648).

It looks like we need to make this faster:

    this.ctx.fillStyle = "rgb("+rValue+", "+gValue+", "+bValue+")";
    this.ctx.fillRect(this.xPosition + x * this.pixelWidth, 
                      this.yPosition + y * this.pixelHeight,  
                      this.pixelWidth,
                      this.pixelHeight);

We seem to spend a lot of time under fillRect and parsing "rgb(..)". Filing this bug in the CSS component but maybe Canvas/Graphics is a better choice; feel free to move it.

Comment 1

[Jan de Mooij [:jandem]]

Bug 920046 will make the Math.round call a good bit faster. With that most of the JS time will be in JIT code and math_round_impl/floor and we are a lot faster than V8.

If we can be as fast as Chrome on the canvas stuff we will do really well on this test.

Comment 2

[Boris Zbarsky [:bzbarsky]]

For fillRect, see bug 601176.

I could have sworn we had existing bugs about color parsing.... I'll see what's going on there.

Comment 3

[Boris Zbarsky [:bzbarsky]]

So for what it's worth, I just tried this on Mac.  The testcase is about 20fps for me, with Chrome at 30fps.

If I comment out the fillRect call, we're at 85-90 fps, while Chrome is at 65-70.

So the non-fillrect bits of this take at most 12ms for us (of which a profile says about half is color parsing), while fillRect takes about 40ms.  For Chrome, the non-fillRect bits take about 14ms, and the fillRect takes about 20ms.

So this is really fillRect-bound.  I'll see what I can do about shaving time off color parsing but even if that gets down to 0 we'll end up at 22fps.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Profile shows nothing really obvious for color parsing other than "it's complicated".  Which it is.  :(

Comment 5

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

Yeah, the place to look is to figure out the fillRect cost.  Color parsing sucks, I've long advocated that we should accept arrays to fillColor/strokeColor too (e.g. canvas.fillColor = [1.0, 0.0, 0.0, 1.0]; for rgba) to entirely skip the parsing.

Comment 6

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

One day we might want a supermagical WebIDL extension such as
  [StringSetter(fillStyleRGB, "rgb(", unsigned long r, ",", unsigned long g, ",", unsigned long b, ")"),
   StringSetter(fillStyleRGBA, "rgba(", unsigned long r, ",", unsigned long g, ",", unsigned long b, ",", float a, ")")]
  attribute (DOMString or CanvasGradient or CanvasPattern) fillStyle;
which could pattern-match JS expressions in the JIT and call through to special stubs, avoiding building the string and parsing it :-). Would also be useful for "element.style.top = y + 'px';" and stuff like that --- very common.

Comment 7

[Guilherme Lima]

I just tested this one, and Nightly is faster than Chrome 33 (18 vs 16fps) on the testcase attached and on the original peacekeeper test.

Comment 8

[Olli Pettay [:smaug]]

On 64 bit Linux Nightly ~33fps vs Chromium ~21fps