Note: There are a few cases of duplicates in user autocompletion which are being worked on.

CustomizableUI.unregisterBuildWindow is slow when closing windows

RESOLVED FIXED in Firefox 55

Status

()

Firefox
Toolbars and Customization
P1
normal
RESOLVED FIXED
3 months ago
2 months ago

People

(Reporter: florian, Assigned: dao)

Tracking

(Blocks: 1 bug)

unspecified
Firefox 55
Points:
---
Dependency tree / graph
Bug Flags:
qe-verify -

Firefox Tracking Flags

(firefox55 fixed)

Details

(Whiteboard: [photon-performance])

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Reporter)

Description

3 months ago
It's strange/unfortunate that the way listeners are implemented in CustomizableUI.jsm causes all windows to receive notifications when one window is closed, and to have lots of listeners check if the notification is relevant to the window they care about.

But specifically, when looking at a profile, a third of the time is spent in the onWidgetInstanceRemoved handler in FullZoomUI.jsm, because it loops over all windows and updates UI in them at http://searchfox.org/mozilla-central/rev/d8496d0a1f6ebef57fe39b9b204475b0eccfb94c/browser/modules/FullZoomUI.jsm#104

A quick look at the history seems to indicate this loop was added recently in bug 1348122.

See this profile of closing several windows: http://perfht.ml/2pr1SDW

Updated

3 months ago
Flags: qe-verify?
Priority: -- → P2
(Assignee)

Comment 1

3 months ago
Maybe the onWidgetRemoved listener is enough. Not sure what else onWidgetInstanceRemoved is good for.
Comment hidden (mozreview-request)
(Assignee)

Updated

3 months ago
Assignee: nobody → dao+bmo
Flags: qe-verify? → qe-verify-

Updated

3 months ago
Status: NEW → ASSIGNED
Priority: P2 → P1

Comment 3

3 months ago
mozreview-review
Comment on attachment 8860060 [details]
Bug 1356911 - Remove unnecessary onWidgetInstanceRemoved listener.

https://reviewboard.mozilla.org/r/132098/#review135010

::: commit-message-ef8d1:1
(Diff revision 1)
> +Bug 1356911 - Stop using a onWidgetInstanceRemoved listener to update the full zoom UI. r?jaws

Please explain in your commit message *why* we are changing event listeners. As-is the commit message tells nothing more than the literal code change.
Attachment #8860060 - Flags: review?(jaws) → review+
Comment hidden (mozreview-request)

Comment 5

3 months ago
Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/9a8c26fc0120
Remove unnecessary onWidgetInstanceRemoved listener. r=jaws
https://hg.mozilla.org/mozilla-central/rev/9a8c26fc0120
Status: ASSIGNED → RESOLVED
Last Resolved: 3 months ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55

Updated

3 months ago
Iteration: --- → 55.4 - May 1
(Reporter)

Updated

2 months ago
No longer blocks: 1348289
(Reporter)

Updated

2 months ago
Blocks: 1348289
You need to log in before you can comment on or make changes to this bug.