1369751 - Initializing browser-sidebar.js is visible in startup profiles before first paint

Status: RESOLVED FIXED

(Firefox :: General, enhancement, P1)

Description

[Florian Quèze [:florian]]

See https://perfht.ml/2s1skYS from a startup profile on a very slow netbook. Most of the browser-sidebar.js time is spent in the <browser> and <label> constructors. They are triggered by the getElementById calls from init().

We shouldn't need to construct the sidebar browser before first paint, especially if the sidebar is going to be hidden.

Comment 1

[Florian Quèze [:florian]]

Attachment: Patch

Comment 2

[Mike de Boer [:mikedeboer]]

Comment on attachment 8873865 [details] [diff] [review]
Patch

Review of attachment 8873865 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with comments addressed. Thanks!

::: browser/base/content/browser-sidebar.js
@@ +27,5 @@
>  
>    _box: null,
> +  // The constructor of this label accesses the browser element due to the
> +  // control="sidebar" attribute, so avoid getting this label during startup.
> +  get _title() { return document.getElementById("sidebar-title"); },

Please cache the element:
```js
if (this.__title)
  return this.__title;
return this.__title = document.getElementById("sidebar-title");
```

And the same for the `browser` getter above. This can potentially be repeated for all the elements that are fetched in the init() method, right? Perhaps we can do something smarter?

Can be done at a later date, but worth a thought just in case.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d1bdd265b00d
avoid constructing the sidebar browser before first paint, r=mikedeboer.

Comment 4

[Florian Quèze [:florian]]

(In reply to Mike de Boer [:mikedeboer] from comment #2)

I cached _title and browser as requested. But note that only the first getElementById call is expensive, because it forces attachment of the XBL binding.

> This can potentially be
> repeated for all the elements that are fetched in the init() method, right?
> Perhaps we can do something smarter?

I'm actually not sure we need to store references to these elements, I think we could do a getElementById call each time we need them.

Comment 5

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

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