merge: land #8623, #8625, #8627 with metadata fixes and a non-vacuous GC fixture

changelog.d/8623-root-spill-threshold.md

@@ -0,0 +1,3 @@
+### Changed
+
+- Native GC-root spilling default raised from 4,000,000 to 32,000,000 estimated statepoint relocations (#8620, #8589). The 4M default fired on moderate-fan-out functions whose native `rewrite-statepoints-for-gc` statepoints optimize fine — on an ~8M-relocation entry function, spilling to the shadow frame was measured *slower* than the fan-out it replaced (303 s spilled vs 180 s fanned out), paying shadow-frame overhead for nothing. Measured synthetic entry functions (`@main` codegen unit, `-Os`, spilling off) fan out in bounded time up to 32M (~8.5 min) and do not finish past 40M (> 20 min), so the default is set to the largest estimate whose fan-out still finished. This is compile-time only — spilling a run-once init entry does not affect the emitted binary's runtime — and is backstopped by the post-RS4GC instruction-budget assertion (`PERRY_LL_RS4GC_MAX_INSTRS`, #8586), which fails loudly rather than hanging if a function this estimate misses still fans out. `PERRY_ROOT_SPILL_RELOCATIONS=<n>` still overrides it; `0` disables spilling.

changelog.d/8625-ta-view-param-number-by-construction.md

@@ -0,0 +1,38 @@
+A hot function that folds a typed-array **parameter** into an accumulator —
+`function reduce(arr: Float64Array) { let s = 0.0; for (…) { let x = arr[i] + 1.0;
+s = s + x; } }` — no longer keeps its numeric locals as NaN-boxed GC roots.
+
+The spec-ABI already proves such a parameter (`TaPtr`) permanently holds one
+specific numeric-kind, non-view typed array, and specializes the body so the
+element read inlines. But the `number_by_construction` fixpoint
+(`collectors/ptr_shape_numeric.rs`) only recognised a typed-array element read as
+"a Number or `undefined`, never a pointer" for a **local** view with a
+compiler-visible `TypedArrayNew` initializer — not for a proven `TaPtr`
+parameter. So the fresh, read-derived `x` failed the numeric proof, which
+cascaded to the loop-carried accumulator `s = s + x`. Both then kept a shadow
+root slot with a per-write `js_write_barrier_root_nanbox`, and `s`'s update
+lowered to the opaque `js_dynamic_string_or_number_add` call instead of an inline
+`fadd`.
+
+The fixpoint now also treats a read off a `spec_ta_lens` binding as
+Number-or-`undefined`. `spec_ta_lens` is keyed exactly by `SpecParamRep::TaPtr`
+parameters, and `collectors::spec_abi_sites` admits a `TaPtr` only for
+`spec_ta_kind_is_numeric` kinds (the BigInt typed arrays — whose elements are
+BigInt pointers — are never `TaPtr`), so `arr[numeric_index]` off one is provably
+a Number in-bounds and `undefined` out of range, which `+`/`-` launders into a
+genuine Number (`NaN` at worst). The `rec(index)` guard is retained — a
+non-numeric key would read a property, which can be a pointer. Soundness rests on
+the entry contract, not on the erased `Float64Array` annotation, so a reassigned
+or unproven receiver is untouched.
+
+Effect on a 200000×4096 `Float64Array` reduction passed by parameter: the
+accumulator's per-iteration `js_dynamic_string_or_number_add` and root barrier
+become a single `fadd` in a raw `double` slot — ~5× faster (measured 5.1–7.3s →
+~1.0s), with byte-identical output to the rooted build under every moving-GC
+configuration and to Node.
+
+Does not yet cover a typed array read through a **module-global** binding (the
+issue #8619 reproducer): on `main` that read is still a runtime call (module-
+global read inlining, #8617, is unmerged), so its accumulator rooting is a
+secondary cost there; extending the same proof to `module_global_proven_types`
+is the natural follow-up once the read inlines.

changelog.d/8627-report-size-duplicate-body-accuracy.md

@@ -0,0 +1,3 @@
+### Fixed
+
+- `perry compile --report-size` (from [#8579](https://github.com/PerryTS/perry/pull/8579)): duplicate function/static-data body detection now keys directly on the exact byte slice instead of an FNV-1a hash + size check — a hash collision at the same size could previously fabricate a false duplicate finding (caught in CodeRabbit review, fixed before this PR rather than just reworded, since the fix was as cheap as the hash was). Also corrected the "duplicate crate instance" finding's framing: verified directly (md5 + objdump on the extracted archive members) that while `perry-runtime`/`perry-stdlib` do redundantly compile some shared dependencies (`gimli` confirmed byte-identical across their two separate `.a` archives), a successful link only pulls one physical copy per symbol — so the attributed bytes are real, in-use code in the shipped binary, not a duplicate sitting in it twice. The report and its "Suggestions" section now say this explicitly (renamed `duplicate_crate_versions` to `duplicate_crate_instances` in the JSON schema, and `estimated_bytes` for this finding is `0` rather than a size claim) instead of overclaiming a shipped-binary-size win that isn't there.

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

@@ -367,14 +367,33 @@ pub(crate) fn inline_hot_small_max_call_sites() -> u32 {
 /// bundle's 68 MB entry body measured 795 root slots × ~106k safepoints ≈ 8.4e7
 /// and grew 439k → 6.5M instructions under RS4GC; without RS4GC the same unit
 /// optimized at `-Os` in ~5s). Real functions sit orders of magnitude below
-/// this: hundreds of call sites times tens of slots is ~1e4–1e5. The default
-/// is set well under the measured pathological point and well over ordinary
-/// code, and the post-RS4GC instruction-budget assertion (#8583, inprocess.rs)
-/// backstops any function the estimate misses.
+/// this: hundreds of call sites times tens of slots is ~1e4–1e5.
+///
+/// The default (#8620) is measured, not guessed. Synthetic entry functions with
+/// a controlled `slots × safepoints` estimate were compiled at `-Os` with
+/// spilling OFF (pure RS4GC fan-out) and the `@main` codegen unit timed:
+///
+/// | estimate | fan-out finish |
+/// |---------:|---------------:|
+/// |     8.0M |         ~325 s |
+/// |    16.0M |         ~235 s |
+/// |    32.0M |  ~511 s (8.5m) |
+/// |    40.0M |  did not finish in 20 min |
+/// |    48.0M |  did not finish in 20 min |
+///
+/// The fan-out cliff sits between 32M and 40M, so the default is the largest
+/// estimate whose fan-out still finished in bounded time. Below it fan-out is
+/// the cheaper lowering — spilling a moderate function costs more than the
+/// fan-out it avoids (an ~8M function spilled in 303 s vs 180 s fanned out,
+/// #8620) — and above it fan-out risks not finishing and the shadow frame wins.
+/// The former 4M default fired on ~8M functions that fan out fine in minutes.
+/// The post-RS4GC instruction-budget assertion (#8586, inprocess.rs) backstops
+/// any function this estimate misses: it fails loudly rather than hanging, so
+/// raising the threshold is safe.
 ///
 /// `PERRY_ROOT_SPILL_RELOCATIONS=<n>` overrides it; `0` disables spilling
 /// (every function stays on native statepoints, the pre-#8583 behavior).
-const DEFAULT_ROOT_SPILL_RELOCATIONS: usize = 4_000_000;
+const DEFAULT_ROOT_SPILL_RELOCATIONS: usize = 32_000_000;
 
 fn root_spill_relocation_threshold() -> usize {
     std::env::var("PERRY_ROOT_SPILL_RELOCATIONS")
@@ -390,6 +409,45 @@ pub(crate) fn root_relocation_estimate(slot_count: usize, safepoint_sites: usize
     slot_count.saturating_mul(safepoint_sites)
 }
 
+#[cfg(test)]
+mod root_spill_default_tests {
+    use super::{root_relocation_estimate, DEFAULT_ROOT_SPILL_RELOCATIONS};
+
+    /// #8620: the default is pinned to the measured RS4GC fan-out cliff — the
+    /// largest estimate whose fan-out finished in bounded time (32M finished in
+    /// ~8.5 min; 40M/48M did not finish in 20 min). Change it only with fresh
+    /// measurement.
+    #[test]
+    fn default_sits_at_the_measured_fan_out_cliff() {
+        assert_eq!(DEFAULT_ROOT_SPILL_RELOCATIONS, 32_000_000);
+    }
+
+    /// The moderate case the old 4M default wrongly spilled (#8620): ~8M
+    /// relocations (4000 root slots × ~2001 safepoints) fans out in minutes, so
+    /// under the new default it stays on native statepoints.
+    #[test]
+    fn moderate_fan_out_stays_on_statepoints() {
+        let est = root_relocation_estimate(4000, 2001);
+        assert_eq!(est, 8_004_000);
+        assert!(
+            est <= DEFAULT_ROOT_SPILL_RELOCATIONS,
+            "moderate estimate {est} must not exceed the default (would spill)",
+        );
+    }
+
+    /// The genuinely-catastrophic case (Claude Code `cli.js` `@main`,
+    /// ~795 slots × ~106k safepoints ≈ 8.4e7, never finishes at `-Os`) must
+    /// still spill under the new default.
+    #[test]
+    fn catastrophic_fan_out_still_spills() {
+        let est = root_relocation_estimate(795, 106_000);
+        assert!(
+            est > DEFAULT_ROOT_SPILL_RELOCATIONS,
+            "catastrophic estimate {est} must exceed the default (should spill)",
+        );
+    }
+}
+
 /// Decide whether `func` should spill its roots to the shadow frame, and if so
 /// mark it (BEFORE its `enable_*_shadow_frame` call) and report it. Only
 /// meaningful under native stack-map roots — the shadow frame is already the

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

@@ -92,12 +92,23 @@ pub(crate) fn collect_number_by_construction_locals(
     if !enabled() {
         return HashSet::new();
     }
+    // #8619: spec-ABI `TaPtr` parameters are proven to permanently hold one
+    // specific NUMERIC-kind, non-view typed array — `spec_ta_lens` is keyed
+    // exactly by those params (its only source is `SpecParamRep::TaPtr`, which
+    // `collectors::spec_abi_sites` admits only for `spec_ta_kind_is_numeric`
+    // kinds; the BigInt kinds are never TaPtr). A read `arr[numeric_index]` off
+    // one is therefore a Number (in-bounds) or `undefined` (OOB), never a
+    // pointer/string, so the fixpoint may treat it like a compiler-visible
+    // local typed-view constructor on one side of `+` (where `undefined`
+    // becomes the Number NaN rather than selecting string concatenation).
+    let numeric_ta_views: HashSet<u32> = spec_ta_lens.keys().copied().collect();
     let mut numeric = super::ptr_shape::collect_numeric_by_construction_locals_for_type_analysis(
         stmts,
         boxed_vars,
         module_globals,
         not_bigint_locals,
         &HashMap::new(),
+        &numeric_ta_views,
     );
     numeric.extend(collect_number_at_read_after_undefined(
         stmts,
@@ -545,4 +556,81 @@ mod tests {
 
         assert!(!run(&stmts).contains(&N));
     }
+
+    // #8619: a spec-ABI `TaPtr` parameter is proven to permanently hold one
+    // specific NUMERIC-kind, non-view typed array, so `arr[numeric_index]` is a
+    // Number (in-bounds) or `undefined` (OOB) — never a pointer. The
+    // number-by-construction fixpoint must therefore admit a fresh
+    // read-derived local `let x = arr[i] + 1.0` (whose value is a genuine
+    // Number, `NaN` at worst) and cascade to the loop-carried accumulator
+    // `s = s + x`, so both drop their GC-root slot and their arithmetic stays
+    // an inline `fadd` instead of `js_dynamic_string_or_number_add`.
+    fn ta_view_stmts(arr: u32, s_id: u32, x_id: u32) -> Vec<Stmt> {
+        vec![
+            Stmt::Let {
+                id: s_id,
+                name: "s".to_string(),
+                ty: HirType::Number,
+                mutable: true,
+                init: Some(Expr::Number(0.0)),
+            },
+            Stmt::Let {
+                id: x_id,
+                name: "x".to_string(),
+                ty: HirType::Number,
+                mutable: false,
+                init: Some(add(
+                    Expr::IndexGet {
+                        object: Box::new(Expr::LocalGet(arr)),
+                        index: Box::new(Expr::Integer(0)),
+                    },
+                    Expr::Number(1.0),
+                )),
+            },
+            Stmt::Expr(Expr::LocalSet(
+                s_id,
+                Box::new(add(Expr::LocalGet(s_id), Expr::LocalGet(x_id))),
+            )),
+        ]
+    }
+
+    fn run_fixpoint(stmts: &[Stmt], ta_views: &HashSet<u32>) -> HashSet<u32> {
+        crate::collectors::ptr_shape::collect_numeric_by_construction_locals_for_type_analysis(
+            stmts,
+            &HashSet::new(),
+            &HashMap::new(),
+            &HashSet::new(),
+            &HashMap::new(),
+            ta_views,
+        )
+    }
+
+    #[test]
+    fn spec_ta_param_view_admits_read_derived_number_locals() {
+        let (arr, s_id, x_id) = (10u32, 20u32, 21u32);
+        let stmts = ta_view_stmts(arr, s_id, x_id);
+
+        let with = run_fixpoint(&stmts, &HashSet::from([arr]));
+        assert!(
+            with.contains(&x_id),
+            "fresh `arr[i] + 1.0` local must be Number by construction"
+        );
+        assert!(
+            with.contains(&s_id),
+            "accumulator must cascade to Number by construction"
+        );
+    }
+
+    #[test]
+    fn ta_read_without_spec_proof_stays_dynamic() {
+        // Same body, but the receiver is NOT a spec-proven typed array: the read
+        // could be a string/property access on an arbitrary receiver, so neither
+        // local may be un-rooted.
+        let (arr, s_id, x_id) = (10u32, 20u32, 21u32);
+        let stmts = ta_view_stmts(arr, s_id, x_id);
+
+        let without = run_fixpoint(&stmts, &HashSet::new());
+        assert!(!without.contains(&x_id));
+        assert!(!without.contains(&s_id));
+    }
 }

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

@@ -479,6 +479,10 @@ pub(crate) fn collect_shape_proven_ptr_locals_and_element_fields(
         module_globals,
         not_bigint_locals,
         &const_local_inits,
+        // #8619: this is the `Ptr<Shape>` provenance pass (feeds `is_numeric_expr`),
+        // not the local rooting proof; it has no specialized `TaPtr` context, so
+        // no view binding is spec-proven here.
+        &HashSet::new(),
     );
     // A spec entry has validated these parameters before entering this body.
     // Unlike a TypeScript annotation, that is runtime evidence, so derived

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

@@ -785,6 +785,7 @@ fn numeric_locals_of(stmts: &[Stmt]) -> HashSet<u32> {
         &HashMap::new(),
         &HashSet::new(),
         &HashMap::new(),
+        &HashSet::new(),
     )
 }
 
@@ -942,6 +943,7 @@ fn non_numeric_writes_and_bindings_are_excluded() {
             &HashMap::new(),
             &HashSet::new(),
             &HashMap::new(),
+            &HashSet::new(),
         )
         .contains(&7),
         "a boxed local's write set is not this region's to enumerate"
@@ -976,6 +978,7 @@ fn update_value_resolves_via_not_bigint() {
         &HashMap::new(),
         &not_bigint,
         &HashMap::new(),
+        &HashSet::new(),
     );
     assert!(with_fact.contains(&21));
     let without_fact = numeric::collect_numeric_by_construction_locals(
@@ -984,6 +987,7 @@ fn update_value_resolves_via_not_bigint() {
         &HashMap::new(),
         &HashSet::new(),
         &HashMap::new(),
+        &HashSet::new(),
     );
     assert!(
         !without_fact.contains(&22),