Closed
Bug 1067588
Opened 10 years ago
Closed 10 years ago
3-20% Win7|8 regressions on inbound (v.35) Sept 12th from push c232720a2847
Categories
(Core :: Graphics, defect)
Tracking
()
RESOLVED
FIXED
mozilla35
People
(Reporter: jmaher, Assigned: mattwoodrow)
References
Details
(Keywords: perf, regression, Whiteboard: [talos_regression])
Attachments
(2 files)
(deleted),
patch
|
bas.schouten
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
bas.schouten
:
review+
|
Details | Diff | Splinter Review |
a big set of regressions came in late last week:
+---------------------+----------------+-------------------------------+---------+-----+--------+
| date | platform | test | percent | bug | status |
+---------------------+----------------+-------------------------------+---------+-----+--------+
| 2014-09-12 16:42:31 | WINNT 6.1 (ix) | TP5 Scroll | -10.6% | | |
| 2014-09-12 16:43:28 | WINNT 6.1 (ix) | Customization Animation Tests | -21.6% | | |
| 2014-09-12 16:44:37 | WINNT 6.1 (ix) | SVG-ASAP | -12.7% | | |
| 2014-09-12 16:43:54 | WINNT 6.1 (ix) | Tab Animation Test | -10.6% | | |
| 2014-09-12 16:46:41 | WINNT 6.1 (ix) | Tp5 Optimized | -3.26% | | |
| 2014-09-12 16:47:09 | WINNT 6.1 (ix) | TResize | -20% | | |
| 2014-09-12 16:47:13 | WINNT 6.2 x64 | TResize | -17.6% | | |
| 2014-09-12 17:06:27 | WINNT 6.2 x64 | Tp5 Optimized | -2.62% | | |
| 2014-09-12 17:23:07 | WINNT 6.2 x64 | Customization Animation Tests | -13.2% | | |
| 2014-09-12 17:23:32 | WINNT 6.2 x64 | Tab Animation Test | -9.03% | | |
| 2014-09-12 17:24:13 | WINNT 6.2 x64 | SVG-ASAP | -12.7% | | |
+---------------------+----------------+-------------------------------+---------+-----+--------+
looking on graph server, you can see that this is the case:
http://graphs.mozilla.org/graph.html#tests=[[254,131,25]]&sel=1410492849802,1410530649802&displayrange=7&datatype=running
We don't have a scenario here where we didn't build or run tests for a few builds, this is the real deal and here is the push that caused the regression:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?changeset=c232720a2847
It appears :mattwoodrow is on PTO for this week, we need to get somebody else to look at this.
Reporter | ||
Comment 1•10 years ago
|
||
:bas, can you comment on this as to what was expected, and what we can do to reduce these regressions?
Flags: needinfo?(bas)
Comment 2•10 years ago
|
||
(In reply to Joel Maher (:jmaher) from comment #1)
> :bas, can you comment on this as to what was expected, and what we can do to
> reduce these regressions?
With Matt on PTO it's my opinion we should probably back out the integration patches. But JWatt is the best person to chime in.
Flags: needinfo?(bas) → needinfo?(jwatt)
Comment 3•10 years ago
|
||
I agree with Bas. (bug 1044702 comment 13 also suspects it of causing crashes.)
Flags: needinfo?(jwatt)
Assignee | ||
Comment 5•10 years ago
|
||
I'm back now!
I've got a fix up for the crash, will look at the perf stuff this week too.
Flags: needinfo?(jwatt)
Flags: needinfo?(bas)
Assignee | ||
Comment 6•10 years ago
|
||
Assignee: nobody → matt.woodrow
Attachment #8496062 -
Flags: review?(bas)
Updated•10 years ago
|
Attachment #8496062 -
Flags: review?(bas) → review+
Assignee | ||
Comment 7•10 years ago
|
||
Reporter | ||
Comment 8•10 years ago
|
||
This shows improvements, in fact the TART, CART, and TResize tests are back to normal. Tp5 is fixed from this regression (although other regressions make the graph look bad, but for this bug it is fixed!)
The svg tests are still regressed:
win7 svgx: http://graphs.mozilla.org/graph.html#tests=%5B%5B281,131,25%5D%5D&sel=none&displayrange=30&datatype=running
win8 svgx: http://graphs.mozilla.org/graph.html#tests=%5B%5B281,131,31%5D%5D&sel=none&displayrange=30&datatype=running
Reporter | ||
Comment 9•10 years ago
|
||
the svg regressions appear to be in Hixie-004.xml:
http://hg.mozilla.org/build/talos/file/53dab58cff84/talos/page_load_test/svgx/hixie-004.xml
Comment 10•10 years ago
|
||
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
Reporter | ||
Comment 11•10 years ago
|
||
I would like to leave this open until we have agreement on what we are doing with the svgx regressions on windows.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Comment 12•10 years ago
|
||
Are there multiple regressions in those graphs?
For svgx on windows 7 it looks like we were averaging around 238, then we jumped up to ~253 briefly before jumping again to ~277 (maybe with yet another step in the middle).
I have a patch on try that looks like it'll get us back to ~253, not sure if that covers all the regressions that I caused or not.
Assignee | ||
Comment 13•10 years ago
|
||
If the subimage covers the entire image then we don't want to enforce the sampling restriction.
This moves the code around to benefit from the checks around CSRD instead of having our own ones.
Attachment #8497938 -
Flags: review?(bas)
Assignee | ||
Comment 14•10 years ago
|
||
Reporter | ||
Comment 15•10 years ago
|
||
the try push shows tsvgx having an improvement :)
Comment 16•10 years ago
|
||
Comment on attachment 8497938 [details] [diff] [review]
Don't extract a subimage if it would be the entire image
Review of attachment 8497938 [details] [diff] [review]:
-----------------------------------------------------------------
Hrm, does this not reduce the cases in which we use the DrawWithSamplingRect function?
Assignee | ||
Comment 17•10 years ago
|
||
(In reply to Bas Schouten (:bas.schouten) from comment #16)
> Comment on attachment 8497938 [details] [diff] [review]
> Don't extract a subimage if it would be the entire image
>
> Review of attachment 8497938 [details] [diff] [review]:
> -----------------------------------------------------------------
>
> Hrm, does this not reduce the cases in which we use the DrawWithSamplingRect
> function?
Yes, it should skip it if the sampling rect matches the image rect.
Updated•10 years ago
|
Attachment #8497938 -
Flags: review?(bas) → review+
Assignee | ||
Comment 18•10 years ago
|
||
Comment 19•10 years ago
|
||
Status: REOPENED → RESOLVED
Closed: 10 years ago → 10 years ago
Resolution: --- → FIXED
Reporter | ||
Comment 20•10 years ago
|
||
this cleared up the win7 regression, but not the win8:
http://graphs.mozilla.org/graph.html#tests=%5B%5B281,131,31%5D%5D&sel=none&displayrange=7&datatype=running
:bas, should we leave this alone, it looks like we have resolved all but one issue!
Flags: needinfo?(bas)
Comment 21•10 years ago
|
||
(In reply to Joel Maher (:jmaher) from comment #20)
> this cleared up the win7 regression, but not the win8:
> http://graphs.mozilla.org/graph.html#tests=%5B%5B281,131,
> 31%5D%5D&sel=none&displayrange=7&datatype=running
>
> :bas, should we leave this alone, it looks like we have resolved all but one
> issue!
Probably still good to look at, although Win8 should now be using D2D 1.1. We should probably open a separate bug.
Flags: needinfo?(bas)
Reporter | ||
Comment 22•10 years ago
|
||
this has caused a 7.3% win8 tp5o_scroll regression:
http://graphs.mozilla.org/graph.html#tests=%5B%5B323,63,31%5D,%5B323,131,31%5D%5D&sel=1411981153079.1162,1412208779141.4392,2.4782608695652177,6&displayrange=30&datatype=running
maybe this is no big deal; it does seem a bit high though.
Comment 23•10 years ago
|
||
(In reply to Joel Maher (:jmaher) from comment #20)
> :bas, should we leave this alone, it looks like we have resolved all but one
> issue!
Joel, can you open a new bug for whatever issue you're referring to here and note the number in this bug?
(In reply to Joel Maher (:jmaher) from comment #22)
> this has caused a 7.3% win8 tp5o_scroll regression:
Also can you clarify what you mean by "this"?
Flags: needinfo?(jmaher)
Reporter | ||
Comment 24•10 years ago
|
||
the tp5 scroll regression appears to be fixed, I believe there is no pending regressions from this bug:
http://graphs.mozilla.org/graph.html#tests=%5B%5B323,63,31%5D,%5B323,131,31%5D%5D&sel=none&displayrange=90&datatype=running
Flags: needinfo?(jmaher)
Updated•10 years ago
|
Flags: qe-verify-
You need to log in
before you can comment on or make changes to this bug.
Description
•