1214629 - 2.69% Linux 64 sessionrestore regression on Fx-Team on October 14, 2015 from push 2a20ae6dda0e56cf0a5f9441c54e76b32babc2fa

Status: RESOLVED FIXED

(DevTools :: JSON Viewer, defect)

Description

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

Talos has detected a Firefox performance regression from commit 2a20ae6dda0e56cf0a5f9441c54e76b32babc2fa in bug 1132203.  We need someone to address this regression.

This is a list of all known regressions and improvements related to your bug:
http://alertmanager.allizom.org:8080/alerts.html?rev=2a20ae6dda0e56cf0a5f9441c54e76b32babc2fa&showAll=1

On the page above you can see Talos 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, please see: https://wiki.mozilla.org/Buildbot/Talos/Tests#sessionrestore.2Fsessionrestore_no_auto_restore

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 linux64,win64 -u none -t other  # add "mozharness: --spsProfile" to generate profile data

To run the test locally and do a more in-depth investigation, first set up a local Talos environment:
https://wiki.mozilla.org/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 sessionrestore

Making a decision:
As the patch author we need your feedback to help us handle this regression.
*** Please let us know your plans by Monday, or the offending patch will be backed out! ***

Our wiki page oulines the common responses and expectations:
https://wiki.mozilla.org/Buildbot/Talos/RegressionBugsHandling

Comment 1

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

I have done a lot of retriggers on treeherder, and am collecting more data to see if other tests are affected.  Looking at compare view we see a lot of noise:
https://treeherder.allizom.org/perf.html#/compare?originalProject=fx-team&originalRevision=c7b79ffe40b2&newProject=fx-team&newRevision=2a20ae6dda0e

the graph for sessionrestore does show what we expect, a clear regression:
https://treeherder.mozilla.org/perf.html#/graphs?series=[fx-team,d302a19fe1f655de7f9db1928a97bda4b3274568,1]

In addition, I have done a series of try pushes to bisect and narrow down the regression:
https://treeherder.mozilla.org/#/jobs?repo=try&author=[e-mail]&fromchange=5d039c369396&tochange=a19a1a778025

Ideally we will be able to determine which changeset is the root cause.

:honza, I see you are the author for the majority of the patches in this bug, can you look into this and help us determine why this is happening and once we understand that we can then determine if this is a regression we need to accept or fix in partial or full.

Comment 2

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Attachment: bug1214629-1.patch

The attached patch lazy loads couple of modules, let's see if it helps.

Here is try push with Talos test
https://treeherder.mozilla.org/#/jobs?repo=try&revision=2c3c6b2a5508
https://treeherder.mozilla.org/perf.html#/comparechooser?newProject=try&newRevision=2c3c6b2a5508

Honza

Comment 3

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

this is the revision where the regression is introduced:
http://hg.mozilla.org/integration/fx-team/rev/1c2a1cba58b5

it is the first patch in the series.  Looking forward to the try results in an hour to see if it fixes it.

Assuming there is no regression, we should see the no change in comparing the previous push on fx-team to the try push:
https://treeherder.mozilla.org/perf.html#/comparesubtest?originalProject=fx-team&originalRevision=c7b79ffe40b2&newProject=try&newRevision=2c3c6b2a5508&originalSignature=32206df28ab3b52e15d26f73178ee78e2af7e760&newSignature=32206df28ab3b52e15d26f73178ee78e2af7e760

Comment 4

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

and the try push fixes it!

Comment 5

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8673669 [details] [diff] [review]
bug1214629-1.patch

Review of attachment 8673669 [details] [diff] [review]:
-----------------------------------------------------------------

I hope it will be this simple to fix! :)

Comment 6

[Jan Honza Odvarko [:Honza] (always need-info? me)]

(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #5)
> Comment on attachment 8673669 [details] [diff] [review]
> bug1214629-1.patch
> 
> Review of attachment 8673669 [details] [diff] [review]:
> -----------------------------------------------------------------
Thanks for the review!

> 
> I hope it will be this simple to fix! :)
It should be (see comment #4)

Try push:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=a73d2452aa3c

Honza

Comment 7

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/df4265195357

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/df4265195357

Comment 9

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

verified in the graphs