Skip to content

perf(codegen): route the class-id dispatch tower to the proven-this clone behind an inline keys check (#7142) - #7169

Merged
proggeramlug merged 8 commits into
mainfrom
perf/7142-tower-pshape-keys-guard
Aug 1, 2026
Merged

perf(codegen): route the class-id dispatch tower to the proven-this clone behind an inline keys check (#7142)#7169
proggeramlug merged 8 commits into
mainfrom
perf/7142-tower-pshape-keys-guard

Conversation

@proggeramlug

@proggeramlug proggeramlug commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Closes #7142. Follow-up to #7141, which fixed Phase 5a's arm-ordering defect at
the two guard-dominated routing sites and deliberately left this one — the
largest single win — on the table because taking it needs a guard that did not
exist.

benchmarks/app-patterns/kernels/batch.ts is the workload Ptr<Shape> exists
for, and it consumed nothing: its hot call is r.rescore(1.5) inside
rows.map((r) => …), whose closure parameter has no static class, so
receiver_class_name() returns None, neither existing routing site is ever
reached, and the call lowers to the class-id switch tower.

Why the tower could not just route on class_id

delete inst.f compacts the packed inline slots while preserving
class_id
. On class Row { a; b; c }, delete row.b moves c from slot 2
into slot 1 and shortens field_count (object/delete_rest.rs — the keys
scan/compaction path, which class instances get no tombstone branch out of). A
tower case that matched class_id has therefore proved the class, not the
layout, and the clone's bare fixed-offset loads would read the wrong slot.

The fix: an inline keys check

delete installs a freshly cloned keys array (js_array_alloc
set_object_keys_array, cloned precisely because the old array is shared across
every instance of the shape), so a pointer compare against the class's
@perry_class_keys_* token catches it exactly. The check is dynamic on purpose:
the delete shape barrier that stands the analysis down is collected per
module while receivers alias across modules (#7143), so at this site a
static proof would be the wrong instrument — which is the same reason the two
existing routing sites are safe and this one was not.

Emitted at idispatch.caseN, in ONE basic block. No gate/deref split is needed
because the dereference is already proven safe by the tower's own non-zero
js_object_get_class_id match, which rejects the handle band, the
Set/Map/RegExp registries, out-of-heap addresses and non-GC_TYPE_OBJECT
allocations:

idispatch.case0.4:
  %r24 = load i64, ptr %r22                                    ; entry-hoisted keys token
  %r25 = load volatile i8, ptr @PERRY_CLASS_FIELD_INLINE_GUARD_DISABLED
  %r26 = icmp eq i8 %r25, 0
  %r27 = inttoptr i64 %r19 to ptr
  %r28 = getelementptr i8, ptr %r27, i64 -7
  %r29 = load i8, ptr %r28
  %r30 = and i8 %r29, -128                                     ; GC_FLAG_FORWARDED
  %r31 = icmp eq i8 %r30, 0
  %r32 = getelementptr i8, ptr %r27, i64 -6
  %r33 = load i16, ptr %r32
  %r34 = and i16 %r33, 2049                                    ; FROZEN | HAS_DESCRIPTORS
  %r35 = icmp eq i16 %r34, 0
  %r36 = load i32, ptr %r27
  %r37 = icmp eq i32 %r36, 1                                   ; OBJECT_TYPE_REGULAR
  %r38 = getelementptr i8, ptr %r27, i64 16
  %r39 = load i64, ptr %r38
  %r40 = icmp eq i64 %r39, %r24                                ; ← the keys token
  %r41 = and i1 %r26, %r31
  %r42 = and i1 %r41, %r35
  %r43 = and i1 %r42, %r37
  %r44 = and i1 %r43, %r40
  br i1 %r44, label %idispatch.case0.pshape.8, label %idispatch.case0.generic.9

21 IR instructions: 4 loads off the receiver (three from the two header words
the tower already touched) plus one entry-hoisted slot reload, three geps, nine
ALU ops, one branch. No calls.

Each conjunct beyond the keys token earns its place, and the set is chosen to be
never weaker than the per-access inline guard the clone deletes:

  • the sticky @PERRY_CLASS_FIELD_INLINE_GUARD_DISABLED latch — flipped when a
    descriptor/accessor lands on a class prototype or Object.prototype for a
    name some class declares as an instance field, when typed-feedback tracing is
    active, or under PERRY_VERIFY_TYPED_INTACT. Two consequences worth naming:
    the routed path stands down in exactly the mode that collects typed feedback
    (so the observations the clone would skip are never actually skipped while
    anyone is looking), and PERRY_DISABLE_CLASS_FIELD_INLINE=1 is already a kill
    switch for this route — no new knob is added.
  • per-object OBJ_FLAG_HAS_DESCRIPTORS — instance-level installs deliberately
    do NOT flip the process-global latch (perf: class-field inline fast path (#5093) is disabled process-wide in normal runs #5654), so they are vetted per receiver.
  • OBJ_FLAG_FROZEN — the clone may contain field WRITES, and a guard-free raw
    store into a frozen receiver would silently succeed where the spec requires a
    strict-mode TypeError. Phase 5a's admission rules that out only through a
    module-scoped freeze-barrier kill, so the receiver is vetted here too. This
    makes the tower route strictly stronger than js_method_direct_shape_guard,
    which checks neither this bit nor the per-object descriptor bit.
  • not-forwarded and object_type == OBJECT_TYPE_REGULAR — the two header
    predicates js_object_get_class_id does not itself check.

Routing is additionally restricted to cases whose receiver class declares
the method (impl_owner), so the clone's this is exactly the class it was
compiled for and can never be a subclass instance with a longer chain — the same
condition the two existing sites rely on.

Profitability

Unlike the two existing sites, this one pays for its own proof, so it consults a
gate rather than routing unconditionally (collectors/repsel_benefit.rs, the
module #7132 added). The re-check IS one instance of the same header check the
public body runs at every this.field site, so the trade is N in-body checks
for 1 at the call site
and the break-even is exactly N == 1: at one field
site the route swaps a check for a check, adds a branch and a second call site,
and is strictly worse when that site sits behind a conditional the call does not
always reach. It routes at N >= 2.

The rule is a count, with no target-specific term — it says the same thing
on AArch64 and x86-64, unlike the fusion term #7146 flags. It under-counts in
both directions that matter (a loop site is counted once; transitively-invoked
this.m() sites are not followed), and under-counting refuses routes rather
than taking them.

Measured — and what the numbers do and do not say

batch.ts, perry-dev, darwin-arm64. The two bodies the tower now chooses
between:

body IR lines basic blocks class-field guard calls by-name field calls total js_* call sites
…__Row__rescore (public) 352 24 3 3 15
…__Row__rescore__pshape 189 15 0 0 6

Correction to the framing in #7142: those js_typed_feedback_class_field_get_guard
and js_object_get_field_by_name_f64 calls are not executed once per
invocation. Every one of them sits in a MISS arm —
class_field_inline.guardcall.* and class_field_get*.fallback.* — so on a
monomorphic receiver they never run. What the routed path actually deletes per
call on the shape-hit path is the three inline shape-check diamonds
(9 of the public body's 24 blocks: 3 × deref/guardcall plus their
fast/fallback/merge fan-in), replaced by one check at the call site,
plus 163 IR lines of cold code that no longer sit in the function.

Call sites, same compiler, routing on vs off — the honest A/B, because
module-wide static totals are unchanged (the public body survives as the miss
arm and as the registered vtable symbol, so counting js_* per module shows
27 guards / 47 by-name in both arms and would be a vacuous "no change"):

before (tower does not route):  perry_method_batch_ts__Row__rescore__pshape  0 call sites
after:                          perry_method_batch_ts__Row__rescore__pshape  1 call site

checksum: 1601043.5000 in both arms.

Census (compiler_output_regression.py census --gate): green, no floor
lowered. batch stays at ptr-shape 2 / ptr-shape-consumed 1 by design
the census counts per value, and a proven-this receiver is reported
separately (consumed_receiver), never as a selection. That is the #7141 lesson
restated: this row could not have moved, which is why the evidence above is call
sites in the emitted IR.

The delete reproducer is the soundness test

test-files/test_gap_repsel_pshape_tower_delete.ts + _helpers/repsel_pshape_tower_rows.ts.
Construct → delete a field through a cross-module alias → call the method.
Built red-first against a class-id-only route:

arm after: line
node 26.5.1 (oracle) 103,206,309,412
this PR 103,206,309,412 (whole file byte-identical)
sabotage: keys compare removed 103,NaN,309,412

The delete lives in the importing module on purpose — a same-module delete
is a §5.2 shape barrier that disables the clone outright and the test would pass
vacuously. That asymmetry is #7143, and it is precisely why the check has to be
an inline dynamic one.

Registered in test-parity/gc_repsel_corpus.txt, and GC-live by
construction ([gc-copy-minor] ran copied_objects=5971 at --pressure 8), so
the evacuating arms have a receiver to move rather than reporting the file
inert: scripts/gc_repsel_matrix.sh --arms all --pressure 8 gives
PASS=21 UNVER=0 XFAIL=0 FAIL=0 on this row, every arm biting (21/21
collected; 14/21 moved objects, the other 7 being the requires=collect
non-moving arms). The rest of the corpus, same settings: PASS=447 UNVER=119
XFAIL=1 FAIL=0
over 567 cells.

Instructions retired (quiet Raspberry Pi 5, perf stat)

Two compilers from the same tree — arm before has tower_pshape_route forced
to None, nothing else — producing two batch.ts binaries with distinct
hashes and the same checksum: 1601043.5000. ASLR off (setarch -R), pinned to
one core, interleaved, 12 reps each, 1-minute load average below 0.30 at start.

The workload is bimodal at ~1% independently of the arm — 3/12 runs in each
arm land in the low mode, and PERRY_GC_TRACE=1 reports zero collections in
both, so it is not a GC-schedule effect. Reporting a single mean across the two
modes is what produced a misleading −0.53% on a first, ASLR-on run where the
modes happened to split 2/12 against 6/12. Within each mode the signal is
consistent:

statistic before after delta
low mode median (n=3 each) 3,621,049,608 3,615,400,137 −0.156%
high mode median (n=9 each) 3,652,688,821 3,646,538,197 −0.168%
overall median 3,652,360,661 3,646,108,175 −0.17%
overall min 3,620,747,563 3,614,785,401 −0.16%

≈6.0M instructions over 40,000 rescore calls ≈ 150 instructions saved per
call
. The whole-program figure is small because rescore is a small share of
batch.ts, which is dominated by allocation, sort and reduce — the point of
the fixture is that its receiver shape is the hard case, not that its method
is the hot spot.

Sabotage table

Three ratchets in collectors/proven_this_routing_tests.rs, each with a red set
disjoint from the others' — verified by building all three arms:

sabotage tower_case_routes… tower_route_is_guarded_by_the_class_keys_token tower_route_refused_when_clone_deletes_one_field_site
none (this PR) pass pass pass
routing reverted to pre-#7142 FAIL FAIL pass
keys compare removed pass FAIL pass
profitability gate removed pass pass FAIL

The keys-guard ratchet traces the token end to end — global → entry-hoisted slot
→ reload in the guard block → icmp eq i64 → the branch that enters the clone's
block — rather than asserting a substring. All three are --lib unit tests, so
they run on every PR (#5960).

GC contract

Unchanged (double this, args…) ABI. The clone still stores its receiver to a
slot and js_shadow_slot_binds it with nothing in between — verified in the
emitted batch.ts IR, not inferred:

store double %this_arg, ptr %r7
call void @js_shadow_slot_bind(i32 0, ptr %r7)

GC_TYPE_OBJECT is movable in the shipped configuration (#7019); the TaPtr
no-bind shortcut does not transfer and is not copied here. The existing
proven_this_clone_binds_its_receiver_slot ratchet enumerates every __pshape
definition, so it covers the newly-routed clone too.

Other gates run locally

  • cargo test -p perry-codegen --lib — 456 passed, 0 failed.
  • census-knob-isolation — every knob moves only its own representation.
  • census-determinism — 27/27 byte-identical, so the object-hash A/B is valid
    on this host.
  • run_parity_tests.sh --filter test_gap_repsel_ — the new test passes through
    the real harness. (repsel_gc_stress and repsel_scalar_replaced_locals time
    out at the harness's 10 s cap only because this run used a perry-dev
    compiler: measured 10.7 s and 18.5 s respectively; both are PASS on every arm
    in the matrix run above, which has a larger budget. Not a regression.)
  • cargo fmt --all -- --check clean.
  • scripts/check_file_size.sh is red on this branch and on main alike, on 16
    files none of which this PR touches — Tests has been failing on main since
    at least 2026-07-28. Pre-existing, flagged not fixed.
  • gc-ratchet was not run locally: it needs a --release build and this
    host is down to 22 GiB free. Expectation for a codegen-only change is 0/72
    gated metrics differing; the PR's own gc-ratchet job is the authoritative
    darwin-arm64 measurement.

Not done

Routing the vdispatch.* virtual-override tower (statically-typed receiver with
overriding subclasses) the same way. It is the identical shape and the same
check would work; it is left out so this PR's measurement stays about the site
that motivated it.

@coderabbitai

coderabbitai Bot commented Aug 1, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@proggeramlug, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 32 seconds

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 9fe78641-3401-45e9-befe-6929f535f5a3

📥 Commits

Reviewing files that changed from the base of the PR and between 5f5006a and 5b3571a.

📒 Files selected for processing (18)
  • changelog.d/7169-tower-pshape-keys-guard.md
  • crates/perry-codegen/src/codegen/closure.rs
  • crates/perry-codegen/src/codegen/entry.rs
  • crates/perry-codegen/src/codegen/function.rs
  • crates/perry-codegen/src/codegen/method.rs
  • crates/perry-codegen/src/codegen/mod.rs
  • crates/perry-codegen/src/codegen/opts.rs
  • crates/perry-codegen/src/collectors/mod.rs
  • crates/perry-codegen/src/collectors/proven_this.rs
  • crates/perry-codegen/src/collectors/proven_this_routing_tests.rs
  • crates/perry-codegen/src/collectors/repsel_benefit.rs
  • crates/perry-codegen/src/collectors/repsel_benefit/tests.rs
  • crates/perry-codegen/src/expr/class_field_inline_guard.rs
  • crates/perry-codegen/src/expr/mod.rs
  • crates/perry-codegen/src/lower_call/property_get/dynamic_dispatch.rs
  • test-files/_helpers/repsel_pshape_tower_rows.ts
  • test-files/test_gap_repsel_pshape_tower_delete.ts
  • test-parity/gc_repsel_corpus.txt

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 7a14d1a into main Aug 1, 2026
7 checks passed
@proggeramlug
proggeramlug deleted the perf/7142-tower-pshape-keys-guard branch August 1, 2026 07:52
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.

repsel Phase 5a: route the class-id dispatch tower to __pshape under an inline keys check (batch.ts: 4 guard calls + 3 by-name field calls per call)

1 participant