1277549 - 2.37 - 2.61% tp5o Main_RSS / tp5o_scroll (osx-10-10, windowsxp) regression on push 952eae508967 (Tue May 31 2016)

Status: RESOLVED FIXED

(Core :: Layout: Tables, defect)

Description

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

Talos has detected a Firefox performance regression from push 952eae508967. As author of one of the patches included in that push, we need your help to address this regression.

This is a list of all known regressions and improvements related to the push:
https://treeherder.mozilla.org/perf.html#/alerts?id=1412

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#tp5o_scroll
https://wiki.mozilla.org/Buildbot/Talos/Tests#tp5

Reproducing and debugging the regression:
If you would like to re-run this Talos test on a potential fix, use try with the following syntax:

try: -b o -p win32,macosx64 -u none -t g1[Windows XP,10.10],tp5o-e10s[Windows XP,10.10] --rebuild 5  # add "mozharness: --spsProfile" to generate profile data

(we suggest --rebuild 5 to be more confident in the results)

To run the test locally and do a more in-depth investigation, first set up a local Talos environment:
https://wiki.mozilla.lorg/Buildbot/Talos/Running#Running_locally_-_Source_Code

Then run the following command from the directory where you set up Talos:
talos --develop -e [path]/firefox -a tp5o_scroll:tp5o

(add --e10s to run tests in e10s mode)

Making a decision:
As the patch author we need your feedback to help us handle this regression.
*** 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

[Markus Stange [:mstange]]

So this is a memory usage regression?

Comment 2

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

I did some retriggers:
https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&group_state=expanded&filter-searchStr=xp%20g1&tochange=952eae508967ae46d36909d179159f2a2205169c&fromchange=9915ff6e7bf778341f46b5bc095ec46deb673416&selectedJob=29165265

and the compare view between revision 952eae508967 and 9915ff6e7bf7 yields 2 clean regressions:
https://treeherder.mozilla.org/perf.html#/compare?originalProject=mozilla-inbound&originalRevision=9915ff6e7bf7&newProject=mozilla-inbound&newRevision=952eae508967&framework=1

While we don't have all the data, I think most of the data is in and this is what we have:
* 2.26 osx10 tp5o main_rss regression
* 2.69 winXP tp5o_scroll regression (note: tp5o_scroll xinhuanet.com opt is the main regression here: https://treeherder.mozilla.org/perf.html#/graphs?timerange=172800&highlightedRevisions=9915ff6e7bf7&highlightedRevisions=952eae508967_

:acomminos, can you take a look at this and help figure out the root cause and what we can do to resolve this bug (accept, backout, fix, etc.)

Comment 3

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

:mstange, this has 2 regressions, 1 is memory, the other is scroll related.  I am not sure why this doesn't affect memory on other platforms or scroll on other platforms.

Comment 4

[Markus Stange [:mstange]]

Scrolling the tp5n copy of xinhuanet.com in today's nightly with the scrollbar (so no APZ) shows lots of invalidations that weren't there before bug 1276734, so this caught a real problem. Nice!

Comment 5

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

cool, thanks for confirming that :mstange!

Comment 6

[Andrew Comminos [:acomminos]]

Able to reproduce these additional invalidations for table cell items' backgrounds on linux64. I believe the switch to using nsDisplayBackgroundImage in 259734abfe3a caused this, as we seem to be invalidating more now on cells with a background image. Inspection of nsDisplayBackgroundImage reveals that it computes an invalidation region more aggressively than nsDisplayItem's implementation did.

I'll continue looking into what's going on.

Comment 7

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

cool, thanks for digging into this.  Let me know if you need help validating numbers from talos, etc.  From the sounds of it just fixing it and running the page locally should tell you if it is fixed.

Comment 8

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

when bug 1153694 landed it looks as though all the tp5o_scroll regressions went away!  Thanks for making the fix needed for those tests.

I don't see any change in the osx tp5o main_rss, possibly we have to live with that.

Comment 9

[Markus Stange [:mstange]]

Yes, a small memory usage increase is expected because we want to be creating a separate layer for the fixed backgrounds.
So I think we're done here! Part FIXED, part WONTFIX.