1438775 - stylo: 2.43 - 12.65% sessionrestore / sessionrestore_many_windows / sessionrestore_no_auto_restore / tart / tpaint / ts_paint / ts_paint_heavy / tsvg_static (linux64) regression on push 4e0d4c91940bb09ead646ef82c8666075175e69a (Thu Feb 15 2018)

Status: RESOLVED WONTFIX

(Core :: CSS Parsing and Computation, defect, P2)

Description

[Ionuț Goldan [:igoldan]]

Talos has detected a Firefox performance regression from push:

https://hg.mozilla.org/integration/autoland/pushloghtml?changeset=4e0d4c91940bb09ead646ef82c8666075175e69a

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

Regressions:

 13%  tpaint linux64 pgo e10s stylo     125.17 -> 141.00
 11%  tsvg_static linux64 pgo e10s stylo44.70 -> 49.44
  8%  sessionrestore_many_windows linux64 pgo e10s stylo935.79 -> 1,011.08
  6%  sessionrestore linux64 pgo e10s stylo269.96 -> 285.67
  5%  sessionrestore_no_auto_restore linux64 pgo e10s stylo314.67 -> 331.08
  3%  ts_paint linux64 pgo e10s stylo   437.83 -> 453.00
  3%  tart linux64 pgo e10s stylo       2.02 -> 2.08
  3%  ts_paint_heavy linux64 pgo e10s stylo442.79 -> 455.92

Improvements:

  3%  tsvgr_opacity linux64 pgo e10s stylo     183.34 -> 177.25


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

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

[Ionuț Goldan [:igoldan]]

:xidorn As you noticed, there are a lot of big regressions which got in after enabling Stylo on chrome also. I leave here to continue the discussions around this issue. Let us know if you need any help from our side.

Comment 2

[Ionuț Goldan [:igoldan]]

Oh, pardon me. I just saw the backout of bug 1417138 [1]. I'll leave this bug open for the regressions that will reappear.

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1417138#c21

Comment 3

[Xidorn Quan [:xidorn] UTC+11]

So there are still more optimizations coming. We want to ship stylo-chrome in 60 to pave the way for removing the old style system in short term, and we don't want to maintain two style systems for another year in 60esr. So we are going to take the regressions as far as they wouldn't be too scary.

The most significant regression on tpaint is mostly because stylo-chrome is several ms slower on some places which makes us more likely to miss one more vsync, so the difference is basically 16ms. I would expect this to be fixed by bug 1435940, which emilio has been working on. sessionrestore ones are probably similar.

The tsvg_static regression is weird. It is a test on content rather than chrome, so theoretically it shouldn't be affected in a negative way by stylo-chrome at all. I have no idea what's going on there. But anyway, according to the subtests, this regression doesn't seem to scale with the size of image, so probably some fixed regression introduced by chrome or harness or initialization code, which is perhaps less concerned.

Comment 4

[Xidorn Quan [:xidorn] UTC+11]

And we want to enable stylo-chrome now so that we have an extra week for people to test it before 60 branches to beta.

Comment 5

[Bobby Holley (:bholley)]

And to be clear - the final decision to ship in 60 hasn't been made, but we're optimistic that emilio's in-flight perf work should make a big dent in those regressions, and so we want to turn on stylo-chrome now to surface any other bugs that might arise.

Comment 6

[Ionuț Goldan [:igoldan]]

The backout returned the baselines to previous values:

== Change summary for alert #11612 (as of Fri, 16 Feb 2018 00:48:35 GMT) ==

Regressions:

  4%  tsvgr_opacity linux64 pgo e10s stylo     176.75 -> 183.43

Improvements:

 11%  tpaint linux64 pgo e10s stylo     140.75 -> 125.67
  9%  tsvg_static linux64 pgo e10s stylo49.55 -> 44.90
  9%  sessionrestore_many_windows linux64 opt e10s stylo1,072.33 -> 977.00
  9%  tsvg_static linux64 opt e10s stylo50.10 -> 45.69
  8%  sessionrestore_many_windows linux64 pgo e10s stylo1,012.31 -> 935.75
  6%  sessionrestore linux64 pgo e10s stylo285.88 -> 269.67
  5%  sessionrestore linux64 opt e10s stylo301.67 -> 285.33
  5%  sessionrestore_no_auto_restore linux64 pgo e10s stylo330.12 -> 313.50
  5%  sessionrestore_no_auto_restore linux64 opt e10s stylo351.67 -> 334.58
  4%  ts_paint_heavy linux64 pgo e10s stylo456.12 -> 438.42
  3%  ts_paint_heavy linux64 opt e10s stylo482.67 -> 466.25
  3%  tart linux64 pgo e10s stylo       2.07 -> 2.02

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=11612

