perf(codegen,runtime): #6011 — packed-f64 range-loop versioning: EMA loop at Node parity (9.3x)#6033
Conversation
…ar-bound multi-array loops run inline
A tight float loop (`for (i = 1; i < N; i++) ema[i] = prices[i]*a +
ema[i-1]*b`) ran ~9x slower than Node: the existing packed-f64 loop
versioning only matched `i < arr.length` bounds on a single array with
exact-`i` indices, so every access went through generic runtime calls
(reads probing four registries per element, stores paying the write
barrier's thread-local preamble per element).
Codegen:
- new range-loop matcher + lowering (stmt/loops.rs): scalar loop-invariant
bounds (`i < N`), multiple arrays per loop, affine `i±c` indices; one
range guard per array at loop entry, hole-checked inline loads with a
side exit to the slow loop
- array-kind facts: seed params and `new Array<number>(n)` locals
(Generic{"Array"} type spelling + New{Array} initializers); facts stay
versioning hints — every fast loop is runtime-guard validated
- `arr[i±c]` offset support in the fast-loop index get/set lowerings
Runtime:
- js_typed_feedback_packed_f64_range_loop_guard: kind/shape/frozen checks
+ static index-range bounds proof + hole-tolerant slot canonicalization
(rebuild_array_numeric_raw_f64_allow_holes)
- GC_ARRAY_RAW_F64_HOLES header bit: `new Array(n)` is raw-f64-or-holes
by construction; the invariant is maintained by the store choke points
and makes the guard's verify walk O(1) (also O(1)-fails the per-store
dense-layout probe while a fill is in flight — the walk there was
O(N²) across a sequential fill)
- write barrier: primitive-child early-exit before the incremental-mark
TLS probe; decode_heap_addr fast-rejects f64 payload bit patterns
before the page-map lookup
EMA benchmark (100k elems × 100 iters): 55.6 → 5.9-6.1 ns/elem — at/below
node v26 (6.0-6.1). Verification: 22-case loop matrix byte-identical to
node (holes, guard-failure fallbacks, param arrays, offset reads);
packed_f64_loop_versioning fixtures green; parity --filter array
unchanged (49 pass, 1 pre-existing failure); cargo test -p perry-runtime
1155/1155 serial; the one perry-codegen failure
(pod_field_read_after_dynamic_materialization_uses_number_coerce) is
pre-existing on main (#6015 revert left tests/native_proof_regressions.rs
uncompilable; this branch restores its compilation).
Known follow-up: an array that later appears as an array-method receiver
(`arr.filter(...)`) is currently rejected by the range matcher, so
fill-then-chain code takes the per-access guarded path (~1.8x slower than
the legacy call on that phase). Tracked for a follow-up relaxation.
Closes #6011
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR adds a hole-tolerant packed-f64 range-loop fast path spanning compiler and runtime layers: fact collection threads function/method params for array-kind seeding, index get/set lowering supports offset-based loop indices (i±c), a new range-versioned for-loop matcher and lowerer emits typed-feedback-guarded fast copies, runtime array headers track a new holes-tolerant raw-f64 layout flag, and a new typed-feedback FFI guard validates range-bounded packed-f64 loops. A GC write-barrier decode optimization and Module test-fixture field initialization are also included. ChangesPacked-f64 range-loop guard optimization
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Codegen as loops.rs (for-loop lowering)
participant Matcher as match_packed_f64_range_loop
participant TypedFeedback as js_typed_feedback_packed_f64_range_loop_guard
participant ArrayHeader as header.rs (rebuild_array_numeric_raw_f64_allow_holes)
participant FastLoop as Compiled fast copy loop
Codegen->>Matcher: attempt match on for-loop body
Matcher-->>Codegen: matched bound/array accesses (min/max offsets)
Codegen->>TypedFeedback: emit guard call per array (min_idx, max_idx_exclusive)
TypedFeedback->>ArrayHeader: rebuild_array_numeric_raw_f64_allow_holes(arr)
ArrayHeader-->>TypedFeedback: dense or holes-tolerant flag set / reject
TypedFeedback-->>Codegen: guard pass/fail (1/0)
alt guard passes
Codegen->>FastLoop: run packed-f64 fast copy with allow_holes fact
FastLoop-->>Codegen: hole detected mid-loop side-exits to slow path
else guard fails
Codegen->>Codegen: fall back to slow JS-semantics loop
end
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 |
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 (2)
crates/perry-codegen/src/codegen/method.rs (1)
330-340: 🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick winThread function params into native-fact collection here
collect_native_region_fact_graphseeds packed-array facts fromparams, butcompile_methodandcompile_static_methodstill pass&[], so array params likeprices: number[]never reach the new range-loop matcher.compile_closurehas the same omission.🤖 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/codegen/method.rs` around lines 330 - 340, The native-fact collection call is not receiving the method’s parameters, so packed-array facts for array-typed params never get seeded. Update `compile_method`, `compile_static_method`, and `compile_closure` to thread the actual params through to `collect_native_region_fact_graph` instead of passing `&[]`, using the existing `method`/closure parameter data so the new range-loop matcher can see array parameters like `prices: number[]`. Keep the change localized to the call sites around `native_facts` and ensure the collector gets the same params source used by the method compilation path.crates/perry-codegen/src/codegen/closure.rs (1)
704-716: 🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick winForward the actual params into native-fact collection
seed_paramsis never exercised here:collect_native_region_fact_graphis called with&[]incodegen/function.rs,codegen/method.rs,codegen/entry.rs, andcodegen/closure.rs, so packed-array params never seed the fast path anywhere. If the new packed-f64 matcher should apply to declared array params, thread the real param slice through these call sites.🤖 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/codegen/closure.rs` around lines 704 - 716, The native-fact collection call is still hardcoded with an empty seed-params slice, so declared array params never reach the fast path. Update the call sites of collect_native_region_fact_graph in codegen/function.rs, codegen/method.rs, codegen/entry.rs, and this closure.rs path to pass the real parameter slice instead of &[], using the existing seed_params/declared params data so the packed-f64 matcher can see them.
🧹 Nitpick comments (4)
crates/perry-codegen/src/expr/index_set.rs (1)
192-215: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider hoisting the twin helpers to eliminate cross-file duplication.
packed_f64_loop_fact_for_index,load_packed_loop_index_i32,packed_f64_loop_fact, andnumeric_index_has_loop_array_index_proofare now byte-for-byte duplicated betweenindex_set.rsandindex_get.rs(onlypacked_f64_loop_index_partswas shared via themod.rsre-export). The comment already calls out "the twin helper inindex_get.rs," which acknowledges the divergence risk. Hoisting these into the parentmod.rs(likepacked_f64_loop_index_parts) keeps the hole-tolerant offset semantics defined once.🤖 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_set.rs` around lines 192 - 215, The helper logic for packed loop index handling is duplicated between the index_set and index_get paths, which creates divergence risk for the hole-tolerant offset behavior. Hoist the shared helpers into the parent module alongside packed_f64_loop_index_parts, and have both packed_f64_loop_fact_for_index, load_packed_loop_index_i32, packed_f64_loop_fact, and numeric_index_has_loop_array_index_proof call the single shared implementation instead of keeping twin copies in index_set.rs and index_get.rs.crates/perry-codegen/src/expr/index_get.rs (1)
141-143: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winShare the packed-f64 offset cap constant
64matchesPACKED_F64_RANGE_LOOP_MAX_OFFSETincrates/perry-codegen/src/stmt/loops.rs. Hoist that cap into a shared constant (or re-export it) so the matcher and range-loop guard stay in sync.🤖 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 141 - 143, The packed-f64 offset limit is duplicated in the index-get matcher, which can drift from the range-loop guard. Replace the hardcoded `64` in `IndexGet` handling with the shared `PACKED_F64_RANGE_LOOP_MAX_OFFSET` from `loops.rs`, or re-export that constant so both `index_get` and the range-loop logic use the same source of truth.crates/perry-codegen/src/type_analysis/pod.rs (1)
388-407: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNear-duplicate of
type_has_numeric_pointer_free_array_layoutinhelpers.rs.
type_has_numeric_pointer_free_array_layout_for_fallbackhas an identical match shape (Array/Generic-Array/Tuple/Union over Number|Int32) toexpr/helpers.rs'stype_has_numeric_pointer_free_array_layout. This PR itself had to add the same newGeneric { base: "Array", .. }arm to both in lockstep — exactly the kind of drift risk duplication creates. Consider extracting the shared predicate into one function (e.g. intype_analysis) that both call sites use, parameterized if the two ever need to diverge.Sketch
-fn type_has_numeric_pointer_free_array_layout_for_fallback(ty: &HirType) -> bool { - match ty { - HirType::Array(elem) => matches!(elem.as_ref(), HirType::Number | HirType::Int32), - HirType::Generic { base, type_args } if base == "Array" && type_args.len() == 1 => { - matches!(type_args[0], HirType::Number | HirType::Int32) - } - HirType::Tuple(elems) => elems - .iter() - .all(|elem| matches!(elem, HirType::Number | HirType::Int32)), - HirType::Union(variants) => variants.iter().all(|variant| { - matches!(variant, HirType::Null | HirType::Void | HirType::Never) - || type_has_numeric_pointer_free_array_layout_for_fallback(variant) - }), - _ => false, - } -} +use crate::expr::helpers::type_has_numeric_pointer_free_array_layout as type_has_numeric_pointer_free_array_layout_for_fallback;🤖 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/type_analysis/pod.rs` around lines 388 - 407, The predicate in type_analysis/pod.rs is duplicated with the near-identical Array/Generic/Tuple/Union shape in expr/helpers.rs, so update the shared logic by extracting one reusable helper for the numeric pointer-free array layout check and have both type_has_numeric_pointer_free_array_layout_for_fallback and type_has_numeric_pointer_free_array_layout call it. Keep the existing behavior for HirType::Array, HirType::Generic with base "Array", HirType::Tuple, and HirType::Union, and place the shared implementation where both callers can use it to avoid future drift.crates/perry-codegen/src/collectors/hir_facts.rs (1)
593-616: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNo direct test coverage for the new
seed_params/param_seeded_localspath.This file's test module never calls
collect_type_facts/collect_hir_facts/collect_native_region_fact_graphwith a non-emptyparamsslice, soseed_params, theparam_seeded_localsbookkeeping, and its interaction withmark_unknown_call_escape's hazard carve-out (lines 1146-1156) are untested at this layer — only exercised indirectly (if at all) through end-to-end fixtures elsewhere in the stack.A helper like
param(id, name, ty)already exists incrates/perry-codegen/tests/native_proof_regressions.rsfor constructingParamfixtures; a unit test here confirming e.g. anumber[]param staysPackedF64/eligible after an in-boundsarr[i]=write but loses eligibility after an unknown call would lock in this subtle new invariant.Want me to draft these unit tests?
🤖 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/collectors/hir_facts.rs` around lines 593 - 616, Add direct unit coverage for the new ArrayFactCollector::seed_params and param_seeded_locals behavior by exercising collect_type_facts/collect_hir_facts/collect_native_region_fact_graph with a non-empty params slice. Create a test that uses a packed numeric array parameter (for example a number[]-style Param) and verifies it is seeded as a local kind, stays eligible after an in-bounds mutation, and then loses eligibility after the unknown-call path that goes through mark_unknown_call_escape. Use the existing Param fixture helper and assert the interaction with the param_seeded_locals carve-out so this new invariant is covered at this layer.
🤖 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 `@crates/perry-codegen/src/codegen/closure.rs`:
- Around line 704-716: The native-fact collection call is still hardcoded with
an empty seed-params slice, so declared array params never reach the fast path.
Update the call sites of collect_native_region_fact_graph in
codegen/function.rs, codegen/method.rs, codegen/entry.rs, and this closure.rs
path to pass the real parameter slice instead of &[], using the existing
seed_params/declared params data so the packed-f64 matcher can see them.
In `@crates/perry-codegen/src/codegen/method.rs`:
- Around line 330-340: The native-fact collection call is not receiving the
method’s parameters, so packed-array facts for array-typed params never get
seeded. Update `compile_method`, `compile_static_method`, and `compile_closure`
to thread the actual params through to `collect_native_region_fact_graph`
instead of passing `&[]`, using the existing `method`/closure parameter data so
the new range-loop matcher can see array parameters like `prices: number[]`.
Keep the change localized to the call sites around `native_facts` and ensure the
collector gets the same params source used by the method compilation path.
---
Nitpick comments:
In `@crates/perry-codegen/src/collectors/hir_facts.rs`:
- Around line 593-616: Add direct unit coverage for the new
ArrayFactCollector::seed_params and param_seeded_locals behavior by exercising
collect_type_facts/collect_hir_facts/collect_native_region_fact_graph with a
non-empty params slice. Create a test that uses a packed numeric array parameter
(for example a number[]-style Param) and verifies it is seeded as a local kind,
stays eligible after an in-bounds mutation, and then loses eligibility after the
unknown-call path that goes through mark_unknown_call_escape. Use the existing
Param fixture helper and assert the interaction with the param_seeded_locals
carve-out so this new invariant is covered at this layer.
In `@crates/perry-codegen/src/expr/index_get.rs`:
- Around line 141-143: The packed-f64 offset limit is duplicated in the
index-get matcher, which can drift from the range-loop guard. Replace the
hardcoded `64` in `IndexGet` handling with the shared
`PACKED_F64_RANGE_LOOP_MAX_OFFSET` from `loops.rs`, or re-export that constant
so both `index_get` and the range-loop logic use the same source of truth.
In `@crates/perry-codegen/src/expr/index_set.rs`:
- Around line 192-215: The helper logic for packed loop index handling is
duplicated between the index_set and index_get paths, which creates divergence
risk for the hole-tolerant offset behavior. Hoist the shared helpers into the
parent module alongside packed_f64_loop_index_parts, and have both
packed_f64_loop_fact_for_index, load_packed_loop_index_i32,
packed_f64_loop_fact, and numeric_index_has_loop_array_index_proof call the
single shared implementation instead of keeping twin copies in index_set.rs and
index_get.rs.
In `@crates/perry-codegen/src/type_analysis/pod.rs`:
- Around line 388-407: The predicate in type_analysis/pod.rs is duplicated with
the near-identical Array/Generic/Tuple/Union shape in expr/helpers.rs, so update
the shared logic by extracting one reusable helper for the numeric pointer-free
array layout check and have both
type_has_numeric_pointer_free_array_layout_for_fallback and
type_has_numeric_pointer_free_array_layout call it. Keep the existing behavior
for HirType::Array, HirType::Generic with base "Array", HirType::Tuple, and
HirType::Union, and place the shared implementation where both callers can use
it to avoid future drift.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ae7bbf28-40cb-4f77-a625-ec05c22f1c1d
📒 Files selected for processing (22)
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/hir_facts.rscrates/perry-codegen/src/expr/helpers.rscrates/perry-codegen/src/expr/index_get.rscrates/perry-codegen/src/expr/index_set.rscrates/perry-codegen/src/expr/mod.rscrates/perry-codegen/src/runtime_decls/objects.rscrates/perry-codegen/src/stmt/loops.rscrates/perry-codegen/src/type_analysis/numeric.rscrates/perry-codegen/src/type_analysis/pod.rscrates/perry-codegen/tests/native_proof_regressions.rscrates/perry-runtime/src/array/alloc.rscrates/perry-runtime/src/array/header.rscrates/perry-runtime/src/array/mod.rscrates/perry-runtime/src/array/tests.rscrates/perry-runtime/src/gc/barrier.rscrates/perry-runtime/src/gc/types.rscrates/perry-runtime/src/typed_feedback.rscrates/perry-runtime/src/typed_feedback/tests.rs
Closes #6011.
Problem
The issue's EMA loop —
for (let i = 1; i < N; i++) ema[i] = prices[i] * alpha + ema[i - 1] * (1 - alpha)— ran ~9x slower than Node (55.6 vs 6.0 ns/elem measured; the issue reports 7x). Perry's existing packed-f64 loop versioning only matchesi < arr.lengthbounds, one array per loop, exact-iindices — the EMA loop (scalar boundi < N, two arrays, ani-1read) fell through to generic per-element runtime calls: reads probing four registries per element, stores paying the write barrier's thread-local preamble per element.Fix
Codegen — a new range versioned-loop path alongside the existing length-bound one:
i < Nwith N a const local or literal), multiple arrays per loop, affinei±cindices[start+min_offset, bound+max_offset) ⊆ [0, length)), then fully inline loads/stores in the fast loop; loads are hole-checked with a side exit to the identical slow loopnew Array<number>(n)locals (theGeneric{"Array"}type spelling andNew{Array}initializers previously classified Unknown). Facts remain versioning hints — every fast loop is validated by its runtime guard, so a caller passing a holey/exotic array just runs the slow loop.Runtime:
js_typed_feedback_packed_f64_range_loop_guard+rebuild_array_numeric_raw_f64_allow_holes: hole-tolerant slot canonicalization (numeric slots → raw f64,TAG_HOLEleft in place for the fast loop's hole checks)GC_ARRAY_RAW_F64_HOLESheader invariant bit:new Array(n)is raw-f64-or-holes by construction; maintained at the store choke points, it makes the guard's verify walk O(1) — and O(1)-fails the per-store dense-layout probe mid-fill, which otherwise walked the filled prefix per store (O(N²) across a sequential fill)decode_heap_addrfast-rejects ordinary f64 payload bits before the page-map lookup (both dominated tight numeric store loops)Results (M-series macOS, release;
nodev26.3.0)A hoisted-buffer variant (no per-call
new Array(100k)) measures Perry 4.7 vs Node 5.6 ns/elem — the pure loop beats V8; the remaining cost in the issue's shape is the allocation churn both engines pay.Verification
new Array()+ push) — byte-identical to Node, and byte-identical to pre-change Perry on the one pre-existing deviation (typed-number[]lowering of a lyinganyelement).packed_f64_loop_versioning+_negativefixture harness gates all pass (checksums, IR checks, no unapproved vectorization).run_parity_tests.sh --filter array: 49 pass / 1 fail — identical to the pre-change report; the failure (test_compat_arrays_iterators,String(array)→ "undefined") reproduces on the pristine branch base (fix: revert the #5874 payload swept onto main — restore 322 test262 regressions #6015-revert fallout).cargo test -p perry-runtime -- --test-threads=1: 1155/1155.cargo test -p perry-codegen: all targets pass exceptpod_field_read_after_dynamic_materialization_uses_number_coerce— pre-existing: the fix: revert the #5874 payload swept onto main — restore 322 test262 regressions #6015 revert lefttests/native_proof_regressions.rsuncompilable on main (CI never sees it); this branch restores its compilation, exposing the failure.cargo fmt --check, file-size gate, and the address-classification audit all green.Known follow-up
An array that later appears as an array-method receiver (
arr.filter(...)after a fill loop) is currently rejected by the range matcher, so fill-then-chain code takes the per-access guarded path (~1.8x slower than the legacy call on that phase — ~18.5 vs ~10 ms/iter on a 100k filter-map-reduce). Tracked for a follow-up relaxation of the receiver-escape condition.Note: per maintainer convention, no version bump / CHANGELOG in this PR.
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Array<number>andArray<Int32>forms.Bug Fixes