1546421 - 9.4 - 9.74% tsvg_static (linux64-shippable, linux64-shippable-qr) regression on push 4b7f613b9372e582bb175a0ed3b8843921ef2817

Status: RESOLVED INCOMPLETE

(Core :: Layout, defect, P3)

Description

[Ionuț Goldan [:igoldan]]

Talos has detected a Firefox performance regression from push:

https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=49c94645be164f0004412b60db6f8d464b39d317&tochange=4b7f613b9372e582bb175a0ed3b8843921ef2817

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

Regressions:

10% tsvg_static linux64-shippable opt e10s stylo 49.98 -> 54.85
9% tsvg_static linux64-shippable-qr opt e10s stylo 54.92 -> 60.08

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

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/Performance_sheriffing/Talos/Tests

For information on reproducing and debugging the regression, either on try or locally, see: https://wiki.mozilla.org/Performance_sheriffing/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/Performance_sheriffing/Talos/RegressionBugsHandling

Comment 1

[Kenny Levinsen :kennylevinsen]

The changeset referenced fixes a regression in bug 1521786. That enhancement intends to make it so that, instead of unconditionally stopping all timers when they are no longer needed, certain timers are granted a 4-second grace period in case they become needed again, in order to decrease latency at the cost of a bit of wasted CPU cycles.

However, instead of simply adding a delayed stop, a third branch was added: A completed/contentful-painted top-level document in a content process would never have its timer stopped. It does not seem like this was intentional.

While not stopping timers can improve latency by a timer tick, here the tradeoff is constant CPU utilization. For ultra-books in low-power modes like my own, the utilization is very significant (easily ~5% cpu before, ~0.7% after this fix in idle). I think it is sensible to leave the patch in, and separately improve the latency.

I have opened bug 1546490 to track the latency improvement.

Comment 2

[Emilio Cobos Álvarez [:emilio]]

Yeah, specially if the regression is linux64-only it seems we could just take this. Olli, opinions?

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

If this is really linux64/tsvg_static only, then that sounds ok.

Comment 4

[Ionuț Goldan [:igoldan]]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #2)

Yeah, specially if the regression is linux64-only it seems we could just take this. Olli, opinions?

It is Linux specific.

Comment 5

[Emilio Cobos Álvarez [:emilio]]

Bug 1546490 could potentially bring this back, but for now then we probably don't need any immediate action. Ionut, what's the best course of action in terms of changing the bug status? This is not necessarily a "WONTFIX" per se (since further work may actually fix this).

Comment 6

[Ionuț Goldan [:igoldan]]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #5)

Bug 1546490 could potentially bring this back, but for now then we probably don't need any immediate action. Ionut, what's the best course of action in terms of changing the bug status? This is not necessarily a "WONTFIX" per se (since further work may actually fix this).

Just set an appropriate priority. We'll leave the status as it is.
I'm adding the backlog-deffered keyword, to know we're waiting for a future fix.

Comment 8

[Greg Mierzwinski [:sparky]]

Hi :emilio, do you think this performance alert bug is still valid? I see that bug 1546490 was resolved.

Comment 9

[Emilio Cobos Álvarez [:emilio]]

It's unclear if the extra work actually fixed it or not... I guess at this point tracking this is kinda moot.