Skip to content

arrays: element facts inside loop regions — one preheader guard, bare in-region reads (S3) - #11671

Merged
proggeramlug merged 7 commits into
mainfrom
perf-array-region-facts
Sep 29, 2026
Merged

proggeramlug merged 7 commits into
mainfrom
perf-array-region-facts

Conversation

@proggeramlug

@proggeramlug proggeramlug commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Array slice S3: array element facts inside loop regions (#11650).

What

  • Preheader guard: for an array the loop never reassigns, read as xs[e & c] or xs[c], one guard is checked in the region preheader. It covers the header guard word, the prototype facts and c < capacity.
  • In-region reads: a read inside the region is a load from a cached element base plus an inline hole select. It no longer marks the region's facts stale. The base is re-derived on the GC poll arm and at every re-check, and the verifier rejects a read if a collection can happen before it.
  • Nested regions: a per-iteration receiver read from the array (const o = objs[k & 7]) gets its body region inside the loop region.
  • Refusals: a plan that would re-check every iteration isn't formed. A failed nested body guard clears the loop's valid flag and re-checks on the next iteration.
  • lead_lit_ctl's packed-f64 tier slow copy goes through the region entry, and a redeclared let no longer loses the plan.

Results (instructions per iteration, outputs identical to node)

row main this PR
lead_lit_ctl 40 21
read1 varying 88 80–81
read4 varying 103 94
overwrite varying 76 66
addkey × ocreate varying 113 78
addkey × lit / factory varying 460 474 (+3%, tracked in #11670)
inherited × ocreate varying 246 257 (+4.5%, tracked in #11670)
tsc / Zod, n=5 — +0.04% / +0.36% (within spread); full collections unchanged

Verification

  • runtime (serial) passes; codegen 2336/0
  • parity: array 156 pass (1 pre-existing compile fail); region 6/6; typed_array_compound 1/1
  • the new test_gap_region_array_facts.ts (11 loop regions) matches node
  • sabotage: removing the capacity bound, the prototype check or the guard word each turns it red. "Static reads bare" can't be observed: the planner refuses any plan with a non-bare read.
  • the hole verifier shows 0 violations on tsc and Zod; gc-root-dominance stale 0; gc_call_effects --check, wasm abi, sso, file size and fmt pass

Summary by CodeRabbit

  • New Features
    • Added optimized handling for eligible array-element reads inside loops, including checks for array validity and safe fallback when assumptions no longer hold.
    • Array reads account for holes, prototype behavior, array changes, and garbage collection during loops.
  • Bug Fixes
    • Nested loop-body guard failures now exit the optimized region and retry when conditions become valid again.
  • Tests
    • Added regression coverage for array contents, holes, prototypes, mutations, growth, and allocations in loops.

Ralph Küpper added 6 commits September 29, 2026 12:30
A loop region (#11650) now takes an array binding the loop cannot
reassign as a receiver of its own when the body reads it as
`xs[e & c]` (or `xs[c]`). The preheader checks the S1 guard word, the
prototype facts a hole read needs and `c <u capacity` once, and derives
the element base from the same header into a slot. In F-body the read is
`load base; load [base + 8 idx]; hole ? undefined` and runs no JS, so it
no longer stales the region's other facts.

The base is an address: the loop poll's arm re-derives it from the
binding's root, and so does every re-check. The verifier judges the
emitted element reads for JS like every bare access, and separately
requires that nothing that may collect lies before a read, and that an
F-body path that collects leaves through a re-check.

When the loop body also has a per-iteration receiver read from the
array (`const o = objs[k & 7]; o.d = k; ...`), that body region is split
inside the loop region's F-body; its G-tail and fact trees' generic arms
set the loop's dirty flag. A plan that would re-check every iteration is
not formed. The packed-f64-range tier's slow copy is lowered through the
same region entry, and a `let` redeclared through `LocalSet` keeps the
region plan for its cloned initializer.
Holes, Array.prototype and own-prototype changes, an accessor element,
length shrink and growth, growth past capacity, stores, allocation in the
body, a bound past capacity, a non-array receiver and a module const,
each in the middle of a region loop, compared with node.
The earlier cases changed a fact in the middle of a loop through calls,
so no array plan formed there and sabotaging the preheader guard left
the test green. These cases keep the body call-free (one element read,
an `undefined` count), so the loop region forms (PERRY_REGION_DIAG=4
prints a plan for each), and break the fact before the loop: an
Array.prototype index over a holey array, a replaced own prototype, a
read past the capacity (JSON.parse allocates exactly 8; a literal gets
the minimum 16, read with `& 31`), a hole written by `delete`, with and
without a prototype element at that index, a length shrink and pops,
and receivers that are not arrays.

An index property on a prototype turns the array fast paths off for the
rest of the process, so the cases that set one run last; before, they
ran first and every later region guard refused.
A loop region with array facts can carry a body region split inside its
F-body (`const o = objs[k & 7]; o.d = k; ...`). Its G-tail set the
loop's dirty flag, so where the body guard fails every iteration (the
matrix addkey and inherited `varying` rows: a spilled key, an inherited
key) the array guard re-ran every iteration, +24 instructions over
main's straight-line read.

The G-tail now clears the loop's valid flag instead, and the loop runs
G-body, which keeps the nested body region (main's loop). This is the
judgement that already refuses a plan which would re-check every
iteration, made where the failure is seen. A receiver that failed only
while its layout was warming up comes back: when the body guard passes
inside G-body, it sets the dirty flag, and the loop's top tests that
flag first and re-checks. Fact trees' generic arms still only set the
dirty flag. The array verifier accepts an exit that left the region
after a collection.
@coderabbitai

coderabbitai Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 055f1c65-ede7-4094-a3b5-1248bf8936fd

📥 Commits

Reviewing files that changed from the base of the PR and between 7bb0c6f and 748c2f5.

📒 Files selected for processing (1)
  • crates/perry-codegen/src/expr/mod.rs
 __________________________________________________________________________________
< Crash early. A dead program normally does a lot less damage than a crippled one. >
 ----------------------------------------------------------------------------------
  \
   \   \
        \ /\
        ( )
      .( o ).

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 27044842-5ab4-491a-83a7-77ed2b98ffca

📥 Commits

Reviewing files that changed from the base of the PR and between b0bf0ae and 7bb0c6f.

📒 Files selected for processing (11)
  • changelog.d/11671-array-region-facts-s3.md
  • crates/perry-codegen/src/expr/index_get.rs
  • crates/perry-codegen/src/expr/index_get/guarded_array.rs
  • crates/perry-codegen/src/expr/mod.rs
  • crates/perry-codegen/src/stmt/let_stmt.rs
  • crates/perry-codegen/src/stmt/loops.rs
  • crates/perry-codegen/src/stmt/region_loop/arrays.rs
  • crates/perry-codegen/src/stmt/region_loop/bare.rs
  • crates/perry-codegen/src/stmt/region_loop/mod.rs
  • crates/perry-codegen/src/stmt/region_loop/plan.rs
  • test-files/test_gap_region_array_facts.ts

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 5 remain after this review.


📝 Walkthrough

Walkthrough

The code generator now plans statically bounded array-element reads as loop-region facts, emits guarded base-relative loads with hole handling, and coordinates validity across collection and nested-region fallback. A TypeScript regression test covers array mutations, prototypes, holes, capacity bounds, and allocation.

Changes

Array Region Facts

Layer / File(s) Summary
Plan bounded array reads
crates/perry-codegen/src/stmt/region_loop/arrays.rs, crates/perry-codegen/src/stmt/region_loop/plan.rs
The planner selects eligible array receivers and records reads with recognized static index bounds as loop-region facts. Nested-tail traversal applies separate handling to bare accesses and fact-tree nodes.
Guard and lower array reads
crates/perry-codegen/src/expr/index_get/guarded_array.rs, crates/perry-codegen/src/stmt/region_loop/arrays.rs, crates/perry-codegen/src/expr/index_get.rs, crates/perry-codegen/src/expr/mod.rs
Guards validate the receiver and array bounds. Planned reads load through saved element bases and convert holes to undefined; poll handling refreshes bases, and emitted reads are checked against collection and region-exit paths.
Coordinate loop and nested-region state
crates/perry-codegen/src/stmt/region_loop/*, crates/perry-codegen/src/stmt/let_stmt.rs, crates/perry-codegen/src/stmt/loops.rs
Loop lowering adds array-only entry, nested body-region state, dirty rechecks, retries, and fallback handling. Safepoints refresh array bases, and clone aliases are tracked while lowering initializers and fallback loops.
Exercise array facts across loop cases
test-files/test_gap_region_array_facts.ts, changelog.d/11671-array-region-facts-s3.md
The regression test covers mutations, holes, prototypes, capacity bounds, and other array inputs. The changelog describes static-index reads and nested-body guard handling.

Priority: ➖ Normal

Estimated code review effort: 4 (Complex) | ~45 minutes

Change: Refactor

Suggested reviewers: claude

Merge Risk: ⚪ Minimal · up to 7bb0c

This change speeds up array element reads inside loops by checking array facts once before the loop. The concern that array bases could become stale after garbage collection was checked, and the refreshed base matches the one computed by the original guard. No outstanding defects remain, so the change appears ready to merge after normal checks.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 71.19% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 59 functions across 10 files. (1 skipped:… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the main change: adding array element facts inside loop regions. It is specific and concise enough to scan in the project history.
Description check ✅ Passed The description explains the change, lists key implementation details, reports performance results, and provides a detailed verification summary. It does not reproduce every template heading or checkl…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 71.19% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 59 functions across 10 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@proggeramlug
proggeramlug merged commit 799fa6c into main Sep 29, 2026
22 of 24 checks passed
@proggeramlug
proggeramlug deleted the perf-array-region-facts branch September 29, 2026 16:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant