Closed Bug 1753633 Opened 4 months ago Closed 4 months ago

Add a fixed-size property cache for megamorphic lookups

Categories

(Core :: JavaScript Engine: JIT, task, P3)

task

Tracking

()

RESOLVED FIXED
99 Branch
Tracking Status
firefox99 --- fixed

People

(Reporter: jandem, Assigned: jandem)

References

(Blocks 1 open bug)

Details

Attachments

(8 files)

Megamorphic property lookups from JIT code are very common on modern JS workloads. We've seen GetNativeDataPropertyPure and similar functions show up in a lot of profiles.

We can optimize this with a per-thread fixed-size lookup cache based on the receiver object's shape + the property key. This works well in practice because JS code often accesses the same kind of objects repeatedly: the cache has a hit rate of > 80% on many websites. V8 has a similar cache, so this also helps us prevent certain perf cliffs in code optimized for Chrome.

Because this cache assumes things about prototype objects, we need to invalidate the cache when prototype objects are mutated in certain ways. This is where Watchtower comes in, it's now fairly easy for us to do that robustly.

This bug will add the cache but disabled by default.

For the megamorphic cache we'll need to handle property mutation/removal. We can ignore
freeze/seal, but this adds the Watchtower machinery in case we need it later for other
optimizations.

Drive-by change: add AutoCheckShapeConsistency to NativeObject::freezeOrSealProperties.

We got rid of almost all of the other JIT optimizations for typed objects. Supporting
them in HasNativeDataPropertyPure complicates later patches and it seems better for
these functions to be restricted to native objects (as implied by the name).

Depends on D137850

Also use isInt instead of JSID_IS_INT while we're there.

Depends on D137851

We can potentially reuse this pref for different Watchtower optimizations over time.

Because the megamorphic lookup path is so hot, we probably don't want to keep a pref
for that for too long, at least on non-Nightly/non-debug builds.

Depends on D137852

Depends on D137854

Attachment #9262329 - Attachment description: Bug 1753633 part 6 - Add a pref for Watchtower, disabled by default. r?iain! → Bug 1753633 part 6 - Add a pref for megamorphic cache, disabled by default. r?iain!
Pushed by jdemooij@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/aeff3654959e
part 1 - Add Watchtower hooks for property mutation/removal. r=iain
https://hg.mozilla.org/integration/autoland/rev/3bb6f9ea44f7
part 2 - Add Watchtower hook for object swapping. r=iain
https://hg.mozilla.org/integration/autoland/rev/b57b87a024c0
part 3 - Add HashAtomOrSymbolPropertyKey. r=iain
https://hg.mozilla.org/integration/autoland/rev/f64a89bf9681
part 4 - Stop handling typed objects in HasNativeDataPropertyPure. r=iain
https://hg.mozilla.org/integration/autoland/rev/25fe67147f22
part 5 - Move/fix a comment. r=iain
https://hg.mozilla.org/integration/autoland/rev/e292f8177315
part 6 - Add a pref for megamorphic cache, disabled by default. r=iain
https://hg.mozilla.org/integration/autoland/rev/917a7fc15044
part 7 - Add megamorphic lookup cache, disabled by default. r=iain
https://hg.mozilla.org/integration/autoland/rev/250fa587c2b0
part 8 - Add tests. r=iain
Regressions: 1754764
You need to log in before you can comment on or make changes to this bug.