perf(codegen/runtime): #6812 — key-table clone lane: w12 beats node 6× - #6902
Merged
Conversation
added 3 commits
July 27, 2026 10:32
… clone tier) The matcher recognizes a single-group body whose sole store's key is K[idx] for a loop-invariant local K and an integer-valued, range-proven index expression (Mod joins the numeric grammar with a [0, c-1] range for nonnegative dividends and constant positive divisors). The preflight wrapper guard validates K (live array, length covers the proven range, entries are strings), materializes each entry to a stable heap string (rooted across SSO materialization), and reuses object_array_numeric_write_slots verbatim for the receiver-prefix proof — spill lanes included — writing resolved lanes into a stack table. The nest indexes the table with the range-proven expression (no bounds check needed) and stores through the same inline/spill shape as compile-time lanes. Rotating-key writes over a constant key array now run call-free. Claude-Session: https://claude.ai/code/session_01QJ5mwMDPc63tNLAFPdthAG
…e lane frem lowers to an fmod library call on AArch64 (~10ns per element). Index expressions are integer-proven, so emit them from the counters' native i32 registers as i64 add/sub/mul/srem — srem equals JS % on the proven nonnegative-dividend positive-divisor domain — and index the slot table directly, no float round-trip. Claude-Session: https://claude.ai/code/session_01QJ5mwMDPc63tNLAFPdthAG
|
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 (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a table-driven fast lane for dense object-array writes, supporting invariant string key tables and integer index expressions including modulo. Runtime preflight resolves table keys to slots, and codegen emits resolved inline or spill stores. ChangesKey-table object-array write lane
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant ObjectArrayWriteMatcher
participant lower_object_array_write_versioned_for
participant js_object_array_keytable_write_guard
participant FastLoop
ObjectArrayWriteMatcher->>lower_object_array_write_versioned_for: match o[K[idx]] = v
lower_object_array_write_versioned_for->>js_object_array_keytable_write_guard: validate table and resolve slots
js_object_array_keytable_write_guard-->>lower_object_array_write_versioned_for: return encoded slot lanes
lower_object_array_write_versioned_for->>FastLoop: compute i64 index and emit slot-based store
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 |
![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 (3)
crates/perry-codegen/src/stmt/loops.rs (3)
3054-3063: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDrop the dead
lanebinding.
laneis only consumed bylet _ = lane;; return the two derived registers directly.♻️ Proposed cleanup
- let (lane, spill_flag, slot) = { + let (spill_flag, slot) = { let blk = ctx.block(); let lane_ptr = blk.gep(I64, out_alloca, &[(I64, &idx_i64)]); let lane = blk.load(I64, &lane_ptr); let spill_flag = blk.and(I64, &lane, "32768"); let low = blk.and(I64, &lane, "32767"); let slot = blk.sub(I64, &low, "1"); - (lane, spill_flag, slot) + (spill_flag, slot) }; - let _ = lane;🤖 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/stmt/loops.rs` around lines 3054 - 3063, Remove the unused lane binding and the trailing let _ = lane statement in the block computing spill_flag and slot. Return only spill_flag and slot from the block, and update the destructuring binding accordingly while preserving their existing calculations.
3107-3117: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueTable mode still emits the group-0 element load per iteration.
In table mode
slotsis empty (decode_lanes == 0) butgroup_plansstill contains the primary group, so this block emits a dead element load/bitcast/inttoptrchain each inner iteration before thezipproduces zero lanes. It is DCE-able and correct, but skipping the primary plan whenkey_table.is_some()makes the invariant explicit rather than relying on the length-mismatchedzip.🤖 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/stmt/loops.rs` around lines 3107 - 3117, Skip the primary group plan in table mode before the per-iteration object pointer construction in the group_plans loop. Use key_table.is_some() to exclude group 0 while retaining all group plans for non-table mode, and preserve the existing zero-lane behavior without emitting the unused load/bitcast/inttoptr chain.
2451-2533: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider flattening this recognition path.
Seven levels of nested
if/if letmake the accept/reject conditions hard to audit. Extracting the table-lane recognition into a helper returningOption<KeyTableLane>(with?-style early exits) would keep the reject-vs-continue distinction explicit — note the subtle contract that a matchedIndexGetkey always terminates the store loop (t2.is_empty()) or returnsNone.🤖 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/stmt/loops.rs` around lines 2451 - 2533, Refactor the table-lane recognition inside the loop’s key handling into a dedicated helper returning Option<KeyTableLane>, using early exits for the existing eligibility checks and index/value validation. Preserve the contract that any matched IndexGet key terminates processing: return None for rejected candidates, while only accepted lanes update key_table, values, cursor, and continue; retain the surrounding key_table rejection behavior.
🤖 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 2612-2649: Reject integer-valued numeric constants outside the
exact i64 range in object_array_write_number_integer_valued before
emit_object_array_write_index_i64 is reached. Preserve the existing
finite-and-integral checks, add an exact representability check for the
constant, and ensure out-of-range values do not take the i64 emission path.
---
Nitpick comments:
In `@crates/perry-codegen/src/stmt/loops.rs`:
- Around line 3054-3063: Remove the unused lane binding and the trailing let _ =
lane statement in the block computing spill_flag and slot. Return only
spill_flag and slot from the block, and update the destructuring binding
accordingly while preserving their existing calculations.
- Around line 3107-3117: Skip the primary group plan in table mode before the
per-iteration object pointer construction in the group_plans loop. Use
key_table.is_some() to exclude group 0 while retaining all group plans for
non-table mode, and preserve the existing zero-lane behavior without emitting
the unused load/bitcast/inttoptr chain.
- Around line 2451-2533: Refactor the table-lane recognition inside the loop’s
key handling into a dedicated helper returning Option<KeyTableLane>, using early
exits for the existing eligibility checks and index/value validation. Preserve
the contract that any matched IndexGet key terminates processing: return None
for rejected candidates, while only accepted lanes update key_table, values,
cursor, and continue; retain the surrounding key_table rejection behavior.
🪄 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: b146ae3c-8043-49d5-9af2-28dadbe22615
📒 Files selected for processing (4)
changelog.d/6812-keytable-lane.mdcrates/perry-codegen/src/stmt/loops.rscrates/perry-runtime/src/proxy/put_value.rsdocs/object-write-matrix.md
An integral-but-huge double (1e19) saturates in the as-i64 lowering, so 1e19 %% 4 would srem to 3 where JS evaluates 0. The integer-valuedness proof now requires the round-trip (c as i64) as f64 == c; regression in dynkey sanity s14 ((1e19 + i) %% 3 through the fallback, byte-identical). Claude-Session: https://claude.ai/code/session_01QJ5mwMDPc63tNLAFPdthAG
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
w12 falls: 3 ms vs node ~18 ms — 6× faster than node (was 82 ms before this campaign arc; 58 ms after the dynamic-key IC in #6895). The scoreboard reaches 15 of 18 matrix rows beating node.
The slice teaches the whole-loop clone the table-driven write
o[K[idx]] = vwhereKis a loop-invariant local holding an array of strings:%joins the numeric matcher with an exact integer range ([0, c-1]for a proven-nonnegative dividend and constant positive divisor); a new integer-valuedness proof gates index expressions sofptosi/truncation can never change semantics.js_object_array_keytable_write_guard): validates the key table (live array, length covers the proven range, entries are strings), materializes each entry to a stable heap string — array/table/keys rooted across SSO materialization — and reusesobject_array_numeric_write_slotsverbatim for the receiver-prefix proof, spill lanes included, writing resolved lanes into a caller-provided stack table.srem—fremlowers to an fmod library call on AArch64, which was the entire 25 → 3 ms residual), indexes the slot table (range-proven, no bounds check), and stores through the same inline/spill shapes as compile-time lanes.Validation
dynkey_ic_sanity.tsgrew to 13 cases: the key-table adversarials cover a SHORT table (guard rejects; fallback preserves the exact TypeError timing node produces) and a non-string table entry (guard rejects; generic path byte-identical) — all byte-identical vs node v26.3.0, including underPERRY_GC_FORCE_EVACUATE=1 PERRY_GC_VERIFY_EVACUATION=1.Refs #6812.
https://claude.ai/code/session_01QJ5mwMDPc63tNLAFPdthAG
Summary by CodeRabbit
Performance Improvements
Documentation