perf(codegen): restore the fast store for class field initializers (94% of #8648)

changelog.d/8648-class-field-init-fast-path.md

@@ -0,0 +1,40 @@
+Restored the optimized store for class field initializers.
+
+#8630 replaced the field-initializer lowering with an unconditional
+`js_class_field_add` — a full `[[DefineOwnProperty]]` behind a handle scope,
+per field, per construction. The motivation was correct: DefineField uses
+CreateDataProperty, so an inherited setter must not run and a Proxy receiver
+must observe its `defineProperty` trap. But a bare declaration (`x: number;`)
+still gets a synthesized `undefined` initializer, so an ordinary class pays the
+full define path for every field of every instance. `shapes.ts` (7 classes,
+~2-3 fields each, 120k constructions) measured **3.11x** the instruction count
+of the pre-#8630 compiler.
+
+The two semantics coincide when neither difference can arise, and both are
+statically decidable:
+
+* **no accessor anywhere on the chain** — `class_field_global_index` already
+  answers exactly this, returning `None` the moment an accessor or a
+  re-declaration appears on the chain (the #5654 machinery); and
+* **the receiver is provably the freshly allocated ordinary instance** — no
+  constructor on the chain returns a value, `js_ctor_return_override` being the
+  only route by which a Proxy can become the field-initializer receiver.
+
+When both hold, lower through the `PropertySet` path (inline shape precheck ->
+direct slot store) as this did before #8630. Everything else keeps the full
+DefineField call. The chain walk is conservative on every edge it cannot see: a
+native base, a dynamic `extends`, an id-only parent edge, or a class missing
+from `ctx.classes` all answer "unsafe".
+
+Measured (instructions retired, vs the pre-#8630 compiler at `00bddb34b`):
+`shapes` 3,662,616,604 -> 1,320,393,329 against a 1,179,031,124 baseline —
+**94% of the regression recovered**, 3.11x -> 1.12x.
+
+Verified against Node 26.5.1 that #8630's fix is preserved: for
+`class Base { set v(x) {...} }` / `class Derived extends Base { v = 5 }`, the
+pre-#8630 compiler printed `SETTER RAN` and `d.v=undefined`; this prints
+`d.v=5`, matching Node. A getter two levels up the chain also matches, and a
+value-returning parent constructor still emits `js_class_field_add`.
+
+Partial for #8648: `cycles`, `deeplist` and a two-class `new B(x, y)` loop are
+essentially unmoved by this, so they have a second, independent cause.

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

@@ -82,7 +82,9 @@ pub(crate) use integer_locals::{
     collect_flat_row_aliases, is_int32_producing_expr, static_index_window,
 };
 pub(crate) use local_refs::{expr_contains_local_get, mark_all_candidate_refs_in_expr};
-pub(crate) use mutation::{body_contains_call, body_contains_closure, has_any_mutation};
+pub(crate) use mutation::{
+    body_contains_call, body_contains_closure, body_returns_value, has_any_mutation,
+};
 pub(crate) use number_by_construction::collect_number_by_construction_locals;
 pub(crate) use param_ranges::{collect_param_int_ranges, ParamIntRanges};
 pub(crate) use pointer_locals::collect_pointer_typed_locals;

crates/perry-codegen/src/collectors/mutation.rs

@@ -57,6 +57,44 @@ pub fn body_contains_closure(stmts: &[perry_hir::Stmt]) -> bool {
     any_top_level_expr(stmts, &mut expr_contains_closure)
 }
 
+/// #8648: can this constructor body hand back a replacement `this`?
+///
+/// ECMAScript lets a constructor `return` an object, which becomes the
+/// construction's result (`js_ctor_return_override`). That replacement may be a
+/// Proxy, and DefineField must reach a Proxy's `defineProperty` trap rather
+/// than its `set`. With no value-returning `return` anywhere on the chain, the
+/// instance a field initializer writes to is provably the freshly allocated
+/// ordinary object, so `CreateDataProperty` and a plain own-slot store agree.
+pub fn body_returns_value(stmts: &[perry_hir::Stmt]) -> bool {
+    stmts_have_value_return(stmts)
+}
+
+/// `Stmt::Return(Some(_))` at any statement depth.
+fn stmts_have_value_return(stmts: &[perry_hir::Stmt]) -> bool {
+    use perry_hir::Stmt;
+    for s in stmts {
+        let hit = match s {
+            Stmt::Return(Some(_)) => true,
+            Stmt::If {
+                then_branch,
+                else_branch,
+                ..
+            } => {
+                stmts_have_value_return(then_branch)
+                    || else_branch
+                        .as_ref()
+                        .is_some_and(|b| stmts_have_value_return(b))
+            }
+            Stmt::While { body, .. } | Stmt::For { body, .. } => stmts_have_value_return(body),
+            _ => false,
+        };
+        if hit {
+            return true;
+        }
+    }
+    false
+}
+
 fn expr_contains_closure(expr: &perry_hir::Expr) -> bool {
     if matches!(expr, perry_hir::Expr::Closure { .. }) {
         return true;

crates/perry-codegen/src/lower_call/field_init.rs

@@ -816,7 +816,39 @@ pub(crate) fn apply_field_initializers_recursive(
 
             // DefineField uses CreateDataProperty semantics: an inherited
             // setter must not run, while a Proxy receiver must observe its
-            // `defineProperty` trap.
+            // `defineProperty` trap. `js_class_field_add` provides both, but it
+            // is a full [[DefineOwnProperty]] behind a handle scope — per field,
+            // per construction. #8648: `shapes.ts` pays it ~2M times (7 classes
+            // x ~2-3 fields x 120k constructions) and measured 3.14x.
+            //
+            // The two semantics coincide when neither difference can arise:
+            //
+            //   * no accessor anywhere on the chain -- `class_field_global_index`
+            //     already answers exactly this, returning `None` the moment an
+            //     accessor (or a re-declaration) appears on the chain
+            //     (`class_field_inline_guard`, #5654); and
+            //   * the receiver is provably the freshly allocated ordinary
+            //     instance -- no constructor on the chain hands back a
+            //     replacement `this` via `js_ctor_return_override`, which is the
+            //     only way a Proxy can become the field-initializer receiver.
+            //
+            // Both hold for an ordinary class, so lower through the optimized
+            // `PropertySet` path (inline shape precheck -> direct slot store)
+            // exactly as this did before #8630. Anything else keeps the full
+            // DefineField call.
+            let chain_can_replace_this = chain_constructor_returns_value(ctx, &class_name_in_chain);
+            let no_accessor_on_chain =
+                crate::type_analysis::class_field_global_index(ctx, &class_name_in_chain, &prop)
+                    .is_some();
+            if no_accessor_on_chain && !chain_can_replace_this {
+                let set_expr = Expr::PropertySet {
+                    object: Box::new(Expr::This),
+                    property: prop,
+                    value: Box::new(init_expr),
+                };
+                let _ = lower_expr(ctx, &set_expr)?;
+                continue;
+            }
             let value = lower_expr(ctx, &init_expr)?;
             let this_val = ctx
                 .this_stack
@@ -882,3 +914,36 @@ pub(crate) fn apply_field_initializers_recursive(
 
 #[cfg(test)]
 mod tests;
+
+/// #8648: does any constructor on `leaf`'s inheritance chain `return` a value?
+///
+/// A value-returning constructor can hand back a replacement `this`
+/// (`js_ctor_return_override`), and that replacement may be a Proxy — which
+/// DefineField must reach through `defineProperty`, not `set`. Conservative on
+/// every edge it cannot see: a native base, a dynamic `extends`, or a class
+/// missing from `ctx.classes` all answer `true`.
+fn chain_constructor_returns_value(ctx: &crate::expr::FnCtx<'_>, leaf: &str) -> bool {
+    let mut name = leaf.to_string();
+    for _ in 0..32 {
+        let Some(class) = ctx.classes.get(&name).copied() else {
+            return true;
+        };
+        if class.native_extends.is_some() || class.extends_expr.is_some() {
+            return true;
+        }
+        if let Some(ctor) = class.methods.iter().find(|m| m.name == "constructor") {
+            if crate::collectors::body_returns_value(&ctor.body) {
+                return true;
+            }
+        }
+        // `extends` is a class id; `extends_name` is the textual parent. Only
+        // the latter can be followed through `ctx.classes`, so an id-only edge
+        // is treated as unknown.
+        match class.extends_name.as_ref() {
+            Some(parent) => name = parent.clone(),
+            None if class.extends.is_some() => return true,
+            None => return false,
+        }
+    }
+    true
+}