Closed
Bug 1454388
Opened 7 years ago
Closed 6 years ago
3.35 - 3.75% about_preferences_basic (linux64, osx-10-10, windows10-64) regression on push 19591cc39b9a (Fri Apr 13 2018)
Categories
(Firefox :: General, defect)
Firefox
General
Tracking
()
RESOLVED
FIXED
Firefox 61
Tracking | Status | |
---|---|---|
firefox-esr52 | --- | unaffected |
firefox-esr60 | --- | unaffected |
firefox59 | --- | unaffected |
firefox60 | --- | unaffected |
firefox61 | + | fixed |
People
(Reporter: igoldan, Assigned: mkohler)
References
Details
(Keywords: perf, regression, talos-regression)
Attachments
(1 file)
Talos has detected a Firefox performance regression from push:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=43855a242386b8b0ab11cede40a6f41302c0dec4&tochange=19591cc39b9a0ea11f0d4e3a314d0fb2461a1ceb
As author of one of the patches included in that push, we need your help to address this regression.
Regressions:
4% about_preferences_basic windows10-64 pgo e10s stylo 132.03 -> 136.98
3% about_preferences_basic osx-10-10 opt e10s stylo 214.63 -> 221.97
3% about_preferences_basic linux64 opt e10s stylo 148.99 -> 153.98
You can find links to graphs and comparison views for each of the above tests at: https://treeherder.mozilla.org/perf.html#/alerts?id=12705
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•7 years ago
|
||
:mkohler Please look over these performance regressions and state whether we can fix them or whether we should consider accepting them/backing them out.
Flags: needinfo?(me)
Updated•7 years ago
|
status-firefox59:
--- → unaffected
status-firefox60:
--- → unaffected
status-firefox-esr52:
--- → unaffected
Assignee | ||
Updated•7 years ago
|
Flags: needinfo?(me) → needinfo?(jhofmann)
Comment 2•7 years ago
|
||
So what happened here is that we started showing the privacy pane by default (https://hg.mozilla.org/integration/autoland/rev/19591cc39b9a#l5.12) instead of keeping it "hidden" initially.
I'm sure what we're recording is just the extra overhead of rendering the site data group with the rest of the UI instead of rendering it later asynchronously (making this a WONTFIX), but I wonder:
1) Where exactly did we remove the "hidden" attribute from the site data group (we only deleted code adding it), i.e. at what point is the group rendered?
2) Did delaying the rendering have a real effect on first meaningful paint to the user or just our Talos numbers? Is there a way we can utilize this to improve about:preferences load times?
Jared, do you have any thoughts (especially regarding question 1)?
Thanks! :)
Flags: needinfo?(jhofmann) → needinfo?(jaws)
Comment 3•7 years ago
|
||
> So what happened here is that we started showing the privacy pane by default
I meant showing the site data group by default...
Comment 4•7 years ago
|
||
It should still have kept the hidden="true" attribute. This attribute gets removed by the search (gotoPref) code,
https://searchfox.org/mozilla-central/rev/11a2ae294f50049e12515b5821f5a396d951aacb/browser/components/preferences/in-content/preferences.js#49,84,179,191,201
As for 2, yes, this is what the Talos test is measuring. So the regression that is noticed here is real and should be fixed. The talos test was added by bug 1384272. The test measures the first non-blank paint, https://hg.mozilla.org/mozilla-central/rev/ecb24bcac0cf#l6.30
Flags: needinfo?(jaws)
Assignee | ||
Comment 5•7 years ago
|
||
I'll ad the hidden="true" again tomorrow then. Anything else that needs to be done?
Assignee: nobody → me
Comment 6•7 years ago
|
||
Nope, that should cover it. Thanks!
Comment 7•6 years ago
|
||
Updated•6 years ago
|
Flags: needinfo?(jhofmann)
Comment 8•6 years ago
|
||
Comment on attachment 8970277 [details]
Bug 1454388 - Re-add hidden="true" in Site Data to fix about_preferences_basic talos regression r=johannh
Johann Hofmann [:johannh] - At a work week, might respond slowly has approved the revision.
https://phabricator.services.mozilla.com/D1016
Attachment #8970277 -
Flags: review+
Updated•6 years ago
|
Flags: needinfo?(jhofmann)
Updated•6 years ago
|
status-firefox-esr60:
--- → unaffected
tracking-firefox61:
--- → +
Comment 9•6 years ago
|
||
I'm going to check this in to avoid uplifting for 61...
Comment 10•6 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/0e7560b8c179034f8a11c63c52c552b76c06c32d
Bug 1454388 - Re-add hidden="true" in Site Data to fix about_preferences_basic talos regression r=johannh
Comment 11•6 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 61
Reporter | ||
Comment 12•6 years ago
|
||
I confirm the perf regression was fixed on all platforms. After a few more jobs are done, Perfherder should automatically catch the improvements.
== Change summary for alert #13017 (as of Wed, 02 May 2018 04:49:20 GMT) ==
Improvements:
3% about_preferences_basic windows7-32 pgo e10s stylo 143.89 -> 139.35
For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=13017
You need to log in
before you can comment on or make changes to this bug.
Description
•