Ionuț Goldan [:igoldan] Reporter
	Description • 6 years ago

Talos has detected a Firefox performance regression from push:

https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?changeset=18937f7c5c2fe8d38b7e479c18f81816368df503

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

Regressions:

  9%  damp osx-10-10 opt e10s stylo     109.74 -> 119.32
  6%  damp windows7-32 pgo e10s stylo   69.26 -> 73.73
  6%  damp windows10-64 opt e10s stylo  71.78 -> 76.37
  6%  damp linux64 opt e10s stylo       63.40 -> 67.18
  6%  damp windows10-64 pgo e10s stylo  67.45 -> 71.45
  6%  damp linux64 pgo e10s stylo       60.44 -> 63.83


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

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

	Ionuț Goldan [:igoldan] Reporter
	Comment 1 • 6 years ago

:gl Looks like bug 1446944 caused big DAMP perf regressions. Is there something we can do to fix or at least lessen this. Or should we rather accept the tooltip penalties?

	Julian Descottes [:jdescottes]
	Comment 2 • 6 years ago

This change made all the settle tests increase significantly, because we trigger the display of this onboarding tooltip on clean profiles. Showing a tooltip is async and we don't wait on the completion of the show to consider the inspector as initialized (waiting on it would not change the actual regression, but would move the load to the "show" tests rather than the settle tests).

Given that this tooltip should not be displayed in most of the runs I think we could simply disable it for our DAMP tests.
Alex: what is your opinion about this?

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

Ahah 20,528.36% regression, nice Gabriel :)

Yes, preffing this off sounds good to me.

We already do this for the new console frontend over here:
  https://searchfox.org/mozilla-central/source/testing/talos/talos/tests/devtools/addon/content/damp.js#83
We may want to make this more generic/discoverable.


Note the existence of this pref file used by all mochitest suites:
  https://searchfox.org/mozilla-central/source/testing/profiles/common/user.js#31-37
  (it may have been better to pref it off from this instead of devtools/client/shared/test/shared-head.js)
  (unfortunately, this isn't shared with xpcshell)
But for DAMP, we want another pref list as we don't want the logging prefs turned on.

	Alexandre Poirot [:ochameau]
	Comment 5 • 6 years ago
mozreview-review

Comment on attachment 8973662 [details]
Bug 1459538 - Pref off the 3 pane inspector onboarding tooltip for DAMP.

https://reviewboard.mozilla.org/r/242028/#review247890

::: testing/talos/talos/tests/devtools/addon/content/damp.js:84
(Diff revision 1)
>  
>  /* globals res:true */
>  
>  function Damp() {
>    Services.prefs.setBoolPref("devtools.webconsole.new-frontend-enabled", true);
> +  Services.prefs.setBoolPref("devtools.inspector.show-three-pane-tooltip", false);

Could you add a comment about why we disable just this? (may be a link to the bug would help)

And a Talos run would help confirming that it fixes the regression before landing it :)

	Gabriel [:gl] (ΦωΦ) Assignee
	Comment 6 • 6 years ago

Base https://treeherder.mozilla.org/#/jobs?repo=try&revision=a9fd2b1e55f8a2193bc27800568be3ff94995ae7
Talos https://treeherder.mozilla.org/perf.html#/comparechooser?newProject=try&newRevision=f617dd6c1fe3bfbf61188ac0590176152bd88998

	Pulsebot
	Comment 7 • 6 years ago

Pushed by gabriel.luong@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c66986ae29bc
Pref off the 3 pane inspector onboarding tooltip for DAMP. r=ochameau

	Raul Gurzau (:RaulG)
	Comment 8 • 6 years ago
bugherder

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

	Ryan VanderMeulen [:RyanVM]
	Comment 9 • 6 years ago

We can uplift this to Beta a=test-only. Thanks for the quick fix, Gabriel!

	Ryan VanderMeulen [:RyanVM]
	Comment 10 • 6 years ago
uplift

https://hg.mozilla.org/releases/mozilla-beta/rev/dd77db86ef180cc68765683a80f92d94cb01b6a6

	Ionuț Goldan [:igoldan] Reporter
	Comment 11 • 6 years ago

I confirm this fixed the regressions:

== Change summary for alert #13115 (as of Mon, 07 May 2018 16:04:01 GMT) ==

Improvements:

 10%  damp osx-10-10 opt e10s stylo     123.05 -> 110.88
  7%  damp windows10-64 opt e10s stylo  78.75 -> 73.10
  6%  damp linux64 opt e10s stylo       69.60 -> 65.10
  6%  damp linux64 pgo e10s stylo       65.48 -> 61.48
  5%  damp windows7-32 pgo e10s stylo   75.01 -> 71.12

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