perf(codegen): count property/index stores as GC safepoints in the spill estimate (#8583)

changelog.d/8583-count-store-safepoints.md

@@ -0,0 +1 @@
+fix(codegen): the RS4GC root-spill estimate (`count_safepoint_sites`, #8583) now counts property/index STORES (`PropertySet`, `PropertyUpdate`, `IndexSet`) as GC safepoints, extending the earlier literal-counting fix. A closed-shape object literal compiles to a constructor that is one long run of `this.field = v` stores, each lowering to a collecting `js_class_field_set_ic` / `js_set_property` call that RS4GC gives a statepoint; with none counted, the constructor's estimate was ~0, it was never spilled, and RS4GC grew one such `__AnonShape_*_constructor` from 34k to 2.28M instructions — overrunning the #8586 per-function budget and refusing the whole module. Reads (`PropertyGet`/`IndexGet`) are deliberately not counted: they frequently inline to a shape-cached load with no call, and counting them would over-spill read-heavy hot loops.

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

@@ -71,6 +71,20 @@ fn is_safepoint(e: &Expr) -> bool {
             | Expr::ObjectAssign { .. }
             | Expr::Array(_)
             | Expr::ArraySpread(_)
+            // #8583 (`__AnonShape_*_constructor`): a property/index STORE lowers
+            // to an allocating, collecting runtime call (`js_class_field_set_ic`
+            // / `js_set_property` / the array-set helpers) that RS4GC gives a
+            // statepoint. A closed-shape object literal compiles to a constructor
+            // that is one long run of `this.field = v` stores (`PropertySet`);
+            // with none counted the estimate was ~0, the constructor was not
+            // spilled, and RS4GC grew it 34k -> 2.28M instructions, overrunning
+            // the #8586 budget. Count the stores (not the reads:
+            // `PropertyGet`/`IndexGet` frequently inline to a shape-cached load
+            // with no call, and counting them would over-spill read-heavy hot
+            // loops). `PropertyUpdate` (`x.f++`) is a read-modify-write store.
+            | Expr::PropertySet { .. }
+            | Expr::PropertyUpdate { .. }
+            | Expr::IndexSet { .. }
     )
 }
 
@@ -257,4 +271,40 @@ mod tests {
         // 1 outer + 10 inner = 11.
         assert_eq!(count_safepoint_sites(&[Stmt::Expr(Expr::Array(rows))]), 11);
     }
+
+    #[test]
+    fn property_and_index_stores_are_safepoints() {
+        // #8583: `this.field = v` / `arr[i] = v` lower to a collecting runtime
+        // call and must count. A closed-shape object literal is a constructor of
+        // many `PropertySet` stores (the `__AnonShape_*_constructor` shape) — the
+        // pre-fix count saw none, so the constructor never spilled and RS4GC
+        // overran the #8586 budget.
+        let this = || Expr::LocalGet(0);
+        let set = |p: &str| Expr::PropertySet {
+            object: Box::new(this()),
+            property: p.to_string(),
+            value: Box::new(Expr::Number(1.0)),
+        };
+        // Three field stores in the constructor body.
+        let body = vec![Stmt::Expr(set("a")), Stmt::Expr(set("b")), Stmt::Expr(set("c"))];
+        assert_eq!(count_safepoint_sites(&body), 3);
+
+        // An index store counts too; the value sub-expression still recurses
+        // (a call in the value is its own safepoint).
+        let idx_set = Expr::IndexSet {
+            object: Box::new(this()),
+            index: Box::new(Expr::Number(0.0)),
+            value: Box::new(call(vec![])),
+        };
+        // 1 for the IndexSet + 1 for the call in `value`.
+        assert_eq!(count_safepoint_sites(&[Stmt::Expr(idx_set)]), 2);
+
+        // A read (`PropertyGet`) is deliberately NOT a safepoint (it inlines).
+        let get = Expr::PropertyGet {
+            object: Box::new(this()),
+            property: "x".to_string(),
+            byte_offset: 0,
+        };
+        assert_eq!(count_safepoint_sites(&[Stmt::Expr(get)]), 0);
+    }
 }