feat(vm): implement polymorphic inline cache (PIC) for property access#4740
feat(vm): implement polymorphic inline cache (PIC) for property access#4740akash-R-A-J wants to merge 5 commits intoboa-dev:mainfrom
Conversation
Test262 conformance changes
|
|
hii @jedel1043, could you please review this pr? happy to make any changes or address feedback. thanks! |
|
Thanks! Before reviewing anything, can you run https://github.com/boa-dev/data/blob/main/bench/bench-v8/combined.js? This should report benchmark results for your current PR's head commit, and you can compare that with a run against the main branch |
|
hey @jedel1043, I tried running the V8 combined benchmark ( Both runs currently overflow the stack on Windows when executing:
I also tried increasing the stack size (RUST_MIN_STACK=33554432) and rerunning the benchmark, but it still crashes with the same stack overflow. If there is a recommended way to run this benchmark on Windows, I'd be happy to try again and report the results. |
|
hey @jedel1043, here's the report after running both branches on wsl2. After running on main branch:
After running on this PR's branch:
|
|
Wow, Splay got a very biiiiig perf boost |
| use std::cell::Cell; | ||
|
|
||
| use boa_gc::GcRefCell; | ||
| use boa_gc::{GcRefCell, Trace as BoaTrace, custom_trace}; |
There was a problem hiding this comment.
Why import Trace as BoaTrace?
There was a problem hiding this comment.
The alias was introduced during development while wiring up the Trace implementation to avoid confusion with other imports. After the refactor it was no longer necessary, so it has been removed in the latest update.
| } | ||
|
|
||
| #[derive(Clone, Debug, Finalize)] | ||
| pub(crate) struct PicEntries(pub(crate) ArrayVec<PicEntry, PIC_CAPACITY>); |
There was a problem hiding this comment.
Maybe it would be better to implement Trace on ArrayVec in the boa_gc crate? Should be basically the same as the custom_trace! you implemented.
There was a problem hiding this comment.
That makes sense. Initially, I used custom_trace! on the PIC container to keep the changes local, since ArrayVec did not implement Trace. I’ve now implemented Trace for ArrayVec in boa_gc as suggested and updated the PIC implementation to use that instead.
d48eaa9 to
207831a
Compare
|
Hey @jedel1043, thanks for the review and the feedback! After incorporating the suggested changes, the benchmark score improved further (179 → 192 → 206). The largest improvement, as you noted earlier, continues to appear in the Splay benchmark. Latest benchmark on this PR branch: |
| if entries.len() < PIC_CAPACITY { | ||
| entries.push(PicEntry { | ||
| shape: shape.into(), | ||
| slot, | ||
| }); |
There was a problem hiding this comment.
Here you should use https://docs.rs/arrayvec/latest/arrayvec/struct.ArrayVec.html#method.try_push
| let shape_addr = shape.to_addr_usize(); | ||
|
|
||
| // If the shape already exists, update its slot. | ||
| // This handles cases where property transitions preserve the shape but change the slot. |
There was a problem hiding this comment.
Is this possible? IIRC any property transition will change the shape, so it should be impossible to have two shapes that are the same without them having the same slots.
| #[cfg(test)] | ||
| mod tests; | ||
|
|
||
| pub(crate) const PIC_CAPACITY: usize = 4; |
There was a problem hiding this comment.
thought: we might have to experiment with this value in the future. Increasing it might increase performance, but it might also cause more cache misses which could decrease performance.



This Pull Request fixes/closes #4656.
Currently Boa uses a monomorphic inline cache for property access sites, storing a single
(Shape → Slot)mapping. When the shape does not match, the cache is reset, which causes frequent cache invalidation when accessing the same property across objects with different shapes.This PR upgrades the inline cache to a polymorphic inline cache (PIC) to better support polymorphic property access patterns commonly seen in JavaScript.
It changes the following:
(Shape → Slot)mappings per access site using a small fixed-capacity cache (PIC_CAPACITY = 4).This change reduces inline cache thrashing for polymorphic property access patterns and aligns Boa's inline cache design more closely with modern JavaScript engines.