perf(codegen): route the class-id dispatch tower to the proven-this clone behind an inline keys check (#7142) - #7169
Conversation
…nd inline keys check (#7142)
|
Warning Review limit reached
Next review available in: 32 seconds Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the 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 configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (18)
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 |
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.tsis the workloadPtr<Shape>existsfor, and it consumed nothing: its hot call is
r.rescore(1.5)insiderows.map((r) => …), whose closure parameter has no static class, soreceiver_class_name()returnsNone, neither existing routing site is everreached, and the call lowers to the class-id switch tower.
Why the tower could not just route on
class_iddelete inst.fcompacts the packed inline slots while preservingclass_id. Onclass Row { a; b; c },delete row.bmovescfrom slot 2into slot 1 and shortens
field_count(object/delete_rest.rs— the keysscan/compaction path, which class instances get no tombstone branch out of). A
tower case that matched
class_idhas therefore proved the class, not thelayout, and the clone's bare fixed-offset loads would read the wrong slot.
The fix: an inline keys check
deleteinstalls a freshly cloned keys array (js_array_alloc→set_object_keys_array, cloned precisely because the old array is shared acrossevery 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
deleteshape barrier that stands the analysis down is collected permodule 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 neededbecause the dereference is already proven safe by the tower's own non-zero
js_object_get_class_idmatch, which rejects the handle band, theSet/Map/RegExp registries, out-of-heap addresses and non-
GC_TYPE_OBJECTallocations:
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:
@PERRY_CLASS_FIELD_INLINE_GUARD_DISABLEDlatch — flipped when adescriptor/accessor lands on a class prototype or
Object.prototypefor aname 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=1is already a killswitch for this route — no new knob is added.
OBJ_FLAG_HAS_DESCRIPTORS— instance-level installs deliberatelydo 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 rawstore into a frozen receiver would silently succeed where the spec requires a
strict-mode
TypeError. Phase 5a's admission rules that out only through amodule-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.
object_type == OBJECT_TYPE_REGULAR— the two headerpredicates
js_object_get_class_iddoes not itself check.Routing is additionally restricted to cases whose receiver class declares
the method (
impl_owner), so the clone'sthisis exactly the class it wascompiled 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, themodule #7132 added). The re-check IS one instance of the same header check the
public body runs at every
this.fieldsite, so the trade is N in-body checksfor 1 at the call site and the break-even is exactly
N == 1: at one fieldsite 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 ratherthan taking them.
Measured — and what the numbers do and do not say
batch.ts, perry-dev, darwin-arm64. The two bodies the tower now choosesbetween:
js_*call sites…__Row__rescore(public)…__Row__rescore__pshapeCorrection to the framing in #7142: those
js_typed_feedback_class_field_get_guardand
js_object_get_field_by_name_f64calls are not executed once perinvocation. Every one of them sits in a MISS arm —
class_field_inline.guardcall.*andclass_field_get*.fallback.*— so on amonomorphic 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/guardcallplus theirfast/fallback/mergefan-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 shows27 guards / 47 by-name in both arms and would be a vacuous "no change"):
checksum: 1601043.5000in both arms.Census (
compiler_output_regression.py census --gate): green, no floorlowered.
batchstays atptr-shape 2 / ptr-shape-consumed 1by design —the census counts per value, and a proven-
thisreceiver is reportedseparately (
consumed_receiver), never as a selection. That is the #7141 lessonrestated: this row could not have moved, which is why the evidence above is call
sites in the emitted IR.
The
deletereproducer is the soundness testtest-files/test_gap_repsel_pshape_tower_delete.ts+_helpers/repsel_pshape_tower_rows.ts.Construct →
deletea field through a cross-module alias → call the method.Built red-first against a class-id-only route:
after:line103,206,309,412103,206,309,412(whole file byte-identical)103,NaN,309,412The
deletelives in the importing module on purpose — a same-moduledeleteis 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 byconstruction (
[gc-copy-minor] ran copied_objects=5971at--pressure 8), sothe evacuating arms have a receiver to move rather than reporting the file
inert:
scripts/gc_repsel_matrix.sh --arms all --pressure 8givesPASS=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=collectnon-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
beforehastower_pshape_routeforcedto
None, nothing else — producing twobatch.tsbinaries with distincthashes and the same
checksum: 1601043.5000. ASLR off (setarch -R), pinned toone 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=1reports zero collections inboth, 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:
≈6.0M instructions over 40,000
rescorecalls ≈ 150 instructions saved percall. The whole-program figure is small because
rescoreis a small share ofbatch.ts, which is dominated by allocation,sortandreduce— the point ofthe 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 setdisjoint from the others' — verified by building all three arms:
tower_case_routes…tower_route_is_guarded_by_the_class_keys_tokentower_route_refused_when_clone_deletes_one_field_siteThe 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'sblock — rather than asserting a substring. All three are
--libunit tests, sothey run on every PR (#5960).
GC contract
Unchanged
(double this, args…)ABI. The clone still stores its receiver to aslot and
js_shadow_slot_binds it with nothing in between — verified in theemitted
batch.tsIR, not inferred:GC_TYPE_OBJECTis movable in the shipped configuration (#7019); theTaPtrno-bind shortcut does not transfer and is not copied here. The existing
proven_this_clone_binds_its_receiver_slotratchet enumerates every__pshapedefinition, 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 validon this host.
run_parity_tests.sh --filter test_gap_repsel_— the new test passes throughthe real harness. (
repsel_gc_stressandrepsel_scalar_replaced_localstimeout at the harness's 10 s cap only because this run used a
perry-devcompiler: 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 -- --checkclean.scripts/check_file_size.shis red on this branch and onmainalike, on 16files none of which this PR touches —
Testshas been failing onmainsinceat least 2026-07-28. Pre-existing, flagged not fixed.
gc-ratchetwas not run locally: it needs a--releasebuild and thishost is down to 22 GiB free. Expectation for a codegen-only change is 0/72
gated metrics differing; the PR's own
gc-ratchetjob is the authoritativedarwin-arm64 measurement.
Not done
Routing the
vdispatch.*virtual-override tower (statically-typed receiver withoverriding 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.