From 9016b4cbb492806882696a565d3463176c22076d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Mon, 24 Aug 2026 04:34:12 +0200 Subject: [PATCH] perf(codegen): count property/index stores as GC safepoints in the spill estimate (#8583) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extends the literal-counting spill estimate (count_safepoint_sites): a property/index STORE (`PropertySet`, `PropertyUpdate`, `IndexSet`) lowers to a collecting runtime call (`js_class_field_set_ic` / `js_set_property` / the array-set helpers) that RS4GC gives a statepoint, but none were counted. A closed-shape object literal compiles to a constructor that is one long run of `this.field = v` stores. With the stores uncounted, such a constructor's estimate was ~0, so it was never spilled to the shadow frame, and RS4GC grew one `__AnonShape_*_constructor` in the Claude Code bundle from 34,009 to 2,280,128 instructions — overrunning the #8586 per-function budget and refusing the whole module. Counting the stores makes the estimate reflect the fan-out. Reads (`PropertyGet`/`IndexGet`) are deliberately NOT counted: they frequently inline to a shape-cached load with no call, so counting them would over-spill read-heavy hot loops. Unit test added. Claude-Session: https://claude.ai/code/session_01TwxRkALrR9HKSF1zKLSTAF --- changelog.d/8583-count-store-safepoints.md | 1 + .../src/collectors/safepoint_sites.rs | 50 +++++++++++++++++++ 2 files changed, 51 insertions(+) create mode 100644 changelog.d/8583-count-store-safepoints.md diff --git a/changelog.d/8583-count-store-safepoints.md b/changelog.d/8583-count-store-safepoints.md new file mode 100644 index 0000000000..56e36504ea --- /dev/null +++ b/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. diff --git a/crates/perry-codegen/src/collectors/safepoint_sites.rs b/crates/perry-codegen/src/collectors/safepoint_sites.rs index 4cb67640f9..853422f36a 100644 --- a/crates/perry-codegen/src/collectors/safepoint_sites.rs +++ b/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); + } }