Comment 7

[Ionuț Goldan [:igoldan]]

We're already getting the 2nd round of expected regressions, from push [1].

== Change summary for alert #11647 (as of Tue, 20 Feb 2018 02:37:21 GMT) ==

Regressions:

 11%  tsvg_static linux64 opt e10s stylo     45.59 -> 50.62
  4%  tart linux64 opt e10s stylo            2.23 -> 2.31
  3%  ts_paint_heavy linux64 opt e10s stylo  465.83 -> 482.00

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=11647

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1417138#c43

Comment 8

[Jet Villegas (inactive)]

[ Triage 2017/02/20: P2 ] Marking this P2 as stylo-chrome is a P2 feature.

Comment 9

[Ionuț Goldan [:igoldan]]

This is a more complete record of the regressions that showed up:

== Change summary for alert #11647 (as of Tue, 20 Feb 2018 02:37:21 GMT) ==

Regressions:

 13%  tsvg_static linux64 pgo e10s stylo     44.56 -> 50.16
 12%  tpaint linux64 pgo e10s stylo          126.17 -> 141.67
 11%  tsvg_static linux64 opt e10s stylo     45.59 -> 50.62
 11%  tpaint linux64 opt e10s stylo          139.42 -> 154.17
  9%  sessionrestore_many_windows windows10-64 pgo e10s stylo1,253.92 -> 1,371.25
  9%  sessionrestore_many_windows linux64 pgo e10s stylo926.50 -> 1,010.50
  9%  sessionrestore_no_auto_restore osx-10-10 opt e10s stylo647.00 -> 703.83
  8%  sessionrestore_many_windows linux64 opt e10s stylo975.58 -> 1,053.17
  8%  tpaint windows7-32 pgo e10s stylo      198.67 -> 214.00
  8%  sessionrestore windows10-64 pgo e10s stylo366.00 -> 393.67
  7%  sessionrestore osx-10-10 opt e10s stylo567.29 -> 608.83
  7%  sessionrestore windows7-32 pgo e10s stylo391.08 -> 418.83
  7%  sessionrestore_many_windows windows7-32 pgo e10s stylo1,260.54 -> 1,349.33
  7%  sessionrestore linux64 pgo e10s stylo  265.00 -> 283.67
  7%  tabpaint windows10-64 pgo e10s stylo   62.34 -> 66.70
  7%  tpaint windows10-64 pgo e10s stylo     211.25 -> 225.50
  7%  ts_paint windows10-64 pgo e10s stylo   475.50 -> 506.75
  7%  ts_paint_webext windows10-64 pgo e10s stylo532.33 -> 567.08
  6%  sessionrestore_no_auto_restore windows10-64 pgo e10s stylo436.46 -> 462.83
  6%  sessionrestore_no_auto_restore windows7-32 pgo e10s stylo459.04 -> 485.83
  5%  sessionrestore linux64 opt e10s stylo  283.38 -> 298.33
  5%  sessionrestore_no_auto_restore windows7-32 opt e10s stylo570.00 -> 599.67
  5%  ts_paint_webext windows7-32 pgo e10s stylo623.42 -> 655.42
  5%  ts_paint osx-10-10 opt e10s stylo      753.17 -> 788.92
  5%  ts_paint_webext osx-10-10 opt e10s stylo826.83 -> 865.17
  5%  ts_paint_webext windows10-64 opt e10s stylo632.42 -> 661.50
  5%  sessionrestore_no_auto_restore linux64 pgo e10s stylo309.75 -> 324.00
  5%  sessionrestore windows10-64 opt e10s stylo439.67 -> 459.67
  4%  ts_paint_heavy linux64 pgo e10s stylo  435.67 -> 454.33
  4%  sessionrestore_no_auto_restore linux64 opt e10s stylo333.00 -> 347.08
  4%  sessionrestore_many_windows windows10-64 opt e10s stylo1,553.42 -> 1,617.25
  4%  ts_paint linux64 pgo e10s stylo        432.33 -> 449.83
  4%  ts_paint windows10-64 opt e10s stylo   562.75 -> 585.33
  4%  sessionrestore_no_auto_restore windows10-64 opt e10s stylo519.33 -> 538.17
  4%  tart linux64 opt e10s stylo            2.23 -> 2.31
  3%  ts_paint_heavy linux64 opt e10s stylo  465.83 -> 482.00
  3%  ts_paint linux64 opt e10s stylo        456.92 -> 471.67
  3%  tart linux64 pgo e10s stylo            2.03 -> 2.09

