1445213 - 8.88 - 11.61% tabpaint (linux64, osx-10-10) regression on push 664c633802d4cd0e2c32a271a2d29107b5b43f38 (Mon Mar 12 2018)

Status: RESOLVED FIXED

(Firefox :: Tabbed Browser, defect)

Description

[Ionuț Goldan [:igoldan]]

Talos has detected a Firefox performance regression from push:

https://hg.mozilla.org/integration/autoland/pushloghtml?changeset=664c633802d4cd0e2c32a271a2d29107b5b43f38

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

Regressions:

 12%  tabpaint osx-10-10 opt e10s stylo     62.53 -> 69.79
  9%  tabpaint linux64 opt e10s stylo       53.62 -> 58.38


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

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]]

:mconley Bug 1423220 caused these perf regressions on the integration branches.
Can you please look over them?

Comment 2

[Mike Conley (:mconley) (:⚙️)]

Try build with logging: https://treeherder.mozilla.org/#/jobs?repo=try&revision=11f288db5283f17de5058ccdaf768bab2a9f2995

Comment 3

[Mike Conley (:mconley) (:⚙️)]

So here are some tabpaint profiles to look at:

Warming disabled: https://perfht.ml/2FKZ9wN
Warming enabled: https://perfht.ml/2DvpSeL

I think the most important thing to pay attention to is the MozLayerTreeReady event firing in the parent, since that's what the async tab switcher is using to know when layers are available to switch to. With warming disabled, we see a paint occur in the content process to paint the new tab, and then the MozLayerTreeReady event fires in the parent after the upload to the compositor completes. Meanwhile, in the content process, a MozAfterPaint event fires. Makes perfect sense.

Then look at when warming is enabled. What's interesting is that the MozLayerTreeReady event fires _waaaaay_ before the MozAfterPaint fires in the content process. The difference is about 23ms in the profile I posted. I suspect this is because, with warming enabled, we only activate the docShell after the switch has completed, and we don't fire MozAfterPaint events until the DocShell is active... but this is _after_ layers have been rendered.

That's just a hypothesis, but the evidence in the profile supports this. Basically, the tabpaint test is broken with warming enabled, but I don't think there's a real regression here - we just need to adjust the test to reflect reality.

Comment 4

[Mike Conley (:mconley) (:⚙️)]

I suspect this will go away shortly with the new patch I landed in bug 1423220. That patch activates the DocShell right away when the tab being switched to isn't in the process of being warmed (which is the case for the tabpaint test).

igoldan, can you confirm?

Comment 5

[Joel Maher ( :jmaher ) (UTC -8)]

we did a backout of bug 1423220 and this problem went away:
== Change summary for alert #12205 (as of Fri, 16 Mar 2018 14:57:17 GMT) ==

Improvements:

 13%  tabpaint osx-10-10 opt e10s stylo     70.38 -> 61.54
  7%  tabpaint linux64 opt e10s stylo       58.24 -> 54.11

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

Comment 6

[Mike Conley (:mconley) (:⚙️)]

(In reply to Joel Maher ( :jmaher) (UTC-5) from comment #5)

Even better - after the backout, I relanded a variation of the patch that both re-enables tab warming, and doesn't cause the talos regression to recur. Victory all around!

Comment 7

[Mike Conley (:mconley) (:⚙️)]

(So I'm going to close this now)