Alison Shiue Reporter
	Description • 8 years ago

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

Regressions:

  7%  tabpaint summary linux64 opt         88.14 -> 94.69
  7%  tabpaint summary windows7-32 opt     90.45 -> 96.37
  6%  tabpaint summary windows8-64 opt     88.45 -> 93.95
  4%  tpaint windowsxp opt                 288.61 -> 299.32
  4%  tpaint windows8-64 opt               269.96 -> 279.82
  4%  tpaint windows7-32 opt               283.09 -> 293.09
  3%  tpaint osx-10-10 opt                 344.42 -> 355.18


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

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

After doing some retriggers, this issue might be caused by the following patch:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=5011ecc316111a8c6a3a6260d2b9dab941405970&tochange=d1bf9267ba7da182771c43aec042f0f5f579de93

Hi Jan, as you are the patch author, can you take a look at this and determine what is the root cause? Thanks!

	Jan de Mooij [:jandem] Assignee
	Comment 2 • 8 years ago

Thanks Alison.

I will do a Try push to turn off relazification, so we can see if that's affecting this. If that improves things, we're probably relazifying too much and end up reparsing the same functions multiple times. If relazification doesn't affect these numbers, maybe reentering the parser for each lazy function is too slow, but that's a bit surprising because there should be a ton of unused functions to compensate for that.

More later today.

	Jan de Mooij [:jandem] Assignee
	Comment 3 • 8 years ago

OK after some retriggers I have the following results for Linux x64 opt no-e10s:

Before bug 1297706 landed:

- tabpaint: 88.64, 88.64, 89.27, 87.18, 89.73, 89.27
- tpaint: 304.48, 313.28, 292.72, 284.52, 300.73, 289.56

Try, after bug 1297706 landed:

- tabpaint: 95.99, 96.47, 94.50, 97.99, 96.50, 95.50
- tpaint: 303.03, 304.89, 295.65, 297.22, 300.11, 301.65

Try, no relazification:

- tabpaint: 89.90, 86.72, 89.55, 90.19, 85.63
- tpaint: 285.49, 279.85, 289.35, 295.94, 277.49

So disabling relazification indeed fixes the regression, and might even be a small win compared to the old situation.

A potential fix here is to relazify only if we're doing a shrinking GC, and not on all GCs. I like that idea, because it will still let us relazify periodically but it could avoid a lot of unnecessary reparses. I'll Try this next and see what the results are.

	Jan de Mooij [:jandem] Assignee
	Comment 4 • 8 years ago

With the "relazify only on shrinking GCs patch" I get the following:

- tabpaint: 89.27, 88.64, 90.65, 87.18, 87.18, 89.80, 87.80
- tpaint: 303.81, 314.84, 305.61, 304.45, 305.10, 298.76, 298.77

Looks like it will fix these regressions.

	Jan de Mooij [:jandem] Assignee
	Comment 5 • 8 years ago

Here's the patch to relazify only when we're doing a shrinking GC. The numbers I posted in this bug indicate that relazifying on every major GC can cause perf issues because we have to reparse more.

	Jan de Mooij [:jandem] Assignee
	Comment 6 • 8 years ago

This one also moves the AutoPhase outside the loop.

	Pulsebot
	Comment 7 • 8 years ago

Pushed by jandemooij@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0febd243c39d
Only relazify functions on shrinking GCs to avoid reparsing too much. r=terrence

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

and a win for tabpaint:
== Change summary for alert #3179 (as of September 15 2016 03:59 UTC) ==

Improvements:

  8%  tabpaint summary windows7-32 opt     95.92 -> 88.02
  7%  tabpaint summary linux64 opt         94.16 -> 87.79

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

possibly we will see more wins as we collect more data

	Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)
	Comment 9 • 8 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/0febd243c39d

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

I am only seeing tabpaint improvements here, not tpaint.  Luckily the tabpaint not only fixes, but does some slight improvements :)

:jandem, do you think there is more to do here?

	Jan de Mooij [:jandem] Assignee
	Comment 11 • 8 years ago

Yeah, the tabpaint wins are pretty nice:

https://treeherder.mozilla.org/perf.html#/alerts?id=3189
https://treeherder.mozilla.org/perf.html#/alerts?id=3194
https://treeherder.mozilla.org/perf.html#/alerts?id=3179

The tpaint regression was smaller and the numbers seem more noisy. I don't have any ideas to improve that, but I will experiment with more changes in this area - I'll definitely keep an eye on these tests.

	Alison Shiue Reporter
	Comment 12 • 8 years ago

As a note, this regression patch has been merged into Aurora, and shows some tpaint test regression.

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