Improvements:

  5%  damp windows7-32 pgo e10s stylo     69.05 -> 65.49
  2%  tsvgr_opacity linux64 pgo e10s stylo183.88 -> 180.78

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=11647

Comment 10

[Ionuț Goldan [:igoldan]]

Be aware that we also have AWSY regressions. The Heap Unclassified regressions seems to have returned to almost previous values, but Images ones are still there.

== Change summary for alert #11656 (as of Tue, 20 Feb 2018 03:50:54 GMT) ==

Regressions:

 16%  Images windows10-64 pgo stylo     6,694,549.40 -> 7,785,785.59
 15%  Images windows7-32 pgo stylo      4,965,264.98 -> 5,727,731.98
 15%  Images windows7-32 opt stylo      4,925,956.74 -> 5,674,843.21
 10%  Images osx-10-10 opt stylo        5,975,981.83 -> 6,589,516.55
  9%  Images linux64-stylo-sequential opt stylo-sequential5,634,647.70 -> 6,149,759.95
  9%  Images linux64 opt stylo          5,637,098.81 -> 6,127,811.37
  2%  Heap Unclassified windows10-64 pgo stylo41,318,245.41 -> 42,319,751.39

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=11656

Comment 11

[Xidorn Quan [:xidorn] UTC+11]

For the Images regression, as I explained in bug 1421374 comment 6, it is a fixed 100KB~200KB regression due to larger style data for SVG images used in UI. It looks scary because when we don't open any page, the Images section was very small, so it has a significant percentage in those subtests. We can probably optimize further for this but it may not that worthwhile.

The heap unclassified one is a fixed 800KB on unreported. I have seen some issue from the DMD report, and investigating why we are not reporting some of the values.

Comment 12

[Ionuț Goldan [:igoldan]]

From this comment [1], it looks like the Heap Unclassified regression is fixed.

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1421374#c12

Comment 13

[Bobby Holley (:bholley)]

So to recap, the proposed plan is to let stylo-chrome ride to beta, and land bug 1439875 immediately after the merge to fix the perf regressions discussed in this bug. If all goes well, we'll uplift it to beta.

When discussing this plan, Yan asked for certainty that bug 1439875 will fix the regressions. So I spent some time this morning digging into the Talos numbers from bug 1439875 comment 44, and trying to extract signal from the noise.

I focused only on windows to make the analysis easier, since that is by far the most important Desktop perf target. I primarily judged PGO results. However, there is bimodal behavior with the win7 PGO tests where we occasionally get extremely slow runs. This means that the averages reported by the perfherder comparison tool are not useful. So for win7 PGO, I judged the result based on the other three configurations (win7-opt, win64-pgo, and win64-opt). There is occasional bimodal behavior in other tests, which I compensated for when it came up.

TL;DR - There are two tests (sessionrestore_many_windows, and tabpaint) where the regressions don't seem (fully) resolved. The next step there is to confirm that the regressions are indeed associated with stylo-chrome, which I'll go ahead and do now with some try pushes.

Full results below:

