perf(runtime): try the transition fast path before rooting in the dyn-IC miss (48 → 44 ms) - #9329
Merged
proggeramlug merged 1 commit intoAug 31, 2026
Conversation
…-IC miss (48 -> 44 ms)
js_put_value_set_dyn_ic_miss opened a RuntimeHandleScope and rooted
target/key/value before trying the learned shape-transition shortcut —
which then opened its own scope and rooted the same three operands again.
On the fresh-object-construction lane (the dominant taker of this entry:
every write to a new key changes the receiver's shape, so no per-site IC
can ever hit) that was two scopes and six roots per property write, one
set of which protected nothing.
The shortcut now runs BEFORE the scope, through a value-returning form of
the transition fast path:
* On success it returns the stored value re-read through its OWN root,
taken after every internal allocation point (key interning, spill
growth) — so the caller needs no roots of its own.
* On a miss it reports, via an out-param, whether it allocated before
missing (interning a fresh key and then finding no learned edge). In
that case the caller's raw f64 operands may point at moved objects,
and the callee hands back re-rooted copies for the fallback ladder.
Without that, this reorder would have been the PerryTS#6655 bug class —
rooting stale bits after a GC the callee triggered.
The i32 entries keep their signatures; the value form is a pub(crate)
twin, and the existing rooted call site is replaced rather than
duplicated.
Measured on the quiet host, 20k fresh objects x 20 computed-key writes,
three runs each, spread <= 1 ms:
concat keys 48 -> 44 ms (node 24)
pre-built keys 43 -> 39 ms (node 12)
bench_object_property ~35 -> ~33 ms (node 12)
Correctness: the intern-then-miss stress (40 generations of objects with
never-seen keys, string values, spill growth) is node-identical under the
default GC and under PERRY_GC_HEAP_LIMIT=8 PERRY_GC_FORCE_EVACUATE=1.
This is one increment, not parity: the remaining ~3x against node on this
shape is the transition install machinery itself (second handle scope in
the fast path, shape-version install, keys handling, spill store), tracked
on PerryTS#9287.
Refs PerryTS#9287.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (4)
Included review availability: Your plan provides up to 8 included reviews per hour; 6 remain after this review. 📝 WalkthroughWalkthroughThe dynamic-write miss handler now attempts the learned shape transition before opening its outer handle scope. The new value-returning fast path roots operands internally, refreshes them after key interning, and returns the stored value on success. ChangesDynamic property write transition
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to This localized runtime optimization reduces redundant rooting work while preserving existing write checks and fallback behavior; no actionable merge-blocking risk remains beyond normal checks and review. Sequence Diagram(s)sequenceDiagram
participant js_put_value_set_dyn_ic_miss
participant object_set_field_by_name_transition_only_fast_value
participant RuntimeHandleScope
js_put_value_set_dyn_ic_miss->>object_set_field_by_name_transition_only_fast_value: pass unrooted operands
object_set_field_by_name_transition_only_fast_value->>object_set_field_by_name_transition_only_fast_value: root operands and attempt transition
object_set_field_by_name_transition_only_fast_value-->>js_put_value_set_dyn_ic_miss: return stored value or refreshed operands
js_put_value_set_dyn_ic_miss->>RuntimeHandleScope: open scope after a miss
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Full details: Description checkExplanation The description is substantive and covers the change, rationale, performance results, safety validation, and reference to issue Full details: Docstring CoverageExplanation Docstring coverage is 80.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 3 files. (1 skipped: 1 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 |
proggeramlug
added a commit
that referenced
this pull request
Aug 31, 2026
* fix(runtime): scope the refresh key read and classify the test hit counter with_const_ptr keeps the key pointer's validity window explicit for the non-allocating tuple build, restoring fast_paths.rs to its ceiling of 8. TEST_TRANSITION_FAST_HITS is a cfg(test) Cell<u64> counter, not a GC root. * fix(runtime): gate the now-test-only transition wrapper #9287 moved the production caller to the value-returning form, leaving the i32 wrapper used only by the transition tests; -D warnings rejects it. * chore: changelog fragment for the #9329 follow-up --------- Co-authored-by: Ralph Küpper <ralph@skelpo.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
One increment on #9287's computed-key residual. Runtime-only, 3 files.
The change
js_put_value_set_dyn_ic_missopened aRuntimeHandleScopeand rooted target/key/value before trying the learned shape-transition shortcut — which then opened its own scope and rooted the same operands again. On the fresh-object-construction lane (the dominant taker: every new-key write changes the receiver's shape, so no per-site IC can ever hit) that was two scopes and six roots per property write, one set protecting nothing.The shortcut now runs before the scope, via a value-returning twin of the transition fast path:
PERRY_GC_HEAP_LIMIT=8 PERRY_GC_FORCE_EVACUATE=1.Numbers
Quiet host, 20k fresh objects × 20 computed-key writes, three runs each, spread ≤ 1 ms:
o["field_" + j])bench_object_propertyStated plainly
~10%, not parity. This removes one of the two scopes; the remaining ~3× lives in the transition install machinery itself — the fast path's own scope (skippable when the key is already interned and no growth occurs), shape-version install, keys handling, spill store. That's the next increment, tracked on #9287, and it's design-shaped rather than shave-shaped.
Also for the record, three levers eliminated by measurement before this one was built: key concat+intern (~5 ms of 48 — the profile's top frame overstated it), the prototype intercept probe (one load when no descriptors exist), and an early-transition shortcut in
js_put_value_set(already present in-tree at the exact call site — found before duplicating it).Refs #9287.
Summary by CodeRabbit