Skip to content

perf(codegen,runtime): inline precheck for boxed class-field stores, arguments-registry emptiness latch, declared-type refinement for property reads (interp 1.236 -> 1.097, iso_miss 1.670 -> 1.465) - #7854

Merged
proggeramlug merged 1 commit into
mainfrom
perf/7850-interp-round4
Aug 11, 2026

Conversation

@proggeramlug

Copy link
Copy Markdown
Contributor

Round 4 of the interp campaign (3.96 s → 1.893 → 1.499 → 1.237 → this). Three
independent changes plus one refutation that closes the lever the previous round's
handoff was built around.

A — the strict class-field SET arm skipped the inline precheck for BOXED fields

expr/property_set.rs gated emit_class_field_inline_precheck on
requires_raw_f64. Every store into a declared field that is not a number — a
string, a class type, a union: most fields of most objects — therefore paid an
unconditional cross-crate js_typed_feedback_class_field_set_guard call. That
includes the synthesized __AnonShape_*_constructor that every closed-shape object
literal runs, which is why js_typed_feedback_class_field_set_guard (2.9%) and
typed_feedback::guards::class_field_fast_contract (2.1%) sat near the top of
interp's profile: { kind: "bin", op, left, right } is four stores, three boxed.

The stated reason for the gate ("its setter-in-chain handling and write barrier
aren't reproduced inline") was already answered by
try_lower_sloppy_class_field_boxed_store, which has taken the boxed inline
precheck since #7288 — the write barrier, layout note and string demote come from
emit_jsvalue_slot_store_pointer_tested, which the shared fast block calls with the
identical value-side predicates; a setter anywhere in the chain is refused upstream
by class_field_global_index's accessor_in_chain. The precheck proves a strict
subset of the runtime's class_field_fast_contract, so on a hit the guard would
have answered "fast" too. Every miss still lands on the guardcall block and the
unchanged strict fallback.

Verified live rather than assumed: interp.ts goes from 3 to 44 emitted
PERRY_CLASS_FIELD_INLINE_GUARD_DISABLED gate loads (41 new prechecks) with the
same 43 guard-call sites, now on the miss arm.

B — is_arguments_object had no emptiness latch

It is a probe, called from the by-name property-get tail, Array.prototype.push,
the array and Symbol.iterator iterator entries, Array.from/concat and class
construction. In a program that never writes arguments it was still 2.8% of
interp: a thread-local resolution (Darwin has no local-exec TLS, so that is a real
_tlv_get_addr — 7.1% of the same profile), a RefCell borrow and a pointer hash,
per call, to prove the absence of a feature the source does not contain.

ARGUMENTS_OBJECTS_EVER_USED is a process-global AtomicBool latched by the one
and only registry insert and checked before the thread-local — the
EXTERNAL_BUFFERS_NONEMPTY / SET_REGISTRY_EVER_USED idiom verbatim (#7474,
#7469).

object/arguments_latch_tests.rs asserts the SUBJECT, not the answer:
latch_off_is_what_makes_the_probe_cheap registers a real arguments object, forces
the latch back off, and requires the probe to answer false — deliberately the
wrong answer, and the only way to show the short-circuit is the arm being taken
rather than dead code in front of a registry that would have answered anyway. Delete
the early-out and it goes red.

C — a property read into an unannotated local threw its type away

const names = e.names left names at Any, so names[i] lowered to a
js_dyn_index_get call (4.3% of interp) whose own miss path calls
js_array_length (2.9%), and names[i] === name lowered to the fully dynamic
js_eq. refine_type_from_init's PropertyGet arm resolves the receiver with
receiver_class_name — which answers None for a reassigned local and for a union
— and then looks only in ctx.classes. A chain-walking cursor over a
type Env = { … } alias therefore failed three separate ways.

declared_property_type_from_annotation resolves through the same class / interface
/ object-alias tables static_type_of already consults, after stripping
null/undefined from the receiver's union (a read that returns at all had a
non-nullish receiver — reading through null throws).

★ It is a claim, not a proof — and one consumer could not take one

Element reads and stores tolerate a violated claim: both re-check GC_TYPE_ARRAY
and fall back. .length does not. Its inline arm is guarded, but its fallback
js_value_length_f64 answers 0 for every value that carries no length, where JS
answers undefined (and where a nullish receiver must throw). That degradation is
pre-existing and documented in place in the runtime — and it is already reachable on
main through a hand-written annotation:

type Bag = { items: string[] };
function mk(v: any): Bag { return { items: v }; }
const items: string[] = mk(42).items;
items.length            // node: undefined      main today: 0

So .length must not be handed a fresh claim. refined_array_type_is_declared_only
records these ids in FnCtx::declared_only_array_locals — the mechanism #7773
introduced for the numeric half — and the .length arm refuses them, leaving them
on exactly the generic path the unrefined Any local takes today.

test-files/test_gap_declared_field_type_refine_guarded.ts is the sabotage test:
one items: string[] declaration handed arrays, strings, plain objects aping
arrays, numbers, null and undefined, through an alias, an interface and a class,
via a nullable reassigned cursor, a nested read chain, an element store and an
element-typed ===/+. Every row must match node byte for byte. It was written
before the guard and it failed
— four rows read len=0 — which is how the hazard
above was found rather than shipped.

Measured — quiet M1 mini, best-of-5, exit-checked

origin/main @ 0321c6554 (perry 0.5.1467) vs this branch. Every output
byte-compared against node --experimental-strip-types before timing; exit code
recorded per cell. Load 1.48 before / 2.11 after, zero foreign processes at both ends.

bench main this PR delta ceiling
interp 1.236 1.097 -11.2%
iso_miss 1.670 1.465 -12.3%
pipeline 0.483 0.465 -3.8% 0.49
asyncpipe 0.134 0.132 -1.3% 0.14
churn_read 0.023 0.022 -0.9% 0.026
deeplist 0.121 0.121 -0.2% 0.13
retain_wide1 0.157 0.157 -0.1%
tree 1.161 1.161 -0.0% 1.20
shapes 0.183 0.183 0.0% 0.19
retain_wide 0.455 0.455 0.0% 0.48
push_num 0.069 0.069 +0.1% 0.075
tree_wide 1.647 1.648 +0.1% 1.70
fib40 0.393 0.394 +0.1% 0.41
retain1 0.134 0.134 +0.1% 0.14
churn 0.289 0.289 +0.2% 0.30
churn_alloc 0.241 0.241 +0.2% 0.25
retain 0.345 0.346 +0.2% 0.36
cycles 0.111 0.111 +0.5% 0.12
push_cls 0.235 0.236 +0.5% 0.25

Zero regressions. The fourteen programs sitting inside +/-0.5% are the run's own
noise floor (med within 1 ms of best in every cell), which is what makes the three
movers credible. All protected ceilings met.

Correctness canaries: interp.ts prints 1708840; iso_miss.ts prints
checksum 437840 misses 0 — gated on the miss counter, and re-verified under
PERRY_GC_PROTECT_FROMSPACE=1 PERRY_GC_PROTECT_FROMSPACE_DEPTH=800 PERRY_GC_VERIFY_EVACUATION=1 (both exit 0, output unchanged).

cmp does not apply here: this touches perry-runtime, so the two arms link
different archives and all 19 binaries differ by construction.

Tests

  • cargo test --release -p perry-runtime arguments — 4 passed, including the two new
    latch tests and the two existing ARGUMENTS_OBJECTS GC-side-table tests.
  • cargo test --release -p perry-codegen — all unit + integration suites pass except
    large_object_barriers::large_local_array_push_inbounds_store_emits_precise_slot_barrier,
    which is already red on main @ 0321c6554 and unrelated: perf(codegen): put the numeric array push's GC bookkeeping behind one live test (push_num 0.149 -> 0.069) #7839
    (a64c5a9eb, the commit immediately before this branch point) moved
    js_gc_note_slot_layout / js_write_barrier_slot out of apush.inbounds into a
    new apush.gc_bookkeeping block emitted after apush.realloc, outside the slice
    the old assertion searches. Verified empirically against a reference build of
    0321c6554, not inferred: neither call appears between apush.inbounds and
    apush.realloc, and apush.gc_bookkeeping is present. perf(codegen): put the numeric array push's GC bookkeeping behind one live test (push_num 0.149 -> 0.069) #7839 wrote its own
    replacement gate and left this one stale; it landed green because
    crates/*/tests/*.rs do not run per-PR.
  • cargo fmt --all -- --check, scripts/check_file_size.sh clean.
  • Gap suite: run locally with this compiler (PERRY_SKIP_BUILD=1, node 26.5.1 per
    .node-version) — see the comment below for the outcome.

Follow-up filed

#7853.length on a receiver whose declared type is an array/Named but whose
runtime value carries no length returns 0 instead of undefined, and returns 0
where JS throws. Pre-existing on main, reachable from a hand-written annotation, and
the reason change C has to carve .length out. Fixing it removes the carve-out and
recovers the remaining ~3%.

Refuted: shape narrowing after a discriminant test

PROFILE-interp-round3.md proposed narrowing n to its matching union member
inside if (n.kind === "bin") so evalNode's surviving diamonds (27.2% of the
program) become class-keyed slot loads. The ceiling was measured before building
it: ~5%, not the projected ~20%.

Three source-level arms of interp.ts, built from one compiler and identical in
every other respect — object literals (the original); each union member as a real
class (isolating allocation from typing); that program plus hand-written narrowing
casts in every evalNode arm. The narrowed arm converts 19 of 31 generic
property diamonds into guarded class-field inline reads and is 1.226 → 1.162 s
against its own control, 5.2%. The reason is structural: a class-field guarded read
is only about a third cheaper than the polymorphic-IC read, because the cost is the
guard, not the lookup — narrowing swaps one guarded diamond for another. A large
win needs the check hoisted out of the branch (one shape test, N unguarded slot
loads), i.e. loop-versioning applied to a discriminant arm.

…arguments-registry emptiness latch, declared-type refinement for property reads

Round 4 of the `interp` campaign. Three independent changes:

A. `expr/property_set.rs` gated the #5093 inline shape precheck on
   `requires_raw_f64`, so every store into a declared field that is not a
   `number` paid an unconditional `js_typed_feedback_class_field_set_guard`
   call — including the synthesized `__AnonShape_*_constructor` behind every
   closed-shape object literal. The sloppy arm has taken the boxed precheck
   since #7288 and its argument applies verbatim: the write barrier, layout
   note and string demote come from `emit_jsvalue_slot_store_pointer_tested`,
   not from the guard, and a setter in the chain is refused upstream by
   `class_field_global_index`. Every miss still reaches the unchanged guard
   call and strict fallback.

B. `is_arguments_object` gets the #7474/#7469 emptiness latch. It is a probe
   run from the by-name property-get tail, array push, the iterator entries
   and class construction; in a program with no `arguments` it was 2.8% of
   `interp` — a thread-local resolution (a real `_tlv_get_addr` on Darwin), a
   `RefCell` borrow and a pointer hash to prove a feature's absence.

C. `refine_type_from_init` recovers a property read's type from the
   receiver's declared annotation through the class / interface / object-alias
   tables `static_type_of` already consults, after stripping nullish union
   arms — so `const names = e.names` on `let e: Env | null` stops being `Any`
   and `names[i]` stops being a `js_dyn_index_get` call.

   That type is a CLAIM, not a proof. Element reads and stores re-check
   `GC_TYPE_ARRAY` and tolerate a violated one; `.length` does not, because
   its `js_value_length_f64` fallback answers 0 where JS answers `undefined`.
   `declared_only_array_locals` (the #7773 mechanism) keeps these ids off the
   `.length` fast arm. The sabotage test was written first and failed, which
   is how that was found.

Refutes the round-3 handoff's primary lever: hand-written shape narrowing
after a discriminant test converts 19 of 31 generic property diamonds in
`evalNode` and is worth 5.2%, not the projected ~20% — the cost is the guard,
not the lookup, so narrowing swaps one guarded diamond for another.

Claude-Session: https://claude.ai/code/session_012B8z92S82sCfqCrVqrFgS2
@coderabbitai

coderabbitai Bot commented Aug 11, 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: 3 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

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: 04983593-f218-4de3-b0b3-487c132727ff

📥 Commits

Reviewing files that changed from the base of the PR and between 079e646 and 5266200.

📒 Files selected for processing (15)
  • changelog.d/7854-interp-round4-store-precheck-arguments-latch-declared-array.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/expr/mod.rs
  • crates/perry-codegen/src/expr/property_get.rs
  • crates/perry-codegen/src/expr/property_set.rs
  • crates/perry-codegen/src/stmt/let_stmt.rs
  • crates/perry-codegen/src/type_analysis.rs
  • crates/perry-codegen/src/type_analysis/refine.rs
  • crates/perry-runtime/src/object/arguments.rs
  • crates/perry-runtime/src/object/arguments_latch_tests.rs
  • crates/perry-runtime/src/object/mod.rs
  • test-files/test_gap_declared_field_type_refine_guarded.ts

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

Copy link
Copy Markdown
Contributor Author

Gap-suite status — partial, and I could not get a quiet host

Stating this plainly rather than claiming a clean sweep.

The suite was run locally against this branch's compiler
(PERRY_SKIP_BUILD=1, PERRY_BIN/PERRY_RUNTIME_DIR pinned to the round-4 build,
node 26.5.1 per .node-version). The dev box sat at load 70–116 throughout from
four concurrent agents, so it ran at roughly 4 s/test and did not finish inside the
window. It reached ~384 of 537. Correctness verdicts are load-independent, so the
results below are meaningful; the coverage is not complete.

Non-passing tests observed, all triaged:

test verdict
test_gap_2159_defineproperty_class_prototype already parity_fail in test-parity/gap_snapshot.json (issue 2159)
test_gap_2514_settracesigint already parity_fail in the snapshot (issue 2514)
test_gap_4510_enum_forward_ref already node_fail in the snapshot (oracle refuses the syntax)
test_gap_derived_param_props already node_fail in the snapshot
test_gap_enum_in_function_body already node_fail in the snapshot
test_gap_backoff_options already node_fail in the snapshot (#1634, npm import)
test_gap_cron_cronjob already node_fail in the snapshot (#1634)
test_gap_dayjs_factory_arg already node_fail in the snapshot (#1634)
test_gap_gc_symbol_local_rooting cleared — TIMEOUT at the harness's 10 s cap on a load-100 box; the binary exits 0 standalone on both this branch and a reference build of 0321c6554
test_gap_fetch_request_from_node_incoming_message not attributable either way — see below
test_gap_http_client_no_redirect_follow ditto
test_gap_http_overloads_3226plus ditto
test_gap_http_req_async_iterator ditto

The four network tests. They SIGABRT on a Rust panic inside perry-ext-http
(server/server.rs:911, "there is no reactor running, must be called from the context
of a Tokio 1.x runtime") — code this PR does not touch. I could not A/B them
against a reference build: the reference compiler resolves its ext archives relative to
its own source tree and finds no libperry_ext_http.a there, so main's arm does not
link at all (Undefined symbols: _js_http_client_request_end_full, …). And the ext
archives this environment auto-built during the run went into the worktree's default
target/release/, i.e. a different target dir from the libperry_runtime.a being
linked — the known "an ext staticlib bundles its own feature-stripped runtime that wins
the link" trap. So this is very likely environmental, but I am not asserting that
without an A/B I could actually run.

What is outstanding: a full gap run on a quiet host. Nothing in the 384 tests
observed is a new failure attributable to this change, but the last ~150 have not been
looked at.

The parts of validation that did complete cleanly are in the PR body: the 19-program
mini sweep (outputs byte-compared to node, exit-checked, zero regressions), both
correctness canaries including under PERRY_GC_PROTECT_FROMSPACE /
PERRY_GC_VERIFY_EVACUATION, cargo test -p perry-runtime, and
cargo test -p perry-codegen (one pre-existing red integration test, diagnosed to
#7839 in the PR body).

@proggeramlug

Copy link
Copy Markdown
Contributor Author

Gap-suite update — final tally for the partial run

Stopped at 404 / 537 (the box hit load 115 and the run had slowed to ~1 test/min;
I killed it rather than keep four other agents starved). Final triage of all 17
non-passing tests:

10 already in test-parity/gap_snapshot.json — not regressions:
2159_defineproperty_class_prototype, 2514_settracesigint (parity_fail);
perfhooks_3088_3008_3010_3011 (parity_fail / module-inventory);
4510_enum_forward_ref, derived_param_props, enum_in_function_body,
backoff_options, cron_cronjob, dayjs_factory_arg, moment_methods
(node_fail — the oracle cannot run them).

1 cleared: gc_symbol_local_rooting — TIMEOUT at the harness's 10 s cap under
load; exits 0 standalone on both this branch and a reference build of
0321c6554.

6 network/server tests I could not A/B, all perry-ext-*:
fetch_request_from_node_incoming_message, http_client_no_redirect_follow,
http_overloads_3226plus, http_req_async_iterator,
http_res_socket_writable_onfinished, net_connect_bound_value. They abort on a
Rust panic inside perry-ext-http ("there is no reactor running, must be called
from the context of a Tokio 1.x runtime", server/server.rs:911) — code this PR does
not touch. The reference compiler cannot link them in this environment at all (it
resolves ext archives relative to its own source tree and finds none:
Undefined symbols: _js_http_client_request_end_full), and the ext archives that were
auto-built here landed in the worktree's default target/release/, a different
target dir from the libperry_runtime.a being linked — the known "an ext staticlib
bundles its own feature-stripped runtime that wins the link" trap. Very likely
environmental; I am not asserting it without an A/B I could actually run.

Bottom line unchanged: no new failure in the 404 tests observed is attributable to
this change, and a full gap run on a quiet host is still outstanding.

@proggeramlug

Copy link
Copy Markdown
Contributor Author

Correction: the partial gap run recorded 412/537, not 404

My earlier two comments quoted 404 — that was the last line I read out of the live
log, not the harness's own tally. Its exit banner is authoritative:

PAUSED after SIGTERM — 412/537 test(s) recorded.
  test-parity/reports/latest.json was left untouched (this run is incomplete).
  Resume with:
      ./run_parity_tests.sh --filter test_gap_ --resume

Nothing else changes. The set of non-passing tests is exactly the 17 already
triaged above — 10 already in test-parity/gap_snapshot.json, 1 cleared as a
load-induced timeout (gc_symbol_local_rooting), 6 perry-ext-* network tests I
could not A/B. No new failure attributable to this change; the last 125 tests remain
unobserved.

Two things worth knowing for whoever finishes it on a quiet host:

  • It is resumable./run_parity_tests.sh --filter test_gap_ --resume picks up
    where this left off rather than restarting the 412.
  • The snapshot was not polluted. The harness left latest.json untouched
    precisely because the run was incomplete, and test-parity/reports/ is gitignored.
    The working tree is clean; this PR contains only the code, the two tests and the
    changelog fragment.

@proggeramlug
proggeramlug merged commit 185915e into main Aug 11, 2026
9 of 53 checks passed
@proggeramlug
proggeramlug deleted the perf/7850-interp-round4 branch August 11, 2026 15:21
proggeramlug added a commit that referenced this pull request Aug 11, 2026
…ld per construction (shapes 0.183 -> 0.146; isolated probe 1.81x) (#7861)

* perf(class): widen the class-field shape guard to the subclass closure

WIP

* wip: temporary class-field guard miss counters (to be reverted)

* wip: keep pre-widening and-chain when the closure is empty (byte-identical IR)

* perf(class): declare the typed shape at allocation for subclassed chains (#7512 followup)

* Revert "wip: temporary class-field guard miss counters (to be reverted)"

* test(codegen): pin the chain prologue analysis

* docs: changelog fragment for the class-field chain fix

* docs: key the changelog fragment to PR #7861

* docs: record the #7854 boxed-store interaction in the fragment
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