(In reply to Ionuț Goldan [:igoldan], Performance Sheriffing from comment #9)
>   7%  tpaint windows10-64 pgo e10s stylo     211.25 -> 225.50
>   8%  tpaint windows7-32 pgo e10s stylo      198.67 -> 214.00

Fixed.

>   4%  sessionrestore_many_windows windows10-64 opt e10s stylo1,553.42 ->
> 1,617.25
>   9%  sessionrestore_many_windows windows10-64 pgo e10s stylo1,253.92 ->
> 1,371.25
>   7%  sessionrestore_many_windows windows7-32 pgo e10s stylo1,260.54 ->
> 1,349.33

Improved but not fixed.

>   4%  sessionrestore_no_auto_restore windows10-64 opt e10s stylo519.33 ->
> 538.17
>   5%  sessionrestore_no_auto_restore windows7-32 opt e10s stylo570.00 ->
> 599.67
>   6%  sessionrestore_no_auto_restore windows10-64 pgo e10s stylo436.46 ->
> 462.83
>   6%  sessionrestore_no_auto_restore windows7-32 pgo e10s stylo459.04 ->
> 485.83

Fixed.

>   5%  sessionrestore windows10-64 opt e10s stylo439.67 -> 459.67
>   8%  sessionrestore windows10-64 pgo e10s stylo366.00 -> 393.67
>   7%  sessionrestore windows7-32 pgo e10s stylo391.08 -> 418.83

A bit noisy, but fixed AFAICT.

>   7%  tabpaint windows10-64 pgo e10s stylo   62.34 -> 66.70

Unfixed.


>   4%  ts_paint windows10-64 opt e10s stylo   562.75 -> 585.33
>   7%  ts_paint windows10-64 pgo e10s stylo   475.50 -> 506.75

Fixed.

>   5%  ts_paint_webext windows10-64 opt e10s stylo632.42 -> 661.50
>   5%  ts_paint_webext windows7-32 pgo e10s stylo623.42 -> 655.42
>   7%  ts_paint_webext windows10-64 pgo e10s stylo532.33 -> 567.08

Fixed.

Comment 14

[Bobby Holley (:bholley)]

Here are two try pushes based on today's mozilla-central:
base: https://treeherder.mozilla.org/#/jobs?repo=try&revision=7a3439fbd50f89f9904788c3048af74f652eeb8c
new: https://treeherder.mozilla.org/#/jobs?repo=try&revision=fe586a83ee70aa6152c2c0be5493fe3a98d8ae57

The "base" revision flips off stylo-chrome, and corresponds to what we would get if we decided to punt the feature to 61. The "new" revision is just mozilla-central (which already has stylo-chrome enabled) plus the patches from bug 1439875.

Here is a comparison:

https://treeherder.mozilla.org/perf.html#/compare?originalProject=try&originalRevision=7a3439fbd50f89f9904788c3048af74f652eeb8c&newProject=try&newRevision=fe586a83ee70aa6152c2c0be5493fe3a98d8ae57&framework=1

(Note that the averages in the comparison view are sometimes unreliable due to bimodal outliers - click through to the graphs to get a better picture).

From this we see that there is no regression in either sessionrestore_many_windows or tabpaint. In fact, there is a significant _improvement_ in both tests. Indeed, we see improvements in tpaint, tabpaint, and all the sessionrestore tests (all highlighted in green).

So we're in good shape here to execute the plan outlined in comment 13.

Comment 15

[Xidorn Quan [:xidorn] UTC+11]

Attachment: talos win7 tpaint change over last 30 days

So I'm wondering are we still slower than what we have already shipped?

The attachment is the graph of tpaint win32 pgo on autoland. The regression of this has been marked on the graph.

As can be seen, the performance has been approaching back to the level it originally has. We landed several optimization in the past days to further knock down the time usage. Every single of them may not be a big performance gain, they could be only some uncertain 1% or 2% change, but they accumulate to fill the gap.

It seems to me from the graph, we are almost approaching the original performance we had.

And actually, the question is, which one is the thing we really care about:
1. performance regression from gecko to stylo
2. performance regression from what we have shipped to what we are shipping

They are different.

For shipping stylo-chrome, we have done some optimization to the frontend, those would benefit Gecko as well. Making Stylo completely on par with Gecko in performance with the same chrome code can be harder due to some design restriction. And given the benefit of shipping stylo everywhere and removing the old style system, I don't think having them fully on par should be a blocker.

So I think what we really want is the second, that we don't regress much from what we have already shipped.

We should probably measure 60beta against 59beta to see if we really regress them, and if we are in an acceptable range, we should probably just ship stylo-chrome as-is, rather than either having to rush in risky patches, or slipping it to 61, maintaining the old style system for another year, and delaying removing the old style system for another version.

Comment 16

[Bobby Holley (:bholley)]

It is true that we landed various low-risk performance improvements last week that may bring us to parity without bug 1439875. That said, I think we should still target an uplift of bug 1439875, assuming it appears stable, since it's a significant performance win which we can bring to the 60 release.

If things go rockier-than-expected with bug 1439875, we should reevaluate, especially since our baseline performance without it is now better than we originally predicted.

Comment 17

[Bobby Holley (:bholley)]

I'm about to head out for two weeks, so I wanted to write up where things stand on stylo-chrome.

Recommendation: Take no action, let the trains ride as they are.

Summary:
* The various performance tweaks we landed at the end of the 60 cycle put us in a good place. Beta60 shows 4-5% wins on tpaint and sessionrestore relative to disabling stylo-chrome.
* Bug 1439875 is no longer required to avoid significant performance regressions, and is now an independent performance improvement, which further improves tpaint and sessionrestore by several percent. We could still uplift bug 1439875 to beta if we wanted that improvement in 60, but there is no pressing need to - so given that the landing was rocky, I recommend letting it ride the trains.
* There is a ~3ms tabpaint regression relative to disabling stylo-chrome on Beta60, but we're flat relative to Beta59. Full analysis below.
* There is a ~6ms ts_paint regression relative to disabling stylo-chrome on Beta60, but we've improved significantly relative to Beta59. Full analysis below.

Regression Analysis:
* The tabpaint regression: ~3ms slower opening a new tab, relative to a baseline of ~59.5ms with stylo-chrome disabled. Using Beta59 as the baseline instead, there is no regression [1] [2]. Bug 1439875 has no effect here. The issue here results from an impedance mismatch between stylo and XBL (Bug 1445566 has the details). Mitigating it would be hard, and the situation will improve naturally given the deXBLification work in 2018. Given that 3ms is far below the perceptibility threshold, I don't recommend additional resource spend here.
* The ts_paint regression: ~6ms slower to paint startup test page, relative to a baseline of ~459ms with stylo-chrome disabled. Using Beta59 as the baseline instead, we see a a 12ms overall improvement [3] [4]. Bug 1439875 also gives us another 12ms improvement here, but I think we can afford to let that extra win ride the trains.

Try pushes:
* Beta60 (stylo-chrome enabled): https://treeherder.mozilla.org/#/jobs?repo=try&revision=44b6785f28c88b4fe5a7b2ae95bdeb0672609542
* Beta60 + Disable stylo-chrome: https://treeherder.mozilla.org/#/jobs?repo=try&revision=e33a57adfdb660b8e6c7ee389223124d351b6af0
* Beta60 + bug 1439875:          https://treeherder.mozilla.org/#/jobs?repo=try&revision=285df7e2f20bc9e3185d40cef9868280a64c9e01
* Beta59:                        https://treeherder.mozilla.org/#/jobs?repo=try&revision=a19d089c8f855889b39d9c5e903dbbea6c5f1b7a

Footnotes:
[1] tabpaint Beta59: https://treeherder.mozilla.org/perf.html#/graphs?timerange=31536000&series=try,1525895,1,1&series=try,1525880,1,1&highlightedRevisions=a19d089c8f85&highlightedRevisions=44b6785f28c8&zoom=1521211606844.3914,1521218790000,55.150391785630404,71.58534984314757
[2] tabpaint Beta60: https://treeherder.mozilla.org/perf.html#/graphs?timerange=31536000&series=try,1540074,1,1&series=try,1539871,1,1&highlightedRevisions=44b6785f28c8&zoom=1521158608204.187,1521158773842.102,59.29569712076395,71.32081217310431
[3] ts_paint Beta59: https://treeherder.mozilla.org/perf.html#/graphs?timerange=31536000&series=try,1525890,1,1&series=try,1525874,1,1&highlightedRevisions=a19d089c8f85&highlightedRevisions=44b6785f28c8&zoom=1521025730965.5637,1521218790000,450,613.8951219422987
[4] ts_paint Beta60: https://treeherder.mozilla.org/perf.html#/graphs?timerange=86400&series=try,1539866,1,1&series=try,1540067,1,1&highlightedRevisions=44b6785f28c8&zoom=1521158641964.0837,1521158730818.8442,448.81537276279255,568.138129429319

Comment 18

[Peter Dolanjski [:pdol]]

Based on Bobby's analysis (3ms impact on tabpaint and 12ms improvement on ts_paint), from a product perspective, I (representing the product team) am comfortable with the recommendation to let stylo-chrome ride the trains.

Comment 19

[Chris Peterson [:cpeterson]]

(In reply to Peter Dolanjski [:pdol] from comment #18)
> Based on Bobby's analysis (3ms impact on tabpaint and 12ms improvement on
> ts_paint), from a product perspective, I (representing the product team) am
> comfortable with the recommendation to let stylo-chrome ride the trains.

firefox60=wontfix based on Peter's comment 18.

If I am reading Bobby's comment 17 correctly, tabpaint and ts_paint regressed in Beta 60 with Stylo-chrome compared to Beta 60 without Stylo-chrome, but are flat (tabpaint) or faster (ts_paint) compared to Beta 59 (without Stylo-chrome).

I'll leave this bug open with firefox61=affected to track restoring tabpaint and ts_paint performance to the Beta 60 baseline (without Stylo-chrome) in Nightly 61, if possible.

Comment 20

[Chris Peterson [:cpeterson]]

After talking with the Stylo engineers, I'm closing this bug as WONTFIX. As Bobby noted in comment 17, there is no Stylo-chrome regression compared to what Release channel users have now with Firefox 59. We expect the fix for XUL sizing bug 1439875 in 61 to restore or exceed 60's performance before Stylo-chrome was enabled, but we won't be specifically profiling these particular tabpaint and ts_paint test cases.