Closed
Bug 1459538
Opened 6 years ago
Closed 6 years ago
5.61 - 8.72% damp (linux64, osx-10-10, windows10-64, windows7-32) regression on push 18937f7c5c2fe8d38b7e479c18f81816368df503 (Thu May 3 2018)
Categories
(DevTools :: Inspector, defect, P1)
Tracking
(firefox-esr52 unaffected, firefox-esr60 unaffected, firefox60 unaffected, firefox61+ fixed, firefox62+ fixed)
RESOLVED
FIXED
Firefox 62
Tracking | Status | |
---|---|---|
firefox-esr52 | --- | unaffected |
firefox-esr60 | --- | unaffected |
firefox60 | --- | unaffected |
firefox61 | + | fixed |
firefox62 | + | fixed |
People
(Reporter: igoldan, Assigned: gl)
References
Details
(Keywords: perf, regression, talos-regression)
Attachments
(1 file)
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
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?
Flags: needinfo?(gl)
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?
Flags: needinfo?(poirot.alex)
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.
Flags: needinfo?(poirot.alex)
Assignee | ||
Updated•6 years ago
|
Assignee: nobody → gl
Status: NEW → ASSIGNED
Flags: needinfo?(gl)
Comment hidden (mozreview-request) |
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 :)
Attachment #8973662 -
Flags: review?(poirot.alex) → review+
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
Updated•6 years ago
|
status-firefox60:
--- → unaffected
status-firefox61:
--- → affected
status-firefox62:
--- → affected
status-firefox-esr52:
--- → unaffected
status-firefox-esr60:
--- → unaffected
tracking-firefox61:
--- → +
tracking-firefox62:
--- → +
Priority: -- → P1
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
Comment 8•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/c66986ae29bc
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 61
![]() |
||
Updated•6 years ago
|
Target Milestone: Firefox 61 → Firefox 62
Comment 9•6 years ago
|
||
We can uplift this to Beta a=test-only. Thanks for the quick fix, Gabriel!
Whiteboard: [checkin-needed-beta]
Comment 10•6 years ago
|
||
uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/dd77db86ef180cc68765683a80f92d94cb01b6a6
Whiteboard: [checkin-needed-beta]
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
Updated•6 years ago
|
Product: Firefox → DevTools
Updated•6 years ago
|
Flags: qe-verify-
You need to log in
before you can comment on or make changes to this bug.
Description
•