Closed Bug 1451974 Opened 2 years ago Closed 2 years ago

opening/closing tab overflow menu *very* slow/janky due to custom elements code

Categories

(Core :: DOM: Core & HTML, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
Tracking Status
firefox61 --- affected

People

(Reporter: dbaron, Unassigned)

References

Details

(Keywords: perf, Whiteboard: [qf])

Recently I'm seeing that opening/closing the tab overflow menu (i.e., the dropdown at the right end of the tab bar that gives access to the tabs that don't fit) in a window with a large number of tabs is *very* slow/janky.  A profile shows that the problem is in custom elements code: https://perfht.ml/2q8m8ve .  The bulk of the time seems to be spent in mozilla::dom::CustomElementRegistry::UnregisterUnresolvedElement and (mostly) a QueryInterface inside it.

Myk suggests the proximate cause of the regression may be bug 1421070, though presumably the problem is older.  Not sure who the right person to investigate here is.
Flags: needinfo?(dtownsend)
This is probably due to Bug 1446247.
Blocks: 1446247
No longer blocks: 1421070
Flags: needinfo?(dtownsend) → needinfo?(bgrinstead)
There's an off chance that the fix from Bug 1451340 will help here if it turns out to be scrollbar related
:smaug - I won't be able to look at this until next week, but is there anything in the profile that stands out to you? Unless if you think there's a general CE optimization to be made I guess we can introduce a blacklist of tag names like in https://reviewboard.mozilla.org/r/228226/diff/2/. Or possibly switch to a whitelist that gets updated right before we do a migration (although I'd prefer to not do this because it makes the frontend changes more inconvenient).
Flags: needinfo?(bugs)
Or maybe we could bypass elements that already have a XBL binding attached in the same way we did with IsInNativeAnonymousSubtree in Bug 1451340.
We need to figure some other setup for XUL than blacklisting.

Obviously we want to back out bug 1446247 for now.
Flags: needinfo?(bugs)
CustomElementRegistry::UnregisterUnresolvedElement looks like a huge performance problem, though -- a linear scan of an array, doing a QueryReferent on each element to test for a match?  Removing everything is O(N^2) in the number of elements in the array.  Isn't that what's happening here?  That doesn't seem like acceptable performance characteristics, XUL or not.
Flags: needinfo?(bugs)
(For the record, I have about 1550 tabs in my browser window.)
Flags: needinfo?(bugs)
there is a bug filed to improve that.
and bug 1446247 was backed out, so this is fixed.
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
(In reply to Olli Pettay [:smaug] (only webcomponents and event handling reviews, please) from comment #8)
> there is a bug filed to improve that.

Which bug?
Flags: needinfo?(bugs)
bug 1452074
Flags: needinfo?(bugs)
Flags: needinfo?(bgrinstead)
Component: DOM → DOM: Core & HTML
Depends on: 1452074
You need to log in before you can comment on or make changes to this bug.