perf(class): a subclassed hierarchy paid a by-name hash store per field per construction (shapes 0.183 -> 0.146; isolated probe 1.81x)

changelog.d/7861-class-field-subclass-chain.md

@@ -0,0 +1,108 @@
+### Performance
+
+**A subclassed class hierarchy paid a by-name hash store for every field
+assignment in every constructor on its chain.** `gc-handoff/apps/shapes.ts`
+issued **528 000** `js_put_value_set` calls per run; after this change it issues
+**48 000**.
+
+Two independent defects, found in that order, with the second 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 runtime
+`class_field_fast_contract` behind it) compared the receiver's `class_id` and
+`keys_array` against the *declared* class of the expression — one pair, exact
+match. Inside a base class's own constructor or method that bet is not merely
+unreliable, it is **guaranteed wrong**: `this` in `Node2D`'s constructor is only
+ever reached through `super(...)` from a subclass, so both compares fail on every
+single `this.x = x`, and every inherited read — `Node2D`'s `get originDist`
+reading `this.x` — missed 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, which is the
+field-side counterpart of the dispatch widening in
+`lower_call/property_get/dynamic_dispatch.rs` (#7800). Soundness does not rest on
+the layout algorithm's root→leaf ordering: the field's slot index and its 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.
+
+All five `emit_class_field_inline_precheck` sites are widened, **including the
+strict BOXED store arm #7854 un-gated**. That one matters on its own: #7854
+removed the `requires_raw_f64` gate so boxed declared fields stop paying an
+unconditional guard call, but in a base class's own constructor the precheck it
+newly emits would still have missed 100% of the time, because `this` there is
+only ever a subclass. Its arms are computed with `requires_raw_f64` rather than a
+literal, so a candidate subclass whose declared type disagrees about the slot's
+representation is dropped.
+
+Measured effect, with per-precondition counters on `shapes.ts`: the runtime get
+guard goes from being called on every inherited read to **never being entered at
+all** (`get_guard_calls=0`) — every read now takes the inline fast path.
+
+#### 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 —
+`contract_cid=0, contract_keys=0, contract_fieldcount=0, set_frozen=0,
+set_notplain=0` but `contract_rawf64=144000`. Not the guard's class test: the
+side table said the slot was not raw-f64 at all.
+
+`typed_shape::class_layout_declarable_at_allocation` consults
+`ctor_prologue_param_assigned_fields`, which returns the empty set the moment a
+class has `extends`. Empty prologue ⇒ no `js_gc_declare_typed_shape_layout` at
+the allocation site ⇒ `GC_OBJ_TYPED_LAYOUT_INTACT` is clear for the whole
+construction ⇒ every raw-f64 field store in every constructor on the chain
+misses its guard. That is exactly #7512's mechanism ("declaring the fields
+`number` is what makes the class slower — more type information selects a
+representation whose guard the construction path has made unsatisfiable"), which
+was fixed for a standalone class and never extended past it.
+
+It is not a base-class-only tax: `Node2D` extends nothing, yet its own
+`this.x = x` misses 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 single `extends`, 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 the two, which is precisely what made a chain
+unanalysable a class at a time (a fieldless `Marker extends Shape` is the second
+case, and is fine). The extra obligations heritage brings:
+
+- A leading `super(...)` is skipped rather than truncating the prologue at
+  statement 0, but only when every argument is `This`-free, so the parent
+  constructor cannot be handed the half-built instance.
+- Every statement **after** a class's prologue run must be a `Stmt::Expr` with no
+  `this` anywhere in it — a non-leaf constructor's trailing statements run
+  *before* the leaf writes its own fields, so a `this.w` read in `Shape`'s body
+  would see a raw-f64-masked slot still holding `undefined`'s NaN-box bits and
+  yield `NaN` instead of `undefined`. (`Shape.made = Shape.made + 1` is the
+  motivating admission.) The expression scan uses
+  `perry_hir::walker::walk_expr_children`, which is exhaustive and drift-checked
+  against its `_mut` twin; the statement side is a deliberate **whitelist**,
+  because the HIR has no shared statement walker and a missed variant here would
+  be a silent wrong answer rather than a missed optimization.
+- Every raw-f64 field anywhere on the chain must be prologue-assigned by its own
+  class, or the declaration is refused.
+
+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-init `undefined` write into one of
+those slots fails `layout_raw_f64_bits` and downgrades the descriptor on the
+spot, which would make the declaration worthless.
+
+#### Validation
+
+Compiling the 19-program `gc-handoff` corpus with both arms against the **same**
+runtime archives and `cmp`-ing the executables (output basename held constant):
+**18 of 19 byte-identical, only `shapes` differs**, and all 19 outputs match
+node byte-for-byte with exit 0.
+
+A semantics probe covering the shapes CLAUDE.md flags as weak — fieldless
+subclass, indirect subclass, an un-assigned `number` field, a `string` field, and
+a post-construction `d.x = "str"` downgrade — is byte-identical to node,
+including `Object.keys` order and `JSON.stringify` output. Eight new unit tests
+pin the chain analysis, including the two soundness refusals (a trailing
+statement mentioning `this`, and a `super()` argument mentioning `this`).

crates/perry-codegen/src/expr/class_field_inline_guard.rs

@@ -54,6 +54,135 @@ const OBJ_FLAG_HAS_DESCRIPTORS_BIT: &str = "2048"; // OBJ_FLAG_HAS_DESCRIPTORS (
 const OBJ_FLAG_FROZEN_OR_DESCRIPTORS: &str = "2049";
 const F64_EXP_MASK: &str = "9218868437227405312"; // 0x7FF0_0000_0000_0000
 
+/// A widening arm for the class-field shape check: one concrete subclass whose
+/// instances put `property` at the SAME packed slot as the declared class does.
+///
+/// `keys_global` names the module global holding that subclass's canonical keys
+/// array; `class_id` is its registered class id.
+#[derive(Clone, Debug)]
+pub(crate) struct ClassFieldSubclassArm {
+    pub class_id: u32,
+    pub keys_global: String,
+}
+
+/// A hierarchy wider than this turns the shape check into a longer compare
+/// chain than the by-name fallback it replaces. Matches the dispatch-side cap
+/// in `lower_call/property_get/dynamic_dispatch.rs`.
+const MAX_CLASS_FIELD_SUBCLASS_ARMS: usize = 8;
+
+/// Every transitive subclass of `class_name` that agrees with it about
+/// `property`'s slot — i.e. every receiver the field fast path may accept
+/// beyond the declared class itself.
+///
+/// ## Why this exists
+///
+/// `emit_class_field_inline_precheck` (and the runtime `class_field_fast_contract`
+/// behind it) speculates that the receiver's dynamic class is EXACTLY the
+/// expression's declared class. Inside a base class's own constructor or method
+/// that bet is not merely unreliable, it is **guaranteed wrong**: `this` in
+/// `Node2D`'s constructor is only ever reached through `super(...)` from a
+/// subclass, so the class-id compare fails on every single store and each
+/// `this.x = x` pays a full by-name `js_put_value_set`. The same holds for every
+/// inherited read — a `Node2D` getter reading `this.x` misses 100% of the time.
+///
+/// This is the field-side counterpart of the dispatch widening in
+/// `lower_call/property_get/dynamic_dispatch.rs` (#7800): one shape probe,
+/// several (class id, keys) pairs.
+///
+/// ## Why it is sound
+///
+/// `class_field_global_index` lays a class out as its init chain's keyable
+/// fields, root → leaf — parent fields first — so an inherited field keeps its
+/// index in every subclass. That is a property of the layout algorithm, not a
+/// promise, so this **re-derives the index for each candidate** and drops any
+/// subclass that disagrees (a shadowing re-declaration lands at its own slot,
+/// and an accessor anywhere on the chain makes `class_field_global_index`
+/// return `None`). The raw-f64 candidacy of the declared type is likewise
+/// re-checked per subclass: the fast path reads/writes the slot as a bare
+/// double, and the per-object typed-layout intact bit only licenses that for a
+/// field the *matched* class declares as a raw-f64 candidate.
+pub(crate) fn class_field_subclass_arms(
+    ctx: &FnCtx<'_>,
+    class_name: &str,
+    property: &str,
+    field_index: u32,
+    requires_raw_f64: bool,
+) -> Vec<ClassFieldSubclassArm> {
+    let Some(&declared_id) = ctx.class_ids.get(class_name) else {
+        return Vec::new();
+    };
+    // Deterministic order: class id, then name. Codegen output must be
+    // byte-reproducible (the corpus `cmp` A/B depends on it).
+    let mut candidates: Vec<(&String, u32)> = ctx.class_ids.iter().map(|(k, &v)| (k, v)).collect();
+    candidates.sort_unstable_by(|a, b| a.1.cmp(&b.1).then_with(|| a.0.cmp(b.0)));
+
+    let mut arms: Vec<ClassFieldSubclassArm> = Vec::new();
+    let mut seen_ids: Vec<u32> = vec![declared_id];
+    for (sub_name, sub_id) in candidates {
+        if sub_name == class_name || sub_id == 0 || seen_ids.contains(&sub_id) {
+            continue;
+        }
+        if !is_transitive_subclass(ctx, sub_name, class_name) {
+            continue;
+        }
+        // A class with computed runtime members has keys the packed layout
+        // does not describe; its sets route through the by-name path anyway.
+        if super::property_set::class_has_computed_runtime_members(ctx, sub_name) {
+            continue;
+        }
+        // The layout algorithm SHOULD put an inherited field at the same index
+        // in every subclass. Verify rather than assume — a shadowing
+        // re-declaration or an accessor on the subclass chain breaks it.
+        if crate::type_analysis::class_field_global_index(ctx, sub_name, property)
+            != Some(field_index)
+        {
+            continue;
+        }
+        // The fast path's representation choice (raw double vs NaN-boxed) is
+        // fixed at this site, so a subclass whose declared type disagrees would
+        // have the slot read at the wrong representation.
+        let sub_raw_f64 = crate::type_analysis::class_field_declared_type(ctx, sub_name, property)
+            .as_ref()
+            .is_some_and(crate::typed_shape::type_is_raw_f64_candidate);
+        if sub_raw_f64 != requires_raw_f64 {
+            continue;
+        }
+        let Some(keys_global) = ctx.class_keys_globals.get(sub_name).cloned() else {
+            continue;
+        };
+        seen_ids.push(sub_id);
+        arms.push(ClassFieldSubclassArm {
+            class_id: sub_id,
+            keys_global,
+        });
+        if arms.len() > MAX_CLASS_FIELD_SUBCLASS_ARMS {
+            return Vec::new();
+        }
+    }
+    arms
+}
+
+/// Is `name` a transitive subclass of `ancestor`? Cycle- and depth-guarded:
+/// heavily-modular packages declare same-named classes across modules, and the
+/// name-keyed `ctx.classes` can then form a parent cycle (see
+/// `type_analysis_class_fields.rs`).
+fn is_transitive_subclass(ctx: &FnCtx<'_>, name: &str, ancestor: &str) -> bool {
+    let mut seen: std::collections::HashSet<String> = std::collections::HashSet::new();
+    let mut parent = ctx.classes.get(name).and_then(|c| c.extends_name.clone());
+    let mut depth = 0usize;
+    while let Some(p) = parent {
+        depth += 1;
+        if depth > 64 || !seen.insert(p.clone()) {
+            return false;
+        }
+        if p == ancestor {
+            return true;
+        }
+        parent = ctx.classes.get(&p).and_then(|c| c.extends_name.clone());
+    }
+    false
+}
+
 /// Emit the `i1` "plain finite number" predicate on a value's raw bits: true
 /// iff the exponent field is not all-ones. Rejects ±Inf, every NaN (canonical
 /// or boxed), and therefore every NaN-box tag — exactly the values the
@@ -312,6 +441,13 @@ pub(crate) fn emit_proven_shape_recheck(
 /// adds the not-frozen and plain-finite-number checks the set fast contract
 /// requires (a non-number must downgrade through the boxed setter, never a raw
 /// store).
+///
+/// `subclass_arms` widens the shape test from "is exactly the declared class"
+/// to "is the declared class or one of these subclasses, each of which puts
+/// this property at this same slot" — see [`class_field_subclass_arms`] for why
+/// the narrow form misses 100% of the time in a base-class body. Pass an empty
+/// slice to keep the single-pair check; a class with no subclasses emits
+/// byte-identical IR either way.
 #[allow(clippy::too_many_arguments)]
 pub(crate) fn emit_class_field_inline_precheck(
     ctx: &mut FnCtx,
@@ -323,6 +459,7 @@ pub(crate) fn emit_class_field_inline_precheck(
     require_raw_f64: bool,
     set_value_bits: Option<&str>,
     fast_label: &str,
+    subclass_arms: &[ClassFieldSubclassArm],
 ) -> String {
     let deref_idx = ctx.new_block("class_field_inline.deref");
     let guardcall_idx = ctx.new_block("class_field_inline.guardcall");
@@ -395,9 +532,32 @@ pub(crate) fn emit_class_field_inline_precheck(
         // before this dereference.)
         let mut acc = blk.and(I1, &gtype_ok, &not_fwd);
         acc = blk.and(I1, &acc, &ot_ok);
-        acc = blk.and(I1, &acc, &cid_ok);
-        acc = blk.and(I1, &acc, &fc_ok);
-        acc = blk.and(I1, &acc, &ka_ok);
+        if subclass_arms.is_empty() {
+            // Byte-for-byte the pre-widening and-chain. A class with no
+            // eligible subclass must emit IDENTICAL IR, so the corpus-wide
+            // `cmp` stays a usable no-regression instrument (a reordered
+            // and-chain alone made 17 of 19 corpus binaries differ for no
+            // behavioural reason).
+            acc = blk.and(I1, &acc, &cid_ok);
+            acc = blk.and(I1, &acc, &fc_ok);
+            acc = blk.and(I1, &acc, &ka_ok);
+        } else {
+            // The declared class's own (class id, keys) pair, OR any subclass
+            // arm's. Each arm is a full pair — matching a class id without its
+            // canonical keys array would accept an instance that has since
+            // grown a property and no longer has the packed layout this slot
+            // index describes.
+            let mut shape_ok = blk.and(I1, &cid_ok, &ka_ok);
+            for arm in subclass_arms {
+                let arm_cid_ok = blk.icmp_eq(I32, &class_id, &arm.class_id.to_string());
+                let arm_keys = blk.load(I64, &format!("@{}", arm.keys_global));
+                let arm_ka_ok = blk.icmp_eq(I64, &keys_array, &arm_keys);
+                let arm_ok = blk.and(I1, &arm_cid_ok, &arm_ka_ok);
+                shape_ok = blk.or(I1, &shape_ok, &arm_ok);
+            }
+            acc = blk.and(I1, &acc, &shape_ok);
+            acc = blk.and(I1, &acc, &fc_ok);
+        }
 
         // #5654: a receiver that has ever had a property / accessor descriptor
         // installed on it (Object.defineProperty / freeze / seal) needs the

crates/perry-codegen/src/expr/mod.rs

@@ -1990,7 +1990,7 @@ mod new_dynamic;
 mod objects_arrays_lit;
 mod os_uri_dates;
 pub(crate) mod property_get;
-mod property_set;
+pub(crate) mod property_set;
 pub(crate) mod proxy_reflect;
 mod static_field_meta;
 mod static_method;

crates/perry-codegen/src/expr/property_get.rs

@@ -1607,6 +1607,14 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result<String> {
                         // branches straight to the fast slot load, skipping the
                         // cross-crate guard call; on a miss it leaves the current
                         // block at the guard-call path below (unchanged).
+                        let subclass_arms =
+                            crate::expr::class_field_inline_guard::class_field_subclass_arms(
+                                ctx,
+                                &class_name,
+                                property,
+                                field_index,
+                                requires_raw_f64,
+                            );
                         let _guardcall_label =
                             crate::expr::class_field_inline_guard::emit_class_field_inline_precheck(
                                 ctx,
@@ -1618,6 +1626,7 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result<String> {
                                 requires_raw_f64,
                                 None,
                                 &fast_label,
+                                &subclass_arms,
                             );
                         let guard_ok = ctx.block().call(
                             I32,

crates/perry-codegen/src/expr/property_get/helpers.rs

@@ -736,6 +736,13 @@ pub(crate) fn lower_raw_f64_class_field_get_for_number_context(
     let fallback_label = ctx.block_label(fallback_idx);
     let merge_label = ctx.block_label(merge_idx);
 
+    let subclass_arms = crate::expr::class_field_inline_guard::class_field_subclass_arms(
+        ctx,
+        &class_name,
+        property,
+        field_index,
+        true,
+    );
     let _guardcall_label = crate::expr::class_field_inline_guard::emit_class_field_inline_precheck(
         ctx,
         &obj_bits,
@@ -746,6 +753,7 @@ pub(crate) fn lower_raw_f64_class_field_get_for_number_context(
         true,
         None,
         &fast_label,
+        &subclass_arms,
     );
     let guard_ok = ctx.block().call(
         I32,