1988776 - MutationObserver's getReceiverFor is slow when there are a lot of observers

Status: RESOLVED FIXED

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

Description

[Julien Wajsberg [:julienw]]

Context: I'm building a Lit-based app using mozilla-central MozLitElement. This is a wrapper on the upstream LitElement class that handles localization through Fluent by using Fluent's connectRoot on every shadow root: https://searchfox.org/firefox-main/rev/938e8f38c6765875e998d5c2965ad5864f5a5ee2/toolkit/content/widgets/lit-utils.mjs#160
This in turns calls observe on this root: https://github.com/projectfluent/fluent.js/blob/9a183312d4db035d6002c93e03f0c169a58f3234/fluent-dom/src/dom_localization.js#L149

Then I noticed that the MutationObserver machinery and especially getObserverFor take a big chunk of the time spent on some page that has a lot of these elements, see this profile:
https://share.firefox.dev/469NE0A

There aren't a lot of these elements, I can count 818 fluent roots on this page. I wonder if there could be some easy optimisation here.

I'm also not sure using fluent-dom is the best model with Lit, and may switch to another (custom?) solution later. Also there may be an issue in Fluent.
I tried to do a simple testcase with ~1000 elements but the problem doesn't appear. So I'll try to make a smaller version of my app.

Comment 1

[Julien Wajsberg [:julienw]]

Attachment: testcase

Not exactly a minimal testcase but this reproduces the problem.

The full project is at https://github.com/julienw/mutationobserver-issue in case you'd like to tweak it.
The html file is generated with npm run build and it's then dropped to dist.

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

I don't see anything obvious to optimize.

Comment 3

[Julien Wajsberg [:julienw]]

Here is a new profile with latest nightly: https://share.firefox.dev/4xOTM9L
It's significatively different now, but still too slow. But maybe the slowness is unavoidable given how MutationObserver works.

I guess one solution to reduce the issue would be to have several observers, not just 1 for all roots.
Another solution could be to have an observer that could pierce shadow roots.
Finally (a solution we'll likely implement for the Enterprise Console), a solution that doesn't use a MutationObserver at all, by hooking into Lit's lifecycle directly. The drawback is that we can't set data-l10n-ids directly, but it sounds like a reasonable constraint.

Comment 4

[Jan Jaeschke [:jjaschke]]

Attachment: Bug 1988776, part 1 - Use main-thread atomization for observer filters. r?smaug!

Comment 5

[Jan Jaeschke [:jjaschke]]

Attachment: Bug 1988776, part 2 - Check the observer telemetry flag first. r?smaug!

Comment 6

[Jan Jaeschke [:jjaschke]]

Attachment: Bug 1988776, part 3 - Inline small observer attribute filters. r?smaug!

Comment 7

[Jan Jaeschke [:jjaschke]]

Attachment: Bug 1988776, part 4 - Preallocate WebIDL sequences from arrays. r?smaug!,#spidermonkey-reviewers

Comment 8

[Jan Jaeschke [:jjaschke]]

Attachment: Bug 1988776, part 5 - Retain single-object node storage when unbinding. r?smaug!

Comment 9

[Jan Jaeschke [:jjaschke]]

Here is a profile of my local build (started after loading the test case, then clicking the button once): https://share.firefox.dev/4hlR8md (~490ms jank)
This is the profile with the patches applied: https://share.firefox.dev/4yVndYf (~371ms jank)

~25% improvement? Not bad I guess. I couldn't see any sp3 improvements though.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/e7bc47e55ddb
https://hg.mozilla.org/integration/autoland/rev/359c2e74b20c
part 1 - Use main-thread atomization for observer filters. r=smaug
https://github.com/mozilla-firefox/firefox/commit/c3ff76070416
https://hg.mozilla.org/integration/autoland/rev/1c44ebe7bfb7
part 2 - Check the observer telemetry flag first. r=smaug
https://github.com/mozilla-firefox/firefox/commit/78ad85001bdd
https://hg.mozilla.org/integration/autoland/rev/f4202d8c0767
part 3 - Inline small observer attribute filters. r=smaug
https://github.com/mozilla-firefox/firefox/commit/7b7d92fcff24
https://hg.mozilla.org/integration/autoland/rev/b95716a5cae3
part 4 - Preallocate WebIDL sequences from arrays. r=smaug,spidermonkey-reviewers,jandem
https://github.com/mozilla-firefox/firefox/commit/edadb6d10bb1
https://hg.mozilla.org/integration/autoland/rev/9e5f4a90767f
part 5 - Retain single-object node storage when unbinding. r=smaug

Comment 11

[agoloman]

https://hg.mozilla.org/mozilla-central/rev/359c2e74b20c
https://hg.mozilla.org/mozilla-central/rev/1c44ebe7bfb7
https://hg.mozilla.org/mozilla-central/rev/f4202d8c0767
https://hg.mozilla.org/mozilla-central/rev/b95716a5cae3
https://hg.mozilla.org/mozilla-central/rev/9e5f4a90767f