perf(codegen): eliminate per-access array-read guard calls in hot masked-index loops - #6750
Merged
proggeramlug merged 3 commits intoJul 22, 2026
Merged
Conversation
…ked-index loops Array element reads in tight loops compiled to one runtime guard call per access (js_typed_feedback_numeric_array_index_get_guard and the typed-array getters), making integer/array-heavy JS 10-15x slower than V8 even though the element load itself was already inline. bcryptjs's Blowfish S-boxes are exactly this shape, turning a login's compareSync into seconds of overhead. Three complementary fixes: 1. Static index windows for the buffer bounds prover (expr/range_facts.rs): int_range_expr now understands 'e & K' (always [0, K] for a non-negative i32 constant mask, by ToInt32 semantics), 'e >>> k' (ToUint32 result bounded by the shift), and non-negative 'a | b' windows. Width-tracked typed-array reads like S[i & 1023] / S[256 + ((x >>> 16) & 0xff)] now prove bounds against the tracked view length and take the existing unchecked inline load instead of an out-of-line getter call. An IndexGet range rule (elem-kind value range, gated on the same bounds proof) feeds the same machinery. The i32 fast path classifies proven in-bounds loads from int-element typed arrays as native i32 leaves, so bit-mixing chains stay in add/xor i32 instead of round-tripping f64 through the branchless ToInt32 tower. 2. Collector support so const typed-array lookup tables behave like ints (collectors/): const S = new Int32Array(<literal>) bindings that never escape element-access receiver position are recorded with their length; 'const a = S[x & 0xff]'-style Lets seed integer_locals (window proven inside [0, length)), and the pointer analysis knows numerically-indexed typed-array reads never yield pointers - killing the per-iteration GC shadow-slot spills and dynamic string_or_number_add dispatch the Blowfish round function was paying for. 3. Read-only DENSE tier for the PerryTS#6011 packed-f64 range loop (stmt/loops.rs, typed_feedback.rs): the versioned-loop matcher now also accepts multi-statement read-only bodies whose array reads carry static masked windows, guarded once at loop entry by a dense window guard (window must be hole-free - loads then need no hole check and no side exit, which is what makes multi-statement bodies safe to version). Two guard tiers: _dense_i32 additionally proves every window value is an i32 integer so loads materialize as a bare exact fptosi and LLVM keeps (s + S[i & k])|0 loops fully in integer registers; _dense keeps raw-double loads for float lookup tables. Read eligibility no longer consults the array-kind / materialization-hazard facts: mark_unknown_call_escape blanket-hazards every function-local array whenever the function contains any call, which kept every locally built lookup table off the fast path forever; the entry guard re-validates the actual runtime array, so a wrong static hint costs one failed guard, never correctness. Benchmarks (Apple M1-class, 20M reads / 5M Blowfish-F rounds, byte-identical outputs vs node --experimental-strip-types): node perry before perry after arith-only (20M) 30 ms 26 ms 26 ms plain-Array read 21 ms 200 ms 4 ms Int32Array read 16 ms 235 ms 10 ms Blowfish-F mimic 18 ms 558 ms 15 ms An edge-case suite (OOB windows, holes, fractional/huge/NaN elements, frozen arrays, string elements, shrunk length, mixed counter+masked indices, all int TA kinds, escape shapes) is byte-identical between this branch and origin/main; the four pre-existing divergences from node it found are unrelated to this change and reproduce on main.
|
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)
📝 WalkthroughWalkthroughChangesThe change adds dense-window and dense-i32 guards for packed-f64 range loops, expands typed-array integer and range analysis, records masked-window facts, and lowers proven indexed loads directly. Compilation contexts now initialize the new fact storage. Dense-window packed-f64 range loops
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant LoopMatcher
participant Codegen
participant TypedFeedback
participant ArrayHeader
participant FastLoop
LoopMatcher->>Codegen: collect dense access windows
Codegen->>TypedFeedback: emit dense or dense-i32 guard
TypedFeedback->>ArrayHeader: validate numeric window
ArrayHeader-->>TypedFeedback: return validation result
TypedFeedback-->>Codegen: provide guard outcome
Codegen->>FastLoop: lower masked-window indexed loads
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
…window.rs The masked-window fact consult + the two load-emission tiers pushed expr/index_get.rs past the 2000-line lint cap (check_file_size.sh); move them to a topical sibling module. Pure move plus factoring the shared raw in-window load emission; no behavior change (benchmarks and the edge-case suite are byte-identical before/after).
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
crates/perry-codegen/src/expr/index_get.rs (1)
158-216: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate raw-load sequence between the f64 and i32 masked-window loaders.
lower_masked_window_index_get(170-180) andlower_masked_window_index_get_i32(250-260) emit the identical bitcast→mask→zext→shl 3→+8→+handle→inttoptr→load-double sequence. Extracting a shared helper (returning the raw f64 SSA value) would avoid the two copies diverging silently.♻️ Suggested extraction
+fn masked_window_raw_f64_load(ctx: &mut FnCtx<'_>, arr_box: &str, idx_i32: &str) -> String { + let blk = ctx.block(); + let arr_bits = blk.bitcast_double_to_i64(arr_box); + let arr_handle = blk.and(I64, &arr_bits, POINTER_MASK_I64); + let idx_i64 = blk.zext(I32, idx_i32, I64); + let byte_offset = blk.shl(I64, &idx_i64, "3"); + let with_header = blk.add(I64, &byte_offset, "8"); + let element_addr = blk.add(I64, &arr_handle, &with_header); + let element_ptr = blk.inttoptr(I64, &element_addr); + blk.load(DOUBLE, &element_ptr) +}Then both
lower_masked_window_index_getandlower_masked_window_index_get_i32callmasked_window_raw_f64_load(ctx, arr_box, idx_i32)instead of repeating the sequence inline.Also applies to: 232-299
🤖 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/index_get.rs` around lines 158 - 216, Extract the duplicated raw masked-window load sequence into a shared helper named masked_window_raw_f64_load, returning the raw f64 SSA value. Update both lower_masked_window_index_get and lower_masked_window_index_get_i32 to call this helper with ctx, arr_box, and idx_i32, while preserving their existing value recording and metadata 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.
Nitpick comments:
In `@crates/perry-codegen/src/expr/index_get.rs`:
- Around line 158-216: Extract the duplicated raw masked-window load sequence
into a shared helper named masked_window_raw_f64_load, returning the raw f64 SSA
value. Update both lower_masked_window_index_get and
lower_masked_window_index_get_i32 to call this helper with ctx, arr_box, and
idx_i32, while preserving their existing value recording and metadata behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 41cc2a8b-2778-42b3-ac2b-4d98b112a172
📒 Files selected for processing (16)
crates/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/collectors/integer_locals.rscrates/perry-codegen/src/collectors/mod.rscrates/perry-codegen/src/collectors/pointer_locals.rscrates/perry-codegen/src/expr/i32_fast_path.rscrates/perry-codegen/src/expr/index_get.rscrates/perry-codegen/src/expr/mod.rscrates/perry-codegen/src/expr/range_facts.rscrates/perry-codegen/src/runtime_decls/objects.rscrates/perry-codegen/src/stmt/loops.rscrates/perry-runtime/src/array/header.rscrates/perry-runtime/src/array/mod.rscrates/perry-runtime/src/typed_feedback.rs
…plit module The module split in the previous commit missed the second consult site (the generic lower IndexGet arm) — it still called the moved functions unqualified and the crate did not compile. Verified with a from-scratch build this time; benchmarks and the edge suite are unchanged.
proggeramlug
pushed a commit
that referenced
this pull request
Jul 22, 2026
…ray params Follow-up to #6750: the dense masked-index range-loop versioning only fired for bindings whose STATIC type proved a number array, so an array arriving as an untyped (any) function parameter — the bcryptjs Blowfish S-box shape — kept paying one guard call per access (~40x slower than Node on S[i & 1023] loops). Two changes: 1. Matcher: read-only DENSE accesses also admit bindings with no usable static type (any/unknown/untracked). Safe because the entry guards re-validate the actual runtime value; a wrong hint costs one failed guard -> slow loop. Untyped plain-Array params now version through the existing dense_i32/dense plain-array tiers unchanged. 2. New typed-array tiers ahead of the plain tiers, emitted only when at least one accessed binding is untyped and every access carries a static window: one O(1) probe per array (js_typed_feedback_masked_window_ta_kind: registry lookup + window/length compare, detached views rejected via length 0) classifies the receiver as Int32Array / Uint32Array / Float64Array; all arrays agreeing on a kind branches into a fast copy whose loads go through the preheader-hoisted element-0 data pointer (js_typed_array_masked_window_data_ptr) with the width-correct inline load: i32 loads stay native i32 in bit-mixing chains (values_i32), u32 materializes unsigned (uitofp, never i32-classified), f64 loads directly. The hoisted pointer is stable because the fast copy is call-free (no allocation -> no GC) and view backings are thread-lifetime allocations.
7 tasks
proggeramlug
added a commit
that referenced
this pull request
Jul 26, 2026
…ops (#6850) * perf(codegen): lower Math.imul and typed-array-param reads to native ops Two integer-math JS primitives were compiled as runtime calls and cascaded a whole hot integer loop into slow f64 ToInt32 towers. - Math.imul(a,b) -> native `mul i32` when both operands are provable i32 (exact: the low 32 bits of the product are identical for signed/unsigned operands). Non-finite / fractional / >2^32 operands keep the js_math_imul helper so JS ToUint32/ToInt32 semantics are preserved. Because `Math.imul` is defined as ToInt32(ToUint32*ToUint32 mod 2^32), an integer-literal operand is exact under low-32 truncation even above i32::MAX, so the imul operand check now accepts 32-bit-representable literals -- fixing the accumulator case `a = Math.imul(a, 0x9e3779b1)` (golden-ratio constant 2654435761 > i32::MAX). This relaxation is confined to Math.imul operands: a plain `x * BIGLIT | 0` evaluates its product in f64 (precision loss above 2^53), so it must stay ToInt32(f64_product), never an exact `mul i32`. - Typed-array element read through a parameter (S[i] where S: Int32Array etc. is a function parameter, in an i32/ToInt32 context) -> checked inline native load: a runtime guard (pointer + inline-storage PERRY_TA_VIEW_GUARD + kind-cache) and a header-length bounds check gate a bare width-correct load; an in-kind out-of-bounds read yields 0 (== ToInt32(undefined)); every rejected shape (view/detached/resizable backing, wrong runtime kind) defers to the new js_typed_array_read_int32 fallback. Extends the #6750 typed-array-local bare load to parameters, whose length/storage are unknown at compile time. Plain-value parameter reads still observe undefined out of bounds. On the 40M-iteration Int32Array-parameter bit-mixer both runtime-call families (js_math_imul x3, js_typed_array_get x3) reach zero and the read/multiply chain stays in native i32. Adds gap tests test_gap_math_imul_native.ts and test_gap_typedarray_param_read.ts (byte-identical to node --experimental-strip-types). * docs(changelog): key fragment to PR #6850 * fix(codegen,runtime): address CodeRabbit review on the native typed-array-param read Two Major findings on #6850: 1. Correctness — the checked typed-array-param i32 fast path gated on the receiver kind but NOT the index: `S[3.9]` in an i32/ToInt32 context lowered the index through `fptosi` (=3) and read element 3, but JS reads a fractional typed-array index as `undefined` (-> 0 in that consumer). Gate the arm on `numeric_index_has_integer_array_index_proof` (made pub(crate)) — the same integer-index proof the sibling typed-array read paths in index_get.rs use. Integer literals, `i & mask`, and i32 loop counters keep the native path. 2. Memory safety — `js_typed_array_read_int32` (cold fallback) called `js_typed_array_get`, which reads `(*ta).length` (a TypedArrayHeader field) before classifying the pointer. That fallback is reached on a wrong-kind / kind-cache miss, which includes a receiver that is NOT a typed array (a `f(S: Int32Array)` called with an arbitrary value — TS types are erased), so a non-typed-array pointer type-confused the first GC-header read. Validate the raw pointer with `lookup_typed_array_kind` (the same gate `strict_typed_array_from_raw` uses; it covers native/inline views) before any header access; a non-array receiver returns 0 (= ToInt32(undefined)). Gap test extended with the fractional-index-in-i32-context regression cases, byte-identical to Node. --------- 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.
Summary
Array element reads (
arr[i]) in tight loops compiled to one runtime guard call per access —js_typed_feedback_numeric_array_index_get_guardfor plain arrays, and out-of-line getters (js_typed_array_get/js_typed_array_index_get_dynamic) for typed arrays. The element load itself was already inline; the per-access call was the tax, making integer/array-heavy JS 10–15× slower than V8 while plain i32 arithmetic was already at parity. bcryptjs's Blowfish S-boxes are exactly this shape (S[x >>> 24],S[0x100 | ((x >> 16) & 0xff)], ~600M reads per cost-10compareSync).This PR removes the per-access call for hot masked-index loops via three complementary changes, keeping every cold/deopt case on the existing runtime fallbacks.
1. Static index windows for the buffer bounds prover (
expr/range_facts.rs)int_range_exprnow understands the bit-mask index idioms:e & K→[0, K]for a non-negative i32 constantK— by ToInt32 semantics this holds for anye(NaN, fractional, negative, non-numeric);e >>> k→ ToUint32 result bounded by the shift amount;a | b/a & bwindows (ones-cover bound).Width-tracked typed-array reads like
S[i & 1023]andS[256 + ((x >>> 16) & 0xff)]now prove bounds against the tracked view length and take the existing unchecked inline load (lower_typed_array_load) instead of an out-of-line getter. These ops are integral by construction, so a fractional index (which would read a named property, not an element) cannot slip through. AnIndexGetrange rule (element-kind value range, gated on the same bounds proof plus the same alias/capture checks the unchecked load requires) feeds the same analysis. The i32 fast path classifies proven in-bounds loads from int-element typed arrays (I8/U8/U8Clamped/I16/U16/I32 — not U32, which doesn't round-trip through a signed i32 slot) as native i32 leaves, so bit-mixing chains stay inadd/xor i32.2. Const typed-array lookup tables behave like integers (
collectors/)const S = new Int32Array(<literal len>)bindings that never escape element-access receiver position are recorded with their length;const a = S[x & 0xff]-style Lets (window statically inside[0, len)) seedinteger_locals, with a matching exemption in the provenance judge. In-bounds int-TA loads are integers by construction; out-of-bounds (→undefined) is excluded by the same window proof.const a/b/c/dlocals.Together these turn
x = (((a + b) ^ c) + d) | 0from 4 dynamic-dispatch calls + ~13 shadow-slot calls per iteration into a straight-line i32 chain.3. Read-only DENSE tier for the #6011 packed-f64 range loop (
stmt/loops.rs, runtime)The versioned-loop matcher now also accepts multi-statement read-only bodies whose plain-array reads carry static masked windows, guarded once at loop entry:
js_typed_feedback_packed_f64_range_loop_guard_dense{,_i32}: same plain-array/window validation as the simple EMA (Exponential Moving Average) loop over a 100k-element array runs **7x slower** in the compiled Perry binary (2300ms) compared to tsx (328ms) or node (343ms). #6011 range guard, but the window must be hole-free — the fast loop's loads then carry no hole check and no side exit, which is what makes multi-statement bodies safe to version (a mid-iteration side exit could double-apply earlier statement effects on re-execution; instead, an iteration runs entirely in one copy)._dense_i32additionally proves every window value is an i32-representable integer, so loads materialize with a bare exactfptosiand LLVM keeps(s + S[i & k]) | 0loops fully in integer registers (and vectorizes them);_densekeeps raw-double loads for float lookup tables. Either guard failing falls through (i32 → f64 → slow).mark_unknown_call_escapeblanket-hazards every function-local tracked array whenever the function contains any call (even aconsole.logafter the loop), which kept every locally built lookup table (const S: number[] = new Array(1024)+ fill loop — the S-box shape) off the fast path forever. The entry guard re-validates the actual runtime array — plain-array shape, raw-f64 packedness, frozen/descriptor/prototype-pollution state, and the whole index window — and the matched body admits no store/call/closure/await, so a wrong static hint costs one failed guard → slow loop, never correctness.Measured results
Apple M1-class,
perry-devprofile,PERRY_NO_AUTO_OPTIMIZE=1, outputs byte-identical tonode --experimental-strip-typesin every run:ArrayreadS[i & 1023]Int32ArrayreadS[i & 1023]Validation
./scripts/run_gap_tests.sh, release build): gate OK — 385 pass / 6 output mismatches / 1 crash, all 7 already triaged inknown_failures.json(zlib params, defineProperty-on-class-prototype, iterator helpers, perf_hooks, stream tee tick parity, v8 module, SIGINT trace — none array/loop related), 98.2% parity, no new untriaged failures.as any, frozen arrays, shrunklength, mixed counter+masked indices, all int TA kinds,256 | (i & 255)windows, negative-operand masks, escape shapes, TA OOB/negative indices): byte-identical between this branch andorigin/main. The four divergences from node it surfaced (string-literal coercion into a declarednumber[], string store throughas anycoercing to NaN, TA out-of-boundsundefinedpropagating through numeric adds asundefined/NaN) all reproduce identically on unpatchedorigin/main— pre-existing, not introduced here.perry-runtimetyped_feedback (49) + array (66),perry-codegenlib (209),native_proof_regressions(242),native_proof_buffer_views(29/30),shadow_slot_hygiene(9) — the two failures observed (native_owned_uint8array_get_fallback_uses_uint8array_helper,typed_feedback_trace_dump_runs_before_entry_return, plus thei64_spec_ternary_recursionfixture that doesn't compile) fail identically on unpatchedorigin/main.test_gap_*{array,loop,index,sort,typed}*with the dev build: no regressions vs baseline.Not covered (follow-up)
Real bcryptjs
compareSyncis still slow (~3.9 s vs node 72 ms, unchanged from baseline): its_encipherreceives the S/P boxes as untyped params (built viaS_ORIG.slice()in plain JS, so no declared types for the matcher), uses awhileloop, and increments the round index inside an index expression (P[++i]). Closing that needs typed-feedback-driven versioning for untyped parameters pluswhile-loop and impure-index support in the dense matcher — worth its own issue/PR.Summary by CodeRabbit
Performance Improvements
Reliability