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

RESOLVED FIXED in Firefox 55

Status

()

Firefox
General
P1
normal
RESOLVED FIXED
3 months ago
a month 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

3 months 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

3 months 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

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

Comment 3

2 months 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

2 months 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

2 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/d1bdd265b00d
Status: ASSIGNED → RESOLVED
Last Resolved: 2 months ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55

Updated

a month ago
Flags: qe-verify? → qe-verify-
You need to log in before you can comment on or make changes to this bug.