perf(codegen): #6812 w8/w13 — clone inlined-helper bodies, integer static keys, first-iteration peel#6841
Merged
Merged
Conversation
added 3 commits
July 25, 2026 21:11
… integer static write keys w8: the call inliner leaves 'let x = r + i; let y = r - i;' between the element alias and the writes, which broke the whole-loop clone body match. Admit a leading run of immutable, unboxed numeric temps parsed in the same counter-expression grammar; write values resolve LocalGet(temp) by substitution, so the emitter and finite-range proof see inline-equivalent trees. Ineligible temps reject the loop (never skipped). w13: a constant integer key is the canonical numeric-string property key. static_write_key (write PIC) and the clone matcher's property extraction accept Expr::Integer via i64 formatting; array/typed-array receivers stay safe (PIC miss handler and clone preflight both require REGULAR heap objects, so they fall to the generic write). Claude-Session: https://claude.ai/code/session_01QJ5mwMDPc63tNLAFPdthAG
…the write nest A first-write loop appends its key to every receiver (shape transition), so the preflight taken before any iteration rejects the whole nest and every round runs generically. Peel round #1 through the ordinary lowering (exact source semantics, cannot re-enter versioning), then version the remaining [start+1, bound) rounds — the guard now sees the primed shapes and the clone covers 99.9% of the writes. Cost when the guard would have passed anyway: one ordinary round. Claude-Session: https://claude.ai/code/session_01QJ5mwMDPc63tNLAFPdthAG
Contributor
Author
|
Follow-up evidence on the final tip:
|
📝 WalkthroughWalkthroughThe object-array write codegen matcher resolves immutable numeric temps, recognizes integer property keys canonically, and peels the first outer iteration before versioning. Changelog and performance matrix documentation reflect the expanded whole-loop clone coverage and updated measurements. ChangesObject-write codegen
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant ObjectWriteLoopMatcher
participant NumericExpressionMatcher
participant StoreMatcher
participant VersionedLoopLowerer
ObjectWriteLoopMatcher->>NumericExpressionMatcher: Parse and substitute immutable numeric temps
ObjectWriteLoopMatcher->>StoreMatcher: Match remaining writes and integer keys
StoreMatcher->>NumericExpressionMatcher: Parse substituted write values
VersionedLoopLowerer->>ObjectWriteLoopMatcher: Lower the peeled first outer iteration
Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@crates/perry-codegen/src/stmt/loops.rs`:
- Around line 2245-2278: Cap the number of leading immutable numeric temps
peeled by the loop in the temp-collection logic before substitution can build
deeper expressions. Add a dedicated bound alongside
MAX_OBJECT_ARRAY_WRITE_FIELDS, reject the loop when that bound is exceeded, and
preserve the existing rejection behavior for boxed or unparsable temps.
In `@docs/object-write-matrix.md`:
- Line 40: Update the post-ratio value in the w3_mul_rhs row to match the
displayed post-change timings: 6 ms divided by 7 ms, approximately 0.86. Leave
the baseline ratio and surrounding benchmark data unchanged.
🪄 Autofix (Beta)
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: 9d35b146-536a-4e56-9884-55ed57942662
📒 Files selected for processing (4)
changelog.d/6812-w8-temps-w13-intkeys.mdcrates/perry-codegen/src/expr/proxy_reflect.rscrates/perry-codegen/src/stmt/loops.rsdocs/object-write-matrix.md
… ratio typo Cap the temp run at 8 and every parsed tree (temps and store values) at 64 nodes, counted iteratively with early exit — chained substitution (let b = a + a; let c = b + b) compounds tree size per level, and the range/emit walkers recurse over these trees. Claude-Session: https://claude.ai/code/session_01QJ5mwMDPc63tNLAFPdthAG
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)
docs/object-write-matrix.md (1)
52-52: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winReconcile the w16 timing and ratio with the post-peel result.
This row reports
4163 → 3ms and0.26, but its own note says the peel changes3 → 6ms. The PR objective also reports the final w16 result as 6 ms. If the displayed 29 ms node baseline is used, the post-peel ratio is approximately6/29 = 0.21, not0.26; otherwise, expose the separate baseline used for the ratio.🤖 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 `@docs/object-write-matrix.md` at line 52, Update the w16_overflow_slot row in the object-write performance matrix to consistently report the post-peel 6 ms result and its corresponding ratio, approximately 0.21 against the displayed 29 ms node baseline. If retaining 0.26, explicitly identify the separate baseline used for that calculation.
🤖 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 `@docs/object-write-matrix.md`:
- Line 52: Update the w16_overflow_slot row in the object-write performance
matrix to consistently report the post-peel 6 ms result and its corresponding
ratio, approximately 0.21 against the displayed 29 ms node baseline. If
retaining 0.26, explicitly identify the separate baseline used for that
calculation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3f648a05-a032-4e53-b9a3-22fd6f0b2a0d
📒 Files selected for processing (2)
crates/perry-codegen/src/stmt/loops.rsdocs/object-write-matrix.md
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/perry-codegen/src/stmt/loops.rs
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.
Summary
Third slice of the #6812 beat-node campaign, widening the whole-loop write clone to the shapes real code produces. The scoreboard moves to 11 of 18 matrix rows beating node.
1. Inliner temp
lets in the clone matcher — w8: 47 ms → 6 ms, 0.75× (beats node)The call inliner already inlines write helpers (
setCD(objs[i], r + i, r - i)), but leaveslet x = r + i; let y = r - i;between the element alias and the writes — which broke the clone matcher's body shape, stranding every inlined-helper loop on the per-write path. The matcher now admits a leading run of immutable, unboxed numeric temps parsed in the same counter-expression grammar; write values resolveLocalGet(temp)by substitution, so the emitter and the finite-range proof see the trees the user could have written inline (recomputing a pure numeric expression is unobservable). An ineligible temp rejects the loop — statements are never skipped.2. Constant integer keys are static write keys —
o[7]≡ property"7"static_write_key(write PIC) and the clone matcher's property extraction acceptExpr::Integervia i64 formatting (canonical for every integer, including negatives). Array/typed-array receivers stay safe: the PIC miss handler and the clone preflight both require REGULAR heap objects, so real arrays fall to the generic write, which performs the element store (covered in sanity).3. Peel outer iteration #1 before versioning — first-write appends stop vetoing the nest
A loop whose write introduces a new key (
o[7] = von{a,b,c}objects) appends that key to every receiver — a shape transition — so a preflight taken before any iteration rejected with "target key is absent from the shared shape" and the entire nest ran generically. The first outer round is now peeled through the ordinary lowering (exact source semantics; callslower_for_after_initdirectly so it cannot re-enter versioning), then the guard proves the remaining[start+1, bound)rounds. Within-capacity append variant: 5 ms vs node 12 ms (0.42×, beats node).Measured (release, default pipeline, node v26.3.0 arm64, 3 alternated runs, identical checksums)
Known trade: the unconditional peel costs w16 one ordinary outer round (its fallback writes are the slow ones). Reclaimable with a guard-first second-chance layout; planned together with the object-owned spill slice, which is also what the canonical w13 cell (append past inline capacity → overflow side-table) needs.
Full 18-cell matrix: no other row moved; all previous beat-node rows hold.
Semantics
New
intkey_temps_sanity.ts(10 cases): numeric-key enumeration order (7beforea), negative/huge/fractional keys, helper-fn temps, any-typed array receiver (element write,length,Array.isArray), temp-reads-alias rejection fallback, typed-array receiver (Uint8Array clamping), literal-declared integer key overwrite, closure-captured temp (boxed → rejected → fallback), mutable temp rejection — byte-identical vs node, plus the three existing sanity batteries (empty-site, builder-fold, mul-index) all byte-identical on this build.perry-hir/codegen/runtime suites green (serial protocol); gap gate results in follow-up comment.
Also refreshes the stale w3/w4/w5/w7 rows in
docs/object-write-matrix.md(they beat node since #6830 but the doc still said GAP).Refs #6812.
https://claude.ai/code/session_01QJ5mwMDPc63tNLAFPdthAG
Summary by CodeRabbit
Performance Improvements
Documentation