perf(class): a subclassed hierarchy paid a by-name hash store per field per construction (shapes 0.183 -> 0.146; isolated probe 1.81x) - #7861
Merged
Conversation
proggeramlug
added a commit
that referenced
this pull request
Aug 11, 2026
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (10)
📝 WalkthroughWalkthroughClass-field inline guards now support compatible subclasses with bounded arms. Constructor analysis and typed-shape allocation now validate inherited chains, constructor prologues, raw-f64 assignments, and field-initializer elision. ChangesSubclass-aware class-field guards
Constructor-chain typed layouts
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant ClassFieldAccess
participant ClassFieldSubclassArms
participant InlinePrecheck
participant ReceiverShape
ClassFieldAccess->>ClassFieldSubclassArms: Discover compatible subclass arms
ClassFieldSubclassArms->>InlinePrecheck: Pass bounded arm list
InlinePrecheck->>ReceiverShape: Match class ID, keys, and field count
ReceiverShape-->>ClassFieldAccess: Use fast path or fallback
sequenceDiagram
participant FieldInitializer
participant ConstructorChainAnalysis
participant TypedShapeAllocation
FieldInitializer->>ConstructorChainAnalysis: Resolve and validate inheritance chain
ConstructorChainAnalysis->>TypedShapeAllocation: Provide per-class assignments
TypedShapeAllocation-->>FieldInitializer: Return allocation-layout eligibility
Possibly related PRs
Suggested reviewers: ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
proggeramlug
added a commit
that referenced
this pull request
Aug 11, 2026
proggeramlug
force-pushed
the
perf/class-field-subclass-closure
branch
from
August 11, 2026 15:35
3f3169c to
bcd176a
Compare
proggeramlug
force-pushed
the
perf/class-field-subclass-closure
branch
from
August 11, 2026 16:31
bcd176a to
991bae3
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A subclassed class hierarchy paid a by-name hash store for every field assignment in every constructor on its chain.
gc-handoff/apps/shapes.tsissued 528 000js_put_value_setcalls per run; it now issues 48 000.shapes0.1829 → 0.1463 s (−20.0%) on the quiet mini, best-of-5, exit-checked, VERDICT CLEAN (load 1.69 at both ends). A standalone probe isolating just the defect goes 0.3613 → 0.2000 s (1.81×).Two defects, found in that order; the second was only visible once the first was fixed.
1. The class-field shape guard bet on the DECLARED class
expr/class_field_inline_guard.rs's inline precheck — and the runtimeclass_field_fast_contractbehind it — compared the receiver'sclass_idandkeys_arrayagainst the declared class of the expression. One pair, exact match.Inside a base class's own constructor that bet is not unreliable, it is guaranteed wrong:
thisinNode2D's constructor is only ever reached throughsuper(...)from a subclass, so both compares fail on every singlethis.x = x. Same for every inherited read —Node2D'sget originDistreadingthis.xmissed 100% of the time.class_field_subclass_arms()collects the base's transitive subclass closure and the emitter turns the single equality into a disjunction over it. This is the field-side counterpart of the dispatch widening inlower_call/property_get/dynamic_dispatch.rs(#7800).Soundness does not rest on the layout algorithm's root→leaf field ordering: the slot index and the raw-f64 candidacy are re-derived per candidate subclass, so a shadowing re-declaration or an accessor on the subclass chain drops that arm. Capped at 8 arms. A class with no eligible subclass emits byte-identical IR to before — deliberately, so the corpus-wide
cmpstays a usable no-regression instrument.Instrumented with temporary per-precondition counters: the runtime get guard goes from being entered on every inherited read to never being entered at all (
get_guard_calls=0). The widening is taken, not merely emitted.2. #7512, one level up: no subclass instance ever got an at-allocation typed shape
Fixing (1) left the store side almost unmoved, and the counters said why:
typed_shape::class_layout_declarable_at_allocationconsultsctor_prologue_param_assigned_fields, which returns the empty set the moment a class hasextends. Empty prologue ⇒ nojs_gc_declare_typed_shape_layoutat the allocation site ⇒GC_OBJ_TYPED_LAYOUT_INTACTstays clear for the whole construction ⇒ every raw-f64 field store in every constructor on the chain misses its guard and falls back tojs_put_value_set.That is exactly #7512's mechanism — "declaring the fields
numberis what makes the class slower: more type information selects a representation whose guard the construction path has made unsatisfiable" — fixed for a standalone class and never extended past it.It is not a base-class-only tax.
Node2Dextends nothing, yet its ownthis.x = xmisses too, because the eligibility question is asked of the allocated class. Four TypeScript probes isolate it: a monomorphic class and a hand-flattened two-field class take the fast path on 100% of constructor stores; adding a singleextends— even a fieldless one — puts every store on the chain onto the by-name path.chain_prologue_assigned_fields()answers the same question for a whole chain and distinguishes disqualified from qualified but assigns nothing. The old single-set API conflated those, which is precisely what made a chain unanalysable a class at a time (a fieldlessMarker extends Shapeis the second case and is fine).The extra obligations heritage brings:
super(...)is skipped rather than truncating the prologue at statement 0 — but only when every argument isThis-free, so the parent constructor cannot be handed the half-built instance.Stmt::Exprwith nothisanywhere in it. A non-leaf constructor's trailing statements run before the leaf writes its own fields, so athis.wread inShape's body would see a raw-f64-masked slot still holdingundefined's NaN-box bits and yieldNaNinstead ofundefined. (Shape.made = Shape.made + 1is the motivating admission.)The expression scan uses
perry_hir::walker::walk_expr_children, which is exhaustive and drift-checked against its_muttwin. The statement side is a deliberate whitelist — the HIR has no shared statement walker, and a missed variant there would be a silent wrong answer rather than a missed optimization.The field-init dead-
undefined-write elision consumes the same chain set exactly when the chain form is what authorized the declaration. The two must agree: with the raw-f64 mask live from birth, a field-initundefinedwrite into one of those slots failslayout_raw_f64_bitsand downgrades the descriptor on the spot, which would make the declaration worthless.Validation
Corpus
cmp, both arms against the SAME runtime archives, output basename held constant: 18 of 19 byte-identical, onlyshapesdiffers. All 19 outputs match node byte-for-byte with exit 0. Because 18 binaries are identical machine code, their ±0.6 ms timing spread on the mini is the in-run noise floor, and no ceiling is approached.gc-handoff/apps/iso_miss.tsprintschecksum 437840 misses 0.Semantics. A probe covering the shapes CLAUDE.md flags as weak — fieldless subclass, indirect subclass, an un-assigned
numberfield, astringfield, and a post-constructiond.x = "str"downgrade — is byte-identical to node, includingObject.keysorder (x,y,unset,w— the elision does not drop a slot) andJSON.stringifyoutput.Unit tests. 20/20 in
lower_call::field_init::tests, 8 of them new, including the two soundness refusals (a trailing statement mentioningthis; asuper()argument mentioningthis) and a pin that the single-class predicate is unchanged.Gap suite. The full run flagged 6 rows. Every one of them was re-run through the harness with the base
0321c6554compiler and reproduces identically — twoparity_fail(byte-identical output under both compilers) and fourSIGABRTwith the samethere is no reactor running, must be called from the context of a Tokio 1.x runtimepanic inperry-ext-http/src/server/server.rs:911. A further 10 rows movednode_fail -> parity_fail, which is an oracle-classification change on a contended host, not a behaviour change: node here is v26.5.1 matching.node-version, and it still refusestest_gap_derived_param_propswithERR_UNSUPPORTED_TYPESCRIPT_SYNTAXwhile both arms print that test's documented expected output exactly. Zero gap regressions are attributable to this change.cargo fmt --check,scripts/check_file_size.sh,scripts/addr_class_inventory.pyandscripts/gc_runtime_root_holders.pyare all clean.What this does NOT reach
The campaign target was
shapes ≤ 0.075 s. This gets to 0.1463. Re-profiling the fixed binary shows why, and the remainder is not in this lever:_tlv_get_addr)shapesis no longer a property-access problem — that family is spent. It is now ~74% GC bookkeeping:layout_forget_objectis the #2 leaf (6.4%) reached fromArenaSweepObjectsState::reclaim_dead_objectandjs_array_alloc;per_object_slot_mask(4.9%) fromHeapChildSlotIterator::newduring the trace;layout_addr_filter_add(3.0%) still 100% fromlayout_transferduring evacuation. That is the open per-object-layout workstream, shared withchurn/tree/retain.Note for #7854
#7854 adds the inline precheck to the strict boxed class-field store arm and touches the same two files. The two are complementary and the conflict is mechanical:
emit_class_field_inline_precheckgained a trailingsubclass_arms: &[ClassFieldSubclassArm]parameter, and that new call site should passclass_field_subclass_arms(ctx, &class_name, property, field_index, false)so boxed stores get the widening too.Summary by CodeRabbit
Performance Improvements
Bug Fixes
Tests