1496425 - 2.1 - 4.56% about_preferences_basic (linux64, osx-10-10, windows7-32) regression on push 05e791fb2c6b (Tue Oct 2 2018)

Status: VERIFIED FIXED

(Toolkit :: UI Widgets, defect, P2)

Description

[Ionuț Goldan [:igoldan]]

Talos has detected a Firefox performance regression from push:

https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=a1cfc8fd44eefa562343373b6175902774a5ca31&tochange=05e791fb2c6b2fe7f967adf86e55517231bcf59d

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

Regressions:

  5%  about_preferences_basic linux64 pgo e10s stylo         137.11 -> 143.37
  3%  about_preferences_basic windows7-32 opt e10s stylo     141.51 -> 145.47
  2%  about_preferences_basic linux64 opt e10s stylo         152.09 -> 155.71
  2%  about_preferences_basic osx-10-10 opt e10s stylo       219.43 -> 224.03


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

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

Comment 1

[Brian Grinstead [:bgrins]]

The main issue here is that we have a bunch of hidden <radiogroup>s in the markup for prefs, and with XBL they don't get constructed but with custom elements they do get connected. Additionally, it looks like there's quite a bit of churn where radiogroups get moved around during initialization, like https://searchfox.org/mozilla-central/rev/924e3d96d81a40d2f0eec1db5f74fc6594337128/browser/components/preferences/in-content/privacy.js#549.

I'll see if delaying the connectedCallback until DOMContentLoaded or similar will solve this. I've talked with Emilio, and providing some kind of notification about when an element gets a layout frame (so we can delay connection until then and more closely match XBL behavior) is a no-go.

Comment 2

[Brian Grinstead [:bgrins]]

Still working on it, but have a patch that restores ~1.5% on linux64 and osx and ~2% on linux pgo:

https://treeherder.mozilla.org/perf.html#/compare?originalProject=try&originalRevision=b64afde0926ad2bf831ea85aff1a1ba982737cc1&newProject=try&newRevision=f21dd23b6bebb4bbc7e31d0c74ad62809d5228a1&framework=1

Comment 3

[Brian Grinstead [:bgrins]]

Attachment: Bug 1496425 - Provide a mechanism for Custom Elements to delay connectedCallback until after DOMContentLoaded;r=paolo

There are two reasons for this:
1) It's faster than running the connectedCallback in the middle of document parse, at least for
   <radiogroups> in about:preferences
2) It provides a construction sequence more similar to XBL, so the translation from XBL <constructor>
   to CE connectedCallback is more likely to be sound. This is because when there is markup like:
       <parent-ce><child-ce></child-ce></parent-ce>
   the parent-ce node is empty during the first connectedCallback. If we wait for DOMContentLoaded
   then the parent-ce has the child-ce node below it.

Comment 4

[Brian Grinstead [:bgrins]]

Paolo, this is an implementation of the pattern we discussed earlier. Please feel free to suggest API or naming changes as you see fit.

Comment 5

[Brian Grinstead [:bgrins]]

Huh, I guess adding you as a reviewer in phab didn't cc you on the bug. See Comment 4 :).

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7a32e3740263
Provide a mechanism for Custom Elements to delay connectedCallback until after DOMContentLoaded;r=paolo

Comment 7

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/7a32e3740263

Comment 8

[Ionuț Goldan [:igoldan]]

== Change summary for alert #16568 (as of Mon, 08 Oct 2018 21:47:04 GMT) ==

Improvements:

  4%  about_preferences_basic linux64 pgo e10s stylo     141.09 -> 135.64

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

Rest of regressions mentioned in comment 0 were fixed as well. Thank you!