Closed
Bug 985017
Opened 11 years ago
Closed 11 years ago
10% svgx regression on b2g-inbound on march 7th for osx 10.8 and win 8 platforms
Categories
(Core :: Graphics: ImageLib, defect, P1)
Core
Graphics: ImageLib
Tracking
()
Tracking | Status | |
---|---|---|
firefox29 | --- | unaffected |
firefox30 | + | fixed |
firefox31 | + | fixed |
b2g-v1.3 | --- | unaffected |
b2g-v1.3T | --- | fixed |
b2g-v1.4 | --- | fixed |
b2g-v2.0 | --- | fixed |
People
(Reporter: jmaher, Assigned: mwu)
References
Details
(Keywords: perf, regression, Whiteboard: [c= p= s= u=1.4] [talos_regression][qa-])
Attachments
(2 files, 1 obsolete file)
(deleted),
patch
|
glandium
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
seth
:
review+
|
Details | Diff | Splinter Review |
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.
Reporter | ||
Updated•11 years ago
|
Keywords: perf,
regression
Whiteboard: [talos_regression]
Assignee | ||
Comment 1•11 years ago
|
||
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.
Assignee | ||
Comment 2•11 years ago
|
||
Assignee | ||
Updated•11 years ago
|
blocking-b2g: --- → 1.4?
status-b2g-v1.3:
--- → unaffected
status-b2g-v1.3T:
--- → affected
status-b2g-v1.4:
--- → affected
status-b2g-v2.0:
--- → affected
status-firefox30:
--- → affected
status-firefox31:
--- → affected
Reporter | ||
Comment 3•11 years ago
|
||
thanks for looking into this, this affects aurora and is seen on graph server!
Assignee | ||
Comment 4•11 years ago
|
||
Assignee | ||
Comment 5•11 years ago
|
||
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.
Attachment #8395892 -
Attachment is obsolete: true
Attachment #8396106 -
Flags: review?(mh+mozilla)
Assignee | ||
Comment 6•11 years ago
|
||
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)
Reporter | ||
Comment 7•11 years ago
|
||
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•11 years ago
|
||
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+
Updated•11 years ago
|
Severity: normal → major
Priority: -- → P1
Whiteboard: [talos_regression] → [c= p= s= u=1.4] [talos_regression]
Updated•11 years ago
|
tracking-firefox30:
--- → ?
tracking-firefox31:
--- → ?
Updated•11 years ago
|
Attachment #8396106 -
Flags: review?(mh+mozilla) → review+
Updated•11 years ago
|
Reporter | ||
Comment 9•11 years ago
|
||
are we going to uplift this to Aurora?
Comment 10•11 years ago
|
||
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+
Assignee | ||
Comment 11•11 years ago
|
||
Comment 12•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/d7fcc0c1a744
https://hg.mozilla.org/mozilla-central/rev/2c4d1aa4e976
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
Reporter | ||
Comment 13•11 years ago
|
||
this is resolved fixed, but we have a regression on aurora, are we going to land there or let this go?
Assignee | ||
Comment 14•11 years ago
|
||
(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.
Assignee | ||
Updated•11 years ago
|
Reporter | ||
Comment 15•11 years ago
|
||
thanks!
Comment 16•11 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/1266d70f2951
https://hg.mozilla.org/releases/mozilla-aurora/rev/60c9101d5304
status-firefox29:
--- → wontfix
Assignee | ||
Updated•11 years ago
|
Assignee | ||
Comment 17•11 years ago
|
||
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.
Description
•