Alison Shiue Reporter
	Description • 8 years ago

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

Regressions:

  6%  damp summary windows7-32 pgo          244.15 -> 258.62
  4%  damp summary windows7-32 opt e10s     289.69 -> 300.06
  3%  damp summary windows8-64 opt e10s     265.34 -> 274.14
  3%  damp summary osx-10-10 opt            313.32 -> 323.59
  3%  damp summary linux64 opt              317.75 -> 328.07
  3%  damp summary osx-10-10 opt e10s       293.89 -> 302.28
  3%  damp summary windows8-64 opt          317.96 -> 326.98
  3%  damp summary windows7-32 opt          332.1 -> 341.31
  2%  damp summary linux64 pgo              261.97 -> 268.48
  2%  damp summary linux64 opt e10s         289.71 -> 295.91


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

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

	Alison Shiue Reporter
	Comment 1 • 8 years ago

Hi Alexandre, as you are the patch author, can you take a look at this and determine what is the root cause?
Are these damp regressions expected? Thanks!

	Alexandre Poirot [:ochameau]
	Comment 2 • 8 years ago

Looking at subtests:
https://treeherder.mozilla.org/perf.html#/comparesubtest?originalProject=autoland&originalRevision=d9413eb3db64787916c60efcb7b766cc24420d94&newProject=autoland&newRevision=2268d466af6cbf62fc48006d3ff00d087c339e8b&originalSignature=8df20bbe30a5c9f6cf87fc14608b83b41c03551e&newSignature=8df20bbe30a5c9f6cf87fc14608b83b41c03551e&showOnlyImportant=1&framework=1

The regression happens in close.

I think it relates to this call:
https://hg.mozilla.org/integration/autoland/rev/24775cdd79fb#l15.522
+++ b/devtools/client/framework/toolbox.js
+            win.location.replace("about:blank");

We weren't doing that before bug 1266134, or anything similar.
But I think it is worth the slowdown. It ensure cleaning up the toolbox document and hopefully free some more memory.

:jryans, what do you think? Hopefully the try push is going to confirm that. I removed that call to location.replace.

	Alexandre Poirot [:ochameau]
	Comment 3 • 8 years ago

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

	Alexandre Poirot [:ochameau]
	Comment 4 • 8 years ago

The try push seems to confirm it is just that location.replace:
https://treeherder.mozilla.org/perf.html#/comparesubtest?originalProject=mozilla-central&originalRevision=21153294d3a0&newProject=try&newRevision=41bdfde95082&originalSignature=b97a8a540045b6139d2ce67b9c8b9a2f5c2918de&newSignature=b97a8a540045b6139d2ce67b9c8b9a2f5c2918de&showOnlyImportant=1&framework=1

	J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)
	Comment 5 • 8 years ago

I think we should accept the regression and take this as the new baseline.  Like you said, this should be freeing up memory and doing additional cleanup that needs to happen anyway, but now it just happens that we're triggering it earlier, so it's more likely to appear during the Talos subtest than before (in the past we probably relied on GC / CC to do the cleanup which can run whenever).

Additionally, I would say that close pref is less critical for users than the others, so that's another reason it seems acceptable.

	Joel Maher ( :jmaher ) (UTC -8)
	Comment 6 • 8 years ago

thanks :jryans!