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

RESOLVED FIXED in Firefox 55

Status

()

Firefox
General
P1
normal
RESOLVED FIXED
22 days ago
14 days ago

People

(Reporter: florian, Assigned: florian)

Tracking

(Blocks: 1 bug)

Trunk
Firefox 55
Points:
---
Bug Flags:
qe-verify ?

Firefox Tracking Flags

(firefox55 fixed)

Details

(Whiteboard: [photon-performance])

Attachments

(1 attachment)

(Assignee)

Description

22 days ago
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.
(Assignee)

Comment 1

22 days ago
Created attachment 8873865 [details] [diff] [review]
Patch
Attachment #8873865 - Flags: review?(mdeboer)
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.
Attachment #8873865 - Flags: review?(mdeboer) → review+

Updated

18 days ago
Status: NEW → ASSIGNED
Iteration: --- → 55.7 - Jun 12
Flags: qe-verify?
Priority: -- → P1
Whiteboard: [photon-performance]

Comment 3

15 days ago
Pushed by florian@queze.net:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d1bdd265b00d
avoid constructing the sidebar browser before first paint, r=mikedeboer.
(Assignee)

Comment 4

15 days ago
(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

14 days ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/d1bdd265b00d
Status: ASSIGNED → RESOLVED
Last Resolved: 14 days ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
You need to log in before you can comment on or make changes to this bug.