perf(codegen,runtime): polymorphic property-read cache + arr.length short-circuit — interp.ts 3.96s → 2.39s - #7753
Conversation
📝 WalkthroughWalkthroughThe property-read inline cache now supports four polymorphic ways in a 12-word layout. It validates tokens, slots, and GC epochs. Array ChangesPolymorphic property-read cache
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant GeneratedPropertyGet
participant GenericDispatch
participant PicCache
participant IC_MissHandler
participant ArrayLength
GeneratedPropertyGet->>GenericDispatch: Check MRU and polymorphic ways
GenericDispatch->>PicCache: Validate epoch, token, and slot
PicCache-->>GenericDispatch: Return cached field on hit
GenericDispatch->>IC_MissHandler: Call on cache miss
IC_MissHandler->>ArrayLength: Resolve exact array length key
ArrayLength-->>IC_MissHandler: Return array length
IC_MissHandler->>PicCache: Prime token and slot
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
… short-circuit (#7753) A tree-walking interpreter ran 12.3x Node — three times worse than any synthetic benchmark in the corpus, and the only program in it that resembles real software. GC was 1% of the run. The cause was that the per-site property cache holds exactly ONE entry, so a receiver with more than one shape misses on essentially every read and pays a full re-derivation of the receiver kind plus a linear keys scan. Two changes, one root cause: * `@perry_ic_N` grows from `[8 x i64]` to `[12 x i64]` — the MRU entry keeps its exact prior meaning, followed by four `(token, slot)` ways. The miss handler cascades the shape it evicts into a way instead of discarding it, and the emitted way compares sit inside the miss block above the call, so a monomorphic site's instruction sequence is unchanged. Ways accept keys-POINTER tokens (an ID-token-only way set never fills for object literals, and shipped that way it was a 6% REGRESSION), so they inherit #6080a and share word 2's epoch snapshot: a new epoch wipes every way and drops the evicted token. * The inline cache cannot cache `arr.length` by construction (#72 requires a GC_TYPE_OBJECT receiver), so every dynamic `.length` misses permanently and then walks an object ladder. On a variable lookup written `for (i = 0; i < names.length; i++)` that one read was 22% of total run time. The miss handler now answers it directly for a GC_TYPE_ARRAY receiver, via the same expression the by-name array arm already computes. interp.ts 3.96s -> 2.39s on the quiet M1 mini; all twelve protected benchmark floors hold; outputs byte-identical to node 26.5.1. Claude-Session: https://claude.ai/code/session_015JgLM9UWGa6WAMix7CvhQJ
f82f1a6 to
be0a1f3
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
crates/perry-codegen/src/expr/property_get/generic_dispatch.rs (1)
533-543: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider extracting the inline-capacity bound into a shared helper.
Lines 533-542 repeat the exact instruction sequence emitted at lines 433-438: load
field_countfromsafe_obj_handle + 12, zero-extend, clamp toINLINE_SLOT_FLOOR, then compare. The two copies must stay in agreement, because a divergence would let one path load past a receiver's field region while the other rejects it. A small helper that emits the(limit)value and is called from both the MRU hit block and the way block would make that agreement structural.The duplicated load also sits between two
js_typed_feedback_record_*calls, so LLVM cannot always CSE it away.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/perry-codegen/src/expr/property_get/generic_dispatch.rs` around lines 533 - 543, Extract the duplicated inline-capacity calculation into a shared helper that emits the clamped field-count limit from safe_obj_handle. Use this helper in both the MRU hit path around the existing bound logic and the way block before comparing way_slot, preserving the current INLINE_SLOT_FLOOR behavior and bounds checks.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/perry-codegen/src/expr/property_get/tests.rs`:
- Around line 199-205: Update the assertion in the property-get test around emit
so it inspects only global declarations whose names start with `@perry_ic_`, then
verify every such declaration uses PIC_CACHE_WORDS. Ensure the test cannot pass
from an unrelated matching width and fails if any perry_ic cache has a different
width.
---
Nitpick comments:
In `@crates/perry-codegen/src/expr/property_get/generic_dispatch.rs`:
- Around line 533-543: Extract the duplicated inline-capacity calculation into a
shared helper that emits the clamped field-count limit from safe_obj_handle. Use
this helper in both the MRU hit path around the existing bound logic and the way
block before comparing way_slot, preserving the current INLINE_SLOT_FLOOR
behavior and bounds checks.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 1f387b7e-a13e-4bfb-ac1b-745099560df9
📒 Files selected for processing (13)
changelog.d/7753-polymorphic-property-read-cache.mdcrates/perry-codegen/src/codegen/closure.rscrates/perry-codegen/src/codegen/entry.rscrates/perry-codegen/src/codegen/function.rscrates/perry-codegen/src/codegen/method.rscrates/perry-codegen/src/expr/mod.rscrates/perry-codegen/src/expr/property_get.rscrates/perry-codegen/src/expr/property_get/generic_dispatch.rscrates/perry-codegen/src/expr/property_get/tests.rscrates/perry-runtime/src/node_submodules/tests.rscrates/perry-runtime/src/object/field_get_set.rscrates/perry-runtime/src/object/field_get_set/ic_miss.rscrates/perry-runtime/src/value/dynamic_object.rs
| let ir = emit(false, None); | ||
| assert!( | ||
| ir.contains(&format!( | ||
| "= private global [{PIC_CACHE_WORDS} x i64] zeroinitializer" | ||
| )), | ||
| "every @perry_ic_N must be emitted at the width the runtime writes:\n{ir}" | ||
| ); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Check every @perry_ic_N declaration.
This assertion searches only for a width fragment. It does not require an @perry_ic_N declaration, and it does not reject another cache declaration with a different width. A matching unrelated global can make the test pass while pic_prime_get writes past a cache global. Filter the declarations by @perry_ic_ and assert that every declaration uses PIC_CACHE_WORDS.
Suggested assertion
- assert!(
- ir.contains(&format!(
- "= private global [{PIC_CACHE_WORDS} x i64] zeroinitializer"
- )),
- "every `@perry_ic_N` must be emitted at the width the runtime writes:\n{ir}"
- );
+ let expected = format!("[{PIC_CACHE_WORDS} x i64] zeroinitializer");
+ let ic_globals: Vec<_> = ir
+ .lines()
+ .filter(|line| line.contains("`@perry_ic_`") && line.contains("global ["))
+ .collect();
+ assert!(!ic_globals.is_empty(), "expected an emitted `@perry_ic_N` global:\n{ir}");
+ assert!(
+ ic_globals.iter().all(|line| line.contains(&expected)),
+ "every `@perry_ic_N` must use {expected}:\n{ir}"
+ );📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let ir = emit(false, None); | |
| assert!( | |
| ir.contains(&format!( | |
| "= private global [{PIC_CACHE_WORDS} x i64] zeroinitializer" | |
| )), | |
| "every @perry_ic_N must be emitted at the width the runtime writes:\n{ir}" | |
| ); | |
| let ir = emit(false, None); | |
| let expected = format!("[{PIC_CACHE_WORDS} x i64] zeroinitializer"); | |
| let ic_globals: Vec<_> = ir | |
| .lines() | |
| .filter(|line| line.contains("`@perry_ic_`") && line.contains("global [")) | |
| .collect(); | |
| assert!(!ic_globals.is_empty(), "expected an emitted `@perry_ic_N` global:\n{ir}"); | |
| assert!( | |
| ic_globals.iter().all(|line| line.contains(&expected)), | |
| "every `@perry_ic_N` must use {expected}:\n{ir}" | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/perry-codegen/src/expr/property_get/tests.rs` around lines 199 - 205,
Update the assertion in the property-get test around emit so it inspects only
global declarations whose names start with `@perry_ic_`, then verify every such
declaration uses PIC_CACHE_WORDS. Ensure the test cannot pass from an unrelated
matching width and fails if any perry_ic cache has a different width.
…amorphic latch (#7753) The ways are a trade, not a free win, and just past capacity the trade inverts. Holding everything else fixed and varying only the number of shapes at one site (bench/poly_read{5,,7}.ts): shapes at the site 5 (=capacity) 6 7 v0.5.1434 1.98 s 1.97 s 1.98 s ways, no off-switch 0.79 s 1.87 s 2.71 s 2.5x faster at capacity, 37% SLOWER one shape past it — four dependent loads per read that can never hit. So the compares now sit behind their own branch on a way-state word, and a site that keeps evicting a way by capacity latches them off, leaving no readable way behind. Two policy failures, both measured, both now tests: * Count CONSECUTIVE capacity evictions, not cumulative. A cumulative count latches any long-running site that ever sees a stray shape — evalNode handles let/fun twice per round, 80 strays across a run — and turned the ways off on the very site they were built for (2.39 -> 3.03 s). * The latch must not be permanent. "Megamorphic" is a property of a program PHASE, not a site: interp.ts's string-building sub-program drives evalNode through a different shape set, and a sticky latch let that phase kill the site for the rest of the process (3.02 s). It is a countdown instead — each miss while latched adds one, and after PIC_LATCH_RETRY misses the ways get another chance. The codegen ordering test also becomes structural rather than textual: it now asserts pic.ways ends in a branch choosing between the way load and the miss call, so block emission order cannot make it vacuous. Claude-Session: https://claude.ai/code/session_015JgLM9UWGa6WAMix7CvhQJ
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/perry-runtime/src/object/field_get_set/ic_miss.rs (1)
329-423: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftRegister the pointer-token cache with the GC.
tokencan be a keys-array heap address and Lines 339 and 421 store it inPicCache. The cache is emitted in globals, but this change does not register a mutable root scanner for those pointer-bearing entries. Epoch invalidation only prevents later cache hits. It does not make the pointers visible during collection or relocation.Register the cache storage with
gc_register_mutable_root_scanner, or change the cache representation so it does not retain raw heap pointers. Keep the epoch invalidation behavior after this change. As per coding guidelines, “Any runtime cache or side table holding raw heap pointers must be registered withgc_register_mutable_root_scannerin the same change.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/perry-runtime/src/object/field_get_set/ic_miss.rs` around lines 329 - 423, Register the pointer-bearing PicCache storage with gc_register_mutable_root_scanner so tokens stored by pic_prime_get are visible and updated during GC collection or relocation. Add the mutable-root scanner for the emitted global cache representation, while preserving the existing epoch invalidation and cache reset behavior.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@crates/perry-runtime/src/object/field_get_set/ic_miss.rs`:
- Around line 329-423: Register the pointer-bearing PicCache storage with
gc_register_mutable_root_scanner so tokens stored by pic_prime_get are visible
and updated during GC collection or relocation. Add the mutable-root scanner for
the emitted global cache representation, while preserving the existing epoch
invalidation and cache reset behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 71c9e8e4-8368-4122-a046-892114ab6a22
📒 Files selected for processing (4)
changelog.d/7753-polymorphic-property-read-cache.mdcrates/perry-codegen/src/expr/property_get/generic_dispatch.rscrates/perry-codegen/src/expr/property_get/tests.rscrates/perry-runtime/src/object/field_get_set/ic_miss.rs
🚧 Files skipped from review as they are similar to previous changes (2)
- crates/perry-codegen/src/expr/property_get/tests.rs
- crates/perry-codegen/src/expr/property_get/generic_dispatch.rs
… short-circuit (#7753) A tree-walking interpreter ran 12.3x Node — three times worse than any synthetic benchmark in the corpus, and the only program in it that resembles real software. GC was 1% of the run. The cause was that the per-site property cache holds exactly ONE entry, so a receiver with more than one shape misses on essentially every read and pays a full re-derivation of the receiver kind plus a linear keys scan. Two changes, one root cause: * `@perry_ic_N` grows from `[8 x i64]` to `[12 x i64]` — the MRU entry keeps its exact prior meaning, followed by four `(token, slot)` ways. The miss handler cascades the shape it evicts into a way instead of discarding it, and the emitted way compares sit inside the miss block above the call, so a monomorphic site's instruction sequence is unchanged. Ways accept keys-POINTER tokens (an ID-token-only way set never fills for object literals, and shipped that way it was a 6% REGRESSION), so they inherit #6080a and share word 2's epoch snapshot: a new epoch wipes every way and drops the evicted token. * The inline cache cannot cache `arr.length` by construction (#72 requires a GC_TYPE_OBJECT receiver), so every dynamic `.length` misses permanently and then walks an object ladder. On a variable lookup written `for (i = 0; i < names.length; i++)` that one read was 22% of total run time. The miss handler now answers it directly for a GC_TYPE_ARRAY receiver, via the same expression the by-name array arm already computes. interp.ts 3.96s -> 2.39s on the quiet M1 mini; all twelve protected benchmark floors hold; outputs byte-identical to node 26.5.1. Claude-Session: https://claude.ai/code/session_015JgLM9UWGa6WAMix7CvhQJ
…amorphic latch (#7753) The ways are a trade, not a free win, and just past capacity the trade inverts. Holding everything else fixed and varying only the number of shapes at one site (bench/poly_read{5,,7}.ts): shapes at the site 5 (=capacity) 6 7 v0.5.1434 1.98 s 1.97 s 1.98 s ways, no off-switch 0.79 s 1.87 s 2.71 s 2.5x faster at capacity, 37% SLOWER one shape past it — four dependent loads per read that can never hit. So the compares now sit behind their own branch on a way-state word, and a site that keeps evicting a way by capacity latches them off, leaving no readable way behind. Two policy failures, both measured, both now tests: * Count CONSECUTIVE capacity evictions, not cumulative. A cumulative count latches any long-running site that ever sees a stray shape — evalNode handles let/fun twice per round, 80 strays across a run — and turned the ways off on the very site they were built for (2.39 -> 3.03 s). * The latch must not be permanent. "Megamorphic" is a property of a program PHASE, not a site: interp.ts's string-building sub-program drives evalNode through a different shape set, and a sticky latch let that phase kill the site for the rest of the process (3.02 s). It is a countdown instead — each miss while latched adds one, and after PIC_LATCH_RETRY misses the ways get another chance. The codegen ordering test also becomes structural rather than textual: it now asserts pic.ways ends in a branch choosing between the way load and the miss call, so block emission order cannot make it vacuous. Claude-Session: https://claude.ai/code/session_015JgLM9UWGa6WAMix7CvhQJ
ee2059b to
2c53cd0
Compare
… real numbers (#7753) PIC_WAY_STATE moves from word 11 (byte 88, a second 64-byte line) to word 3, alongside the MRU entry the miss path has already touched; the ways start at 4 and now fill the 12-word global exactly, which the layout tests assert. Measured, the move made NO difference — poly_read7 is 2.16 s either way — so the changelog says that rather than claiming the ~9% the cache-line argument predicted. It is kept for the exact layout. Also corrects the measurement tables to the final build, run interleaved against the same-host v0.5.1434 binaries: interp.ts 3.96 -> 2.47 s (12.3x -> 7.7x node) poly_read5 (5 shapes, = capacity) 2.04 -> 0.82 s 2.5x poly_read7 (7 shapes, > capacity) 2.04 -> 2.16 s +5.9%, latched Nine of the twelve protected benchmarks are identical to the same-host baseline; churn, tree and retain each read one timer tick slower and none reads faster, which is at the measurement floor but one-sided, so it is reported as "no regression I can measure" rather than "no regression". Claude-Session: https://claude.ai/code/session_015JgLM9UWGa6WAMix7CvhQJ
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@changelog.d/7753-polymorphic-property-read-cache.md`:
- Around line 195-199: Update the benchmark summary paragraph in the changelog
to accurately reflect the table: state that seven results are identical,
acknowledge the 0.01-second improvements for push_num and retain_wide, and
remove the incorrect claim that no benchmark reads faster.
In `@crates/perry-codegen/src/expr/property_get/generic_dispatch.rs`:
- Around line 31-34: Update the documentation above PIC_WAY_STATE to describe
negative latch states generally, rather than claiming a sticky -1 state. State
that any negative state temporarily skips way probes and may increment toward
zero, while zero and positive states retain their documented behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b5da77a0-9f64-4d01-aa3e-ef1b9f466b18
📒 Files selected for processing (4)
changelog.d/7753-polymorphic-property-read-cache.mdcrates/perry-codegen/src/expr/property_get/generic_dispatch.rscrates/perry-codegen/src/expr/property_get/tests.rscrates/perry-runtime/src/object/field_get_set/ic_miss.rs
🚧 Files skipped from review as they are similar to previous changes (2)
- crates/perry-codegen/src/expr/property_get/tests.rs
- crates/perry-runtime/src/object/field_get_set/ic_miss.rs
| Nine of twelve are identical. `churn`, `tree` and `retain` each read one | ||
| centisecond — one timer tick — slower, and none reads faster; an earlier | ||
| interleaved run at the same load had `churn` 0.43 vs 0.43 and `tree` 1.76 vs | ||
| 1.75. So this is at or below the measurement floor, but it is one-sided, and the | ||
| honest statement is "no regression I can measure", not "no regression". |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Correct the protected-benchmark summary.
The table in Lines 192-193 has seven identical results, not nine. It also shows push_num and retain_wide improving by 0.01 seconds, so the statement that no benchmark reads faster is incorrect. Update this paragraph to match the table.
Suggested wording
-Nine of twelve are identical. `churn`, `tree` and `retain` each read one
-centisecond — one timer tick — slower, and none reads faster;
+Seven of twelve are identical. `churn`, `tree` and `retain` each read one
+centisecond — one timer tick — slower; `push_num` and `retain_wide` each read
+one centisecond faster;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Nine of twelve are identical. `churn`, `tree` and `retain` each read one | |
| centisecond — one timer tick — slower, and none reads faster; an earlier | |
| interleaved run at the same load had `churn` 0.43 vs 0.43 and `tree` 1.76 vs | |
| 1.75. So this is at or below the measurement floor, but it is one-sided, and the | |
| honest statement is "no regression I can measure", not "no regression". | |
| Seven of twelve are identical. `churn`, `tree` and `retain` each read one | |
| centisecond — one timer tick — slower; `push_num` and `retain_wide` each read | |
| one centisecond faster; an earlier interleaved run at the same load had `churn` | |
| 0.43 vs 0.43 and `tree` 1.76 vs 1.75. So this is at or below the measurement | |
| floor, but it is one-sided, and the honest statement is "no regression I can | |
| measure", not "no regression". |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@changelog.d/7753-polymorphic-property-read-cache.md` around lines 195 - 199,
Update the benchmark summary paragraph in the changelog to accurately reflect
the table: state that seven results are identical, acknowledge the 0.01-second
improvements for push_num and retain_wide, and remove the incorrect claim that
no benchmark reads faster.
| /// Way-state word: `> 0` means at least one way is populated and the compares | ||
| /// are worth running; `0` (fresh / epoch-wiped) and `-1` (sticky megamorphic) | ||
| /// both skip them. Mirrors the runtime's `PIC_WAY_STATE`. | ||
| pub(crate) const PIC_WAY_STATE: usize = 3; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Correct the PIC_WAY_STATE documentation.
Line 31-34 describes the negative state as -1 and sticky. In crates/perry-runtime/src/object/field_get_set/ic_miss.rs lines 335-429, the runtime stores -PIC_LATCH_RETRY and increments negative states toward zero. Document the condition as any negative latch state that temporarily skips way probes.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/perry-codegen/src/expr/property_get/generic_dispatch.rs` around lines
31 - 34, Update the documentation above PIC_WAY_STATE to describe negative latch
states generally, rather than claiming a sticky -1 state. State that any
negative state temporarily skips way probes and may increment toward zero, while
zero and positive states retain their documented behavior.
Merging as v0.5.1442I went after the soundness risk firstThe ways hold raw heap addresses in a global no GC scanner can see, so the epoch discipline is the whole safety argument. Three checks:
All eight named tests present and passing; a 5-shape A/B on my (loaded) box gives 0.65 → 0.51 s. The diagnosis is the best part, and it includes a correction
Re-reading a refutation as a confirmation is rare and worth naming. Three hypotheses were killed with measurements first (string-keyed tags 4%, interning +0.01 s, numeric opcodes −0.01 s), and It also explains the profile lines that made no sense: Two design traps, each caught by a test rather than reasoning
The megamorphic latch is the right answer to the asymmetry it names (+37% on a 7-shape site vs 2.5× on a 5-shape one), and making it a countdown rather than a one-way door is what stops a transiently-wide site from being punished forever. Change 2
Verification postureGating Protected floors all hold, A/B'd against a same-host v0.5.1434 build. The "what is left" section is honest — 7.4× Node, not scriptc's ~5× — and correctly says none of the residue is a single-mechanism gap the way this one was. Gates 21/21. |
1 participant
The gap
gc-handoff/apps/interp.ts— a tree-walking interpreter (lexer, precedence-climbing parser, recursivelet, closures over environments), 189 statements, no dynamic remainder — ran 3.96 s against Node 26.5.1's 0.32 s. That is 12.3× Node, three times worse than any synthetic benchmark in the corpus (all now 2.3–4.0×), 2.3× worse than scriptc, and it is the only program in the corpus that resembles real software.GC is not involved: 0.04 s of pause across the whole run, 38 minors, zero fulls.
It is now 2.39 s (1.66× faster, 12.3× → 7.4× Node).
Root cause
The per-site property-read cache holds exactly one entry.
evalNodedispatches onn.kind === "num" | "str" | "var" | "bin" | "if" | …, so the receiver at those sites cycles through five shapes and the single entry is wrong on essentially every read. Each miss re-derives the receiver kind from scratch — proxy band, closure magic, the registered-buffer and typed-array registries (both behind thread-locals), the accessors latch — then linear-scans the keys array with ajs_string_equalsper key. That was 34% of the reduced program.It also explains the profile lines that made no sense on their face:
typedarray::lookup_typed_array_kindat 3.4% andbuffer::header::is_registered_bufferat 1.9% in a program that uses neither typed arrays nor Buffers. They were not a separate problem — they were inside the miss handler.Three hypotheses died first, and one of the refutations was misread
js_jsvalue_equals+from_utf8+memcmp+js_string_equals= 23% of the profile. Refuted:tag_str.tsvstag_num.tsare both 0.06 s, and converting the whole interpreter to numeric tags moved 3.88 → 3.71 s (4%). Most of that 23% was the keys scan inside the miss handler, not the user's===.streq_sub.tsis 0.05 s vs Node's 0.07. Interning every identifier at lex time: 3.88 → 3.87. Numeric opcodes: 3.88 → 3.89.meg{2..12}.ts, loop body and array size held fixed) showed a flat 3.0–3.7× at every arity, and that was read as "no cliff, so not the cause". It is the opposite: flat-and-already-3× from two shapes onward is exactly the signature of a one-entry cache. The sweep could never show a cliff because there is nothing to fall off.The discriminating probe was
bench/a_flatnode.ts— the same interpreter with every AST node built from one shape, so everyn.*read is monomorphic, with recursion depth, allocation count, string traffic and the environment chain all held constant. 3.88 → 2.84 s. That is what this PR went after.Change 1 — polymorphic ways
@perry_ic_Nwidens[8 x i64]→[12 x i64]: the MRU entry[token, slot, epoch]keeps its exact prior meaning, followed by four(token, slot)ways and a victim counter. The miss handler cascades the shape it evicts into a way instead of discarding it.The way compares are emitted inside the miss block, below the feedback records and above the call, so a monomorphic site executes the identical instruction sequence it did before — the new work is reached only by a site that was already going to call the runtime, and the typed-feedback counters are unchanged.
Two things had to be right, and a test found each:
__AnonShape_*constructor, so it has a realclass_idand primes a keys pointer. Shipped that way it was a measured 6% regression, the compare sequence running on every miss and never once hitting.Admitting pointer tokens inherits #6080a — a freed keys-array address can be recycled under a different shape, and a stale way would pointer-match and load the wrong slot silently. So the ways share word 2's epoch snapshot: the emitted predicate requires
cache[2] == @PERRY_IC_EPOCH, andpic_prime_getwipes every way whenever it writes a new epoch, dropping the evicted token too. A readable way is always one primed in the epoch word 2 still holds.→ 3.96 → 3.01 s.
js_object_get_field_ic_missfell from 9.0% of the profile to 1.0%.Change 2 —
arr.lengthshort-circuitWith
evalNodefixed, the entire remaining miss cost moved to one place: 1143 of 5241 leaf samples, all fromlookup, none of it a polymorphic object read. It wasnames.length.The inline cache requires a
GC_TYPE_OBJECTreceiver by construction (#72), so every dynamic.lengthmisses permanently, by design — and then walks a ladder built for objects, which repeats the registry probes injs_object_get_field_by_namebefore reaching the array arm. For a variable lookup written the ordinary way,for (i = 0; i < names.length; i++), that single read was 22% of total run time.js_object_get_field_ic_missnow answers it directly when the receiver'sGcHeadersaysGC_TYPE_ARRAY— a genuine dense array, since buffers, typed arrays, lazy arrays, Sets and Maps all carry distinctobj_types and anArraysubclass instance is anObjectHeader.js_array_lengthstill resolves growth-forwarding stubs, proxies and subclass receivers, and the expression returned is the one the by-name array arm already computes — a short-circuit, not a second implementation.→ 3.01 → 2.39 s.
Tests
New, and each one fails if the thing it names is undone:
alternating_shapes_all_become_inline_resolvablepointer_tokens_do_reach_a_wayan_epoch_change_wipes_every_waymonomorphic_site_never_fills_a_wayoverflow_rotates_without_corrupting_pairsarray_length_short_circuit_agrees_with_the_full_ladderjs_object_get_field_by_name_f64for empty/small/grown arrays, a same-length non-lengthkey, andlengthon a plain objectpic_cache_words_match_codegen/pic_cache_layout_matches_runtimegeneric_property_get_tries_ways_before_calling_the_miss_handlerMeasurements
Quiet M1 mini, best-of-5, absolute seconds. Outputs verified byte-identical to
node --experimental-strip-typesbefore timing.interp.tsb_fib.ts(reduced case)Protected floors — all hold, each also A/B'd against a same-host v0.5.1434 build:
gc-handoff/apps/iso_miss.tsprintschecksum 437840 misses 0— gated on the miss counter, not the aggregate, because a perf change has previously madeinterp.ts's total read correct while a silent-wrong-answer GC bug (#7682) was fully intact. Also clean underPERRY_GC_VERIFY_EVACUATION=1andPERRY_GC_PROTECT_FROMSPACE=1 PERRY_GC_PROTECT_FROMSPACE_DEPTH=800, which matter here because the ways hold raw heap addresses in a global no GC scanner can see.What is left
interp.tsis 7.4× Node, not the ~5× scriptc reaches. The remaining profile isjs_jsvalue_equals(15.7% —===is still a runtime call per comparison, and an inline fast path only resolves the ~20% of comparisons that are true), the per-object GC layout tables on the allocation path (7.7%, the #7510/#7469 area), write barriers (5.9%), andjs_dyn_index_get's registry probes (~3% — the same "route byGcHeaderbefore probing address registries" shape fixed here twice). None of those is a single-mechanism gap the way this one was.Refs #5094, #6759, #7469.
https://claude.ai/code/session_015JgLM9UWGa6WAMix7CvhQJ
Summary by CodeRabbit
Performance
.lengthreads while preserving proxy, forwarding, and subclass behavior.Bug Fixes
Tests