Skip to content

perf(codegen): revive the class-field versioned loop for canonical-i32 counters - #7425

Merged
proggeramlug merged 4 commits into
mainfrom
perf/7287-hoist-field-guards
Aug 5, 2026
Merged

perf(codegen): revive the class-field versioned loop for canonical-i32 counters#7425
proggeramlug merged 4 commits into
mainfrom
perf/7287-hoist-field-guards

Conversation

@proggeramlug

@proggeramlug proggeramlug commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Fixes #7287. 09_method_calls 82ms → 10ms — parity with Node (8.2×, from 8× behind).

The bug: an optimization that had never once fired

Perry already had this hoist. emit_class_field_loop_preheader_check + lower_class_field_versioned_for were written for this exact benchmark — the comment at stmt/loops.rs:4657 names it: "counter.value = counter.value + 1 after method inlining". The HIR for 09_method_calls is precisely the shape its matcher describes.

It matched nothing. Instrumenting every bail site: repsel Phase 1 (expr/slot_rep.rs) made the canonical i32 slot the only storage for a proven-integer local — registered in ctx.local_slot_reps, with no ctx.locals entry. The matcher gated both its counter and its bound on ctx.locals.contains_key(..), so it declined every loop.

Turning Phase 1 off doesn't recover it either: the counter regains ctx.locals, but a bare i++ never earns an i32 shadow, which the lowering separately requires. Dead in both configurations, with zero tests — CLAUDE.md's fourth failure mode. A sibling matcher had already been repaired for exactly this (local_bound_storage_accessible); this one was missed.

Reviving the matcher alone still changed nothing: sloppy module scope routes the store through try_lower_sloppy_class_field_raw_store (#7423, merged today), which had no loop-fact branch, so the fast clone contained a js_put_value_set call and the call-freeness check branched around it. Both halves were needed.

The volatile-load hypothesis measured zero

I briefed this as the strongest lead — load volatile @PERRY_CLASS_FIELD_INLINE_GUARD_DISABLED can't be hoisted by LLVM, so it looked like it pinned the chain. Tested directly: editing load volatileload and re-running opt -O3 gives 71 hot-path instructions either way. Marking every guard-arm call memory(none) nounwind willreturn as well still leaves both deref blocks in the loop. LLVM won't speculate loads out of a conditionally-executed block whose receiver it can't prove dereferenceable. The "do not de-volatilize the gate" note at class_field_inline_guard.rs:17-26 stands.

Verification

before after
Perry-emitted hot loop 132 instrs 24
post-opt -O3 per iteration 71 10
09_method_calls 82ms 10ms (node 10ms)

The 29-instruction guard chain now sits in class_field_loop.preheader.deref, run once, and LLVM promotes the field load into a loop-carried register.

  • Correctness: 25-case differential — defineProperty non-writable/mid-run, freeze before/mid-run, seal, own accessor, prototype accessor, delete before/mid-run, subclass with different layout, receiver alternating shape, Infinity/NaN mid-loop side exits, zero/one-trip — byte-identical to Node in both arms; the diff was mutated once to confirm it can fail
  • The guard still fires: the slow clone is emitted in every module and demonstrably taken — frozen receiver 3186→3686ms, prototype accessor 10.1→11.3s, both pre-existing cliffs, unchanged
  • Also verified under ESM/strict (12ms vs node 14ms), which uses the other store lowering
  • Regressions: perry-codegen --lib 635/635; 68 class/field/proto gap tests plus a wider 187-test set identical in both arms; perry-runtime --lib 3 failures are gc::tests::teardown::map_set_* parallel flakes — the branch touches zero perry-runtime files
  • 3 new tests assert the versioned blocks appear in emitted IR and that the fast clone contains neither the volatile gate nor a guard call; all three fail on the parent commit

Separately worth filing

reach_frozen surfaced that Perry silently no-ops a frozen field write where Node throws TypeError — the inliner drops the class body's strictness. Pre-existing and identical in both arms, so out of scope here.

Summary by CodeRabbit

  • Performance

    • Improved performance for loops that repeatedly access class fields by enabling optimized fast paths in eligible cases.
    • Added optimized handling for numeric class-field updates, with safe fallback behavior when values do not meet optimization requirements.
  • Bug Fixes

    • Restored optimized loop matching for supported class-field access patterns.
    • Preserved correct behavior across strict and non-strict code, including different loop-bound scenarios.
  • Tests

    • Added regression coverage validating optimized execution and fallback behavior.

Ralph Küpper added 4 commits August 5, 2026 10:46
…cal-i32 counters

Repsel Phase 1 made the canonical i32 slot the ONLY storage for a proven-integer
local, so such a local has no `ctx.locals` entry. The #5093 matcher gated its
counter and its bound on `ctx.locals`, so it matched nothing.

Also teach the sloppy class-field store (#7423) about the loop fact, so the fast
clone stays call-free.
@proggeramlug
proggeramlug merged commit e0a698f into main Aug 5, 2026
@proggeramlug
proggeramlug deleted the perf/7287-hoist-field-guards branch August 5, 2026 08:46
@coderabbitai

coderabbitai Bot commented Aug 5, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: d0528780-cea4-458f-b17a-b943a668da9d

📥 Commits

Reviewing files that changed from the base of the PR and between d255ae6 and 02f9292.

📒 Files selected for processing (5)
  • changelog.d/7425-class-field-loop-guard-hoist.md
  • crates/perry-codegen/src/expr/property_set.rs
  • crates/perry-codegen/src/stmt/class_field_loop_tests.rs
  • crates/perry-codegen/src/stmt/loops.rs
  • crates/perry-codegen/src/stmt/mod.rs

📝 Walkthrough

Walkthrough

Changes

Class-field loop guard hoisting

Layer / File(s) Summary
Readable-slot loop matching
crates/perry-codegen/src/stmt/loops.rs, changelog.d/...
Class-field loop bounds and counters now accept canonical representation-selected readable slots through a shared storage check.
Fast raw-f64 field stores
crates/perry-codegen/src/expr/property_set.rs, changelog.d/...
Sloppy-mode fast clones validate finite numbers, store directly into raw field slots, and side-exit invalid values to the slow clone.
Regression coverage and integration
crates/perry-codegen/src/stmt/class_field_loop_tests.rs, crates/perry-codegen/src/stmt/mod.rs, changelog.d/...
LLVM IR tests cover literal, module-constant, and strict-mode loops, including guard-free fast clones and slow fallbacks.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant LoopMatcher
  participant FastPreheader
  participant FastClone
  participant SlowClone
  LoopMatcher->>FastPreheader: Match readable loop slots
  FastPreheader->>FastClone: Enter after class-field guard
  FastClone->>FastClone: Validate and store finite raw-f64 value
  FastClone->>SlowClone: Side-exit invalid value
Loading

Possibly related PRs

  • PerryTS/perry#6903: Both modify readable local-slot handling for class-field loop matching.

Suggested reviewers: thehypnoo, jdalton

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch perf/7287-hoist-field-guards

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.

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.

perf: 09_method_calls is not a dispatch problem — dispatch costs 0 ms and the whole 7.9x is un-hoistable per-access field guards

1 participant