perf(runtime,codegen): SSO computed keys + megamorphic write stub cache (dynamic-property overwrite 5.3×) - #8965
Conversation
Computed string keys (`o["k" + i]`) stopped defeating every property cache in the pipeline: the dynamic-property overwrite loop goes from ~450 ms to ~86 ms — 5.3x, 8/8 interleaved A/B pairs at stable load — against 28-30 ms for node on the same host. Two changes that only pay off together. 1. `js_string_concat_value_box`: the fused string+number concat now returns SSO inline when the result fits (<= 5 ASCII bytes) instead of always minting a StringHeader. That removes a heap allocation per iteration from the hot `"prefix" + i` shape, but the bigger effect is that the result's BITS become content-stable, so caches that compare key values can hit. 2. A thread-local 4096-way megamorphic stub cache keyed on (shape_token, key_bits) for dynamic string-keyed writes. A site's inline IC holds DYN_IC_WAYS = 3; a loop rotating 500 keys through it evicts permanently. The stub is probed after the per-site ways miss and fed at every prime site, including for overflow slots. Supporting: SSO->heap materialization interns instead of minting, so a computed key's ADDRESS is stable too (address-keyed read plans hit); short heap keys are folded to their SSO bits before keying; the way index uses multiplicative mixing (a low-bit XOR collapsed 500 string keys onto 125 ways, worst bucket 10 deep, and was the single reason the cache measured as a wash). Safety: the stub stores only content-derived bits — never an address — so no entry can be invalidated by an intern eviction recycling a heap address, and the table holds no GC roots. Stale entries are rejected by dyn_ic_try_store's per-hit shape-token/flags/slot-bound revalidation. Runtime-side; the binary grows 4 KB (+0.04%). Claude-Session: https://claude.ai/code/session_01Ay8VyLkKbm8Hkc1xmvTEsP
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (10)
Included review availability: Your plan provides up to 8 included reviews per hour; 5 remain after this review. 📝 WalkthroughWalkthroughChangesThe computed-key write path now produces short ASCII keys as SSO immediates and caches dynamic writes by shape token and content-derived key bits. The cache validates entries and supports overflow slots. An optional hardened allocator feature and GC-root metadata were added. Computed-key write optimization
Estimated code review effort: 4 (Complex) | ~45 minutes Merge Risk: ⚪ Minimal · up to This PR optimizes short computed-key writes without changing their validated behavior; the supplied test and differential results show no actionable merge-blocking risk remains. Sequence Diagram(s)sequenceDiagram
participant GeneratedCode
participant js_string_concat_value_box
participant js_put_value_set_dyn_ic
participant WRITE_STUB
participant dyn_ic_try_store
GeneratedCode->>js_string_concat_value_box: build computed property key
GeneratedCode->>js_put_value_set_dyn_ic: perform dynamic property write
js_put_value_set_dyn_ic->>WRITE_STUB: probe shape token and key bits
WRITE_STUB->>dyn_ic_try_store: validate cached slot
dyn_ic_try_store-->>GeneratedCode: return stored value
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Description checkExplanation The description provides detailed technical context, performance results, safety rationale, and verification results. However, it does not use the required template sections or provide the required test-plan and checklist entries. Resolution Restructure the description using the repository template. Add Summary, Changes, Related issue, Test plan, Screenshots / output, and Checklist sections. Include the executed verification commands and mark the applicable checklist items. Set Related issue to an issue reference or "n/a" if standalone. Do not include the Claude session URL as a substitute for the required sections. Full details: Docstring CoverageExplanation Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 17 functions across 7 files. (3 skipped: 3 unsupported.)
✨ Finishing Touches🧪 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 |
|
Merged (batched with #8966). The two changes do depend on each other — an SSO result from the fused concat is what lets the megamorphic write stub cache hit at all — and both are additive fast paths with the existing routes as the fallback. Validation: codegen 1341/0, runtime 2779/0 ( The 5.3× is not re-measured here. |
1 participant
Computed string keys (
o["k" + i]) stopped defeating every property cache in the pipeline.Dynamic-property overwrite loop: ~450 ms → ~86 ms (5.3×), 8/8 interleaved A/B pairs at stable load; node on the same host runs it in 28–30 ms, so this workload goes from ~16× node to ~3.0× node. A second 10/10 run under heavy co-tenant load (everything inflated ~1.8×) reproduced the same ratio. Binary size +4 KB (+0.04%) — runtime-side, no generated-code growth.
Two changes that only pay off together
1.
js_string_concat_value_box— SSO result when it fits. The fused string+number concat now returns an inline SSO immediate for results of ≤ 5 ASCII bytes instead of always minting aStringHeader. That removes a heap allocation per iteration from the hot"prefix" + ishape, but the bigger effect is that the result's bits become content-stable, so caches that compare key values can hit. ASCII-only, for the sameutf16_lensoundness reason asjs_string_concat_box's existing SSO arm.2. A megamorphic stub cache for dynamic string-keyed writes. A site's inline IC holds
DYN_IC_WAYS = 3; a loop rotating 500 computed keys through it evicts permanently and pays the full miss walk every write. Added a thread-local 4096-way direct-mapped cache keyed on(shape_token, key_bits)— V8's megamorphic stub pattern — probed after the per-site ways miss and fed at every prime site, including for overflow slots (a wide object keeps every data property there, so gating on the inline region would starve the cache for exactly the receivers it exists for).Supporting: the SSO→heap materialization every
*const StringHeaderconsumer crosses now interns instead of minting, so a computed key's address is stable too and address-keyed read plans can hit; short heap keys are folded to their SSO bits before keying.Safety
The stub stores only content-derived bits, never an address; keys that don't fit the inline form are rejected rather than cached under their pointer. This is load-bearing, not conservatism:
dyn_ic_try_storerevalidates the receiver's current shape token, blocking flags and slot bound on every hit, but it confirms the shape, not that the cached slot belongs to this key. A pointer-keyed entry could be primed, evicted from the (direct-mapped, evict-on-collision) intern table, collected, and its address recycled by an unrelated string whose write would hit the stale entry and overwrite the wrong slot. Content-only keying removes that class entirely and leaves the table holding no GC roots (registered ingc_runtime_root_holders.jsonwith that verdict).Method note — two wrong verdicts before the right one
The stub alone measured as a wash, twice, and no profile explained why. Counters did, in two stages:
"k0".."k499"collapsed onto 125 ways with buckets 10 deep, evicting each other continuously. Multiplicative mixing over the top bits gives 480/500 distinct ways, and that single change is what produced the 5.3×.Verification
perry-runtime2774 passed / 0 failed;perry-codegen1340 passed / 0 failed.scripts/run_lint_gates.sh: all gates pass (ci_cargo_test_shard.pyneeds--total-shards 8, which passes when supplied).Map/Setkeys, the 5→6 byte SSO boundary, non-ASCII, floats, negatives, 1e21, numeric-looking keys: byte-identical output._boxtwin; their intent (fused concat, no tag test) is unchanged.benchmarks/bench_populated_delete.ts(the ~200× populated-delete gap) is unaffected by this path and remains open for dictionary mode.https://claude.ai/code/session_01Ay8VyLkKbm8Hkc1xmvTEsP
Summary by CodeRabbit
Performance
Diagnostics