2082419 - Return already-populated HTMLCollection entries directly on safe indexed-getter cache hits, and batch ContentList population only for sequential next-index lazy misses.

Status: RESOLVED FIXED

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

Description

[perf-ai-bot]

Return already-populated HTMLCollection entries directly on safe indexed-getter cache hits, and batch ContentList population only for sequential next-index lazy misses.

Problem

Firefox retains matched live-collection elements but re-enters the virtual Item path for indexed getter access. Sequential indexed access also discovers one new match per miss, so this patch returns cached populated entries directly and populates up to 64 matches when the caller requests exactly the next uncached index after an existing valid prefix.

Performance

• Benchmark: speedometer3
• Suite: speedometer3
• Test: sp3-full
• Component: DOM
• Local Improvement: 2.62%

Validation

• Try push
• Performance comparison

Comment 1

[perf-ai-bot]

Please move this bug to the correct component.

Once the Phabricator patch is ready for review, please commandeer it.

Comment 2

[perf-ai-bot]

Attachment: WIP: Bug 2082419 - Return already-populated HTMLCollection entries directly on safe indexed-getter cache hits, and batch ContentList population only for sequential next-index lazy misses.

Return already-populated HTMLCollection entries directly on safe indexed-getter cache hits, and batch ContentList population only for sequential next-index lazy misses.

Comment 3

[Vincent Hilla [:vhilla]]

Bug 2075738 changed something here recently and is relevant for rebasing.

Comment 4

[Vincent Hilla [:vhilla]]

Rebased, there might be fewer performance gains without mFlushesNeeded being removed. Though the patch still does some pre-fetching in PopulateSelf and perhaps a more direct IndexedGetter.

https://perf.compare/compare-lando-results?landoInstance=lando-prod-2025&baseLando=106959&newLando=106960&baseRepo=try&newRepo=try&framework=13

Comment 5

[Vincent Hilla [:vhilla]]

Linux try profile before https://share.firefox.dev/4sducJP
Linux try profile after https://share.firefox.dev/4zLhogp

mozilla::dom::HTMLCollection::IndexedGetter went from 520 to 407 samples, of which mozilla::dom::ContentList::PopulateSelf went from 460 to 345 samples.

mozilla::dom::ContentList::PopulateSelf in total went from 474 to 360 samples.

So this still seems worthwhile.

Comment 6

[Vincent Hilla [:vhilla]]

I think the improvement came from HTMLCollection::IndexedGetter being non-virtual and not having to call a virtual method in the case where the element is already in mElements. I tried around a bit with the prefetching in PopulateSelf but didn't see clear wins. At the same time, prefetching has the risk of doing unneeded work that might be expensive for some collections. Edit: oops but comment 5 pointed to the opposite. So prefetching looked promising on try, being non-virtual was useful locally, we might do both.

Try push for my WIP https://perf.compare/compare-lando-results?landoInstance=lando-prod-2025&baseLando=106959&newLando=107221&baseRepo=try&newRepo=try&framework=13

Tests: https://treeherder.mozilla.org/jobs?repo=try&revision=cd388755d53d6267e2eb122ce95a3b47fe882b6e

Comment 7

[Vincent Hilla [:vhilla]]

Attachment: Bug 2082419 - Avoid virtual calls for HTMLCollection::{Item, IndexedGetter}. r=#dom-core

Comment 8

[Vincent Hilla [:vhilla]]

https://perf.compare/compare-lando-results?landoInstance=lando-prod-2025&baseLando=106959&newLando=107603&baseRepo=try&newRepo=try&framework=13

Comment 9

[Vincent Hilla [:vhilla]]

I had claude add a bit of logging and do some analysis for an sp3 run.

There are no ContentList::Item calls where aIndex > mElements.Length() && mState != State::UpToDate, i.e. the "frontier" always moves forward sequentially (though access overall might not be).

Histogram of highest indices needed per "epoch" (one list instance after creation / becoming dirty)

Highest index + 1	Epochs
1	1,091
5-8	10
9-16	3,010
17-64	160
65-256	640
>256	2,150

-> There are lots of lists that get populated to a large size.

Simulated prefetch policies

Policy	PopulateSelf calls	Fetched items (upper bound)
none	1,235,901	1,235,901
min(count,4)	259,401	1,244,881
min(count,8) (current)	155,641	1,266,681
min(count,16)	94,671	1,272,681
min(count,64)	48,151	1,355,201
fixed 64	24,331	1,581,515
count (doubling, uncapped)	39,601	1,948,641

The number of fetched items might be lower because we might not have more items in the tree that match the selector.

-> I considered doing min(count, 8) while the original did a fixed 64. Try shows a statistically significant improvement between them. Even though the number of PopulateSelf calls saved only goes from 88% to 98%.

Comment 10

[Vincent Hilla [:vhilla]]

Looking at profiles, min(count, 64) spends a comparable time to fixed 64 in IndexedGetter. But the try perf results are still worse.

Comment 11

[Vincent Hilla [:vhilla]]

I guess the cached base try run was too old again, a new push shows a good improvement: https://perf.compare/compare-lando-results?landoInstance=lando-prod-2025&baseLando=108881&newLando=108882&baseRepo=try&newRepo=try&framework=13&initialized=1

Comment 12

[Vincent Hilla [:vhilla]]

Attachment: Bug 2082419 - Prefetch successive elements for ContentList::PopulateSelf. r=#dom-core

Comment 13

[Vincent Hilla [:vhilla]]

https://treeherder.mozilla.org/jobs?repo=try&revision=fe31dd1e9fd67b8be7545f88f65499ef671d3a09

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/4936a6a87b79
https://hg.mozilla.org/integration/autoland/rev/b107c03b073f
Avoid virtual calls for HTMLCollection::{Item, IndexedGetter}. r=dom-core-reviewers,webidl,hsivonen
https://github.com/mozilla-firefox/firefox/commit/bafd2a4450b0
https://hg.mozilla.org/integration/autoland/rev/fe8f54024938
Prefetch successive elements for ContentList::PopulateSelf. r=dom-core-reviewers,hsivonen

Comment 15

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/b107c03b073f
https://hg.mozilla.org/mozilla-central/rev/fe8f54024938