1476964 - 2.23 - 9.1% displaylist_mutate / glterrain / tscrollx / tsvg_static (linux64) regression on push 72725d9980b366c47b3c1cff39c3a737d0d3c8d8 (Wed Jul 18 2018)

Status: RESOLVED WONTFIX

(Core :: Graphics: Layers, defect, P2)

Description

[Ionuț Goldan [:igoldan]]

Talos has detected a Firefox performance regression from push:

https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?changeset=72725d9980b366c47b3c1cff39c3a737d0d3c8d8

As author of one of the patches included in that push, we need your help to address this regression.

Regressions:

  9%  tsvg_static linux64 pgo e10s stylo     49.45 -> 53.95
  8%  tsvg_static linux64 opt e10s stylo     50.74 -> 54.99
  8%  tscrollx linux64 opt e10s stylo        0.72 -> 0.77
  7%  tscrollx linux64 pgo e10s stylo        0.70 -> 0.75
  4%  displaylist_mutate linux64 pgo e10s stylo2,795.41 -> 2,920.95
  2%  displaylist_mutate linux64 opt e10s stylo3,095.91 -> 3,169.03
  2%  glterrain linux64 opt e10s stylo       5.46 -> 5.58
  2%  glterrain linux64 pgo e10s stylo       5.41 -> 5.54

Improvements:

 15%  rasterflood_svg linux64 pgo e10s stylo     12,449.62 -> 10,595.10
 10%  rasterflood_svg linux64 opt e10s stylo     11,898.26 -> 10,730.76
  5%  tp5o_webext responsiveness linux64 pgo e10s stylo1.48 -> 1.41
  5%  tp5o_webext responsiveness linux64 opt e10s stylo1.70 -> 1.61
  4%  tp5o linux64 pgo e10s stylo                130.42 -> 124.72
  4%  tp5o linux64 opt e10s stylo                143.50 -> 138.06
  3%  tp5o_scroll linux64 pgo e10s stylo         0.59 -> 0.57
  3%  tp5o_webext linux64 opt e10s stylo         225.82 -> 219.88
  2%  tp5o_webext linux64 pgo e10s stylo         207.28 -> 202.42


You can find links to graphs and comparison views for each of the above tests at: https://treeherder.mozilla.org/perf.html#/alerts?id=14427

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

Comment 1

[Ryan Hunt [:rhunt] (on leave until early May)]

From a discussion with Matt Woodrow on irc, we're not very worried about the displaylist_mutate test. He is concerned about DisplayList/FrameLayerBuilder changes that regress that, and tiling isn't one of those.

I'm skeptical about glterrain. It's a small absolute and percentage change, and I haven't seen that on any other platform when enabling tiling. I'd lean towards accepting it for the other gains, if it's real.

tscrollx is more significant, but also a small absolute change. I'd lean towards accepting that as well.

tsvg_static is significant and I will look into that to see if there are any improvements we can make there.

Comment 2

[Ryan Hunt [:rhunt] (on leave until early May)]

Not all of the subtests in tsvg_static are worse. It looks like composite-scale-rotate and composite-scale are the ones that actually regressed.

Oddly enough though, I wasn't able to see any difference when running it locally. They actually improve with tiling enabled.

I also noticed that those two tests don't actually display anything for me. It seems like the images they're loading aren't actually available. If I copied the images from another svg test using them into the tsvg_static folder, then the test actually displays correctly.

I did a try run to see if that made a difference for some reason, and it looks like it improves the test. [1]

I'm not sure if that makes up the whole regression, it's about 4 points or 5.5% improvement. I'll put up the patch here.

[1] https://treeherder.mozilla.org/perf.html#/compare?originalProject=mozilla-central&originalRevision=4f12d77b4f9b%26&newProject=try&newRevision=3e6dd59723cfdc949fe3723e2c29d04c3481d6a6&framework=1&filter=tsvg

Comment 3

[Ionuț Goldan [:igoldan]]

(In reply to Ryan Hunt [:rhunt] from comment #2)
> Oddly enough though, I wasn't able to see any difference when running it
> locally. They actually improve with tiling enabled.
> 
> I also noticed that those two tests don't actually display anything for me.
> It seems like the images they're loading aren't actually available. If I
> copied the images from another svg test using them into the tsvg_static
> folder, then the test actually displays correctly.
> 
> I did a try run to see if that made a difference for some reason, and it
> looks like it improves the test. [1]

:jwatt this sounds like an issue with the tsvg_static tests. Could you look over this?

Comment 4

[Ionuț Goldan [:igoldan]]

Bug 1480139 basically backed out bug 1471704. So the regressions/improvements from comment 0 were canceled.

Comment 5

[Jonathan Watt [:jwatt]]

It looks like bug 1318530 broke all four of the testing/talos/talos/tests/svg_static/composite-scale-*.svg tests a couple of years ago and nobody noticed. I filed bug 1481402 and put up a patch there.

Comment 6

[Jonathan Watt [:jwatt]]

Regarding the perf regression on that test, some sort of regression seems like it would be expected with tiling enabled. Tiling adds more overhead for initial rendering (the svg_static tests check the speed of initial rendering) to help improve the performance of dynamic changes and scrolling. Whether the amount of the regression is expected I can't say. Someone from the GFX team who's been working on the tiling would have to comment on that.

Comment 7

[Ionuț Goldan [:igoldan]]

(In reply to Jonathan Watt [:jwatt] from comment #6)
> Regarding the perf regression on that test, some sort of regression seems
> like it would be expected with tiling enabled. Tiling adds more overhead for
> initial rendering (the svg_static tests check the speed of initial
> rendering) to help improve the performance of dynamic changes and scrolling.
> Whether the amount of the regression is expected I can't say. Someone from
> the GFX team who's been working on the tiling would have to comment on that.

:rhunt can you comment on this?

Comment 8

[Ryan Hunt [:rhunt] (on leave until early May)]

Ideally we'd have very comparable initial rendering performance for tiling vs. buffer rotation, and that's what I saw locally when testing tsvgx. But regardless, tiling has been backed out on linux because of a high crash rate so this shouldn't be an issue anymore.

No one has time to work on re-enabling it and fixing the crashes, so I'd say we wait and see what happens if/when it get's re-enabled.

Comment 9

[Ionuț Goldan [:igoldan]]

Nothing to fix here, as causing bug got backed out.