Closed
Bug 1464046
Opened 7 years ago
Closed 7 years ago
8.16 - 17.48% tsvg_static / tsvgr_opacity / tsvgx (linux64, linux64-qr, windows7-32) regression on push 133a13c44abedac2e448d315a32068ce1a5568f4 (Wed May 23 2018)
Categories
(Core :: SVG, defect)
Core
SVG
Tracking
()
RESOLVED
WONTFIX
Tracking | Status | |
---|---|---|
firefox60 | --- | unaffected |
firefox61 | --- | unaffected |
firefox62 | + | wontfix |
People
(Reporter: igoldan, Unassigned)
References
Details
(Keywords: perf, regression, talos-regression)
Talos has detected a Firefox performance regression from push:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?changeset=133a13c44abedac2e448d315a32068ce1a5568f4
As author of one of the patches included in that push, we need your help to address this regression.
Regressions:
17% tsvgx linux64-qr opt e10s stylo 328.21 -> 385.57
13% tsvgr_opacity windows7-32 pgo e10s stylo109.28 -> 123.77
11% tsvgr_opacity linux64 opt e10s stylo164.81 -> 183.73
10% tsvgr_opacity linux64 pgo e10s stylo158.97 -> 174.93
9% tsvgr_opacity windows7-32 opt e10s stylo126.23 -> 138.16
8% tsvg_static linux64-qr opt e10s stylo54.73 -> 59.19
You can find links to graphs and comparison views for each of the above tests at: https://treeherder.mozilla.org/perf.html#/alerts?id=13394
On the page above you can see an alert for each affected platform as well as a link to a graph showing the history of scores for this test. There is also a link to a treeherder page showing the Talos jobs in a pushlog format.
To learn more about the regressing test(s), please see: https://wiki.mozilla.org/Buildbot/Talos/Tests
For information on reproducing and debugging the regression, either on try or locally, see: https://wiki.mozilla.org/Buildbot/Talos/Running
*** Please let us know your plans within 3 business days, or the offending patch(es) will be backed out! ***
Our wiki page outlines the common responses and expectations: https://wiki.mozilla.org/Buildbot/Talos/RegressionBugsHandling
Reporter | ||
Updated•7 years ago
|
Component: General → SVG
Product: Testing → Core
Reporter | ||
Updated•7 years ago
|
Flags: needinfo?(mstange)
Reporter | ||
Comment 1•7 years ago
|
||
Here are the Gecko profiles:
before:
tsvgx: https://perf-html.io/from-url/https%3A%2F%2Fqueue.taskcluster.net%2Fv1%2Ftask%2FUdD2zsh_RGGKGMotMQMQfA%2Fruns%2F0%2Fartifacts%2Fpublic%2Ftest_info%2Fprofile_tsvgx.zip
tsvgr_opacity: https://perf-html.io/from-url/https%3A%2F%2Fqueue.taskcluster.net%2Fv1%2Ftask%2FUdD2zsh_RGGKGMotMQMQfA%2Fruns%2F0%2Fartifacts%2Fpublic%2Ftest_info%2Fprofile_tsvgr_opacity.zip
tsvg_static: https://perf-html.io/from-url/https%3A%2F%2Fqueue.taskcluster.net%2Fv1%2Ftask%2FUdD2zsh_RGGKGMotMQMQfA%2Fruns%2F0%2Fartifacts%2Fpublic%2Ftest_info%2Fprofile_tsvg_static.zip
Reporter | ||
Comment 2•7 years ago
|
||
Unfortunately, I don't yet have profiles after bug 1458968 landed. Somehow, the test fails.
Comment 3•7 years ago
|
||
after:
tsvgx: https://perf-html.io/from-url/https%3A%2F%2Fqueue.taskcluster.net%2Fv1%2Ftask%2FLMJ1E8RoTiis42gMu97ADg%2Fruns%2F0%2Fartifacts%2Fpublic%2Ftest_info%2Fprofile_tsvgx.zip
tsvgr_opacity: https://perf-html.io/from-url/https%3A%2F%2Fqueue.taskcluster.net%2Fv1%2Ftask%2FLMJ1E8RoTiis42gMu97ADg%2Fruns%2F0%2Fartifacts%2Fpublic%2Ftest_info%2Fprofile_tsvgr_opacity.zip
tsvg_static: https://perf-html.io/from-url/https%3A%2F%2Fqueue.taskcluster.net%2Fv1%2Ftask%2FLMJ1E8RoTiis42gMu97ADg%2Fruns%2F0%2Fartifacts%2Fpublic%2Ftest_info%2Fprofile_tsvg_static.zip
Comment 4•7 years ago
|
||
Before: https://perfht.ml/2kmm609
After: https://perfht.ml/2xcowI8
Assignee: nobody → mstange
Status: NEW → ASSIGNED
Flags: needinfo?(mstange)
Comment 5•7 years ago
|
||
(In reply to Ionuț Goldan [:igoldan], Performance Sheriffing from comment #0)
> Regressions:
>
> 17% tsvgx linux64-qr opt e10s stylo 328.21 -> 385.57
> 13% tsvgr_opacity windows7-32 pgo e10s stylo109.28 -> 123.77
> 11% tsvgr_opacity linux64 opt e10s stylo164.81 -> 183.73
> 10% tsvgr_opacity linux64 pgo e10s stylo158.97 -> 174.93
> 9% tsvgr_opacity windows7-32 opt e10s stylo126.23 -> 138.16
> 8% tsvg_static linux64-qr opt e10s stylo54.73 -> 59.19
The tsvgx and tsvg_static regressions only happened with WebRender. We can ignore those for now.
The non-WebRender regressions are all in tsvgr_opacity, and they are entirely in the tsvgr_opacity small-group-opacity-2500.svg subtest. Here's a graph of the tsvgr_opacity numbers over the past 60 days:
https://treeherder.mozilla.org/perf.html#/graphs?timerange=5184000&series=mozilla-central,1648239,1,1
You can see a big dip on April 12, which is when bug 1442190 landed. So it looks like bug 1458968 reverted almost the entire performance win from bug 1442190.
And that makes sense: This test has lots of opacity items which could get flattened into a ClientPaintedLayer. After bug 1458968, all of those opacity items are inside an inactive transform item. And there's this comment in FrameLayerBuilder.cpp:
// Currently we do not support flattening effects within nested inactive
// layer trees.
That's exactly the case we're running into here.
I think we have two options for fixing this:
1. We could make opacity flattening work inside inactive layer trees.
2. Or we could wait for bug 1462672, which will flatten the wrapping transform and then hopefully also flatten the opacity items inside that transform.
I don't think we should back out 1458968 to address this regression. The benefits from bug 1458968 are real and make the WebRender invalidation story much simpler. This regression is only regressing a recent win back to the old numbers, and we have an expected way forward to address it (bug 1462672).
Updated•7 years ago
|
Assignee: mstange → nobody
Status: ASSIGNED → NEW
Comment 6•7 years ago
|
||
I've also been able to reproduce the WebRender regression, interestingly enough it only seems to show up when using software opengl.
Updated•7 years ago
|
status-firefox60:
--- → unaffected
status-firefox61:
--- → unaffected
status-firefox62:
--- → affected
Reporter | ||
Comment 7•7 years ago
|
||
Anything new on this bug?
Comment 8•7 years ago
|
||
(In reply to Markus Stange [:mstange] from comment #5)
> I don't think we should back out 1458968 to address this regression. The
> benefits from bug 1458968 are real and make the WebRender invalidation story
> much simpler. This regression is only regressing a recent win back to the
> old numbers, and we have an expected way forward to address it (bug 1462672).
Markus, just to verify, you think we should WONTFIX this?
Flags: needinfo?(mstange)
Comment 9•7 years ago
|
||
Tracking for 62 to make sure we follow up on this decision.
tracking-firefox62:
--- → +
Comment 10•7 years ago
|
||
Yes, I think we should WONTFIX this. I've filed bug 1467169 for the WebRender-part of this regression which probably has a different cause.
Flags: needinfo?(mstange)
Comment 11•7 years ago
|
||
Thanks Markus. Maire, given Comment 10... wanna WONTFIX this?
Flags: needinfo?(mreavy)
Comment 12•7 years ago
|
||
per comment 10 from mstange
Status: NEW → RESOLVED
Closed: 7 years ago
Flags: needinfo?(mreavy)
Resolution: --- → WONTFIX
Updated•7 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•