Closed Bug 1464046 Opened 3 years ago Closed 3 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)

defect
Not set
normal

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
Component: General → SVG
Product: Testing → Core
Flags: needinfo?(mstange)
Unfortunately, I don't yet have profiles after bug 1458968 landed. Somehow, the test fails.
Before: https://perfht.ml/2kmm609
After: https://perfht.ml/2xcowI8
Assignee: nobody → mstange
Status: NEW → ASSIGNED
Flags: needinfo?(mstange)
(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).
Assignee: mstange → nobody
Status: ASSIGNED → NEW
I've also been able to reproduce the WebRender regression, interestingly enough it only seems to show up when using software opengl.
Anything new on this bug?
(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)
Tracking  for 62 to make sure we follow up on this decision.
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)
Thanks Markus. Maire, given Comment 10... wanna WONTFIX this?
Flags: needinfo?(mreavy)
per comment 10 from mstange
Status: NEW → RESOLVED
Closed: 3 years ago
Flags: needinfo?(mreavy)
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.