Closed Bug 985017 Opened 6 years ago Closed 6 years ago
10% svgx regression on b2g-inbound on march 7th for osx 10
.8 and win 8 platforms
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.
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.
thanks for looking into this, this affects aurora and is seen on graph server!
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.
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.
Attachment #8396107 - Flags: review?(seth)
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!
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
blocking-b2g: 1.4? → 1.4+
Severity: normal → major
Priority: -- → P1
Whiteboard: [talos_regression] → [c= p= s= u=1.4] [talos_regression]
Attachment #8396106 - Flags: review?(mh+mozilla) → review+
are we going to uplift this to Aurora?
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.
Attachment #8396107 - Flags: review?(seth) → review+
this is resolved fixed, but we have a regression on aurora, are we going to land there or let this go?
(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.
Whiteboard: [c= p= s= u=1.4] [talos_regression] → [c= p= s= u=1.4] [talos_regression][qa-]
You need to log in before you can comment on or make changes to this bug.