Talos has detected a Firefox performance regression from push 4a9ad284a59665dab2889370800b1f45a1d784bc. As author of one of the patches included in that push, we need your help to address this regression. Regressions: 1% damp summary linux64 opt e10s 317.8 -> 322.28 You can find links to graphs and comparison views for each of the above tests at: https://treeherder.mozilla.org/perf.html#/alerts?id=5377 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 * removing the 3 day window as this is <2%, but probably worth documenting as this is a 1.4% regression

Thanks for filing. It looks like the patch made the inspector close twice as slow in the "complicated" case (damp complicated.inspector.close.DAMP). If a grid highlighter was displayed, we now save the selector for the currently highlighted element. However that shouldn't have any impact when no highlighter is used.

Comment on attachment 8846816 [details] Bug 1346781 - do not attempt to hide highlighters if currentNode is falsy; https://reviewboard.mozilla.org/r/119816/#review121724 ::: devtools/server/actors/highlighters/auto-refresh.js:122 (Diff revision 2) > > /** > * Hide the highlighter > */ > hide: function () { > - if (Cu.isDeadWrapper(this.highlighterEnv.window)) { > + if (!this.currentNode || Cu.isDeadWrapper(this.highlighterEnv.window)) { That's make sense, if we attempt to call `hide` when `show` was never called in first place. Notice that the `window` getter is changed now, so you want probably to build your patch on top of this one: https://hg.mozilla.org/integration/autoland/rev/b1b91f7861ec And having something like: ```js if (!this.currentNode || !this.highlighterEnv.window) ```

Thanks for the review! Updated the patch accordingly, I'll wait for the other changeset to reach central before landing.

Pushed by jdescottes@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/9555f7ced9de do not attempt to hide highlighters if currentNode is falsy;r=zer0

thanks, verified on the graph!