985017 - 10% svgx regression on b2g-inbound on march 7th for osx 10.8 and win 8 platforms

Status: RESOLVED FIXED

(Core :: Graphics: ImageLib, defect, P1)

Description

[Joel Maher ( :jmaher ) (UTC -8)]

10.8:
http://graphs.mozilla.org/graph.html#tests=[[281,201,24]]&sel=none&displayrange=30&datatype=running

win8:
http://graphs.mozilla.org/graph.html#tests=[[281,203,31]]&sel=none&displayrange=30&datatype=running

these both show a jump in the steady stream of numbers starting with the landing of bug 962670.  I am not sure if this is related to svg, but it doesn't look like a far stretch.

I see hixie-004.xml as taking a permanent hit from this push while many other tests take a more noisy approach:
https://datazilla.mozilla.org/?start=1394018401&stop=1394591478&product=Firefox&repository=B2G-Inbound-Non-PGO&os=win&os_version=6.2.9200&test=tsvgx&graph_search=2429468e82dd&tr_id=4745823&graph=hixie-004.xml&project=talos

in fact you can see the original push, the backout and the repush.

Comment 1

[Michael Wu [:mwu]]

Attachment: Avoid excessive gfxImageSurface creation in drawing path

Turns out creating gfxImageSurfaces can be very expensive. This patch creates a gfxImageSurface for the drawing path that isn't thrown out. However, it also doesn't hold a lock on the volatile buffer, so the drawing path needs to grab a lock before using this surface.

Going to run on try before requesting review. However, this appears to fix the regression locally on my mac mini.

Comment 2

[Michael Wu [:mwu]]

https://tbpl.mozilla.org/?tree=Try&rev=41db73114369

Comment 3

[Joel Maher ( :jmaher ) (UTC -8)]

thanks for looking into this, this affects aurora and is seen on graph server!

Comment 4

[Michael Wu [:mwu]]

https://tbpl.mozilla.org/?tree=Try&rev=99cd40328ad7

Comment 5

[Michael Wu [:mwu]]

Attachment: [1/2] Support null VolatileBufferPtrs

This makes it easier to put VolatileBufferPtr in the right scope (see next patch for better context). mPurged is set to false, but it really has no meaning when we're working with a null VolatileBufferPtr. What it does do is make things fail faster if the code didn't consider the possibility of VolatileBufferPtr being null.

Comment 6

[Michael Wu [:mwu]]

Attachment: [2/2] Cache a gfxImageSurface for the drawing path

This patch creates a gfxImageSurface/gfxQuartzImageSurface so the drawing path doesn't have to create a new one on every draw. In exchange, the drawing path needs to lock the volatile buffer manually in order to use this cached surface.

Comment 7

[Joel Maher ( :jmaher ) (UTC -8)]

this same regression from hg.mozilla.org/integration/b2g-inbound/rev/965fe5b195ab seems to be causing 10.8 and win8 tp5 regressions as well.  Looking forward to this landing!

Comment 8

[bhavana bajaj [:bajaj]]

Please make sure to get this uplifted on 1.3T,central and aurora to resolve it on all branches. I'll sync-up with sheriffs if needed

Comment 9

[Joel Maher ( :jmaher ) (UTC -8)]

are we going to uplift this to Aurora?

Comment 10

[Seth Fowler [:seth] [:s2h]]

Comment on attachment 8396107 [details] [diff] [review]
[2/2] Cache a gfxImageSurface for the drawing path

Review of attachment 8396107 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good.

Comment 11

[Michael Wu [:mwu]]

https://hg.mozilla.org/integration/b2g-inbound/rev/d7fcc0c1a744
https://hg.mozilla.org/integration/b2g-inbound/rev/2c4d1aa4e976

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/d7fcc0c1a744
https://hg.mozilla.org/mozilla-central/rev/2c4d1aa4e976

Comment 13

[Joel Maher ( :jmaher ) (UTC -8)]

this is resolved fixed, but we have a regression on aurora, are we going to land there or let this go?

Comment 14

[Michael Wu [:mwu]]

(In reply to Joel Maher (:jmaher) from comment #13)
> this is resolved fixed, but we have a regression on aurora, are we going to
> land there or let this go?

See comment 8.

Comment 15

[Joel Maher ( :jmaher ) (UTC -8)]

thanks!

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/1266d70f2951
https://hg.mozilla.org/releases/mozilla-aurora/rev/60c9101d5304

Comment 17

[Michael Wu [:mwu]]

https://hg.mozilla.org/releases/mozilla-b2g28_v1_3t/rev/0d50e378158f
https://hg.mozilla.org/releases/mozilla-b2g28_v1_3t/rev/d465edb2b579