From 68b96bc1904fda60c378448fe92bac3e8961b0e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 6 Sep 2026 02:45:07 +0200 Subject: [PATCH 01/26] docs(changelog): fragment for PR 9833 Claude-Session: https://claude.ai/code/session_014UZWia6L37DpA93VLtNK9m --- changelog.d/9833-probe10-margin.md | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) create mode 100644 changelog.d/9833-probe10-margin.md diff --git a/changelog.d/9833-probe10-margin.md b/changelog.d/9833-probe10-margin.md new file mode 100644 index 0000000000..faebd01afb --- /dev/null +++ b/changelog.d/9833-probe10-margin.md @@ -0,0 +1,29 @@ +**The `10_store_receiver_across_alloc` GC-ratchet probe was running no +collection at all, and has been given margin** (#9833, fixes #9832). + +The probe exists to catch a store receiver held in a register across an +evacuating minor — the stale-root class of #6970 / #9523 — and its own header +lists three conditions that must all hold for it to bite, the third being an +allocating right-hand side. On `main` it reported `minor_cycles = 0`: no minor +ran, so no evacuation happened, so there was no window and the probe measured +nothing. `freed_bytes = 0` alongside `copied_objects = 0` rules out "a minor ran +and found nothing live". + +The cause was margin rather than a bug. At 200,000 iterations the probe crossed +the nursery threshold exactly once, and #8313 — shrinking a two-field object +from 56 to 40 bytes — put it under. A probe that fires exactly one collection is +one optimisation away from firing none. It is now 2,400,000 iterations, which +measured 9 minors and keeps several after a further eightfold reduction in bytes +per object, for about 120 ms on `wall_ms`, which the gate does not band. + +Verified by sabotage rather than by the counter moving: removing the allocating +RHS returns `minor_cycles=0 copied_objects=0 freed_bytes=0`, the exact signature +the probe had while broken. + +`heap_used_bytes` returns from 464,072 to 244,648 against a pinned baseline of +220,384 — the +110.57 % that cell showed on `main` was the post-`gc()` residue +of a run in which nothing was ever collected, not retention. + +Five further probes (`01`, `02`, `03`, `09`, `11`) currently sit at +`minor_cycles == 1` and are one allocation win away from the same silent state; +that is recorded in #9832 and not addressed here. From b41c4b9269cc1766b80ca3680dda5d96623887bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 6 Sep 2026 09:21:23 +0200 Subject: [PATCH 02/26] diag(hir): report every native-instance tag at the two entry points that create them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `PERRY_NATIVEINST_DIAG=1` prints one line per native-instance registration: [nativeinst] REGISTER push_module name="O" -> child_process::Instance `register_native_instance` and `push_module_native_instance` are the only two entry points through which a native-instance tag can come into existence, so a diagnostic on them cannot miss a tag. That placement is the point of the change: an earlier attempt at the same question instrumented four plausible construction sites out of the 165 that build `Expr::NativeMethodCall`, printed zero, and the zero was uninterpretable. What it is for (#9847). The tag table is keyed by identifier TEXT with module-wide scope. On a minified bundle that compiles as one module the same short name is routinely claimed by several unrelated native classes, and every method call on any local with that name is then lowered as a native-instance call of whichever class won. On claude-code's `cli_2.1.112.js` this report prints 795 registrations whose most-registered identifiers are Y(71), z(65), K(65), _(65), A(54), O(52), w(37), q(35) — every one a single letter — with `O` registered as `stream::Instance`, `child_process::Instance`, `transform_stream::TransformStream` and `readable_stream::ReadableStream` at once. Reading that took one 30-second compile; deriving it from source took a day of hypotheses, four of which were wrong. `PERRY_NATIVEINST_DIAG` is excluded from the build-level cache for the same reason `PERRY_OPT_REPORT` is: a cached build reuses the finished binary and never lowers HIR, so the report would come up empty — and empty is indistinguishable from "no tag was ever registered", which is precisely the reading this diagnostic exists to make impossible. Off, the cost is one relaxed atomic load per registration and nothing else. --- changelog.d/9847-nativeinst-registry-diag.md | 16 +++++++++ crates/perry-hir/src/lower/context.rs | 35 +++++++++++++++++++ .../perry/src/commands/compile/build_cache.rs | 8 +++++ 3 files changed, 59 insertions(+) create mode 100644 changelog.d/9847-nativeinst-registry-diag.md diff --git a/changelog.d/9847-nativeinst-registry-diag.md b/changelog.d/9847-nativeinst-registry-diag.md new file mode 100644 index 0000000000..43102427a8 --- /dev/null +++ b/changelog.d/9847-nativeinst-registry-diag.md @@ -0,0 +1,16 @@ +`PERRY_NATIVEINST_DIAG=1` reports every native-instance tag as it is created. + +`register_native_instance` and `push_module_native_instance` are the only two +entry points through which such a tag can come into existence, so a diagnostic +on them cannot miss one the way a diagnostic on guessed construction sites can +— which is why it is placed there. One line per registration: + +``` +[nativeinst] REGISTER push_module name="O" -> child_process::Instance +``` + +The env var is excluded from the build-level cache, because a cached build +reuses the finished binary and never lowers HIR, so the report would print +nothing — and nothing is indistinguishable from "no tag was ever registered". + +Off, the cost is one relaxed atomic load per registration. diff --git a/crates/perry-hir/src/lower/context.rs b/crates/perry-hir/src/lower/context.rs index 338b27193f..0f392b3e71 100644 --- a/crates/perry-hir/src/lower/context.rs +++ b/crates/perry-hir/src/lower/context.rs @@ -1461,6 +1461,7 @@ impl LoweringContext { module_name: String, class_name: String, ) -> bool { + nativeinst_registry_diag("register", &local_name, &module_name, &class_name); // #5137: if the user opted this package into `perry.compilePackages`, // its real npm source is being compiled and the binding resolves to // the compiled-from-source class. Registering a native instance here @@ -1679,6 +1680,7 @@ impl LoweringContext { /// scans these in reverse (last-match-wins), so the index stores the LAST /// pushed entry per name (overwrite). pub(crate) fn push_module_native_instance(&mut self, entry: (String, String, String)) { + nativeinst_registry_diag("push_module", &entry.0, &entry.1, &entry.2); let idx = self.module_native_instances.len(); self.module_native_instances_index .insert(entry.0.clone(), idx); @@ -1909,3 +1911,36 @@ pub(crate) fn perry_ui_factory_returns_handle(name: &str) -> bool { || perry_dispatch::perry_ui_lookup(name) .is_some_and(|row| row.ret == perry_dispatch::ReturnKind::Widget) } + +/// #9847: report every native-instance tag as it is created. +/// +/// `register_native_instance` and `push_module_native_instance` are the only +/// two entry points through which a native-instance tag can come into +/// existence, so a diagnostic on *them* cannot miss a tag the way one on +/// guessed construction sites can — which is the whole reason this exists. +/// +/// What it prints, one line per registration: +/// +/// ```text +/// [nativeinst] REGISTER push_module name="O" -> child_process::Instance +/// ``` +/// +/// The tag table is keyed by identifier TEXT with module-wide scope, so on a +/// minified single-module bundle the same short name is routinely claimed by +/// several unrelated native classes and every method call on any local with +/// that name is lowered as a native-instance call of whichever won. This +/// report is what makes that visible: on `cli_2.1.112.js` it prints 795 lines +/// whose most-registered identifiers are `Y`(71), `z`(65), `K`(65), `_`(65), +/// `A`(54), `O`(52), `w`(37), `q`(35) — every one a single letter. +/// +/// Enable with `PERRY_NATIVEINST_DIAG=1`. Off, this is one relaxed atomic load +/// per registration and nothing else. +pub(crate) fn nativeinst_registry_diag(kind: &str, name: &str, module: &str, class: &str) { + static ON: std::sync::OnceLock = std::sync::OnceLock::new(); + let on = *ON.get_or_init( + || matches!(std::env::var("PERRY_NATIVEINST_DIAG"), Ok(v) if !v.is_empty() && v != "0"), + ); + if on { + eprintln!("[nativeinst] REGISTER {kind} name={name:?} -> {module}::{class}"); + } +} diff --git a/crates/perry/src/commands/compile/build_cache.rs b/crates/perry/src/commands/compile/build_cache.rs index 3fc67c1a83..f2eb61c696 100644 --- a/crates/perry/src/commands/compile/build_cache.rs +++ b/crates/perry/src/commands/compile/build_cache.rs @@ -848,6 +848,14 @@ fn eligibility(args: &CompileArgs, project_root: &Path) -> Result<(), String> { if std::env::var("PERRY_OUTLINE_ENTRY_REPORT").is_ok() { return Err("outline-entry-report".to_string()); } + // #9847: same reasoning as `opt-report` above. A cached build reuses the + // finished binary and never lowers HIR, so the native-instance report + // would print nothing — and nothing is indistinguishable from "no tag was + // ever registered", which is the reading this diagnostic exists to make + // impossible. + if std::env::var("PERRY_NATIVEINST_DIAG").is_ok() { + return Err("nativeinst-diag".to_string()); + } if args.verify_native_regions || args.emit_attest || args.emit_sandbox { return Err("sidecar-or-verify".to_string()); } From 42cadbbc29bcdb343560578d3c58fae8d6f9de35 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 6 Sep 2026 09:33:01 +0200 Subject: [PATCH 03/26] fix(codegen): root_reload's cost cap counts root loads, not derived values MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `MAX_BLOCK_LOAD_PRODUCT` guards the reachability walk in `apply_to_function`, and that walk runs once per `groups` entry — one per ROOT LOAD. The check multiplied by `values.len()` instead, which since #7664 counts every pure-bit-op derivation as its own `Reloadable`. On a wide function that is several times the group count, so the pass declined on functions whose real cost was well inside the bound. Declining is not correctness-neutral under the native root lowering. The constant's comment claimed "the pass is an improvement, not a correctness precondition, so declining is safe"; that is false, and it is why the cliff went unnoticed. When the pass does not run, nothing re-reads the slot: a receiver read out of its root, unmasked to an i64/double and carried across a call is a value RS4GC cannot relocate, so the store lands in a from-space object. Found on Claude-of-Duty's `Arm.constructor` (4924 blocks, 747 root loads, 2102 values): 10,350,248 by the old metric against an 8M cap, 3,678,228 by the new one. It declined, and `this.upper = buildSleeve(...)` wrote through a stale receiver — a SIGBUS under `PERRY_GC_PROTECT_FROMSPACE=1`, and silent field corruption without it (`THREE.Object3D.add: object not an instance of THREE.Object3D. undefined` two frames later). The bound itself is unchanged; only the term it is measured against. A function that genuinely exceeds `blocks x groups` still declines and can still carry a stale register — that residual risk is now stated at the constant rather than denied. Note that `scripts/gc_root_dominance_check.py --stale-registers --moving-only` does NOT flag this shape: it reported 0 stale uses on the faulting module (17 found, all `source=global`), so it cannot serve as a guard here. The regression test replicates the `masked_receiver` shape across 1100 blocks with a MAX_RECIPE-length derivation, sized to clear the cap by the old metric and sit an order of magnitude inside it by the new one. It inserts 0 reloads before this change and 1100 after. --- crates/perry-codegen/src/root_reload.rs | 67 +++++++++++----- crates/perry-codegen/src/root_reload_tests.rs | 76 +++++++++++++++++++ 2 files changed, 124 insertions(+), 19 deletions(-) diff --git a/crates/perry-codegen/src/root_reload.rs b/crates/perry-codegen/src/root_reload.rs index 6f2f8df37c..efc23f6f5f 100644 --- a/crates/perry-codegen/src/root_reload.rs +++ b/crates/perry-codegen/src/root_reload.rs @@ -253,13 +253,37 @@ const NON_COLLECTING: &[&str] = &[ ]; /// Guard against a pathological function turning this pass into the compile's -/// bottleneck: the reachability walk is O(blocks) per slot load, so the product -/// is what matters. Above the cap the function keeps today's IR — the pass is -/// an improvement, not a correctness precondition, so declining is safe. +/// bottleneck: the reachability walk is O(blocks) per ROOT LOAD — one walk per +/// `groups` entry below, not one per reloadable value — so `blocks × groups` is +/// the product that bounds the cost. /// /// Sized from the corpora: the largest function in the dependency-scale corpus /// (`zod`'s parse core) is ~1400 blocks with ~90 slot loads, an order of /// magnitude under this. +/// +/// # Declining is NOT correctness-neutral (#9135 follow-up) +/// +/// The previous comment here read "the pass is an improvement, not a +/// correctness precondition, so declining is safe". Under the NATIVE root +/// lowering that is false, and Claude-of-Duty's `Arm.constructor` is the +/// counterexample: the receiver of an inline class-field store is read out of +/// its slot, unmasked to an `i64`/`double`, and carried across `buildSleeve`. +/// RS4GC relocates the `addrspace(1)` load but cannot touch the unmasked copy +/// (the "case 2" shape in this module's header), so when this pass declines +/// NOTHING re-reads the slot and the store lands in a from-space object. It +/// faults under `PERRY_GC_PROTECT_FROMSPACE=1` and silently corrupts the field +/// without it. +/// +/// That function measured 4924 blocks × 747 root loads = 3.7M — comfortably +/// under this cap — but the check multiplied by `values.len()` (2102, every +/// pure-bit-op derivation counted separately since #7664) for 10.4M, so it +/// declined on a metric ~2.8x larger than the cost it was guarding. +/// +/// So the bound stays (a real pathological function must still be able to opt +/// out), but it is measured against the walk's actual driver. A function that +/// genuinely exceeds it still keeps today's IR and can still carry a stale +/// register: the cap trades a compile-time cliff for a correctness risk, and +/// that residual risk is why the number is generous rather than tight. const MAX_BLOCK_LOAD_PRODUCT: usize = 8_000_000; /// How long a derivation may get before the pass declines to re-materialise it. @@ -549,7 +573,27 @@ pub(crate) fn apply_to_function(func: &mut LlFunction) -> usize { break; } } - if blocks.len().saturating_mul(values.len()) > MAX_BLOCK_LOAD_PRODUCT { + // Built BEFORE the cap check: `groups` is what the reachability walk below + // iterates (one walk per root load), so it is the term the cap must be + // measured against. See `MAX_BLOCK_LOAD_PRODUCT`. + // + // Keyed on `(recipe[0], root_ptr)`, not `recipe[0]` alone — #7725's capture-GET extension is + // the one case where a chain's `root_ptr` CHANGES partway through (the closure-ptr slot + // becomes a synthetic per-index key once the derivation passes through + // `js_closure_get_capture_bits`), so two sub-chains sharing one root LOAD can need two + // different invalidation conditions. Before #7725 every member of a `recipe[0]` group had + // the identical `root_ptr` by construction (it was always inherited, never overridden), so + // this is additive: it can only split a group that would otherwise have mixed two + // conditions under one, never change the grouping of any existing (non-capture) chain. + let mut groups: HashMap<((usize, usize), String), Vec> = HashMap::new(); + for (i, v) in values.iter().enumerate() { + groups + .entry((v.recipe[0], v.root_ptr.clone())) + .or_default() + .push(i); + } + + if blocks.len().saturating_mul(groups.len()) > MAX_BLOCK_LOAD_PRODUCT { return 0; } @@ -604,21 +648,6 @@ pub(crate) fn apply_to_function(func: &mut LlFunction) -> usize { // // Grouping by root load also puts the cost back at O(blocks × loads). // - // Keyed on `(recipe[0], root_ptr)`, not `recipe[0]` alone — #7725's capture-GET extension is - // the one case where a chain's `root_ptr` CHANGES partway through (the closure-ptr slot - // becomes a synthetic per-index key once the derivation passes through - // `js_closure_get_capture_bits`), so two sub-chains sharing one root LOAD can need two - // different invalidation conditions. Before #7725 every member of a `recipe[0]` group had - // the identical `root_ptr` by construction (it was always inherited, never overridden), so - // this is additive: it can only split a group that would otherwise have mixed two - // conditions under one, never change the grouping of any existing (non-capture) chain. - let mut groups: HashMap<((usize, usize), String), Vec> = HashMap::new(); - for (i, v) in values.iter().enumerate() { - groups - .entry((v.recipe[0], v.root_ptr.clone())) - .or_default() - .push(i); - } let mut group_keys: Vec<((usize, usize), String)> = groups.keys().cloned().collect(); group_keys.sort_unstable(); diff --git a/crates/perry-codegen/src/root_reload_tests.rs b/crates/perry-codegen/src/root_reload_tests.rs index 47303a4e73..28b192c378 100644 --- a/crates/perry-codegen/src/root_reload_tests.rs +++ b/crates/perry-codegen/src/root_reload_tests.rs @@ -1076,3 +1076,79 @@ fn the_slot_layout_note_helpers_are_non_collecting() { transposition of js_gc_note_slot_layout" ); } + +/// The cost cap must be measured against the number of ROOT LOADS, not the +/// number of reloadable values. +/// +/// `MAX_BLOCK_LOAD_PRODUCT` guards the reachability walk, and that walk runs +/// once per `groups` entry — one per root load — not once per reloadable +/// value. Since #7664 extended a recipe through pure bit ops, `values` counts +/// every derivation separately, so `blocks * values` can be several times +/// `blocks * groups`. Measuring the cap against `values` declined functions +/// whose real cost was well inside the bound, and a declined function keeps +/// every stale register: under the native root lowering nothing else re-reads +/// the slot. +/// +/// Each block below carries one root load plus a `MAX_RECIPE`-length pure +/// derivation off it — the `masked_receiver` shape, replicated — so `values` +/// is 8x `groups`. At 1100 blocks that is ~9.7M by the old metric against an +/// 8M cap, and ~1.2M by the new one. The reloads must still be inserted. +/// +/// Found on Claude-of-Duty's `Arm.constructor` (4924 blocks, 747 root loads, +/// 2102 values): 10.35M by the old metric, 3.68M by the new. Declining there +/// left the receiver of an inline class-field store unrooted across a call, +/// and the store landed in a from-space object. +#[test] +fn the_cap_counts_root_loads_not_derived_values() { + // `load + bitcast + 6 * and` is exactly MAX_RECIPE (8) steps. A longer + // chain is refused outright by the recipe fixpoint, which is a different + // decline and would not exercise the cap at all. + const BLOCKS: usize = 1100; + const MASKS: usize = 6; + + let mut f = LlFunction::new("wide", DOUBLE, vec![(DOUBLE, "%arg".into())]); + let mut labels = Vec::with_capacity(BLOCKS + 1); + labels.push(f.create_block("entry").label.clone()); + for i in 0..BLOCKS { + labels.push(f.create_block(&format!("b{i}")).label.clone()); + } + + let slot; + { + let b = f.block_mut(0).unwrap(); + slot = b.alloca(DOUBLE); + b.store(DOUBLE, "%arg", &slot); + b.call_void( + "js_shadow_slot_bind", + &[(crate::types::I32, "0"), (PTR, &slot)], + ); + b.br(&labels[1]); + } + + for i in 0..BLOCKS { + let next = labels.get(i + 2).cloned(); + let b = f.block_mut(i + 1).unwrap(); + let boxed = b.load(DOUBLE, &slot); + let mut cur = b.bitcast_double_to_i64(&boxed); + for _ in 0..MASKS { + cur = b.and(I64, &cur, "281474976710655"); + } + b.call(DOUBLE, "js_object_get_field_by_name_f64", &[]); + b.call_void( + "js_object_set_field_by_name", + &[(I64, &cur), (DOUBLE, "0.0")], + ); + match next { + Some(n) => b.br(&n), + None => b.ret(DOUBLE, "0.0"), + } + } + + let rewrites = apply_to_function(&mut f); + assert_eq!( + rewrites, BLOCKS, + "every block holds one stale masked receiver across a collecting call; \ + measuring the cap against the derived-value count declines the whole \ + function and silently leaves all of them in place" + ); +} From 6d3216205dec9635508770cc2297a722e03d3efa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 6 Sep 2026 06:35:13 +0200 Subject: [PATCH 04/26] fix(runtime): prove an unpatched iterator prototype without allocating `call_overridden_iterator_next` minted a fresh 4-byte "next" key string on every built-in iterator step, purely to run a by-name prototype lookup that concluded nothing was patched. The `ITERATOR_PROTOTYPE_PTR == 0` early-out that was supposed to prevent this is dead after the first iterator any program allocates: every iterator allocator calls `attach_iterator_prototype` -> `ensure_iterator_prototypes`, which materializes the tower. Adds `prototype_next_is_canonical`: the prototype's own `next` slot holds a closure whose native entry is the canonical thunk, and no accessor descriptor is recorded for "next". Both reads are non-allocating. Any other state falls through to the by-name path, unchanged. This is the third-ranked site by count in the 2026-09-06 claude-code allocation census (~122,880 x 32 B per 400-character reply), which had attributed it to `Intl.Segmenter` substring copying. Caller walk in the shipped binary `cc_relink/cc_int_0905`: js_for_of_next+0xd0 -> dispatch_array_iterator_method_inner+0x218 (bl call_overridden_iterator_next) -> call_overridden_iterator_next+0x67c (bl js_string_from_bytes_with_capacity) -> string_storage_alloc Measured on a relinked claude-code binary carrying this fix plus a measurement-only hit/miss counter. Before the fix every probe allocated, so `hits + byname` is the pre-fix count and `byname` is what survives: 400-char reply, run A 144,189 probes byname 0 400-char reply, run B 144,303 probes byname 0 3300-char reply 887,076 probes byname 0 `byname = 0` on every one of the 173 per-minor reports across the three runs: the proof answers 100 % of probes on a real program, which is what rules out the one silent failure mode (the accessor half is a per-key Bloom bit, so a colliding accessor on the prototype would disable the fast path with no test failing). `cargo test -p perry-runtime --release --lib -- --test-threads=1`: 3,171 passed, 0 failed. Four sabotage arms, each failing only its named assertion: removing the fast path entirely reads exactly 32,000 bytes over 1,000 probes; dropping only the accessor half fails only the accessor test; dropping only the native-entry comparison fails only the replaced-`next` test. --- ...iterator-next-override-probe-allocation.md | 47 ++++ .../src/object/iterator_prototypes.rs | 243 ++++++++++++++++++ .../test_gap_iterator_prototype_next_patch.ts | 173 +++++++++++++ 3 files changed, 463 insertions(+) create mode 100644 changelog.d/9840-iterator-next-override-probe-allocation.md create mode 100644 test-files/test_gap_iterator_prototype_next_patch.ts diff --git a/changelog.d/9840-iterator-next-override-probe-allocation.md b/changelog.d/9840-iterator-next-override-probe-allocation.md new file mode 100644 index 0000000000..c19c44f80d --- /dev/null +++ b/changelog.d/9840-iterator-next-override-probe-allocation.md @@ -0,0 +1,47 @@ +### Fixed + +- **Every built-in iterator step allocated a `"next"` key string to learn that + nothing was patched.** `call_overridden_iterator_next` — the per-step probe + that lets a user replacement of `%ArrayIteratorPrototype%.next` (and the Map + / Set / String family prototypes) drive `for…of`, spread, `Array.from` and + manual `.next()` — ended in a by-name prototype lookup that minted a fresh + 4-byte `"next"` string on every call. One 32-byte allocation per iteration + step of every array, Map, Set and string iterator in the program. + + The existing early-out could not prevent it. `ITERATOR_PROTOTYPE_PTR == 0` + ("the tower was never materialized, so no override can exist") is **dead on + any program that has allocated one iterator**: every iterator allocator calls + `attach_iterator_prototype`, which calls `ensure_iterator_prototypes`, which + builds the tower. The guard is true exactly once and false forever after. + + Replaced by an allocation-free proof that runs on the path every real program + takes: the prototype's OWN `next` slot still holds a closure whose native + entry is the canonical thunk (the certified non-allocating own-field read, + #9480), AND no accessor descriptor is recorded for `"next"` on it (the + per-key Bloom bit `set_accessor_descriptor` sets before inserting, #6759 C2 — + needed because `defineProperty(proto, "next", {get})` leaves the old closure + in the data slot and puts the accessor in the side table). Anything else — + replaced, deleted, an accessor, a bound copy — takes the by-name path + unchanged. + + Affected files: + + - `crates/perry-runtime/src/object/iterator_prototypes.rs` — the + `prototype_next_is_canonical` probe, ahead of the by-name lookup. + + Measured: the 2026-09-06 claude-code allocation census ranked this site third + by count — ~122,880 allocations of 32 bytes per 400-character reply, 17.1 % + of the top-30 allocation count — and misattributed it to `Intl.Segmenter` + substring copying. Resolved by an explicit caller walk in the shipped binary: + `js_for_of_next+0xd0` → `dispatch_array_iterator_method_inner+0x218` (a `bl` + to `call_overridden_iterator_next`) → `+0x67c` (a `bl` to + `js_string_from_bytes_with_capacity`) → `string_storage_alloc`. + + Validation: `test-files/test_gap_iterator_prototype_next_patch.ts` drives a + replaced `next` through `for…of`, spread, `Array.from` and manual `.next()` + on all four families, and covers restore-by-identity, a second replace after + a restore, a bound copy of the original (which must NOT be mistaken for the + builtin), an accessor `next`, and a deleted `next`. The unit counter asserts + that 1,000 probes on an unpatched iterator with the tower materialized move + the arena by ZERO bytes, with the minor-cycle count pinned so a collection + inside the window cannot manufacture a zero delta. diff --git a/crates/perry-runtime/src/object/iterator_prototypes.rs b/crates/perry-runtime/src/object/iterator_prototypes.rs index 98e53d1fbe..1865aa9fed 100644 --- a/crates/perry-runtime/src/object/iterator_prototypes.rs +++ b/crates/perry-runtime/src/object/iterator_prototypes.rs @@ -494,6 +494,31 @@ pub(crate) unsafe fn call_overridden_iterator_next( if proto.with_const_ptr::(|proto| proto.is_null()) { return None; } + // The null-tower proof above is dead on any program that has allocated + // one iterator: `attach_iterator_prototype` materializes the tower at the + // FIRST iterator allocation, so every builtin advance after that reached + // the by-name lookup below and minted a fresh "next" key string just to + // learn nothing was patched — one 24-byte string per `for…of` step, on + // every array / Map / Set / string iterator in the program (~137,000 per + // 400-character claude-code reply; the second 32-byte site of the + // 2026-09-06 allocation census). + // + // Allocation-free proof of "not overridden": the prototype's OWN `next` + // slot still holds a closure whose native entry is the canonical thunk, + // AND no accessor descriptor is recorded for "next" on it. The own read + // is the certified non-allocating leaf (#9480); the accessor check is + // the per-key Bloom bit `set_accessor_descriptor` sets BEFORE inserting + // (#6759 C2), needed because `defineProperty(proto, "next", {get})` on + // an existing data property leaves the old closure in the slot and puts + // the accessor in the side table. Anything else — replaced, deleted, + // accessor, a bound copy — takes the by-name path, unchanged. + // The closure body is NOT covered by the enclosing `unsafe fn`'s implicit + // unsafe block, so the call is spelled out. + if proto.with_const_ptr::(|proto| unsafe { + prototype_next_is_canonical(proto, canonical) + }) { + return None; + } let key = scope.root_raw_const_ptr(crate::string::js_string_from_bytes(b"next".as_ptr(), 4)); let method = proto.with_const_ptr::(|proto| { key.with_const_ptr::(|key| { @@ -517,3 +542,221 @@ pub(crate) unsafe fn call_overridden_iterator_next( Err(error) => crate::exception::js_throw(error), } } + +/// Does `proto`'s OWN `next` data slot hold a closure whose native entry is +/// `canonical`, with no accessor descriptor recorded for `"next"`? A `true` +/// proves the prototype's `next` is the builtin (a user restoring the +/// original closure object after a patch matches too, by entry rather than +/// by object identity); a `false` proves nothing and the caller must run the +/// full by-name lookup. Reads only: no allocation, no collection point. +#[inline] +unsafe fn prototype_next_is_canonical(proto: *const ObjectHeader, canonical: *const u8) -> bool { + let own = super::js_object_get_own_field_or_undef( + crate::value::js_nanbox_pointer(proto as i64), + b"next".as_ptr(), + 4, + ); + if !JSValue::from_bits(own.to_bits()).is_pointer() { + return false; + } + let own_ptr = crate::value::js_nanbox_get_pointer(own) as *const crate::closure::ClosureHeader; + if own_ptr.is_null() || crate::closure::get_valid_func_ptr(own_ptr) != canonical { + return false; + } + !super::descriptor_state::may_have_descriptor_entry(proto as usize, "next", true) +} + +/// The prototype-override probe must be free on the path every real program +/// takes: tower materialized (any iterator allocation does that), nothing +/// patched. Before this module's `prototype_next_is_canonical`, that path +/// allocated a "next" key string per call — the second-largest 32-byte +/// allocation site of a claude-code reply (2026-09-06 census, ~137,000 per +/// 400 characters), mislabelled there as a substring copy. +#[cfg(test)] +mod override_probe_allocation_tests { + use super::*; + use crate::closure::ClosureHeader; + use crate::value::{js_nanbox_get_pointer, js_nanbox_pointer, TAG_UNDEFINED}; + + const PATCHED_SENTINEL: f64 = 4242.0; + + extern "C" fn patched_next_thunk(_closure: *const ClosureHeader) -> f64 { + PATCHED_SENTINEL + } + + extern "C" fn accessor_getter_thunk(_closure: *const ClosureHeader) -> f64 { + f64::from_bits(TAG_UNDEFINED) + } + + /// One array iterator, rooted; materializes the tower as a side effect. + unsafe fn rooted_array_iterator( + scope: &crate::gc::RuntimeHandleScope, + ) -> crate::gc::RuntimeHandle<'_> { + let arr = crate::array::js_array_alloc(1); + crate::array::js_array_push_f64(arr, 1.0); + let iter = crate::array::array_values_iter(js_nanbox_pointer(arr as i64)); + assert!( + iterator_prototypes_materialized(), + "premise: allocating an iterator materializes the tower" + ); + scope.root_nanbox_f64(iter) + } + + unsafe fn array_proto() -> *mut ObjectHeader { + ARRAY_ITERATOR_PROTOTYPE_PTR.load(Ordering::Acquire) as *mut ObjectHeader + } + + unsafe fn set_proto_next(value: f64) { + let key = crate::string::js_string_from_bytes(b"next".as_ptr(), 4); + super::super::js_object_set_field_by_name(array_proto(), key, value); + } + + unsafe fn own_next(proto: *const ObjectHeader) -> f64 { + super::super::js_object_get_own_field_or_undef( + js_nanbox_pointer(proto as i64), + b"next".as_ptr(), + 4, + ) + } + + /// The counter, and the falsifier for the fix: N probes on an unpatched + /// iterator with the tower up must bump the arena by ZERO bytes. Before + /// the fix every probe minted a 24-byte "next" string (32 B rounded), so + /// this read N × 32 — the number the census reported per grapheme. + #[test] + fn probe_on_an_unpatched_iterator_allocates_nothing() { + unsafe { + let scope = crate::gc::RuntimeHandleScope::new(); + let iter_h = rooted_array_iterator(&scope); + let iter_obj = || js_nanbox_get_pointer(iter_h.get_nanbox_f64()) as *mut ObjectHeader; + + // Warm once: a first call may lazily build anything it builds. + assert!( + call_overridden_iterator_next(iter_obj(), crate::array::ARRAY_ITERATOR_CLASS_ID) + .is_none() + ); + const N: usize = 1000; + let minors_before = crate::gc::instruments::copying_minor_cycles(); + let bytes_before = crate::arena::arena_in_use_bytes(); + for _ in 0..N { + assert!( + call_overridden_iterator_next( + iter_obj(), + crate::array::ARRAY_ITERATOR_CLASS_ID + ) + .is_none(), + "nothing is patched, so the probe must decline" + ); + } + let bytes_after = crate::arena::arena_in_use_bytes(); + assert_eq!( + crate::gc::instruments::copying_minor_cycles(), + minors_before, + "a collection inside the window would make a zero delta prove nothing" + ); + assert_eq!( + bytes_after.saturating_sub(bytes_before), + 0, + "the override probe allocated {} bytes over {N} calls on an unpatched \ + iterator with the tower materialized (it minted a \"next\" key string per call)", + bytes_after.saturating_sub(bytes_before) + ); + } + } + + /// The fast path must not be too eager: a replaced prototype `next` is + /// still honoured, and restoring the ORIGINAL closure object (what + /// `test_gap_array_iterator_manual_next.ts` (7) does) returns the probe + /// to its allocation-free decline — by native entry, not by identity. + #[test] + fn probe_honours_a_replaced_prototype_next_and_a_restored_one() { + unsafe { + let scope = crate::gc::RuntimeHandleScope::new(); + let iter_h = rooted_array_iterator(&scope); + let iter_obj = || js_nanbox_get_pointer(iter_h.get_nanbox_f64()) as *mut ObjectHeader; + + let original = scope.root_nanbox_f64(own_next(array_proto())); + assert!( + JSValue::from_bits(original.get_nanbox_f64().to_bits()).is_pointer(), + "premise: the prototype carries an own `next` closure" + ); + + let patched = crate::closure::js_closure_alloc(patched_next_thunk as *const u8, 0); + crate::closure::js_register_closure_arity(patched_next_thunk as *const u8, 0); + let patched_h = scope.root_nanbox_f64(js_nanbox_pointer(patched as i64)); + set_proto_next(patched_h.get_nanbox_f64()); + assert!( + !prototype_next_is_canonical(array_proto(), array_iterator_next_thunk as *const u8), + "a replaced prototype `next` must defeat the allocation-free proof" + ); + assert_eq!( + call_overridden_iterator_next(iter_obj(), crate::array::ARRAY_ITERATOR_CLASS_ID), + Some(PATCHED_SENTINEL), + "the replacement installed on the prototype must be the one called" + ); + + set_proto_next(original.get_nanbox_f64()); + assert!( + prototype_next_is_canonical(array_proto(), array_iterator_next_thunk as *const u8), + "restoring the original closure must re-enable the allocation-free proof" + ); + assert!( + call_overridden_iterator_next(iter_obj(), crate::array::ARRAY_ITERATOR_CLASS_ID) + .is_none(), + "after the restore the builtin advance is back" + ); + } + } + + /// `Object.defineProperty(proto, "next", { get })` records the accessor in + /// the descriptor side table and leaves the old data slot behind, so the + /// own-slot read alone would still see the canonical closure. The per-key + /// accessor bit is what makes the proof decline; without it the getter + /// would be silently bypassed. + #[test] + fn probe_declines_when_an_accessor_next_is_defined_on_the_prototype() { + unsafe { + let scope = crate::gc::RuntimeHandleScope::new(); + let _iter_h = rooted_array_iterator(&scope); + let original = scope.root_nanbox_f64(own_next(array_proto())); + assert!( + prototype_next_is_canonical(array_proto(), array_iterator_next_thunk as *const u8), + "premise: unpatched prototype passes the proof" + ); + + let getter = crate::closure::js_closure_alloc(accessor_getter_thunk as *const u8, 0); + crate::closure::js_register_closure_arity(accessor_getter_thunk as *const u8, 0); + let getter_h = scope.root_nanbox_f64(js_nanbox_pointer(getter as i64)); + let key = scope.root_string_ptr(crate::string::js_string_from_bytes(b"next".as_ptr(), 4)); + super::super::js_object_define_accessor( + js_nanbox_pointer(array_proto() as i64), + key.with_const_ptr::(|k| { + f64::from_bits(JSValue::string_ptr(k as *mut crate::StringHeader).bits()) + }), + getter_h.get_nanbox_f64(), + f64::from_bits(TAG_UNDEFINED), + ); + assert!( + !prototype_next_is_canonical(array_proto(), array_iterator_next_thunk as *const u8), + "an accessor `next` on the prototype must defeat the allocation-free proof \ + even though the data slot may still hold the canonical closure" + ); + + // Delete the accessor and put the data property back. The Bloom + // bit is sticky (zeroed only at meta creation), so the PROOF stays + // declined on this prototype for good — conservative: the by-name + // path runs, exactly as before the fix. Only the semantics are + // pinned here: the builtin advance is back. + key.with_const_ptr::(|k| { + super::super::js_object_delete_field(array_proto(), k); + }); + set_proto_next(original.get_nanbox_f64()); + let iter_obj = js_nanbox_get_pointer(_iter_h.get_nanbox_f64()) as *mut ObjectHeader; + assert!( + call_overridden_iterator_next(iter_obj, crate::array::ARRAY_ITERATOR_CLASS_ID) + .is_none(), + "after delete + restore the builtin advance must be back" + ); + } + } +} diff --git a/test-files/test_gap_iterator_prototype_next_patch.ts b/test-files/test_gap_iterator_prototype_next_patch.ts new file mode 100644 index 0000000000..a0f1917382 --- /dev/null +++ b/test-files/test_gap_iterator_prototype_next_patch.ts @@ -0,0 +1,173 @@ +// A replaced `%ArrayIteratorPrototype%.next` (and the Map / Set / String +// family prototypes) must drive for-of, spread, Array.from and manual calls; +// restoring the ORIGINAL closure must hand iteration back to the builtin. +// The runtime proves "not patched" allocation-free by comparing the +// prototype's own `next` against the builtin thunk, so both directions of a +// replace / restore cycle are exercised on every family, plus an accessor +// `next` on the prototype, which that proof must decline. + +const arrayProto: any = Object.getPrototypeOf([][Symbol.iterator]()); +const mapProto: any = Object.getPrototypeOf(new Map().entries()); +const setProto: any = Object.getPrototypeOf(new Set().values()); +const stringProto: any = Object.getPrototypeOf(""[Symbol.iterator]()); + +function withPatched(proto: any, patch: (orig: any) => any, body: () => void) { + const orig = proto.next; + proto.next = patch(orig); + try { + body(); + } finally { + proto.next = orig; + } +} + +// A: array family, every driver, doubled values through the patch. +withPatched( + arrayProto, + (orig) => + function (this: any) { + const r = orig.call(this); + if (!r.done) r.value = (r.value as number) * 2; + return r; + }, + () => { + const got: number[] = []; + for (const v of [1, 2, 3]) got.push(v); + console.log("A-forof", got.join(",")); + console.log("A-spread", [...[4, 5]].join(",")); + console.log("A-from", Array.from([6].values()).join(",")); + const it = [7, 8].values(); + console.log("A-manual", it.next().value, it.next().value, it.next().done); + }, +); + +// B: after the restore the builtin is back, in every driver. +{ + const got: number[] = []; + for (const v of [1, 2, 3]) got.push(v); + console.log("B-forof", got.join(",")); + console.log("B-spread", [...[4, 5]].join(",")); + const it = [7, 8].values(); + console.log("B-manual", it.next().value, it.next().value, it.next().done); +} + +// C: a second replace after the restore is honoured again (the proof is a +// per-call read, not a one-shot latch). +withPatched( + arrayProto, + () => + function () { + return { done: true, value: undefined }; + }, + () => { + const got: number[] = []; + for (const v of [1, 2]) got.push(v); + console.log("C-forof-empty", got.length); + }, +); +console.log("C-restored", [...[9]].join(",")); + +// D: Map and Set family prototypes, patched and restored. +withPatched( + mapProto, + (orig) => + function (this: any) { + const r = orig.call(this); + if (!r.done) r.value = [r.value[0], (r.value[1] as number) + 100]; + return r; + }, + () => { + const got: string[] = []; + for (const [k, v] of new Map([["a", 1], ["b", 2]])) got.push(k + "=" + v); + console.log("D-map", got.join(",")); + }, +); +console.log("D-map-restored", [...new Map([["a", 1]])].join(",")); +withPatched( + setProto, + (orig) => + function (this: any) { + const r = orig.call(this); + if (!r.done) r.value = "s" + r.value; + return r; + }, + () => { + console.log("D-set", [...new Set([1, 2])].join(",")); + }, +); +console.log("D-set-restored", [...new Set([3])].join(",")); + +// E: String family prototype. +withPatched( + stringProto, + (orig) => + function (this: any) { + const r = orig.call(this); + if (!r.done) r.value = (r.value as string).toUpperCase(); + return r; + }, + () => { + console.log("E-string", [..."ab"].join(",")); + }, +); +console.log("E-string-restored", [..."cd"].join(",")); + +// F: restoring by assigning the very same closure object, then a patch that +// is a bound copy of the original (same algorithm, different function +// object) — the proof compares by builtin entry, so the bound copy must NOT +// be mistaken for the builtin: its `this` is fixed to a different iterator. +{ + const orig = arrayProto.next; + arrayProto.next = orig; + console.log("F-same-object", [...[1, 2]].join(",")); + const other = [100, 200].values(); + arrayProto.next = orig.bind(other); + try { + console.log("F-bound-copy", [...[1, 2]].join(",")); + } finally { + arrayProto.next = orig; + } + console.log("F-restored", [...[3]].join(",")); +} + +// G: an accessor `next` on the prototype is consulted on every step. +{ + const orig = arrayProto.next; + let gets = 0; + Object.defineProperty(arrayProto, "next", { + configurable: true, + get() { + gets++; + return orig; + }, + }); + try { + console.log("G-accessor", [...[1, 2]].join(","), gets > 0); + } finally { + Object.defineProperty(arrayProto, "next", { + value: orig, + writable: true, + enumerable: false, + configurable: true, + }); + } + console.log("G-restored", [...[4]].join(",")); +} + +// H: a deleted prototype `next` makes for-of throw a TypeError; restoring +// it by plain assignment brings the builtin back. +{ + const orig = arrayProto.next; + delete arrayProto.next; + try { + for (const _v of [1]) { + console.log("H-unexpected"); + } + console.log("H", "no-throw"); + } catch (e: any) { + console.log("H", e instanceof TypeError); + } finally { + arrayProto.next = orig; + } + console.log("H-restored", [...[5, 6]].join(",")); +} From e7cbb3b6647a9927d0ef433d7053fe0a5564caf7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 6 Sep 2026 06:46:48 +0200 Subject: [PATCH 05/26] test: pin the iterator-prototype `next` patch surface against node MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An integration arm for the allocation-free proof: compiles `test-files/test_gap_iterator_prototype_next_patch.ts` and byte-compares stdout against node v26.5.1, captured 2026-09-06 on this box. Three of the lines are the ones that can only pass if the proof is exactly right: F-bound-copy 100,200 a `bind` of the original has the SAME native entry as the builtin thunk but a different `this`; a proof that compared native entries without first reading the prototype's own slot would print `1,2`. G-accessor 1,2 true `defineProperty(proto,"next",{get})` leaves the old closure in the data slot, so the own read alone still sees the canonical closure — only the per-key accessor Bloom bit makes the proof decline. H true a deleted `next` must throw a TypeError, never fall through to the builtin advance. --- ...ssue_9840_iterator_prototype_next_patch.rs | 99 +++++++++++++++++++ 1 file changed, 99 insertions(+) create mode 100644 crates/perry/tests/issue_9840_iterator_prototype_next_patch.rs diff --git a/crates/perry/tests/issue_9840_iterator_prototype_next_patch.rs b/crates/perry/tests/issue_9840_iterator_prototype_next_patch.rs new file mode 100644 index 0000000000..276e126b3e --- /dev/null +++ b/crates/perry/tests/issue_9840_iterator_prototype_next_patch.rs @@ -0,0 +1,99 @@ +//! Regression coverage for the allocation-free "is `next` still the builtin?" +//! proof in `object/iterator_prototypes.rs`. +//! +//! The probe that lets a user replacement of `%ArrayIteratorPrototype%.next` +//! (and the Map / Set / String family prototypes) drive `for…of`, spread, +//! `Array.from` and manual `.next()` used to end in a by-name prototype lookup +//! that minted a fresh `"next"` key string on EVERY built-in iterator step. +//! The proof that removed it reads the prototype's own `next` slot and the +//! per-key accessor Bloom bit instead — so every way of *defeating* that proof +//! has to keep working, and every way of *restoring* it has to hand iteration +//! back to the builtin. +//! +//! The expected output below is `node v26.5.1` running the same source +//! (`test-files/test_gap_iterator_prototype_next_patch.ts`), captured +//! 2026-09-06. The discriminating lines are: +//! +//! * `F-bound-copy 100,200` — `orig.bind(other)` has the SAME native entry as +//! the builtin thunk but a different `this`. A proof that compared by native +//! entry alone, without first reading the prototype's own slot, would call +//! the builtin and print `1,2`. +//! * `G-accessor … true` — an accessor `next` installed by `defineProperty` +//! leaves the old closure in the data slot, so the own-slot read alone still +//! sees the canonical closure. Only the accessor Bloom bit makes the proof +//! decline; without it the getter is silently bypassed and `gets` stays 0. +//! * `H true` — a deleted `next` must throw a TypeError, not fall through to +//! the builtin advance. + +use std::path::PathBuf; +use std::process::Command; + +const SOURCE: &str = include_str!("../../../test-files/test_gap_iterator_prototype_next_patch.ts"); + +const EXPECTED: &str = "A-forof 2,4,6\n\ +A-spread 8,10\n\ +A-from 12\n\ +A-manual 14 16 true\n\ +B-forof 1,2,3\n\ +B-spread 4,5\n\ +B-manual 7 8 true\n\ +C-forof-empty 0\n\ +C-restored 9\n\ +D-map a=101,b=102\n\ +D-map-restored a,1\n\ +D-set s1,s2\n\ +D-set-restored 3\n\ +E-string A,B\n\ +E-string-restored c,d\n\ +F-same-object 1,2\n\ +F-bound-copy 100,200\n\ +F-restored 3\n\ +G-accessor 1,2 true\n\ +G-restored 4\n\ +H true\n\ +H-restored 5,6\n"; + +fn perry_bin() -> PathBuf { + PathBuf::from(env!("CARGO_BIN_EXE_perry")) +} + +#[test] +fn patched_iterator_prototype_next_drives_every_iteration_form() { + let dir = tempfile::tempdir().expect("tempdir"); + let entry = dir.path().join("iterator_next_patch.ts"); + let output = dir.path().join("iterator_next_patch_bin"); + std::fs::write(&entry, SOURCE).expect("write entry"); + + let compile = Command::new(perry_bin()) + .current_dir(dir.path()) + .arg("compile") + .arg(&entry) + .arg("-o") + .arg(&output) + .arg("--no-cache") + .output() + .expect("run perry compile"); + assert!( + compile.status.success(), + "perry compile failed\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&compile.stdout), + String::from_utf8_lossy(&compile.stderr) + ); + + let run = Command::new(&output) + .current_dir(dir.path()) + .output() + .expect("run compiled binary"); + assert!( + run.status.success(), + "compiled binary failed\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&run.stdout), + String::from_utf8_lossy(&run.stderr) + ); + assert_eq!( + String::from_utf8_lossy(&run.stdout), + EXPECTED, + "output must match node v26.5.1\nstderr:\n{}", + String::from_utf8_lossy(&run.stderr) + ); +} From d8a8e8c6c41ad3395a4bd69a453005b2aede2071 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 6 Sep 2026 06:48:32 +0200 Subject: [PATCH 06/26] test: cover a non-callable prototype `next` in the patch fixture The allocation-free proof reads the prototype's own `next` slot as a RAW value before deciding anything, so a number, a string, `undefined`, `null` and a plain object each have to defeat it and throw a TypeError rather than be mistaken for the builtin closure. Node v26.5.1 throws for all five; pinned in the integration arm. --- ...ssue_9840_iterator_prototype_next_patch.rs | 14 +++++++++++++- .../test_gap_iterator_prototype_next_patch.ts | 19 +++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/crates/perry/tests/issue_9840_iterator_prototype_next_patch.rs b/crates/perry/tests/issue_9840_iterator_prototype_next_patch.rs index 276e126b3e..18bcdae86e 100644 --- a/crates/perry/tests/issue_9840_iterator_prototype_next_patch.rs +++ b/crates/perry/tests/issue_9840_iterator_prototype_next_patch.rs @@ -24,6 +24,12 @@ //! decline; without it the getter is silently bypassed and `gets` stays 0. //! * `H true` — a deleted `next` must throw a TypeError, not fall through to //! the builtin advance. +//! * `I true` x5 — a non-callable prototype `next` (a number, a string, +//! `undefined`, `null`, a plain object) must throw a TypeError. The proof +//! reads the own slot as a RAW value, so each of these has to defeat it: the +//! number and the string never reach `is_pointer`/`get_valid_func_ptr` as a +//! closure, and the plain object passes `is_pointer` but fails the +//! CLOSURE_MAGIC probe inside `get_valid_func_ptr`. use std::path::PathBuf; use std::process::Command; @@ -51,7 +57,13 @@ F-restored 3\n\ G-accessor 1,2 true\n\ G-restored 4\n\ H true\n\ -H-restored 5,6\n"; +H-restored 5,6\n\ +I number true\n\ +I string true\n\ +I undefined true\n\ +I object true\n\ +I object true\n\ +I-restored 7,8\n"; fn perry_bin() -> PathBuf { PathBuf::from(env!("CARGO_BIN_EXE_perry")) diff --git a/test-files/test_gap_iterator_prototype_next_patch.ts b/test-files/test_gap_iterator_prototype_next_patch.ts index a0f1917382..7f66bdac27 100644 --- a/test-files/test_gap_iterator_prototype_next_patch.ts +++ b/test-files/test_gap_iterator_prototype_next_patch.ts @@ -171,3 +171,22 @@ console.log("E-string-restored", [..."cd"].join(",")); } console.log("H-restored", [...[5, 6]].join(",")); } + +// I: a NON-CALLABLE prototype `next` must throw a TypeError, not be mistaken +// for a pointer. The allocation-free proof reads the own slot as a raw value +// first, so a number, a string and `undefined` each have to defeat it. +for (const bad of [42, "not a function", undefined, null, {}]) { + const orig = arrayProto.next; + arrayProto.next = bad; + try { + for (const _v of [1]) { + console.log("I-unexpected"); + } + console.log("I", typeof bad, "no-throw"); + } catch (e: any) { + console.log("I", typeof bad, e instanceof TypeError); + } finally { + arrayProto.next = orig; + } +} +console.log("I-restored", [...[7, 8]].join(",")); From 1fd247f9ffc898c549a5c5629ebab7f88763d697 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 6 Sep 2026 08:35:10 +0200 Subject: [PATCH 07/26] chore: renumber the changelog fragment to the filed issue (#9846) The fragment was written before the issue existed and carried 9840, which is an unrelated open GC issue. #9846 is the filed report for this defect. --- ...md => 9846-iterator-next-override-probe-allocation.md} | 8 ++++++++ ...tch.rs => issue_9846_iterator_prototype_next_patch.rs} | 0 2 files changed, 8 insertions(+) rename changelog.d/{9840-iterator-next-override-probe-allocation.md => 9846-iterator-next-override-probe-allocation.md} (84%) rename crates/perry/tests/{issue_9840_iterator_prototype_next_patch.rs => issue_9846_iterator_prototype_next_patch.rs} (100%) diff --git a/changelog.d/9840-iterator-next-override-probe-allocation.md b/changelog.d/9846-iterator-next-override-probe-allocation.md similarity index 84% rename from changelog.d/9840-iterator-next-override-probe-allocation.md rename to changelog.d/9846-iterator-next-override-probe-allocation.md index c19c44f80d..b127d8fd7c 100644 --- a/changelog.d/9840-iterator-next-override-probe-allocation.md +++ b/changelog.d/9846-iterator-next-override-probe-allocation.md @@ -45,3 +45,11 @@ that 1,000 probes on an unpatched iterator with the tower materialized move the arena by ZERO bytes, with the minor-cycle count pinned so a collection inside the window cannot manufacture a zero delta. + + Counter on a relinked claude-code binary (this fix plus a measurement-only + hit/miss counter; before the fix every probe allocated, so `hits + byname` is + the pre-fix count and `byname` is what survives): a 400-character reply runs + **144,189 / 144,303** probes and a 3300-character reply **887,076**, with + **`byname = 0` on every one of the 173 per-minor reports across three runs** + — the proof answers 100 % of probes on a real program. At 32 B a string that + is 4.6 MB and 28.4 MB of allocation removed per process respectively. diff --git a/crates/perry/tests/issue_9840_iterator_prototype_next_patch.rs b/crates/perry/tests/issue_9846_iterator_prototype_next_patch.rs similarity index 100% rename from crates/perry/tests/issue_9840_iterator_prototype_next_patch.rs rename to crates/perry/tests/issue_9846_iterator_prototype_next_patch.rs From 2385be52078c85dfc46a1efe8496db165ad1fb8e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 5 Sep 2026 23:48:34 +0200 Subject: [PATCH 08/26] perf(buffer): give the buffer-registry probe the set filter its window can no longer be MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `is_registered_buffer` is the largest single leaf in cc's profile (`is_registered_buffer_slow`, 3.19 % of active main-thread CPU on `cc_main_0905`), and it is reached from property access rather than I/O: a "is this value a buffer?" test run on values that are not buffers. Its gate is `BUFFER_LIKE_ADDR_WINDOW`, a process-global min/max span. The 98.0 % rejection rate in its doc comment is measured on `claude-code --help`, which registers **10** buffers. A streaming turn registers **213**, scattered across a **527 MB** span, so `[lo, hi]` covers half a gigabyte of ordinary heap and stops rejecting. `PERRY_BUFFER_DIAG` (added here), one 400-char reply: probes=34,603,009 admits=25,476,705 (73.63 %) rejected 26.37 % true_positives=53,109 (0.208 % of admits) window [0x4c95a298460, 0x4c979e7db80] span 507.9 MB registrations=213 unregistrations=12 live_max=201 25.5 million out-of-line probes per reply, 99.79 % of which find nothing. That is the failure `RegistryAddrFilter` was built for after #9272 — its doc names "entries are ordinary heap objects interleaved with everything else" as the case a window cannot serve, and measured `is_registered_symbol` at 38.3 % (window) against 99.58 % (filter). Buffers kept the window because it rejected 100 % of `is_uint8array_buffer`'s calls ON `--help`. The capacity question that structure demands was asked BEFORE adopting it. `RegistryAddrFilter` accrues bits per admission and never clears them, so a high-churn set saturates it — the trap #9807 documented, where a 4,096-bit filter held 162,258 keys and answered "may hold" to every probe. Buffers are the opposite case: probing is hot, registration is rare. **213 cumulative admissions against 1,024 bits and 3 hashes is a 10.0 % false-positive rate.** The counter that establishes this ships with the change. One binary, one environment variable apart: PERRY_BUFFER_ADDR_FILTER=0 admits 25,476,705 (73.63 %) rejected 26.37 % filter on admits 1,223,944 ( 3.54 %) rejected 96.46 % **24.25 million out-of-line calls removed per 400-character reply**, true positives preserved (53,109 vs 53,092 — the difference tracks one fewer registration in that run; a Bloom filter has no false negatives). Soundness is machine-checked, not argued: the existing debug assertion re-derives every rejection from the authoritative tables, so a false negative panics. The whole suite in DEBUG — 3,171 tests — passes with it armed. Stacked on the `for-in` branch (#9823) only because both add counters to `hot_diag.rs`; the two changes are otherwise independent. Claude-Session: https://claude.ai/code/session_014UZWia6L37DpA93VLtNK9m --- crates/perry-runtime/src/buffer/header.rs | 72 ++++++++++- crates/perry-runtime/src/hot_diag.rs | 139 +++++++++++++++++++++ crates/perry-runtime/src/registry_latch.rs | 14 ++- 3 files changed, 220 insertions(+), 5 deletions(-) diff --git a/crates/perry-runtime/src/buffer/header.rs b/crates/perry-runtime/src/buffer/header.rs index d3f380ef67..d1224faefc 100644 --- a/crates/perry-runtime/src/buffer/header.rs +++ b/crates/perry-runtime/src/buffer/header.rs @@ -217,6 +217,56 @@ static BUFFER_LIKE_EVER_REGISTERED: RegistryLatch = RegistryLatch::new(); /// [`RegistryAddrWindow`] for the ordering rule that makes it so. static BUFFER_LIKE_ADDR_WINDOW: RegistryAddrWindow = RegistryAddrWindow::new(); +/// The set filter behind the window, for the addresses `[lo, hi]` cannot +/// discriminate. +/// +/// The window's 98.0 % rejection rate above is measured on `claude-code +/// --help`, which registers **10** buffers. On a streaming turn cc registers +/// **213**, scattered across a **527 MB** span — so `[lo, hi]` covers half a +/// gigabyte of ordinary heap and stops rejecting. `PERRY_BUFFER_DIAG`, one +/// 400-character reply: +/// +/// ```text +/// probes=34,603,009 admits=25,627,160 (74.06 %) rejected=8,975,849 (25.94 %) +/// true_positives=53,109 (0.207 % of admits) +/// window [0x5b718eb73e8, 0x5b739e1c0b8] span 527.4 MB +/// registrations=213 unregistrations=12 live_max=201 +/// ``` +/// +/// 25.6 million out-of-line probes per reply, 99.79 % of which find nothing. +/// That is the failure [`RegistryAddrFilter`] was built for after #9272 +/// (`is_registered_symbol`: a window rejects 38.3 %, the filter 99.58 %) — its +/// entries are ordinary heap objects interleaved with everything else, which +/// its doc comment names as the case a window cannot serve. +/// +/// **The capacity question this structure demands was asked before adopting +/// it.** `RegistryAddrFilter` accrues bits per ADMISSION and never clears them, +/// so a high-churn set saturates it — the trap #9807 documented for the +/// per-object layout filter, which held 162,258 keys against 4,096 bits and +/// answered "may hold" to every probe. Buffers are not that case: probing is +/// hot but registration is rare, and **213 cumulative admissions against 1,024 +/// bits and 3 hashes is a 10.0 % false-positive rate**, so the filter rejects +/// about nine of every ten addresses the window admits. The counter that says +/// so ships with it. +/// +/// The window stays in front: two static loads reject 25.94 % for less than +/// the filter's three hashes cost. +static BUFFER_LIKE_ADDR_FILTER: crate::registry_latch::RegistryAddrFilter = + crate::registry_latch::RegistryAddrFilter::new(); + +/// `PERRY_BUFFER_ADDR_FILTER=0` restores the window-only probe, so one binary +/// carries both and the A/B is one environment variable. +fn buffer_addr_filter_enabled() -> bool { + use std::sync::OnceLock; + static CACHED: OnceLock = OnceLock::new(); + *CACHED.get_or_init(|| { + !matches!( + std::env::var("PERRY_BUFFER_ADDR_FILTER").as_deref(), + Ok("0") | Ok("off") | Ok("false") + ) + }) +} + #[cfg(test)] thread_local! { /// Test-only count of `is_registered_buffer` calls that got past the address @@ -256,6 +306,7 @@ pub(crate) fn note_buffer_like_registered(addr: usize) { // checks the latch and then the window, so both must already cover this // address by the time it becomes findable. BUFFER_LIKE_ADDR_WINDOW.admit(addr); + BUFFER_LIKE_ADDR_FILTER.admit(addr); BUFFER_LIKE_EVER_REGISTERED.arm(); } @@ -405,12 +456,17 @@ pub fn register_buffer(ptr: *const BufferHeader) { // the idle fast path and denies it. See `crate::registry_latch`. let addr = ptr as usize; BUFFER_LIKE_ADDR_WINDOW.admit(addr); + BUFFER_LIKE_ADDR_FILTER.admit(addr); BUFFER_LIKE_EVER_REGISTERED.arm(); BUFFER_ADDR_RANGE.with(|r| { let (lo, hi) = r.get(); r.set((lo.min(addr), hi.max(addr))); }); BUFFER_REGISTRY.with(|r| r.borrow_mut().insert(addr)); + if crate::hot_diag::buffer_on() { + let live = BUFFER_REGISTRY.with(|r| r.borrow().len()); + crate::hot_diag::buffer_note_registration(live); + } } /// Historical tier boundary, retained for callers that size test fixtures @@ -442,7 +498,12 @@ pub fn is_registered_buffer(addr: usize) -> bool { // call, the thread-local resolution, the `RefCell` borrow or the hash. // Every writer widens the window before it publishes, which is what makes // rejecting sound; see `BUFFER_LIKE_ADDR_WINDOW`. - if !BUFFER_LIKE_ADDR_WINDOW.may_contain(addr) { + let admitted = BUFFER_LIKE_ADDR_WINDOW.may_contain(addr) + && (!buffer_addr_filter_enabled() || BUFFER_LIKE_ADDR_FILTER.may_contain(addr)); + if crate::hot_diag::buffer_on() { + crate::hot_diag::buffer_note_probe(addr, admitted, BUFFER_LIKE_ADDR_WINDOW.bounds()); + } + if !admitted { // Machine-check the completeness of the writer set instead of trusting // an enumeration of it. The window is only sound if EVERY route into // the three tables below calls `admit` first; an enumeration of those @@ -468,7 +529,11 @@ pub fn is_registered_buffer(addr: usize) -> bool { } #[cfg(test)] TEST_BUFFER_REGISTRY_PROBES.with(|c| c.set(c.get().wrapping_add(1))); - is_registered_buffer_slow(addr) + let found = is_registered_buffer_slow(addr); + if found && crate::hot_diag::buffer_on() { + crate::hot_diag::buffer_note_true_positive(); + } + found } /// `PERRY_BUFFER_RANGE_FILTER=0` restores the unconditional hash lookup. @@ -1081,6 +1146,9 @@ pub(crate) fn finalize_collected_dead_buffer(addr: usize) { BUFFER_REGISTRY.with(|r| { r.borrow_mut().remove(&addr); }); + if crate::hot_diag::buffer_on() { + crate::hot_diag::buffer_note_unregistration(); + } FOREIGN_BACKING_REGISTRY.with(|r| { r.borrow_mut().remove(&addr); }); diff --git a/crates/perry-runtime/src/hot_diag.rs b/crates/perry-runtime/src/hot_diag.rs index 82b4bdaee9..9c7ab13ae7 100644 --- a/crates/perry-runtime/src/hot_diag.rs +++ b/crates/perry-runtime/src/hot_diag.rs @@ -944,3 +944,142 @@ impl EnumDiag { out } } + +// --------------------------------------------------------------------------- +// `is_registered_buffer`: is the min/max window still rejecting? +// --------------------------------------------------------------------------- + +use std::sync::atomic::{AtomicU64, AtomicUsize}; + +static BUFFER_SINK: OnceLock> = OnceLock::new(); +static BUFFER_ON: AtomicBool = AtomicBool::new(false); + +fn buffer_sink() -> &'static Option { + BUFFER_SINK.get_or_init(|| { + let sink = sink_from_env("PERRY_BUFFER_DIAG"); + BUFFER_ON.store(sink.is_some(), Ordering::Relaxed); + sink + }) +} + +/// Is the buffer-probe instrument armed? One relaxed load once initialised. +#[inline] +pub fn buffer_on() -> bool { + if BUFFER_SINK.get().is_none() { + buffer_sink(); + } + BUFFER_ON.load(Ordering::Relaxed) +} + +// Plain relaxed atomics rather than the thread-local `RefCell` the other +// instruments use: this probe runs millions of times per turn, and a borrow +// per probe would dominate the thing being measured. +static BUF_PROBES: AtomicU64 = AtomicU64::new(0); +static BUF_ADMITS: AtomicU64 = AtomicU64::new(0); +static BUF_TRUE_POS: AtomicU64 = AtomicU64::new(0); +static BUF_ADDR_MIN: AtomicUsize = AtomicUsize::new(usize::MAX); +static BUF_ADDR_MAX: AtomicUsize = AtomicUsize::new(0); +static BUF_WIN_LO: AtomicUsize = AtomicUsize::new(usize::MAX); +static BUF_WIN_HI: AtomicUsize = AtomicUsize::new(0); +static BUF_REGS: AtomicU64 = AtomicU64::new(0); +static BUF_UNREGS: AtomicU64 = AtomicU64::new(0); +static BUF_LIVE_MAX: AtomicUsize = AtomicUsize::new(0); + +/// One `is_registered_buffer` probe that got past the "ever registered" latch. +/// `admitted` is what the inline min/max window answered — the whole question, +/// because only an admitted address pays the out-of-line call. +#[inline] +pub fn buffer_note_probe(addr: usize, admitted: bool, window: Option<(usize, usize)>) { + let n = BUF_PROBES.fetch_add(1, Ordering::Relaxed); + if admitted { + BUF_ADMITS.fetch_add(1, Ordering::Relaxed); + } + BUF_ADDR_MIN.fetch_min(addr, Ordering::Relaxed); + BUF_ADDR_MAX.fetch_max(addr, Ordering::Relaxed); + if let Some((lo, hi)) = window { + BUF_WIN_LO.store(lo, Ordering::Relaxed); + BUF_WIN_HI.store(hi, Ordering::Relaxed); + } + // Dump roughly every million probes; the rig SIGKILLs, so an exit hook + // would never fire. + if n & 0xF_FFFF == 0 { + buffer_dump(); + } +} + +/// The slow path found a real registered buffer. +#[inline] +pub fn buffer_note_true_positive() { + BUF_TRUE_POS.fetch_add(1, Ordering::Relaxed); +} + +/// One buffer registration, with the registry's size after it. Registrations +/// are what a Bloom filter would have to hold, and `RegistryAddrFilter` accrues +/// bits **per admission, not per live entry** — so for a high-churn set the +/// number that decides whether that structure can work is the CUMULATIVE +/// count, not the live one. Both are recorded. +pub fn buffer_note_registration(live_now: usize) { + BUF_REGS.fetch_add(1, Ordering::Relaxed); + BUF_LIVE_MAX.fetch_max(live_now, Ordering::Relaxed); +} + +/// One buffer leaving the registry. +pub fn buffer_note_unregistration() { + BUF_UNREGS.fetch_add(1, Ordering::Relaxed); +} + +#[cold] +fn buffer_dump() { + let probes = BUF_PROBES.load(Ordering::Relaxed); + let admits = BUF_ADMITS.load(Ordering::Relaxed); + let tp = BUF_TRUE_POS.load(Ordering::Relaxed); + let amin = BUF_ADDR_MIN.load(Ordering::Relaxed); + let amax = BUF_ADDR_MAX.load(Ordering::Relaxed); + let wlo = BUF_WIN_LO.load(Ordering::Relaxed); + let whi = BUF_WIN_HI.load(Ordering::Relaxed); + let pct = |a: u64, b: u64| if b == 0 { 0.0 } else { 100.0 * a as f64 / b as f64 }; + let mb = |n: usize| n as f64 / (1024.0 * 1024.0); + let win_span = whi.saturating_sub(wlo); + let probe_span = amax.saturating_sub(amin); + let mut out = String::with_capacity(768); + use std::fmt::Write as _; + let _ = writeln!( + out, + "[buffer-diag] probes={probes} admits={admits} ({:.2} %) rejected={} ({:.2} %) \ + true_positives={tp} ({:.6} % of admits)", + pct(admits, probes), + probes - admits, + pct(probes - admits, probes), + pct(tp, admits) + ); + let _ = writeln!( + out, + " window [{wlo:#x}, {whi:#x}] span {:.1} MB", + mb(win_span) + ); + let _ = writeln!( + out, + " probed [{amin:#x}, {amax:#x}] span {:.1} MB -- window covers {:.1} % of the probed range", + mb(probe_span), + if probe_span == 0 { 0.0 } else { 100.0 * win_span as f64 / probe_span as f64 } + ); + let regs = BUF_REGS.load(Ordering::Relaxed); + let unregs = BUF_UNREGS.load(Ordering::Relaxed); + let live_max = BUF_LIVE_MAX.load(Ordering::Relaxed); + // A 1,024-bit, 3-hash Bloom filter (`RegistryAddrFilter`) accrues bits per + // ADMISSION and never clears them, so `regs` — not `live_max` — is what it + // would have to hold. (1 - e^(-3n/1024))^3 at that n: + let fp = |n: f64| { + let x = 1.0 - (-3.0 * n / 1024.0).exp(); + 100.0 * x * x * x + }; + let _ = writeln!( + out, + " registrations={regs} unregistrations={unregs} live_max={live_max} => a 1024-bit/3-hash Bloom holding all admissions would be {:.1} % false-positive (and {:.1} % if it could hold only the live set)", + fp(regs as f64), + fp(live_max as f64) + ); + if let Some(sink) = buffer_sink() { + write_sink(sink, &out); + } +} diff --git a/crates/perry-runtime/src/registry_latch.rs b/crates/perry-runtime/src/registry_latch.rs index dd6adcadcc..8e64271625 100644 --- a/crates/perry-runtime/src/registry_latch.rs +++ b/crates/perry-runtime/src/registry_latch.rs @@ -225,13 +225,21 @@ impl RegistryAddrWindow { self.hi.fetch_max(addr, Ordering::AcqRel); } - /// Test hook: the current `[lo, hi]` pair, or `None` while empty. - #[cfg(test)] - pub(crate) fn bounds_for_tests(&self) -> Option<(usize, usize)> { + /// The live `[lo, hi]` pair, or `None` while the window is still empty. + /// + /// Diagnostic use: `PERRY_BUFFER_DIAG` reports it, so a window that has + /// widened until it covers the heap is visible rather than inferred. + pub(crate) fn bounds(&self) -> Option<(usize, usize)> { let lo = self.lo.load(Ordering::Acquire); let hi = self.hi.load(Ordering::Acquire); (lo <= hi).then_some((lo, hi)) } + + /// Test hook: the current `[lo, hi]` pair, or `None` while empty. + #[cfg(test)] + pub(crate) fn bounds_for_tests(&self) -> Option<(usize, usize)> { + self.bounds() + } } /// A monotone "which addresses have ever been registered?" **set filter** — From 08279ec948e0ad5254e2b65f46757b0776c4eca8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 5 Sep 2026 23:49:21 +0200 Subject: [PATCH 09/26] docs(changelog): fragment for PR 9828 Claude-Session: https://claude.ai/code/session_014UZWia6L37DpA93VLtNK9m --- .../9828-buffer-registry-addr-filter.md | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) create mode 100644 changelog.d/9828-buffer-registry-addr-filter.md diff --git a/changelog.d/9828-buffer-registry-addr-filter.md b/changelog.d/9828-buffer-registry-addr-filter.md new file mode 100644 index 0000000000..2792840957 --- /dev/null +++ b/changelog.d/9828-buffer-registry-addr-filter.md @@ -0,0 +1,32 @@ +**The buffer-registry probe stops answering "maybe" to three quarters of the +addresses it is asked about** (#9828). + +`is_registered_buffer` guards its three registries with +`BUFFER_LIKE_ADDR_WINDOW`, a process-global min/max span, and the 98.0 % +rejection rate in its doc comment is measured on `claude-code --help` — a run +that registers **10** buffers. A streaming turn registers **213**, scattered +across a **527 MB** span, so `[lo, hi]` covers half a gigabyte of ordinary heap +and stops discriminating: on one 400-character reply, 34.6 million probes, of +which the window admits **73.63 %** to the out-of-line lookup, and **99.79 % of +those find nothing**. + +The probe now consults `RegistryAddrFilter` behind the window — the set filter +added after #9272 for exactly this failure, where a registry's entries are +ordinary heap objects interleaved with everything else. Rejection goes from +26.37 % to **96.46 %**, removing **24.25 million out-of-line calls per reply**, +each of which cost a thread-local resolution and a hash. True positives are +unchanged. + +The saturation question that structure demands was answered before adopting it: +`RegistryAddrFilter` accrues bits per admission and never clears them, so a +high-churn set would degrade it into the state #9807 documented for the +per-object layout filter. Buffers are the opposite case — probing is hot, +registration is rare — and 213 cumulative admissions against 1,024 bits gives a +10.0 % false-positive rate. `PERRY_BUFFER_DIAG` reports the occupancy, the +window bounds and the rejection rate so the question stays answerable. + +In the profile, `is_registered_buffer_slow` falls from 169 to 25 leaf samples +(−85 %); its inline caller rises 96 to 123 as the filter's hashes move there, +so the pair falls 44 % overall. That is roughly half of the 3.19 % the profile +attributed to the slow path, and it is below the streaming rig's resolution, so +turn CPU is unchanged. From e7064b283ac90c71cdf2767d45d7e59b1b4bd294 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 5 Sep 2026 19:38:42 +0200 Subject: [PATCH 10/26] test: audit Solid client reactivity under native compilation --- .../packages/solid-reactivity/.gitignore | 3 + .../packages/solid-reactivity/README.md | 76 +++++++++ .../packages/solid-reactivity/entry.ts | 145 ++++++++++++++++++ .../packages/solid-reactivity/expected.txt | 20 +++ .../packages/solid-reactivity/fixture.sh | 21 +++ .../solid-reactivity/package-lock.json | 53 +++++++ .../packages/solid-reactivity/package.json | 18 +++ 7 files changed, 336 insertions(+) create mode 100644 tests/release/packages/solid-reactivity/.gitignore create mode 100644 tests/release/packages/solid-reactivity/README.md create mode 100644 tests/release/packages/solid-reactivity/entry.ts create mode 100644 tests/release/packages/solid-reactivity/expected.txt create mode 100755 tests/release/packages/solid-reactivity/fixture.sh create mode 100644 tests/release/packages/solid-reactivity/package-lock.json create mode 100644 tests/release/packages/solid-reactivity/package.json diff --git a/tests/release/packages/solid-reactivity/.gitignore b/tests/release/packages/solid-reactivity/.gitignore new file mode 100644 index 0000000000..8861e2a33d --- /dev/null +++ b/tests/release/packages/solid-reactivity/.gitignore @@ -0,0 +1,3 @@ +/out +/*.log +/*-out.txt diff --git a/tests/release/packages/solid-reactivity/README.md b/tests/release/packages/solid-reactivity/README.md new file mode 100644 index 0000000000..4c6d5920a4 --- /dev/null +++ b/tests/release/packages/solid-reactivity/README.md @@ -0,0 +1,76 @@ +# Solid client-runtime audit + +This is the first prerequisite for the native UI bridge in [#4644](https://github.com/PerryTS/perry/issues/4644). +It compiles the installed, unmodified Solid 1.9.15 core, store, and universal +renderer to native code and compares their output with Node. No display server +or JavaScript runtime is needed by the resulting binary. + +```sh +PERRY_BIN=/absolute/path/to/perry \ + tests/release/packages/_harness.sh --filter solid-reactivity +``` + +Use the repository's `.node-version` for the oracle. The release sweep's +package tier discovers this fixture automatically. Its lockfile pins Solid +and its transitive dependencies; `fixture.sh` uses `npm ci` on a fresh checkout. + +The fixture exercises: + +- Signals, memo invalidation, batched updates, equality suppression, and disposal. +- Dynamic effect dependencies and cleanup before reruns and disposal. +- Store proxies, nested reads, updater functions, `produce`, keyed `reconcile`, + preserved item identity, and `unwrap`. +- The real universal renderer over an in-memory host: keyed insertion, + anchored reordering, replacement, removal, owner cleanup, and text updates + that preserve node identity. + +## Selecting the reactive build + +Solid's `node` export selects its server implementation. That implementation +does not subscribe effects to updates. Perry's normal Node-compatible package +resolution therefore needs these existing project aliases for a reactive app: + +```json +{ + "perry": { + "compilePackages": ["solid-js"], + "allow": { "compilePackages": ["solid-js"] }, + "packageAliases": { + "solid-js": "solid-js/dist/solid.js", + "solid-js/store": "solid-js/store/dist/store.js" + } + } +} +``` + +The core alias also applies to imports inside the store and universal renderer, +so they share the same owner and dependency state. Importing the client core +only in the app would leave those internal imports pointing at the server +build. The Node oracle uses `--conditions=browser` to select the corresponding +client exports. + +This fixture does not establish a `perry/ui` adapter, native event handling, +or a Solid JSX compiler mode. It also does not audit resources, transitions, +hydration, or every store operation. Solid's bundled `h` and `html` entry +points use its web renderer; they are not a native hyperscript API. See +[Solid's universal-renderer contract](https://github.com/solidjs/solid/blob/main/packages/solid/universal/README.md) +for the host operations and the separate universal JSX transform. + +## Moving-GC verification finding + +On main `d36a1af0c`, the normal fixture matches the Node oracle. A forced +copying run with seed 4644 and protected from-space also matches (22 copying +minors, 14,022 moved objects on macOS arm64). Enabling the evacuation verifier +exposes a failure during store creation: + +```sh +PERRY_GC_SCHEDULE_SEED=4644 PERRY_GC_SCHEDULE_RATE=1 \ +PERRY_GC_PROTECT_FROMSPACE=1 PERRY_GC_VERIFY_EVACUATION=1 \ + tests/release/packages/solid-reactivity/out +``` + +The verifier reports `stale forwarded pointer in remembered dirty ranges` at +the fifth scheduled collection. A store-only reduction also reproduces a +stale pointer in `heap fields`. The signal-only probe passes verification. +This finding needs resolution before declaring the native bridge's GC audit +complete; normal output parity alone does not resolve it. diff --git a/tests/release/packages/solid-reactivity/entry.ts b/tests/release/packages/solid-reactivity/entry.ts new file mode 100644 index 0000000000..caa14d4984 --- /dev/null +++ b/tests/release/packages/solid-reactivity/entry.ts @@ -0,0 +1,145 @@ +// #4644: exercise the installed client runtime, including the shared owner +// and listener state used by solid-js/store and solid-js/universal. +import { + batch, createComponent, createMemo, createRenderEffect, createRoot, + createSignal, For, onCleanup, untrack, +} from "solid-js"; +import { createStore, produce, reconcile, unwrap } from "solid-js/store"; +import { createRenderer } from "solid-js/universal"; + +const state = createRoot(dispose => { + const [count, setCount] = createSignal(1); + const doubled = createMemo(() => count() * 2); + const values: number[] = []; + createRenderEffect(() => { values.push(doubled()); }); + onCleanup(() => console.log("signal cleanup")); + return { count, setCount, doubled, values, dispose }; +}); +console.log("initial", state.count(), state.doubled(), state.values.join(",")); +state.setCount(2); +console.log("update", state.count(), state.doubled(), state.values.join(",")); +batch(() => { state.setCount(3); state.setCount(4); }); +state.setCount(4); // Equal writes must not notify. +console.log("batch", state.count(), state.doubled(), state.values.join(",")); +console.log("untrack", untrack(state.count)); +state.dispose(); +state.setCount(5); +console.log("disposed", state.values.join(",")); + +const branch = createRoot(dispose => { + const [left, setLeft] = createSignal(true); + const [a, setA] = createSignal(1); + const [b, setB] = createSignal(10); + const seen: number[] = []; + let cleanups = 0; + createRenderEffect(() => { + seen.push(left() ? a() : b()); + onCleanup(() => { cleanups++; }); + }); + return { setLeft, setA, setB, seen, cleanups: () => cleanups, dispose }; +}); +branch.setB(11); // The inactive dependency is not subscribed. +branch.setA(2); +branch.setLeft(false); +branch.setA(3); // The old dependency must have been removed. +branch.setB(12); +console.log("branches", branch.seen.join(","), branch.cleanups()); +branch.dispose(); +console.log("branch cleanup", branch.cleanups()); + +const store = createRoot(dispose => { + const [value, setValue] = createStore({ + user: { name: "Ada", score: 1 }, + items: [{ id: 1, value: "one" }, { id: 2, value: "two" }], + }); + const seen: string[] = []; + createRenderEffect(() => { + seen.push(value.user.name + ":" + value.user.score + ":" + + value.items.map(item => item.value).join(",")); + }); + return { value, setValue, seen, dispose }; +}); +console.log("store", store.seen.join("|")); +store.setValue("user", "score", score => score + 1); +store.setValue("user", produce(user => { user.name = "Grace"; })); +console.log("store updates", store.seen.join("|")); +const first = store.value.items[0]; +store.setValue("items", reconcile([{ id: 2, value: "TWO" }, { id: 1, value: "ONE" }])); +console.log("reconcile", store.seen.join("|")); +console.log("store identity", first === store.value.items[1]); +console.log("unwrap", unwrap(store.value).user.name); +store.dispose(); +store.setValue("user", "score", 9); +console.log("store disposed", store.seen.length); + +// An in-memory host keeps this audit independent of a display server. The +// actual Solid renderer drives these operations, including anchored moves. +interface HostNode { + kind: string; + value: string; + children: HostNode[]; + parent: HostNode | undefined; +} +function hostNode(kind: string, value = ""): HostNode { + return { kind, value, children: [], parent: undefined }; +} +function remove(parent: HostNode, node: HostNode): void { + const index = parent.children.indexOf(node); + if (index < 0) throw new Error("removing a non-child"); + parent.children.splice(index, 1); + node.parent = undefined; +} +const renderer = createRenderer({ + createElement: kind => hostNode(kind), + createTextNode: value => hostNode("text", value), + replaceText: (node, value) => { node.value = value; }, + setProperty: (node, name, value) => { + if (name !== "value") throw new Error("unexpected property " + name); + node.value = String(value); + }, + insertNode(parent, node, anchor) { + if (node === anchor) return; + if (node.parent) remove(node.parent, node); + const index = anchor ? parent.children.indexOf(anchor) : parent.children.length; + if (index < 0) throw new Error("anchor is not a child"); + parent.children.splice(index, 0, node); + node.parent = parent; + }, + isTextNode: node => node.kind === "text", + removeNode: remove, + getParentNode: node => node.parent, + getFirstChild: node => node.children[0], + getNextSibling: node => node.parent?.children[node.parent.children.indexOf(node) + 1], +}); + +const root = hostNode("root"); +const [rows, setRows] = createSignal(["a", "b", "c"]); +let creations = 0; +let rowCleanups = 0; +const disposeRows = renderer.render(() => createComponent(For, { + get each() { return rows(); }, + children: item => { + creations++; + onCleanup(() => { rowCleanups++; }); + return renderer.createTextNode(item); + }, +}), root); +const originalA = root.children[0]; +console.log("rows", root.children.map(node => node.value).join(",")); +setRows(["c", "a", "b"]); +console.log("moved", root.children.map(node => node.value).join(","), root.children[1] === originalA, creations); +setRows(["b", "d"]); +console.log("replaced", root.children.map(node => node.value).join(","), creations, rowCleanups); +setRows([]); +console.log("empty", root.children.map(node => node.value).join(","), rowCleanups); +disposeRows(); +setRows(["after disposal"]); +console.log("rows disposed", creations, rowCleanups); + +const labelRoot = hostNode("root"); +const [label, setLabel] = createSignal("before"); +const disposeLabel = renderer.render(() => label, labelRoot); +const originalLabel = labelRoot.children[0]; +setLabel("after"); +console.log("text update", labelRoot.children[0].value, labelRoot.children[0] === originalLabel); +disposeLabel(); diff --git a/tests/release/packages/solid-reactivity/expected.txt b/tests/release/packages/solid-reactivity/expected.txt new file mode 100644 index 0000000000..cfccf83928 --- /dev/null +++ b/tests/release/packages/solid-reactivity/expected.txt @@ -0,0 +1,20 @@ +initial 1 2 2 +update 2 4 2,4 +batch 4 8 2,4,8 +untrack 4 +signal cleanup +disposed 2,4,8 +branches 1,2,11,12 3 +branch cleanup 4 +store Ada:1:one,two +store updates Ada:1:one,two|Ada:2:one,two|Grace:2:one,two +reconcile Ada:1:one,two|Ada:2:one,two|Grace:2:one,two|Grace:2:TWO,ONE +store identity true +unwrap Grace +store disposed 4 +rows a,b,c +moved c,a,b true 3 +replaced b,d 4 2 +empty 4 +rows disposed 4 4 +text update after true diff --git a/tests/release/packages/solid-reactivity/fixture.sh b/tests/release/packages/solid-reactivity/fixture.sh new file mode 100755 index 0000000000..2f97ec7aab --- /dev/null +++ b/tests/release/packages/solid-reactivity/fixture.sh @@ -0,0 +1,21 @@ +#!/usr/bin/env bash +set -euo pipefail +[[ "${1:-}" == "--__did-skip-marker" ]] && exit 1 +cd "$(dirname "$0")" +source ../_fixture_lib.sh +if [[ ! -d node_modules ]]; then + npm ci --ignore-scripts --no-audit --no-fund +fi +fixture_setup "solid-reactivity" + +# Solid's default Node export is intentionally non-reactive SSR. Use the +# client condition for the oracle, matching packageAliases in package.json. +node --conditions=browser entry.ts > node-out.txt +diff -u expected.txt node-out.txt +# A whole-build cache hit emits no module census. Keep object-cache reuse, +# but perform module collection so the native-only assertion is meaningful. +PERRY_DISABLE_BUILD_CACHE=1 fixture_compile_run_diff "solid-reactivity" +if ! grep -Eq 'Found [0-9]+ module\(s\): [1-9][0-9]* native, 0 JavaScript' perry-compile.log; then + echo "FAIL solid-reactivity — expected every module to compile natively" + exit 1 +fi diff --git a/tests/release/packages/solid-reactivity/package-lock.json b/tests/release/packages/solid-reactivity/package-lock.json new file mode 100644 index 0000000000..ba2d9fd35b --- /dev/null +++ b/tests/release/packages/solid-reactivity/package-lock.json @@ -0,0 +1,53 @@ +{ + "name": "perry-release-fixture-solid-reactivity", + "version": "0.0.0", + "lockfileVersion": 3, + "requires": true, + "packages": { + "": { + "name": "perry-release-fixture-solid-reactivity", + "version": "0.0.0", + "dependencies": { + "solid-js": "1.9.15" + } + }, + "node_modules/csstype": { + "version": "3.2.3", + "resolved": "https://registry.npmjs.org/csstype/-/csstype-3.2.3.tgz", + "integrity": "sha512-z1HGKcYy2xA8AGQfwrn0PAy+PB7X/GSj3UVJW9qKyn43xWa+gl5nXmU4qqLMRzWVLFC8KusUX8T/0kCiOYpAIQ==", + "license": "MIT" + }, + "node_modules/seroval": { + "version": "1.5.6", + "resolved": "https://registry.npmjs.org/seroval/-/seroval-1.5.6.tgz", + "integrity": "sha512-rVQVWjjSvlINzaQPZH5JFqsqEsIWdTxY3iJZCnTL/5gQbXIRooVZKI60tVCkOVfzcRPejboxO2t0P89dg5mQaA==", + "license": "MIT", + "engines": { + "node": ">=10" + } + }, + "node_modules/seroval-plugins": { + "version": "1.5.6", + "resolved": "https://registry.npmjs.org/seroval-plugins/-/seroval-plugins-1.5.6.tgz", + "integrity": "sha512-HXuLAX2pu/UByPpaeo/TaMfvMIi+1QqIoPJYCcAtU8QkVNwgR6MPlGuCQTErV1JwraaMbYaWVIBX7mppzGLATQ==", + "license": "MIT", + "engines": { + "node": ">=10" + }, + "peerDependencies": { + "seroval": "^1.0" + } + }, + "node_modules/solid-js": { + "version": "1.9.15", + "resolved": "https://registry.npmjs.org/solid-js/-/solid-js-1.9.15.tgz", + "integrity": "sha512-EeiY2xfpZJqPLjXspVEKjAII4yv8NyG//NxZ3IpOFHdUNnnTyL0uJOeS9LWGvA7cFCz5y94cjFwYlmw5Luncsg==", + "license": "MIT", + "dependencies": { + "csstype": "^3.1.0", + "seroval": "~1.5.4", + "seroval-plugins": "~1.5.4" + } + } + } +} diff --git a/tests/release/packages/solid-reactivity/package.json b/tests/release/packages/solid-reactivity/package.json new file mode 100644 index 0000000000..71469be13b --- /dev/null +++ b/tests/release/packages/solid-reactivity/package.json @@ -0,0 +1,18 @@ +{ + "name": "perry-release-fixture-solid-reactivity", + "version": "0.0.0", + "private": true, + "type": "module", + "description": "Native Solid core, store, and universal-renderer audit for #4644", + "dependencies": { + "solid-js": "1.9.15" + }, + "perry": { + "compilePackages": ["solid-js"], + "allow": { "compilePackages": ["solid-js"] }, + "packageAliases": { + "solid-js": "solid-js/dist/solid.js", + "solid-js/store": "solid-js/store/dist/store.js" + } + } +} From d3d8cb63e27818e1bfe6f4a3191f65b7ac5e90fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 5 Sep 2026 19:50:19 +0200 Subject: [PATCH 11/26] docs: identify the retained array-growth alias in the Solid audit --- tests/release/packages/solid-reactivity/README.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/release/packages/solid-reactivity/README.md b/tests/release/packages/solid-reactivity/README.md index 4c6d5920a4..01d0df118a 100644 --- a/tests/release/packages/solid-reactivity/README.md +++ b/tests/release/packages/solid-reactivity/README.md @@ -74,3 +74,10 @@ the fifth scheduled collection. A store-only reduction also reproduces a stale pointer in `heap fields`. The signal-only probe passes verification. This finding needs resolution before declaring the native bridge's GC audit complete; normal output parity alone does not resolve it. + +Further isolation identified the reported field as the effect computation's +`sources` array after growing from capacity 8 to 16. Both the old forwarding +stub and its target are tenured. Array growth deliberately retains such +aliases for `clean_arr_ptr` to follow, whereas the verifier currently rejects +any forwarded reference. The follow-up needs to distinguish those retained +growth aliases from evacuation originals that are about to be reclaimed. From 37a78bba13f098ba1c7b6a31fec7674b659c0a3d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 5 Sep 2026 20:16:31 +0200 Subject: [PATCH 12/26] docs: link verified Solid GC follow-up --- tests/release/packages/solid-reactivity/README.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/release/packages/solid-reactivity/README.md b/tests/release/packages/solid-reactivity/README.md index 01d0df118a..6ee2f59cfa 100644 --- a/tests/release/packages/solid-reactivity/README.md +++ b/tests/release/packages/solid-reactivity/README.md @@ -81,3 +81,11 @@ stub and its target are tenured. Array growth deliberately retains such aliases for `clean_arr_ptr` to follow, whereas the verifier currently rejects any forwarded reference. The follow-up needs to distinguish those retained growth aliases from evacuation originals that are about to be reclaimed. + +The correction is proposed in [#9822](https://github.com/PerryTS/perry/pull/9822). +With that change, this fixture matches all 20 oracle lines with both protected +from-space and evacuation verification enabled: seed 4644/rate 1 completes +22 copying minors and moves 14,022 objects. Seeds 1/rate 0.25 and 42/rate 0.1 +also pass, completing six and one copying minors respectively. The runtime +regressions also check that direct and indirect nursery forwarding references +are still rejected. From 7832e61ebbd33bd6aee26ea060ebb22f144ff673 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 5 Sep 2026 21:25:27 +0200 Subject: [PATCH 13/26] feat(ui): bridge Solid universal rendering to native widgets --- Cargo.lock | 1 + changelog.d/4644-solid-native-renderer.md | 1 + crates/perry-dispatch/src/ui_table/part_a.rs | 2 +- crates/perry-ui-macos/Cargo.toml | 8 + crates/perry-ui-macos/src/widgets/mod.rs | 125 +++++------ .../tests/native_widget_order.rs | 89 ++++++++ .../src/ffi/widget_layout_extras.rs | 4 +- crates/perry-ui-windows/src/widgets/mod.rs | 24 +++ packages/perry-solid/.gitignore | 3 + packages/perry-solid/README.md | 154 ++++++++++++++ packages/perry-solid/examples/counter.ts | 24 +++ packages/perry-solid/package-lock.json | 94 +++++++++ packages/perry-solid/package.json | 43 ++++ packages/perry-solid/src/index.ts | 91 ++++++++ packages/perry-solid/src/renderer.ts | 196 ++++++++++++++++++ packages/perry-solid/test/native-smoke.py | 105 ++++++++++ packages/perry-solid/test/native-smoke.ts | 27 +++ packages/perry-solid/test/renderer.test.ts | 125 +++++++++++ packages/perry-solid/tsconfig.json | 21 ++ tests/release/packages/perry-solid/.gitignore | 3 + .../release/packages/perry-solid/expected.txt | 1 + tests/release/packages/perry-solid/fixture.sh | 20 ++ 22 files changed, 1101 insertions(+), 60 deletions(-) create mode 100644 changelog.d/4644-solid-native-renderer.md create mode 100644 crates/perry-ui-macos/tests/native_widget_order.rs create mode 100644 packages/perry-solid/.gitignore create mode 100644 packages/perry-solid/README.md create mode 100644 packages/perry-solid/examples/counter.ts create mode 100644 packages/perry-solid/package-lock.json create mode 100644 packages/perry-solid/package.json create mode 100644 packages/perry-solid/src/index.ts create mode 100644 packages/perry-solid/src/renderer.ts create mode 100644 packages/perry-solid/test/native-smoke.py create mode 100644 packages/perry-solid/test/native-smoke.ts create mode 100644 packages/perry-solid/test/renderer.test.ts create mode 100644 packages/perry-solid/tsconfig.json create mode 100644 tests/release/packages/perry-solid/.gitignore create mode 100644 tests/release/packages/perry-solid/expected.txt create mode 100755 tests/release/packages/perry-solid/fixture.sh diff --git a/Cargo.lock b/Cargo.lock index 56323cdbe2..4c4dd6b80a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6675,6 +6675,7 @@ dependencies = [ "objc2-core-foundation", "objc2-foundation", "perry-ffi", + "perry-runtime", "perry-ui", "perry-ui-testkit", ] diff --git a/changelog.d/4644-solid-native-renderer.md b/changelog.d/4644-solid-native-renderer.md new file mode 100644 index 0000000000..18328f3940 --- /dev/null +++ b/changelog.d/4644-solid-native-renderer.md @@ -0,0 +1 @@ +- Add `perry-solid`, a Solid universal-renderer bridge for native stacks, text, buttons, spacers, and dividers, with hyperscript authoring, reactive properties, keyed widget moves, and owner disposal. Add a counter/list example, a Node/native release fixture, and a macOS Geisterhand smoke test. Correct macOS indexed stack insertion and retained layout metadata, match the compiler's reorder arguments to the native floating-point ABI, and implement Windows child reordering. Solid JSX compilation remains a separate stage of #4644. diff --git a/crates/perry-dispatch/src/ui_table/part_a.rs b/crates/perry-dispatch/src/ui_table/part_a.rs index 93eb4dccd4..19c6a4b086 100644 --- a/crates/perry-dispatch/src/ui_table/part_a.rs +++ b/crates/perry-dispatch/src/ui_table/part_a.rs @@ -934,7 +934,7 @@ pub(crate) const PERRY_UI_TABLE_PART_A: &[MethodRow] = &[ MethodRow { method: "widgetReorderChild", runtime: "perry_ui_widget_reorder_child", - args: &[ArgKind::Widget, ArgKind::I64Raw, ArgKind::I64Raw], + args: &[ArgKind::Widget, ArgKind::F64, ArgKind::F64], ret: ReturnKind::Void, }, MethodRow { diff --git a/crates/perry-ui-macos/Cargo.toml b/crates/perry-ui-macos/Cargo.toml index 8afe3c9b72..98771917cc 100644 --- a/crates/perry-ui-macos/Cargo.toml +++ b/crates/perry-ui-macos/Cargo.toml @@ -69,3 +69,11 @@ objc2-app-kit = { version = "0.3", features = [ "NSStatusItem", "NSStatusBarButton", ] } + +[target.'cfg(target_os = "macos")'.dev-dependencies] +perry-runtime.workspace = true + +[[test]] +name = "native_widget_order" +path = "tests/native_widget_order.rs" +harness = false diff --git a/crates/perry-ui-macos/src/widgets/mod.rs b/crates/perry-ui-macos/src/widgets/mod.rs index 0409a9e937..7e52675e66 100644 --- a/crates/perry-ui-macos/src/widgets/mod.rs +++ b/crates/perry-ui-macos/src/widgets/mod.rs @@ -47,7 +47,7 @@ pub mod zstack; use objc2::rc::Retained; use objc2::runtime::{AnyClass, AnyObject}; use objc2::{msg_send, AnyThread, DefinedClass}; -use objc2_app_kit::{NSStackView, NSView}; +use objc2_app_kit::{NSStackView, NSStackViewGravity, NSView}; use objc2_foundation::NSObjectProtocol; use std::cell::RefCell; @@ -284,13 +284,13 @@ pub fn set_hidden(handle: i64, hidden: bool) { if is_stack { let stack: &NSStackView = unsafe { &*(Retained::as_ptr(&parent) as *const NSStackView) }; - let count = stack.arrangedSubviews().len(); - let insert_idx = index.min(count); - unsafe { - let _: () = objc2::msg_send![ - stack, insertArrangedSubview: &*view, atIndex: insert_idx - ]; - } + let count = stack.viewsInGravity(NSStackViewGravity::Top).len(); + stack.insertView_atIndex_inGravity( + &view, + index.min(count), + NSStackViewGravity::Top, + ); + refresh_stack_parent_map(parent_handle, stack); } } } @@ -486,27 +486,50 @@ pub fn clear_children(handle: i64) { } } -/// Add a child view to a parent view at a specific index. +/// Refresh positions used when AppKit detaches and later reattaches hidden views. +fn refresh_stack_parent_map(parent_handle: i64, stack: &NSStackView) { + let views = stack.viewsInGravity(NSStackViewGravity::Top); + WIDGETS.with(|widgets| { + let widgets = widgets.borrow(); + PARENT_MAP.with(|parents| { + let mut parents = parents.borrow_mut(); + for (index, view) in views.iter().enumerate() { + if let Some(handle_index) = widgets + .iter() + .position(|registered| Retained::as_ptr(registered) == Retained::as_ptr(&view)) + { + parents.insert(handle_index as i64 + 1, (parent_handle, index)); + } + } + }); + }); +} + +/// Insert or move a child at an index, retaining its own layout metadata. +/// Perry stacks use the top/leading gravity area for both orientations. pub fn add_child_at(parent_handle: i64, child_handle: i64, index: i64) { if let (Some(parent), Some(child)) = (get_widget(parent_handle), get_widget(child_handle)) { - let is_stack = if let Some(cls) = AnyClass::get(c"NSStackView") { - parent.isKindOfClass(cls) - } else { - false - }; - + let is_stack = AnyClass::get(c"NSStackView") + .map(|class| parent.isKindOfClass(class)) + .unwrap_or(false); if is_stack { - let stack: &NSStackView = - unsafe { &*(Retained::as_ptr(&parent) as *const NSStackView) }; - // Use addView:inGravity: with top/leading gravity for consistent packing - unsafe { - let _: () = objc2::msg_send![stack, addView: &*child, inGravity: 1i64]; + // A move must detach from the previous arranged-view list without + // remove_child's disposal cleanup (which deactivates width/height). + let previous = PARENT_MAP.with(|parents| parents.borrow().get(&child_handle).copied()); + if let Some((old_handle, _)) = previous { + if let Some(old_view) = get_widget(old_handle) { + let old_stack = + unsafe { &*(Retained::as_ptr(&old_view) as *const NSStackView) }; + old_stack.removeView(&child); + refresh_stack_parent_map(old_handle, old_stack); + } } - // Track parent-child for re-attachment after hide/show - PARENT_MAP.with(|m| { - m.borrow_mut() - .insert(child_handle, (parent_handle, index as usize)); - }); + child.removeFromSuperview(); + let stack = unsafe { &*(Retained::as_ptr(&parent) as *const NSStackView) }; + let count = stack.viewsInGravity(NSStackViewGravity::Top).len(); + let index = index.max(0) as usize; + stack.insertView_atIndex_inGravity(&child, index.min(count), NSStackViewGravity::Top); + refresh_stack_parent_map(parent_handle, stack); } else if zstack::is_zstack(parent_handle) { zstack::add_child(parent_handle, child_handle); } else { @@ -530,17 +553,8 @@ pub fn add_child(parent_handle: i64, child_handle: i64) { // Safety: we verified the type with isKindOfClass let stack: &NSStackView = unsafe { &*(Retained::as_ptr(&parent) as *const NSStackView) }; - let index = stack.arrangedSubviews().len(); - // Use addView:inGravity: with Top/Leading gravity (1) so children - // pack tightly from the top (VStack) or leading edge (HStack) - // instead of defaulting to center gravity area. - unsafe { - let _: () = objc2::msg_send![stack, addView: &*child, inGravity: 1i64]; - } - // Track parent-child for re-attachment after hide/show - PARENT_MAP.with(|m| { - m.borrow_mut().insert(child_handle, (parent_handle, index)); - }); + let count = stack.viewsInGravity(NSStackViewGravity::Top).len(); + add_child_at(parent_handle, child_handle, count as i64); } else if zstack::is_zstack(parent_handle) { zstack::add_child(parent_handle, child_handle); } else { @@ -573,6 +587,10 @@ pub fn remove_child(parent_handle: i64, child_handle: i64) { // Clean up metadata maps cleanup_widget_maps(&handles_to_clean); + if is_stack { + let stack = unsafe { &*(Retained::as_ptr(&parent) as *const NSStackView) }; + refresh_stack_parent_map(parent_handle, stack); + } } } @@ -601,31 +619,22 @@ pub fn set_overlay_frame(handle: i64, x: f64, y: f64, w: f64, h: f64) { } } -/// Reorder a child within an NSStackView by moving from one index to another. +/// Reorder a child within a stack, preserving gravity and hidden-view positions. pub fn reorder_child(parent_handle: i64, from_index: i64, to_index: i64) { if let Some(parent) = get_widget(parent_handle) { - let is_stack = if let Some(cls) = AnyClass::get(c"NSStackView") { - parent.isKindOfClass(cls) - } else { - false - }; - + let is_stack = AnyClass::get(c"NSStackView") + .map(|class| parent.isKindOfClass(class)) + .unwrap_or(false); if is_stack { - let stack: &NSStackView = - unsafe { &*(Retained::as_ptr(&parent) as *const NSStackView) }; - let subviews = stack.arrangedSubviews(); - let count = subviews.len(); - let fi = from_index as usize; - let ti = to_index as usize; - if fi < count && ti < count { - let child: *const NSView = - unsafe { objc2::msg_send![&subviews, objectAtIndex: fi] }; - let child_ref: &NSView = unsafe { &*child }; - stack.removeArrangedSubview(child_ref); - unsafe { - let _: () = - objc2::msg_send![stack, insertArrangedSubview: child_ref, atIndex: ti]; - } + let stack = unsafe { &*(Retained::as_ptr(&parent) as *const NSStackView) }; + let views = stack.viewsInGravity(NSStackViewGravity::Top); + let from = from_index as usize; + let to = to_index as usize; + if from < views.len() && to < views.len() && from != to { + let child = views.objectAtIndex(from); + stack.removeView(&child); + stack.insertView_atIndex_inGravity(&child, to, NSStackViewGravity::Top); + refresh_stack_parent_map(parent_handle, stack); } } } diff --git a/crates/perry-ui-macos/tests/native_widget_order.rs b/crates/perry-ui-macos/tests/native_widget_order.rs new file mode 100644 index 0000000000..8544636ff2 --- /dev/null +++ b/crates/perry-ui-macos/tests/native_widget_order.rs @@ -0,0 +1,89 @@ +#[cfg(target_os = "macos")] +fn main() { + use objc2::rc::Retained; + use objc2_app_kit::{NSApplication, NSStackView, NSView}; + use objc2_foundation::MainThreadMarker; + use perry_runtime as _; + use perry_ui_macos::widgets; + + fn children(handle: i64) -> Vec { + let view = widgets::get_widget(handle).unwrap(); + let stack = unsafe { &*(Retained::as_ptr(&view) as *const NSStackView) }; + stack + .arrangedSubviews() + .iter() + .map(|v| Retained::as_ptr(&v) as usize) + .collect() + } + fn ptr(handle: i64) -> usize { + Retained::as_ptr(&widgets::get_widget(handle).unwrap()) as usize + } + + if std::env::args().any(|arg| arg == "--list") { + println!("native_widget_order: test"); + return; + } + let mtm = MainThreadMarker::new().expect("native widget test runs on the main thread"); + let _app = NSApplication::sharedApplication(mtm); + let parent = widgets::vstack::create(0.0); + let other = widgets::hstack::create(0.0); + let a = widgets::spacer::create(); + let b = widgets::spacer::create(); + let c = widgets::spacer::create(); + widgets::add_child(parent, a); + widgets::add_child(parent, b); + widgets::add_child_at(parent, c, 1); + assert_eq!( + children(parent), + vec![ptr(a), ptr(c), ptr(b)], + "indexed insertion must affect native order" + ); + + widgets::add_child_at(parent, a, 2); + assert_eq!(children(parent), vec![ptr(c), ptr(b), ptr(a)]); + widgets::set_width(b, 80.0); + widgets::add_child_at(other, b, 0); + assert_eq!(children(parent), vec![ptr(c), ptr(a)]); + assert_eq!(children(other), vec![ptr(b)]); + let b_view = widgets::get_widget(b).unwrap(); + assert!( + b_view + .constraints() + .iter() + .any(|constraint| constraint.constant() == 80.0 && constraint.isActive()), + "moving a widget preserves its width constraint" + ); + + widgets::add_child_at(parent, b, -1); + assert_eq!(children(parent), vec![ptr(b), ptr(c), ptr(a)]); + assert!(children(other).is_empty()); + widgets::reorder_child(parent, 0, 2); + assert_eq!(children(parent), vec![ptr(c), ptr(a), ptr(b)]); + + // Simulate a stack-detached hidden child, then exercise the cached position + // used by set_hidden. Reordering must update that position for every child. + let parent_view = widgets::get_widget(parent).unwrap(); + let stack = unsafe { &*(Retained::as_ptr(&parent_view) as *const NSStackView) }; + let a_view: Retained = widgets::get_widget(a).unwrap(); + stack.removeArrangedSubview(&a_view); + a_view.removeFromSuperview(); + widgets::set_hidden(a, false); + assert_eq!(children(parent), vec![ptr(c), ptr(a), ptr(b)]); + widgets::remove_child(parent, c); + stack.removeArrangedSubview(&a_view); + a_view.removeFromSuperview(); + widgets::set_hidden(a, false); + assert_eq!( + children(parent), + vec![ptr(a), ptr(b)], + "removal refreshes surviving cached positions" + ); + widgets::add_child_at(parent, c, i64::MAX); + assert_eq!(children(parent), vec![ptr(a), ptr(b), ptr(c)]); + println!( + "PASS native widget ordering, reparenting, retained constraints, and hidden reattachment" + ); +} + +#[cfg(not(target_os = "macos"))] +fn main() {} diff --git a/crates/perry-ui-windows/src/ffi/widget_layout_extras.rs b/crates/perry-ui-windows/src/ffi/widget_layout_extras.rs index 17dd7e11e5..b0247200db 100644 --- a/crates/perry-ui-windows/src/ffi/widget_layout_extras.rs +++ b/crates/perry-ui-windows/src/ffi/widget_layout_extras.rs @@ -143,7 +143,9 @@ pub extern "C" fn perry_ui_stack_set_distribution(handle: i64, distribution: f64 } #[no_mangle] -pub extern "C" fn perry_ui_widget_reorder_child(_parent: i64, _child: i64, _index: i64) {} +pub extern "C" fn perry_ui_widget_reorder_child(parent: i64, from: f64, to: f64) { + widgets::reorder_child(parent, from as i64, to as i64); +} // perry_debug_trace_init and perry_debug_trace_init_done are provided by perry_runtime diff --git a/crates/perry-ui-windows/src/widgets/mod.rs b/crates/perry-ui-windows/src/widgets/mod.rs index 04ef2182a6..11f025f390 100644 --- a/crates/perry-ui-windows/src/widgets/mod.rs +++ b/crates/perry-ui-windows/src/widgets/mod.rs @@ -679,6 +679,30 @@ pub fn add_child_at(parent_handle: i64, child_handle: i64, index: i64) { crate::app::request_layout(); } +/// Move an existing child without changing its native window or layout metadata. +pub fn reorder_child(parent_handle: i64, from_index: i64, to_index: i64) { + if parent_handle <= 0 { + return; + } + let changed = WIDGETS.with(|widgets| { + let mut widgets = widgets.borrow_mut(); + let Some(parent) = widgets.get_mut((parent_handle - 1) as usize) else { + return false; + }; + let from = from_index as usize; + let to = to_index as usize; + if from >= parent.children.len() || to >= parent.children.len() || from == to { + return false; + } + let child = parent.children.remove(from); + parent.children.insert(to, child); + true + }); + if changed { + crate::app::request_layout(); + } +} + /// Remove a specific child from a parent container. pub fn remove_child(parent_handle: i64, child_handle: i64) { // Remove from children list diff --git a/packages/perry-solid/.gitignore b/packages/perry-solid/.gitignore new file mode 100644 index 0000000000..c654d16c90 --- /dev/null +++ b/packages/perry-solid/.gitignore @@ -0,0 +1,3 @@ +node_modules/ +*.log +out diff --git a/packages/perry-solid/README.md b/packages/perry-solid/README.md new file mode 100644 index 0000000000..fae27497f6 --- /dev/null +++ b/packages/perry-solid/README.md @@ -0,0 +1,154 @@ +# Solid for Perry native UI + +`perry-solid` connects Solid's universal renderer to Perry's native widget +handles. Signals update existing widgets directly. The renderer keeps parent +and sibling information in TypeScript so keyed lists can move native widgets +without recreating them. + +This is the runtime bridge from [#4644](https://github.com/PerryTS/perry/issues/4644). +It provides native hyperscript; Solid JSX compilation remains a separate stage. +Solid's bundled `solid-js/h` and `solid-js/html` use its web renderer and are +not substitutes for this package's `h`. + +## Use from this checkout + +In an application project, install the local package and Solid: + +```sh +npm install /path/to/perry/packages/perry-solid solid-js@1.9.15 +``` + +Select Solid's reactive client runtime in the application's `package.json`: + +```json +{ + "perry": { + "compilePackages": ["solid-js"], + "allow": { "compilePackages": ["solid-js"] }, + "packageAliases": { + "solid-js": "solid-js/dist/solid.js", + "solid-js/store": "solid-js/store/dist/store.js" + } + } +} +``` + +These aliases also apply inside Solid's universal renderer and stores, so they +share the same reactive owner. Solid's default Node entry is an intentionally +nonreactive server build. `perry-solid` declares `nativeModule: true`; its +TypeScript is compiled natively along with Solid. + +```ts +import { App, VStack } from "perry/ui"; +import { createSignal } from "solid-js"; +import { h, render } from "perry-solid"; + +const body = VStack([]); +const dispose = render(() => { + const [count, setCount] = createSignal(0); + return h("VStack", { padding: 16 }, + h("Text", { fontSize: 24 }, () => `Count: ${count()}`), + h("Button", { onPress: () => setCount(n => n + 1) }, "Increment"), + ); +}, body); + +App({ title: "Solid + Perry", width: 400, height: 240, body }); +// Call dispose() when unmounting this root: it stops effects and detaches nodes. +``` + +[examples/counter.ts](examples/counter.ts) adds a keyed list and a rotate button. +Compile it from the package directory with: + +```sh +perry examples/counter.ts -o counter +./counter +``` + +## Components and properties + +Use `h(Component, props)` for functions returning native children. Reactive +children are accessors (`() => count()`); reactive properties are getters: + +```ts +h("Text", { get opacity() { return dimmed() ? 0.5 : 1; } }, "Status") +``` + +Supported elements are `VStack`, `HStack`, `Text`, `Button`, `Spacer`, and +`Divider`. Stacks use an initial spacing of eight points. `Text` and `Button` +accept text children (including arrays and reactive text); stacks accept +widgets and text. A primitive text child gets its own native Text widget only +when inserted into a stack. + +| Property | Native behavior | +| --- | --- | +| `text` | Set a Text value or Button title; use this or text children. | +| `onPress` | Button callback; a reactive getter can replace it. | +| `width`, `height` | Fixed native dimensions. | +| `opacity`, `hidden`, `disabled` | Native widget state. | +| `padding`, `cornerRadius` | Uniform padding and corner radius. | +| `backgroundColor` | Four numeric RGBA channels: `[r, g, b, a]`. | +| `tooltip` | Native tooltip text. | +| `fontSize` | Text font size. | + +Properties map to native setters, not CSS. Unsupported element/property names +throw. `ref` follows Solid's spread contract and receives a `NativeNode`; its +`handle` is an opaque Perry widget handle, not an ordinary serializable number. + +Import `For` from `perry-solid` for Solid's keyed list behavior with native +child types: + +```ts +h("VStack", null, For({ + get each() { return items(); }, + children: item => h("Text", null, item.name), +})) +``` + +Mount with `render(component, emptyStackHandle)`. Use a dedicated empty native +VStack or HStack; the renderer owns its mounted child order. The returned disposer is +idempotent, runs Solid cleanup, releases stored user callbacks, and detaches +the mounted nodes. Native +widget allocation and reclamation otherwise follow Perry's widget registry. + +The low-level universal helpers (`createElement`, `createTextNode`, `insert`, +`spread`, `setProp`, `createComponent`, `effect`, `memo`, `mergeProps`, and `use`) +are also exported. `perry-solid/renderer` exposes `createNativeRenderer` and its +`NativeDriver` interface for testing host behavior without a display server. + +## Validation + +```sh +npm ci --ignore-scripts +npm test +npm run typecheck +PERRY_BIN=/absolute/path/to/perry ../../tests/release/packages/_harness.sh --filter perry-solid +``` + +The release fixture copies the actual package sources and pinned dependencies, +then checks the same assertions in Node's browser condition and Perry. It +covers reactive properties/text, callback replacement, keyed identity/order, +reparenting, invalid tree operations, and disposal; it also requires zero +JavaScript modules in the native build. + +`test/native-smoke.ts` is a real widget app for Geisterhand checks. The macOS +backend's `native_widget_order` Cargo target runs on the main thread and checks +actual AppKit ordering, moves between stacks, retained dimensions, and hidden +reattachment. Other backends use their existing native insertion/removal APIs; +this change does not establish executed platform coverage outside macOS. + +From this package directory, with a Geisterhand-enabled Perry installation: + +```sh +perry compile test/native-smoke.ts --geisterhand-port 19764 -o /tmp/perry-solid-smoke +python3 test/native-smoke.py /tmp/perry-solid-smoke --output-dir /tmp/perry-solid-smoke-results +``` + +The runner checks updates to the same native Text handles, button callbacks, +keyed row order with retained widget identities, and stopped effects after +disposal. It saves screenshots and widget snapshots, then exits the app cleanly. +GC scheduling and verifier environment variables are inherited by the app. + +The client runtime's separate GC verifier correction is in +[#9822](https://github.com/PerryTS/perry/pull/9822). Use that correction for +`PERRY_GC_VERIFY_EVACUATION=1` when testing workloads with retained array-growth +aliases. diff --git a/packages/perry-solid/examples/counter.ts b/packages/perry-solid/examples/counter.ts new file mode 100644 index 0000000000..f1ed4e4526 --- /dev/null +++ b/packages/perry-solid/examples/counter.ts @@ -0,0 +1,24 @@ +import { App, VStack } from "perry/ui"; +import { createSignal } from "solid-js"; +import { h, render, For } from "../src/index.ts"; + +function Counter() { + const [count, setCount] = createSignal(0); + const [items, setItems] = createSignal(["Alpha", "Beta", "Gamma"]); + return h("VStack", { padding: 16 }, + h("Text", { fontSize: 24 }, () => `Count: ${count()}`), + h("HStack", null, + h("Button", { onPress: () => setCount(n => n + 1) }, "Increment"), + h("Button", { onPress: () => setCount(0) }, "Reset"), + h("Button", { onPress: () => setItems(rows => [rows[2], rows[0], rows[1]]) }, "Rotate"), + ), + h("VStack", null, For({ + get each() { return items(); }, + children: item => h("Text", null, item), + })), + ); +} + +const body = VStack([]); +render(Counter, body); +App({ title: "Solid + Perry", width: 420, height: 300, body }); diff --git a/packages/perry-solid/package-lock.json b/packages/perry-solid/package-lock.json new file mode 100644 index 0000000000..7b6a3f49d8 --- /dev/null +++ b/packages/perry-solid/package-lock.json @@ -0,0 +1,94 @@ +{ + "name": "perry-solid", + "version": "0.1.0", + "lockfileVersion": 3, + "requires": true, + "packages": { + "": { + "name": "perry-solid", + "version": "0.1.0", + "license": "MIT", + "devDependencies": { + "@types/node": "26.4.1", + "solid-js": "1.9.15", + "typescript": "5.9.3" + }, + "peerDependencies": { + "solid-js": "^1.9.15" + } + }, + "node_modules/@types/node": { + "version": "26.4.1", + "resolved": "https://registry.npmjs.org/@types/node/-/node-26.4.1.tgz", + "integrity": "sha512-k97ENvZWtvA6yqz5/FS6a7duDgOPEeOQOc2iKS/nY6mX6qJUKtLnWzQS+Xj6tXweyj6ZcTAK2Qecetnvi9nCLA==", + "dev": true, + "license": "MIT", + "dependencies": { + "undici-types": "~8.3.0" + } + }, + "node_modules/csstype": { + "version": "3.2.3", + "resolved": "https://registry.npmjs.org/csstype/-/csstype-3.2.3.tgz", + "integrity": "sha512-z1HGKcYy2xA8AGQfwrn0PAy+PB7X/GSj3UVJW9qKyn43xWa+gl5nXmU4qqLMRzWVLFC8KusUX8T/0kCiOYpAIQ==", + "dev": true, + "license": "MIT" + }, + "node_modules/seroval": { + "version": "1.5.6", + "resolved": "https://registry.npmjs.org/seroval/-/seroval-1.5.6.tgz", + "integrity": "sha512-rVQVWjjSvlINzaQPZH5JFqsqEsIWdTxY3iJZCnTL/5gQbXIRooVZKI60tVCkOVfzcRPejboxO2t0P89dg5mQaA==", + "dev": true, + "license": "MIT", + "engines": { + "node": ">=10" + } + }, + "node_modules/seroval-plugins": { + "version": "1.5.6", + "resolved": "https://registry.npmjs.org/seroval-plugins/-/seroval-plugins-1.5.6.tgz", + "integrity": "sha512-HXuLAX2pu/UByPpaeo/TaMfvMIi+1QqIoPJYCcAtU8QkVNwgR6MPlGuCQTErV1JwraaMbYaWVIBX7mppzGLATQ==", + "dev": true, + "license": "MIT", + "engines": { + "node": ">=10" + }, + "peerDependencies": { + "seroval": "^1.0" + } + }, + "node_modules/solid-js": { + "version": "1.9.15", + "resolved": "https://registry.npmjs.org/solid-js/-/solid-js-1.9.15.tgz", + "integrity": "sha512-EeiY2xfpZJqPLjXspVEKjAII4yv8NyG//NxZ3IpOFHdUNnnTyL0uJOeS9LWGvA7cFCz5y94cjFwYlmw5Luncsg==", + "dev": true, + "license": "MIT", + "dependencies": { + "csstype": "^3.1.0", + "seroval": "~1.5.4", + "seroval-plugins": "~1.5.4" + } + }, + "node_modules/typescript": { + "version": "5.9.3", + "resolved": "https://registry.npmjs.org/typescript/-/typescript-5.9.3.tgz", + "integrity": "sha512-jl1vZzPDinLr9eUt3J/t7V6FgNEw9QjvBPdysz9KfQDD41fQrC2Y4vKQdiaUpFT4bXlb1RHhLpp8wtm6M5TgSw==", + "dev": true, + "license": "Apache-2.0", + "bin": { + "tsc": "bin/tsc", + "tsserver": "bin/tsserver" + }, + "engines": { + "node": ">=14.17" + } + }, + "node_modules/undici-types": { + "version": "8.3.0", + "resolved": "https://registry.npmjs.org/undici-types/-/undici-types-8.3.0.tgz", + "integrity": "sha512-j375ScV60dom+YkPFIfTLcOiPxkN/buHz5GobjLhixFuANaNs3C9l4GmrWqejgXWJ7BbJcFYpTEUkS1Ge8bpZQ==", + "dev": true, + "license": "MIT" + } + } +} diff --git a/packages/perry-solid/package.json b/packages/perry-solid/package.json new file mode 100644 index 0000000000..76f86c56b6 --- /dev/null +++ b/packages/perry-solid/package.json @@ -0,0 +1,43 @@ +{ + "name": "perry-solid", + "version": "0.1.0", + "description": "Solid's universal renderer for Perry native widgets", + "type": "module", + "main": "./src/index.ts", + "types": "./src/index.ts", + "exports": { + ".": "./src/index.ts", + "./renderer": "./src/renderer.ts" + }, + "files": [ + "src", + "README.md" + ], + "license": "MIT", + "peerDependencies": { + "solid-js": "^1.9.15" + }, + "devDependencies": { + "@types/node": "26.4.1", + "solid-js": "1.9.15", + "typescript": "5.9.3" + }, + "scripts": { + "test": "node --conditions=browser test/renderer.test.ts", + "typecheck": "tsc --noEmit" + }, + "perry": { + "nativeModule": true, + "compilePackages": [ + "solid-js" + ], + "allow": { + "compilePackages": [ + "solid-js" + ] + }, + "packageAliases": { + "solid-js": "solid-js/dist/solid.js" + } + } +} diff --git a/packages/perry-solid/src/index.ts b/packages/perry-solid/src/index.ts new file mode 100644 index 0000000000..9757460a30 --- /dev/null +++ b/packages/perry-solid/src/index.ts @@ -0,0 +1,91 @@ +import { + VStack, HStack, Text, Button, Spacer, Divider, + textSetString, buttonSetTitle, textSetFontSize, + widgetAddChildAt, widgetRemoveChild, widgetReorderChild, + widgetSetWidth, widgetSetHeight, widgetSetOpacity, + widgetSetHidden, widgetSetEnabled, widgetSetTooltip, + widgetSetBackgroundColor, setCornerRadius, setPadding, + type Widget, +} from "perry/ui"; +import { createNativeRenderer, type NativeDriver, type ElementName } from "./renderer.ts"; +export type { NativeNode, Child, Component, Props, ElementName } from "./renderer.ts"; +export { For } from "./renderer.ts"; + +// Perry injects this target constant (0 = macOS, 1 = iOS, 2 = Android, 3 = Windows, 4 = Linux). +declare const __platform__: number; + +function numeric(value: unknown, fallback: number): number { + if (value == null) return fallback; + if (typeof value !== "number" || !Number.isFinite(value)) throw new Error("Expected a finite native widget value"); + return value; +} + +const driver: NativeDriver = { + create(kind: ElementName, onPress: () => void): number { + switch (kind) { + case "VStack": return VStack(8, []); + case "HStack": return HStack(8, []); + case "Text": return Text(""); + case "Button": return Button("", onPress); + case "Spacer": return Spacer(); + case "Divider": return Divider(); + } + }, + setProperty(handle, kind, name, value) { + const widget = handle as Widget; + switch (name) { + case "text": + if (kind === "Text") textSetString(widget, value == null ? "" : String(value)); + else if (kind === "Button") buttonSetTitle(widget, value == null ? "" : String(value)); + else throw new Error(`text is unsupported on ${kind}`); + return; + case "width": widgetSetWidth(widget, numeric(value, 0)); return; + case "height": widgetSetHeight(widget, numeric(value, 0)); return; + case "opacity": widgetSetOpacity(widget, numeric(value, 1)); return; + case "hidden": widgetSetHidden(widget, value ? 1 : 0); return; + case "disabled": widgetSetEnabled(widget, value ? 0 : 1); return; + case "tooltip": widgetSetTooltip(widget, value == null ? "" : String(value)); return; + case "cornerRadius": setCornerRadius(widget, numeric(value, 0)); return; + case "padding": { + const amount = numeric(value, 0); + setPadding(widget, amount, amount, amount, amount); + return; + } + case "fontSize": + if (kind !== "Text") throw new Error("fontSize is supported on Text"); + textSetFontSize(widget, numeric(value, 13)); + return; + case "backgroundColor": { + const color = value == null ? [0, 0, 0, 0] : value; + if (!Array.isArray(color) || color.length !== 4) throw new Error("backgroundColor expects [r, g, b, a]"); + widgetSetBackgroundColor(widget, numeric(color[0], 0), numeric(color[1], 0), numeric(color[2], 0), numeric(color[3], 0)); + return; + } + default: throw new Error(`Unsupported Perry Solid property: ${name}`); + } + }, + insert(parent, child, index, previousParent) { + // AppKit's indexed insertion detaches without destroying retained layout + // metadata. Other backends need the old parent explicitly cleared first. + if (previousParent !== null && __platform__ !== 0) { + widgetRemoveChild(previousParent as Widget, child as Widget); + } + widgetAddChildAt(parent as Widget, child as Widget, index); + }, + move(parent, from, to) { widgetReorderChild(parent as Widget, from, to); }, + remove(parent, child) { widgetRemoveChild(parent as Widget, child as Widget); }, +}; + +const native = createNativeRenderer(driver); +export const h = native.h; +export const render = native.render; +export const createElement = native.createElement; +export const createTextNode = native.createTextNode; +export const insert = native.insert; +export const spread = native.spread; +export const setProp = native.setProp; +export const createComponent = native.createComponent; +export const effect = native.effect; +export const memo = native.memo; +export const mergeProps = native.mergeProps; +export const use = native.use; diff --git a/packages/perry-solid/src/renderer.ts b/packages/perry-solid/src/renderer.ts new file mode 100644 index 0000000000..a1e8cdac0f --- /dev/null +++ b/packages/perry-solid/src/renderer.ts @@ -0,0 +1,196 @@ +import { createRenderer } from "solid-js/universal"; +import { createRoot, getOwner, onCleanup, mergeProps, For as SolidFor, type Accessor } from "solid-js"; + +export type ElementName = "VStack" | "HStack" | "Text" | "Button" | "Spacer" | "Divider"; +export type Props = Record; +export type Child = NativeNode | string | number | boolean | null | undefined | Child[] | (() => Child); +export type Component

= (props: P) => Child; + +/** Solid For with native children instead of DOM-specific JSX declarations. */ +export const For = SolidFor as (props: { + each: readonly T[] | false | null | undefined; + fallback?: Child; + children: (item: T, index: Accessor) => Child; +}) => Child; + +/** Backend operations. Handles belong to the native widget registry. */ +export interface NativeDriver { + create(kind: ElementName, onPress: () => void): number; + setProperty(handle: number, kind: ElementName, name: string, value: unknown, previous: unknown): void; + insert(parent: number, child: number, index: number, previousParent: number | null): void; + move(parent: number, from: number, to: number): void; + remove(parent: number, child: number): void; +} + +/** Retained ordering metadata; native handles themselves have no sibling API. */ +export interface NativeNode { + kind: ElementName | "#text" | "#root"; + handle: number; + materialized: boolean; + parent: NativeNode | null; + children: NativeNode[]; + props: Props; + text: string; +} + +function isLabel(node: NativeNode): boolean { + return node.kind === "Text" || node.kind === "Button"; +} + +function isContainer(node: NativeNode): boolean { + return node.kind === "VStack" || node.kind === "HStack" || node.kind === "#root"; +} + +export function createNativeRenderer(driver: NativeDriver) { + function makeNode(kind: NativeNode["kind"], text = ""): NativeNode { + return { kind, handle: 0, materialized: false, parent: null, children: [], props: {}, text }; + } + + function createElement(name: string): NativeNode { + if (!["VStack", "HStack", "Text", "Button", "Spacer", "Divider"].includes(name)) { + throw new Error(`Unsupported Perry Solid element: ${name}`); + } + const node = makeNode(name as ElementName); + // A native button's dispatcher outlives a removed Solid owner. Release + // user callbacks when that owner is disposed, even before native detach. + if (getOwner()) onCleanup(() => { node.props = {}; }); + node.handle = driver.create(name as ElementName, () => { + const callback = node.props.onPress; + if (typeof callback === "function") callback(); + }); + node.materialized = true; + return node; + } + + function materialize(node: NativeNode): number { + // Text under a Text/Button contributes to its label. Allocate an independent + // native Text only when the text node is actually inserted into a container. + if (node.kind === "#text" && !node.materialized) { + node.handle = driver.create("Text", () => {}); + node.materialized = true; + driver.setProperty(node.handle, "Text", "text", node.text, undefined); + } + return node.handle; + } + + function refreshLabel(node: NativeNode): void { + let text = ""; + for (const child of node.children) text += child.text; + driver.setProperty(node.handle, node.kind as ElementName, "text", text, undefined); + } + + function removeNode(parent: NativeNode, node: NativeNode): void { + if (node.parent !== parent) return; + const index = parent.children.indexOf(node); + parent.children.splice(index, 1); + node.parent = null; + if (isLabel(parent)) refreshLabel(parent); + else driver.remove(parent.handle, materialize(node)); + } + + function insertNode(parent: NativeNode, node: NativeNode, anchor?: NativeNode): void { + if (anchor === node) return; + if (anchor && anchor.parent !== parent) throw new Error("Insertion anchor belongs to another parent"); + if (isLabel(parent)) { + if (node.kind !== "#text") throw new Error("Text and Button children must be text"); + } else if (!isContainer(parent)) { + throw new Error(`${parent.kind} cannot contain children`); + } + for (let ancestor: NativeNode | null = parent; ancestor; ancestor = ancestor.parent) { + if (ancestor === node) throw new Error("Cannot insert a node into its own subtree"); + } + const previousParent = node.parent; + const previousIndex = previousParent ? previousParent.children.indexOf(node) : -1; + if (previousParent) previousParent.children.splice(previousIndex, 1); + const index = anchor ? parent.children.indexOf(anchor) : parent.children.length; + parent.children.splice(index, 0, node); + node.parent = parent; + + if (previousParent && previousParent !== parent && isLabel(previousParent)) refreshLabel(previousParent); + if (isLabel(parent)) { + if (previousParent && !isLabel(previousParent)) driver.remove(previousParent.handle, materialize(node)); + refreshLabel(parent); + } else if (previousParent === parent) { + if (previousIndex !== index) driver.move(parent.handle, previousIndex, index); + } else { + const oldHandle = previousParent && !isLabel(previousParent) ? previousParent.handle : null; + driver.insert(parent.handle, materialize(node), index, oldHandle); + } + } + + const renderer = createRenderer({ + createElement, + createTextNode(value) { return makeNode("#text", String(value)); }, + isTextNode(node) { return node.kind === "#text"; }, + replaceText(node, value) { + node.text = String(value); + if (node.materialized) driver.setProperty(node.handle, "Text", "text", node.text, undefined); + if (node.parent && isLabel(node.parent)) refreshLabel(node.parent); + }, + setProperty(node, name, value, previous) { + if (name === "onPress") { + if (node.kind !== "Button") throw new Error("onPress is supported on Button"); + if (value != null && typeof value !== "function") throw new Error("onPress must be a function"); + } else { + driver.setProperty(node.handle, node.kind as ElementName, name, value, previous); + } + node.props[name] = value; + }, + insertNode, + removeNode, + getParentNode(node) { return node.parent || undefined; }, + getFirstChild(node) { return node.children[0]; }, + getNextSibling(node) { + const parent = node.parent; + return parent ? parent.children[parent.children.indexOf(node) + 1] : undefined; + }, + }); + + // Solid's implementation accepts arrays, primitives and accessors too; + // its universal declaration narrows component results to NodeType. + const createComponent = renderer.createComponent as

(component: (props: P) => Child, props: P) => Child; + + /** Native hyperscript. Reactive properties use getters; children may be accessors. */ + function h(type: ElementName, props?: Props | null, ...children: Child[]): NativeNode; + function h

(type: Component

, props: P, ...children: Child[]): Child; + function h(type: ElementName | Component, props: Props | null = null, ...children: Child[]): Child { + const properties = children.length + ? mergeProps(props || {}, { children: children.length === 1 ? children[0] : children }) + : (props || {}); + if (typeof type === "function") return createComponent(type, properties); + const node = createElement(type); + renderer.spread(node, properties); + return node; + } + + function releaseSubtree(node: NativeNode): void { + for (const child of node.children) releaseSubtree(child); + node.children = []; + node.parent = null; + node.props = {}; + } + + /** Mount into an existing native stack; dispose effects and detach its nodes. */ + function render(code: () => Child, handle: number): () => void { + const root = makeNode("#root"); + root.handle = handle; + root.materialized = true; + const dispose = createRoot(dispose => { + renderer.insert(root, code()); + return dispose; + }); + let disposed = false; + return () => { + if (disposed) return; + disposed = true; + dispose(); + while (root.children.length) { + const child = root.children[root.children.length - 1]; + removeNode(root, child); + releaseSubtree(child); + } + }; + } + + return { ...renderer, createComponent, render, h, removeNode }; +} diff --git a/packages/perry-solid/test/native-smoke.py b/packages/perry-solid/test/native-smoke.py new file mode 100644 index 0000000000..5f4a543cc6 --- /dev/null +++ b/packages/perry-solid/test/native-smoke.py @@ -0,0 +1,105 @@ +"""Exercise native-smoke.ts on macOS through its Geisterhand server.""" + +import argparse +import json +import socket +import subprocess +import sys +import time +import urllib.request +from pathlib import Path + +parser = argparse.ArgumentParser(description=__doc__) +parser.add_argument("binary", type=Path) +parser.add_argument("--port", type=int, default=19764) +parser.add_argument("--output-dir", type=Path, required=True) +args = parser.parse_args() +if sys.platform != "darwin": + parser.error("this test checks AppKit frame coordinates and requires macOS") +with socket.socket() as probe: + probe.bind(("127.0.0.1", args.port)) +args.output_dir.mkdir(parents=True, exist_ok=True) +base = f"http://127.0.0.1:{args.port}" + + +def get(path): + with urllib.request.urlopen(base + path, timeout=4) as response: + return response.read() + + +def wait_value(handle, expected): + deadline = time.monotonic() + 8 + actual = None + while time.monotonic() < deadline: + actual = json.loads(get(f"/value/{handle}"))["value"] + if actual == expected: + return + time.sleep(0.1) + raise AssertionError((handle, expected, actual)) + + +def click(handle): + request = urllib.request.Request(base + f"/click/{handle}", method="POST", data=b"") + with urllib.request.urlopen(request, timeout=4) as response: + assert json.load(response)["ok"] + + +def capture(name): + tree = get("/widgets?tree=true") + (args.output_dir / f"{name}.json").write_bytes(tree) + (args.output_dir / f"{name}.png").write_bytes(get("/screenshot")) + return json.loads(tree) + + +with (args.output_dir / "stdout.log").open("wb") as stdout, (args.output_dir / "stderr.log").open("wb") as stderr: + process = subprocess.Popen([str(args.binary.resolve())], stdout=stdout, stderr=stderr) + try: + deadline = time.monotonic() + 25 + while time.monotonic() < deadline: + if process.poll() is not None: + raise RuntimeError(f"app exited {process.returncode}; see stderr.log") + try: + if json.loads(get("/health"))["status"] == "ok": + buttons = sorted({item["handle"] for item in json.loads(get("/widgets?type=button")) + if item["callback_kind"] == 0}) + if len(buttons) == 4: + break + except OSError: + pass + time.sleep(0.1) + else: + raise RuntimeError("Geisterhand and four smoke-test buttons did not start") + + before = capture("before") + values = {item["handle"]: json.loads(get(f'/value/{item["handle"]}'))["value"] for item in before} + by_text = {value: handle for handle, value in values.items() if value is not None} + counter, raw = by_text["Count: 0"], by_text["Raw: 0"] + rows = [by_text[name] for name in ("Alpha", "Beta", "Gamma")] + increment, rotate, dispose, exit_button = buttons + click(increment) + wait_value(counter, "Count: 1") + wait_value(raw, "Raw: 1") + click(rotate) + time.sleep(0.2) + after = capture("after") + frames = {item["handle"]: item["frame"] for item in after} + # Same widget handles, now Gamma / Alpha / Beta, in AppKit's bottom-up coordinates. + assert frames[rows[2]]["y"] > frames[rows[0]]["y"] > frames[rows[1]]["y"], frames + click(dispose) + time.sleep(0.2) + wait_value(counter, "Count: 1") # disposal also sets the signal to 99 + capture("disposed") + try: + click(exit_button) + except OSError: + pass # process.exit can close the HTTP response first + assert process.wait(timeout=8) == 0 + print("PASS native text identity, button events, keyed widget order, and disposal") + finally: + if process.poll() is None: + process.terminate() + try: + process.wait(timeout=8) + except subprocess.TimeoutExpired: + process.kill() + process.wait() diff --git a/packages/perry-solid/test/native-smoke.ts b/packages/perry-solid/test/native-smoke.ts new file mode 100644 index 0000000000..f7e8302af7 --- /dev/null +++ b/packages/perry-solid/test/native-smoke.ts @@ -0,0 +1,27 @@ +import { App, VStack, widgetAddChild, type Widget } from "perry/ui"; +import { createSignal } from "solid-js"; +import { h, render, For, type NativeNode } from "../src/index.ts"; + +const [count, setCount] = createSignal(0); +const [items, setItems] = createSignal(["Alpha", "Beta", "Gamma"]); +const body = VStack([]); +let counter: NativeNode; +let increment: NativeNode; +let rotate: NativeNode; +let list: NativeNode; +const dispose = render(() => { + counter = h("Text", { fontSize: 24 }, () => `Count: ${count()}`); + increment = h("Button", { onPress: () => setCount(n => n + 1) }, "Increment"); + rotate = h("Button", { onPress: () => setItems(rows => [rows[2], rows[0], rows[1]]) }, "Rotate"); + list = h("VStack", null, For({ + get each() { return items(); }, + children: item => h("Text", null, item), + })); + return h("VStack", { padding: 16 }, counter, increment, rotate, h("VStack", null, () => `Raw: ${count()}`), list); +}, body); +const stop = h("Button", { onPress: () => { dispose(); setCount(99); } }, "Dispose"); +// Keep the disposal control outside the mounted Solid root for the smoke test. +widgetAddChild(body, stop.handle as Widget); +const exit = h("Button", { onPress: () => process.exit(0) }, "Exit"); +widgetAddChild(body, exit.handle as Widget); +App({ title: "Solid native smoke", width: 420, height: 360, body }); diff --git a/packages/perry-solid/test/renderer.test.ts b/packages/perry-solid/test/renderer.test.ts new file mode 100644 index 0000000000..952b6adcd6 --- /dev/null +++ b/packages/perry-solid/test/renderer.test.ts @@ -0,0 +1,125 @@ +import assert from "node:assert/strict"; +import { createSignal, onCleanup } from "solid-js"; +import { createNativeRenderer, For, type NativeDriver, type ElementName, type NativeNode } from "../src/renderer.ts"; + +const widgets: { kind: ElementName; children: number[]; props: Record; press: () => void }[] = []; +const operations: string[] = []; +const driver: NativeDriver = { + create(kind, press) { + widgets.push({ kind, children: [], props: {}, press }); + return widgets.length; + }, + setProperty(handle, kind, name, value) { widgets[handle - 1].props[name] = value; }, + insert(parent, child, index, previousParent) { + if (previousParent) { + const old = widgets[previousParent - 1].children; + old.splice(old.indexOf(child), 1); + } + widgets[parent - 1].children.splice(index, 0, child); + operations.push(`insert ${child} ${index}`); + }, + move(parent, from, to) { + const children = widgets[parent - 1].children; + const child = children.splice(from, 1)[0]; + children.splice(to, 0, child); + operations.push(`move ${child} ${to}`); + }, + remove(parent, child) { + const children = widgets[parent - 1].children; + children.splice(children.indexOf(child), 1); + operations.push(`remove ${child}`); + }, +}; +const renderer = createNativeRenderer(driver); +const { h } = renderer; +const root = driver.create("VStack", () => {}); +const [count, setCount] = createSignal(0); +const [items, setItems] = createSignal(["a", "b", "c"]); +const [handler, setHandler] = createSignal<() => void>(() => setCount(n => n + 1)); +let label: NativeNode; +let button: NativeNode; +let list: NativeNode; +let effects = 0; +let cleanups = 0; +const dispose = renderer.render(() => { + onCleanup(() => cleanups++); + label = h("Text", { get width() { return 100 + count(); } }, () => { + effects++; + return `Count ${count()}`; + }) as NativeNode; + button = h("Button", { get onPress() { return handler(); } }, "Increment") as NativeNode; + list = h("VStack", null, For({ + get each() { return items(); }, + children: (item: string) => h("Text", null, item), + })) as NativeNode; + return h("VStack", null, label, button, list); +}, root); + +assert.equal(widgets[label!.handle - 1].props.text, "Count 0"); +assert.equal(widgets[button!.handle - 1].props.text, "Increment"); +assert.equal(widgets.filter(w => w.kind === "Text").length, 4, "label text nodes allocate no extra widgets"); +const labelHandle = label!.handle; +widgets[button!.handle - 1].press(); +assert.equal(widgets[labelHandle - 1].props.text, "Count 1"); +assert.equal(widgets[labelHandle - 1].props.width, 101); +setHandler(() => () => setCount(n => n + 10)); +widgets[button!.handle - 1].press(); +assert.equal(widgets[labelHandle - 1].props.text, "Count 11"); +assert.equal(label!.handle, labelHandle); + +const original = [...widgets[list!.handle - 1].children]; +setItems(rows => [rows[2], rows[0], rows[1]]); +assert.deepEqual(widgets[list!.handle - 1].children, [original[2], original[0], original[1]]); +setItems(["b", "d", "c"]); +const final = widgets[list!.handle - 1].children; +assert.equal(final[0], original[1]); +assert.equal(final[2], original[2]); +assert.equal(widgets[final[1] - 1].props.text, "d"); +assert.equal(list!.children[0].parent, list!); +assert.ok(operations.some(op => op.startsWith("move "))); + +const other = renderer.createElement("VStack"); +const moved = list!.children[0]; +renderer.insertNode(other, moved); +assert.equal(moved.parent, other); +assert.deepEqual(widgets[other.handle - 1].children, [moved.handle]); +assert.ok(!widgets[list!.handle - 1].children.includes(moved.handle)); +assert.throws(() => renderer.insertNode(other, list!, list!.children[0])); +assert.throws(() => renderer.insertNode(moved, other)); +assert.throws(() => renderer.insertNode(list!, list!)); +renderer.insertNode(list!, moved, list!.children[0]); +assert.equal(widgets[list!.handle - 1].children[0], moved.handle); +assert.deepEqual(widgets[other.handle - 1].children, []); + +const beforeDispose = effects; +dispose(); +dispose(); +setCount(99); +widgets[button!.handle - 1].press(); +assert.equal(count(), 99, "disposed owners release native user callbacks"); +assert.equal(cleanups, 1); +assert.equal(effects, beforeDispose); +assert.deepEqual(widgets[root - 1].children, []); +// Perry widget handles can use NaN-boxed words. They are opaque tokens; +// numeric truthiness must never decide whether a native widget exists. +const opaqueWrites: string[] = []; +const opaque = createNativeRenderer({ + create() { return Number.NaN; }, + setProperty(_handle, _kind, name, value) { + if (name === "text") opaqueWrites.push(String(value)); + }, + insert() {}, move() {}, remove() {}, +}); +const [raw, setRaw] = createSignal("raw 0"); +let rawContainer: NativeNode; +const disposeOpaque = opaque.render(() => { + rawContainer = opaque.h("VStack", null, raw); + return rawContainer; +}, Number.NaN); +const rawNode = rawContainer!.children[0]; +assert.equal(opaqueWrites[opaqueWrites.length - 1], "raw 0"); +setRaw("raw 1"); +assert.equal(opaqueWrites[opaqueWrites.length - 1], "raw 1"); +assert.equal(rawContainer!.children[0], rawNode, "single reactive text preserves its native node"); +disposeOpaque(); +console.log("PASS Solid native renderer: signals, properties, events, keyed order, reparenting, disposal"); diff --git a/packages/perry-solid/tsconfig.json b/packages/perry-solid/tsconfig.json new file mode 100644 index 0000000000..9c101c5918 --- /dev/null +++ b/packages/perry-solid/tsconfig.json @@ -0,0 +1,21 @@ +{ + "compilerOptions": { + "target": "ES2022", + "module": "NodeNext", + "moduleResolution": "NodeNext", + "strict": true, + "noEmit": true, + "allowImportingTsExtensions": true, + "skipLibCheck": true, + "paths": { + "perry/ui": [ + "../../types/perry/ui/index.d.ts" + ] + } + }, + "include": [ + "src/**/*.ts", + "test/**/*.ts", + "examples/**/*.ts" + ] +} diff --git a/tests/release/packages/perry-solid/.gitignore b/tests/release/packages/perry-solid/.gitignore new file mode 100644 index 0000000000..88ff7f851c --- /dev/null +++ b/tests/release/packages/perry-solid/.gitignore @@ -0,0 +1,3 @@ +work/ +*.log +.last-skip diff --git a/tests/release/packages/perry-solid/expected.txt b/tests/release/packages/perry-solid/expected.txt new file mode 100644 index 0000000000..0530703fca --- /dev/null +++ b/tests/release/packages/perry-solid/expected.txt @@ -0,0 +1 @@ +PASS Solid native renderer: signals, properties, events, keyed order, reparenting, disposal diff --git a/tests/release/packages/perry-solid/fixture.sh b/tests/release/packages/perry-solid/fixture.sh new file mode 100755 index 0000000000..84da2669d4 --- /dev/null +++ b/tests/release/packages/perry-solid/fixture.sh @@ -0,0 +1,20 @@ +#!/usr/bin/env bash +set -euo pipefail +[[ "${1:-}" == "--__did-skip-marker" ]] && exit 1 +cd "$(dirname "$0")" +source ../_fixture_lib.sh +fixture_dir="$PWD" +package_dir="$(cd ../../../../packages/perry-solid && pwd)" +mkdir -p work +cp "$package_dir/package.json" "$package_dir/package-lock.json" work/ +cp -R "$package_dir/src" "$package_dir/test" work/ +cd work +npm ci --ignore-scripts --no-audit --no-fund > install.log 2>&1 +fixture_setup perry-solid +node --conditions=browser test/renderer.test.ts > node-out.txt +diff -u "$fixture_dir/expected.txt" node-out.txt +PERRY_DISABLE_BUILD_CACHE=1 fixture_compile_run_diff perry-solid test/renderer.test.ts "$fixture_dir/expected.txt" +if ! grep -Eq 'Found [0-9]+ module\(s\): [1-9][0-9]* native, 0 JavaScript' perry-compile.log; then + echo 'FAIL perry-solid — expected every module to compile natively' + exit 1 +fi From 6ae411ce5e171d6a51602a6cd640ad53e73f1cc5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 5 Sep 2026 21:26:14 +0200 Subject: [PATCH 14/26] docs: number Solid native renderer changeset for PR 9825 --- ...644-solid-native-renderer.md => 9825-solid-native-renderer.md} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename changelog.d/{4644-solid-native-renderer.md => 9825-solid-native-renderer.md} (100%) diff --git a/changelog.d/4644-solid-native-renderer.md b/changelog.d/9825-solid-native-renderer.md similarity index 100% rename from changelog.d/4644-solid-native-renderer.md rename to changelog.d/9825-solid-native-renderer.md From cc2fbba0900baea2c8c1af30cd19acc0a3628d9d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 6 Sep 2026 04:41:02 +0000 Subject: [PATCH 15/26] fix(gc): price the tiny-parse pressure guard by the productivity backoff Issue #9831 measured the ArenaBytes arm firing 51 times in one 66-delta claude-code reply, each collection freeing a median 131 KB, while the adaptive step sat saturated at 1 GiB. The issue located the discarded backoff in the arm's own re-arm arithmetic; correcting that (the issue's refuted branch) bought -10.8 % CPU for +22 % settled footprint and was rightly rejected. The arm's re-arm is not what re-fires it. Between two consecutive firings the arena grows a few hundred KB, against a trigger armed 16 MB (and below the ceiling, up to 128 MB) above the post-collection total. What pulls the trigger back down is the tiny-parse pressure guard: after every `JSON.parse` that grew the arena by <= 1 MB, `gc_bump_malloc_trigger` (and `gc_schedule_parse_boundary_collection_ if_pressure`, and the boundary collector they arm) tests the absolute `arena_in_use_bytes() >= 48 MB` and, if so, sets the trigger to "now". That threshold is a quantity no collection can lower below the live set, so on a program whose live set never drops under it every small parse -- one per SSE delta -- forced a minor at the next safepoint. The step those minors doubled was consulted by nothing. The guard now also requires the arena to have grown, since the last collection of any kind ended, by a headroom priced from the step: the step rescaled so that its power-on value (128 MB, the ceiling) buys the 16 MB floor, and each doubling the arm's ceiling clamp discards buys the guard one more doubling, bounded by the same ceiling. A productive collection halves the step and the guard keeps the cadence it always had; an unproductive one earns it room. The boundary collector re-prices a pending request so a collection that already satisfied it is not followed by a second one. Measured on the compiled claude-code TUI (cli_2.1.112.js, Linux, same perry binary, runtime-only A/B, 7 interleaved rounds, 3300-char streamed reply, chunk 50): turn CPU base 30.2-41.5 s (mean 35.1) fix 27.8-29.2 s (mean 28.6) post-turn RSS base 754-1057 MB (mean 803) fix 733-855 MB (mean 786) post-idle RSS base 527-1073 MB (mean 736) fix 517-843 MB (mean 722) peak RSS 1964-2062 MB both arms The fix wins CPU in every pair (-8 % to -30 %); footprint is flat within the base's own spread. The base arm is bimodal in both, which is what an absolute in-use threshold does. PERRY_GC_DIAG on one reply: copying minors 104 -> 84 (ArenaBytes 41 -> 13), old-gen fulls 19 -> 7, and the guard forced exactly one collection, after a genuine 16 MB of growth (`[gc-tiny-parse]` is the new witness line). test_memory_json_churn -- the guard's motivating shape -- is byte-identical in output and RSS in all four GC modes; 48/48 test_gap_gc_* and 8/8 test_gap_json_* pass. The arm's own arithmetic is left as it was and now says why. Claude-Session: https://claude.ai/code/session_015kqVkH6rHzfvXskGAj3tRv --- crates/perry-runtime/src/gc/policy.rs | 136 ++++++++++- crates/perry-runtime/src/gc/tests/mod.rs | 1 + .../src/gc/tests/tiny_parse_pressure.rs | 219 ++++++++++++++++++ scripts/gc_runtime_root_holders.json | 8 +- 4 files changed, 356 insertions(+), 8 deletions(-) create mode 100644 crates/perry-runtime/src/gc/tests/tiny_parse_pressure.rs diff --git a/crates/perry-runtime/src/gc/policy.rs b/crates/perry-runtime/src/gc/policy.rs index 73f4fc6636..0117ec509f 100644 --- a/crates/perry-runtime/src/gc/policy.rs +++ b/crates/perry-runtime/src/gc/policy.rs @@ -367,6 +367,92 @@ pub(super) fn gc_suppressed_parse_is_tiny(parse_growth: usize) -> bool { parse_growth <= GC_SUPPRESSED_TINY_PARSE_BYTES } +/// #9831: how much the arena must have grown since the last collection ended +/// before tiny-parse churn may force another one — priced by the part of the +/// productivity backoff the `ArenaBytes` arm computes and then discards. +/// +/// `GC_STEP_BYTES` powers on at `GC_THRESHOLD_INITIAL_BYTES`, which equals the +/// trigger ceiling, and doubles on every collection that frees almost nothing. +/// The arm's own re-arm (`gc_finish_arena_trigger_collection`) clamps +/// `new_total + step` at the ceiling, so every doubling past the initial step +/// is backoff the arm has computed, stored, and cannot express. This rescales +/// the step so that its power-on value buys exactly the headroom floor, and +/// each discarded doubling buys the guard one more doubling — up to the same +/// ceiling, so a run of unproductive collections can never let the guard wait +/// longer than the arm itself would. A productive collection halves the step +/// toward its 16 MB floor, which maps below the headroom floor and clamps to +/// it: a churn loop whose collections pay keeps today's cadence. +pub(super) fn tiny_parse_pressure_headroom_bytes(step: usize) -> usize { + let floor = gc_trigger_headroom_floor_bytes(); + let ceiling = gc_trigger_absolute_ceiling_bytes(); + // 64-bit intermediate on purpose: `step` reaches 1 GiB and `floor` 16 MiB, + // whose product does not fit a 32-bit `usize` (watchOS/visionOS are ILP32; + // see `influx_driven_nursery_cap_bytes` for the same trap). + let scaled = (step as u64).saturating_mul(floor as u64) / (GC_THRESHOLD_INITIAL_BYTES as u64); + let scaled = scaled.min(usize::MAX as u64) as usize; + scaled.max(floor).min(ceiling.max(floor)) +} + +/// #9831: whether tiny-parse churn has earned a forced collection. +/// +/// The guard used to be `in_use >= in_use_trigger` alone — an absolute +/// threshold on a quantity no collection can lower below the live set. On a +/// program whose live set sits above it permanently (the compiled claude-code +/// TUI holds 59–297 MB of arena through one streamed reply) that made every +/// tiny `JSON.parse` the SSE stream consists of force a collection at the next +/// safepoint: 51 minors in one 66-delta reply, each freeing a median 131 KB, +/// while the adaptive step sat at its 1 GiB maximum saying "back off" and +/// nothing consulted it. The growth clause is what that step is for. +/// +/// `base` is `arena_in_use_bytes()` as the last collection ended +/// (`GC_TINY_PARSE_PRESSURE_BASE_BYTES`); `in_use` is the same reading now. +pub(super) fn tiny_parse_pressure_due_with( + in_use: usize, + in_use_trigger: usize, + base: usize, + step: usize, +) -> bool { + in_use >= in_use_trigger + && in_use >= base.saturating_add(tiny_parse_pressure_headroom_bytes(step)) +} + +/// The live [`tiny_parse_pressure_due_with`]: current base and step. +pub(super) fn tiny_parse_pressure_due(in_use: usize, in_use_trigger: usize) -> bool { + let base = GC_TINY_PARSE_PRESSURE_BASE_BYTES.with(Cell::get); + let step = GC_STEP_BYTES.with(Cell::get); + tiny_parse_pressure_due_with(in_use, in_use_trigger, base, step) +} + +/// The in-use reading the tiny-parse guard compares against in this collector +/// mode: the generational collector's guard sits higher than the full +/// mark-sweep one because a minor is the cheaper collection to force. +fn tiny_parse_in_use_trigger_for_mode() -> usize { + if gen_gc_enabled() { + gc_tiny_parse_in_use_trigger_dyn_bytes() + } else { + gc_tiny_parse_full_gc_in_use_trigger_dyn_bytes() + } +} + +/// `PERRY_GC_DIAG=1` witness that the guard is the arm that forced a +/// collection (CLAUDE.md: a gate must assert its subject was live). One line +/// per forced collection, tagged with which parse boundary asked for it. +fn diag_tiny_parse_forced_collection(site: &str, in_use: usize) { + if !crate::gc::gc_diag_enabled() { + return; + } + let base = GC_TINY_PARSE_PRESSURE_BASE_BYTES.with(Cell::get); + let step = GC_STEP_BYTES.with(Cell::get); + eprintln!( + "[gc-tiny-parse] forced collection site={} in_use={} base={} headroom={} step={}", + site, + in_use, + base, + tiny_parse_pressure_headroom_bytes(step), + step + ); +} + pub(super) fn gc_bump_arena_trigger_target( bytes_now: usize, step: usize, @@ -980,6 +1066,12 @@ crate::perry_thread_local! { /// reading and still escalate. Nursery garbage is not. pub(super) static GC_LAST_COLLECTION_POST_IN_USE_BYTES: Cell = const { Cell::new(0) }; + /// #9831: `arena_in_use_bytes()` as the most recent collection of ANY kind + /// ended — the base the tiny-parse pressure guard measures growth from. + /// The bump-offset reading rather than the live census above, because the + /// guard compares against `arena_in_use_bytes()` at every parse boundary, + /// and mixing the two would count every swept hole as growth. + pub(super) static GC_TINY_PARSE_PRESSURE_BASE_BYTES: Cell = const { Cell::new(0) }; /// Yield-adaptive backoff for major-GC pacing (#7726). /// /// `arena_growth_full_escalation_due` escalates a minor to a full once the @@ -1347,12 +1439,12 @@ pub fn gc_bump_malloc_trigger() { let is_tiny_parse = gc_bump_malloc_trigger_with_snapshot(current, bytes_now); if is_tiny_parse { let use_gen_gc = gen_gc_enabled(); - let in_use_trigger = if use_gen_gc { - gc_tiny_parse_in_use_trigger_dyn_bytes() - } else { - gc_tiny_parse_full_gc_in_use_trigger_dyn_bytes() - }; - if crate::arena::arena_in_use_bytes() < in_use_trigger { + // #9831: an absolute in-use guard alone forced a collection after + // EVERY tiny parse on a program whose live set never drops below it. + // The guard now also requires the arena to have grown past the + // productivity-priced headroom since the last collection ended. + let in_use = crate::arena::arena_in_use_bytes(); + if !tiny_parse_pressure_due(in_use, tiny_parse_in_use_trigger_for_mode()) { return; } if use_gen_gc { @@ -1361,6 +1453,7 @@ pub fn gc_bump_malloc_trigger() { return; } GC_SUPPRESSED_TINY_PARSE_COLLECTION_PENDING.with(|pending| pending.set(true)); + diag_tiny_parse_forced_collection("parse_end", in_use); GC_NEXT_TRIGGER_BYTES.with(|trigger| { if trigger.get() > bytes_now { trigger.set(bytes_now); @@ -1396,6 +1489,15 @@ pub fn gc_collect_pending_suppressed_parse() { GC_SUPPRESSED_TINY_PARSE_COLLECTION_PENDING.with(|pending| pending.set(true)); return; } + // #9831: the request was priced when it was made; a collection of any kind + // since then has moved the base, and re-pricing here is what keeps the + // boundary collection from stacking a second minor on top of it. A request + // nothing has satisfied is still due and still collects. + let in_use = crate::arena::arena_in_use_bytes(); + if !tiny_parse_pressure_due(in_use, tiny_parse_in_use_trigger_for_mode()) { + return; + } + diag_tiny_parse_forced_collection("parse_boundary", in_use); let total = crate::arena::arena_total_bytes(); GC_NEXT_TRIGGER_BYTES.with(|trigger| { @@ -1419,7 +1521,12 @@ pub fn gc_schedule_parse_boundary_collection_if_pressure() { if !gen_gc_enabled() { return; } - if crate::arena::arena_in_use_bytes() < gc_tiny_parse_in_use_trigger_dyn_bytes() { + // #9831: priced the same way as the post-parse guard above — see + // `tiny_parse_pressure_due_with`. + if !tiny_parse_pressure_due( + crate::arena::arena_in_use_bytes(), + gc_tiny_parse_in_use_trigger_dyn_bytes(), + ) { return; } GC_SUPPRESSED_TINY_PARSE_COLLECTION_PENDING.with(|pending| pending.set(true)); @@ -1911,6 +2018,8 @@ pub(super) fn pacing_arena_in_use_bytes() -> usize { pub(super) fn note_collection_finished_arena_occupancy(full: bool) { let bytes = pacing_arena_in_use_bytes(); GC_LAST_COLLECTION_POST_IN_USE_BYTES.with(|cell| cell.set(bytes)); + // #9831: the same moment, in the units the tiny-parse guard reads. + GC_TINY_PARSE_PRESSURE_BASE_BYTES.with(|cell| cell.set(crate::arena::arena_in_use_bytes())); super::arena_right_size::note_collection_finished(bytes, full); } @@ -2199,6 +2308,19 @@ fn gc_finish_arena_trigger_collection(pre_in_use: usize, outcome: GcCollectOutco // `new_total` so a workload whose post-GC live set already // approaches the ceiling doesn't thrash on every fresh // allocation. + // + // #9831: above `ceiling - floor` this arithmetic re-arms at `new_total + + // floor` whatever `step` says — the productivity backoff the branch above + // just computed is not visible to it. That is deliberate, and measured: + // pricing the arm's own headroom by the step bought -10.8 % turn CPU on + // the compiled claude-code TUI and cost +22 % settled footprint — a + // CPU-for-footprint trade this project does not accept (the issue's own + // refuted branch). The step is instead consumed where it was + // actually being discarded — the tiny-parse pressure guard + // (`tiny_parse_pressure_due_with`), which pulled this trigger down to + // "now" after every small `JSON.parse` on a heap above its in-use + // threshold and so re-fired this arm 51 times in one reply regardless of + // what this function armed. let stepped = new_total.saturating_add(step); let capped = stepped.min(gc_trigger_absolute_ceiling_bytes()); let floor = new_total.saturating_add(gc_trigger_headroom_floor_bytes()); diff --git a/crates/perry-runtime/src/gc/tests/mod.rs b/crates/perry-runtime/src/gc/tests/mod.rs index bac771f752..b181eb1838 100644 --- a/crates/perry-runtime/src/gc/tests/mod.rs +++ b/crates/perry-runtime/src/gc/tests/mod.rs @@ -62,6 +62,7 @@ mod survival_diag; mod teardown; mod telemetry_verifier; mod temp_roots; +mod tiny_parse_pressure; mod tls_fill_reentrancy; mod trigger_path_tls; mod triggers; diff --git a/crates/perry-runtime/src/gc/tests/tiny_parse_pressure.rs b/crates/perry-runtime/src/gc/tests/tiny_parse_pressure.rs new file mode 100644 index 0000000000..fd4be0507d --- /dev/null +++ b/crates/perry-runtime/src/gc/tests/tiny_parse_pressure.rs @@ -0,0 +1,219 @@ +//! #9831: the tiny-parse pressure guard prices the collections it forces. +//! +//! The guard used to be an absolute `arena_in_use_bytes() >= 48 MB` test, so +//! on a program whose live set never drops below that it forced a collection +//! after EVERY tiny `JSON.parse` — the adaptive step's backoff was computed +//! by each of those collections and consulted by none of them. These tests +//! pin the pricing: the headroom the guard demands between collections is the +//! part of the step the `ArenaBytes` arm's ceiling clamp discards, and the +//! guard is not due until the arena has grown that much since the last +//! collection ended. +//! +//! Sabotage-proved: restoring the absolute guard (dropping the growth clause +//! from `tiny_parse_pressure_due_with`) fails +//! `the_absolute_guard_alone_is_the_bug_the_growth_clause_exists_for` and +//! `growth_past_the_headroom_is_due`'s boundary half while the rest pass; +//! pricing the headroom at the raw step (`floor.max(step.min(ceiling))`) +//! fails `power_on_step_buys_exactly_the_headroom_floor`. + +use super::super::heap_budget::{ + gc_trigger_absolute_ceiling_bytes, gc_trigger_headroom_floor_bytes, +}; +use super::super::policy::{ + tiny_parse_pressure_due, tiny_parse_pressure_due_with, tiny_parse_pressure_headroom_bytes, + GC_STEP_BYTES, GC_THRESHOLD_INITIAL_BYTES, GC_THRESHOLD_MAX_BYTES, + GC_TINY_PARSE_PRESSURE_BASE_BYTES, +}; + +const MB: usize = 1024 * 1024; + +/// Restores the two live cells the guard reads, so a test that moves them +/// cannot leak its state into the next one (the suite is single-threaded, but +/// the cells outlive the test). +struct LiveCellsGuard { + step: usize, + base: usize, +} + +impl LiveCellsGuard { + fn set(step: usize, base: usize) -> Self { + Self { + step: GC_STEP_BYTES.with(|cell| cell.replace(step)), + base: GC_TINY_PARSE_PRESSURE_BASE_BYTES.with(|cell| cell.replace(base)), + } + } +} + +impl Drop for LiveCellsGuard { + fn drop(&mut self) { + GC_STEP_BYTES.with(|cell| cell.set(self.step)); + GC_TINY_PARSE_PRESSURE_BASE_BYTES.with(|cell| cell.set(self.base)); + } +} + +#[test] +fn power_on_step_buys_exactly_the_headroom_floor() { + // The step powers on at the trigger ceiling. That is not evidence of an + // unproductive collection — nothing has run yet — so the guard keeps the + // cadence it always had: the headroom floor, not the ceiling. + assert_eq!( + tiny_parse_pressure_headroom_bytes(GC_THRESHOLD_INITIAL_BYTES), + gc_trigger_headroom_floor_bytes(), + "a step that has never been priced must buy the floor, not the ceiling" + ); +} + +#[test] +fn a_productive_collection_keeps_the_headroom_floor() { + // Productive collections halve the step toward its own 16 MB floor. Every + // step at or below the power-on value maps to the headroom floor. + let floor = gc_trigger_headroom_floor_bytes(); + for step in [16 * MB, 32 * MB, 64 * MB, GC_THRESHOLD_INITIAL_BYTES / 2] { + assert_eq!( + tiny_parse_pressure_headroom_bytes(step), + floor, + "step {step} is a productive reading and must keep the floor" + ); + } +} + +#[test] +fn each_doubling_the_ceiling_discards_doubles_the_headroom() { + let floor = gc_trigger_headroom_floor_bytes(); + let ceiling = gc_trigger_absolute_ceiling_bytes(); + let mut previous = tiny_parse_pressure_headroom_bytes(GC_THRESHOLD_INITIAL_BYTES); + for doublings in 1..=3u32 { + let step = GC_THRESHOLD_INITIAL_BYTES << doublings; + let headroom = tiny_parse_pressure_headroom_bytes(step); + let expected = (floor << doublings).min(ceiling); + assert_eq!( + headroom, expected, + "{doublings} discarded doubling(s) must buy floor << {doublings}, bounded by the ceiling" + ); + assert!( + headroom >= previous, + "backing off further must never shrink the headroom" + ); + if ceiling >= (floor << doublings) { + assert!( + headroom > previous, + "an unproductive collection must earn more headroom than the reading before it" + ); + } + previous = headroom; + } +} + +#[test] +fn headroom_is_bounded_by_the_absolute_ceiling() { + let ceiling = gc_trigger_absolute_ceiling_bytes(); + let saturated = tiny_parse_pressure_headroom_bytes(GC_THRESHOLD_MAX_BYTES); + assert!( + saturated <= ceiling, + "a saturated step ({saturated}) must not let the guard wait longer than the arm's ceiling ({ceiling})" + ); + // On the unconstrained desktop budget the saturated step (1 GiB, three + // doublings past the 128 MB initial) reaches the 128 MB ceiling exactly; + // under a small `PERRY_GC_HEAP_LIMIT` the ceiling is lower and the clamp + // binds earlier. Either way the saturated reading IS the ceiling. + if ceiling <= gc_trigger_headroom_floor_bytes() << 3 { + assert_eq!(saturated, ceiling); + } +} + +#[test] +fn below_the_in_use_trigger_is_never_due() { + let trigger = 48 * MB; + // Even with zero base and the most productive step, the guard stays off + // below its in-use trigger: small heaps are the regular arms' business. + assert!(!tiny_parse_pressure_due_with( + trigger - 1, + trigger, + 0, + 16 * MB + )); + assert!(!tiny_parse_pressure_due_with(0, trigger, 0, 16 * MB)); +} + +#[test] +fn the_absolute_guard_alone_is_the_bug_the_growth_clause_exists_for() { + // The measured shape: a 60 MB live set above the 48 MB trigger, a tiny + // parse that grew the arena by a few KB since the collection that just + // ran, and a step saturated at its maximum because those collections free + // nothing. The old guard said "collect" here after every parse. + let trigger = 48 * MB; + let base = 60 * MB; + let in_use = base + 4096; + assert!( + !tiny_parse_pressure_due_with(in_use, trigger, base, GC_THRESHOLD_MAX_BYTES), + "a few KB of growth past a collection that freed nothing must not force another" + ); + // Nor with a productive step: 4 KB is below the headroom floor too. + assert!(!tiny_parse_pressure_due_with( + in_use, + trigger, + base, + 16 * MB + )); +} + +#[test] +fn growth_past_the_headroom_is_due() { + let trigger = 48 * MB; + let base = 60 * MB; + for step in [16 * MB, GC_THRESHOLD_INITIAL_BYTES, GC_THRESHOLD_MAX_BYTES] { + let headroom = tiny_parse_pressure_headroom_bytes(step); + let boundary = base + headroom; + assert!( + tiny_parse_pressure_due_with(boundary, trigger, base, step), + "growth of exactly the headroom ({headroom}) at step {step} is due" + ); + assert!( + !tiny_parse_pressure_due_with(boundary - 1, trigger, base, step), + "one byte short of the headroom ({headroom}) at step {step} is not" + ); + } +} + +#[test] +fn the_live_predicate_reads_the_step_and_the_base() { + let trigger = 48 * MB; + let base = 60 * MB; + let floor = gc_trigger_headroom_floor_bytes(); + let ceiling = gc_trigger_absolute_ceiling_bytes(); + + // Saturated step: the guard waits for the ceiling's worth of growth. + let _cells = LiveCellsGuard::set(GC_THRESHOLD_MAX_BYTES, base); + if ceiling > floor { + assert!(!tiny_parse_pressure_due(base + floor, trigger)); + } + assert!(tiny_parse_pressure_due(base + ceiling.max(floor), trigger)); + + // Productive step: the floor is enough again. + GC_STEP_BYTES.with(|cell| cell.set(16 * MB)); + assert!(tiny_parse_pressure_due(base + floor, trigger)); + assert!(!tiny_parse_pressure_due(base + floor - 1, trigger)); +} + +#[test] +fn a_finished_collection_moves_the_base_to_the_post_collection_reading() { + use super::super::js_gc_collect; + // Whatever the base was, a completed collection re-baselines it to the + // arena's post-collection in-use reading — the same reading the guard + // compares against at the next parse boundary. Assert the identity of the + // two readings, not merely that the cell moved: a base recorded in other + // units (the live census) would count every swept hole as growth. + let _cells = LiveCellsGuard::set(GC_STEP_BYTES.with(|cell| cell.get()), usize::MAX); + js_gc_collect(); + let base = GC_TINY_PARSE_PRESSURE_BASE_BYTES.with(|cell| cell.get()); + assert_ne!( + base, + usize::MAX, + "a finished collection must record the base" + ); + assert_eq!( + base, + crate::arena::arena_in_use_bytes(), + "the base must be the post-collection `arena_in_use_bytes()` reading" + ); +} diff --git a/scripts/gc_runtime_root_holders.json b/scripts/gc_runtime_root_holders.json index 237b198a2c..9f1488bd20 100644 --- a/scripts/gc_runtime_root_holders.json +++ b/scripts/gc_runtime_root_holders.json @@ -294,7 +294,7 @@ "crates/perry-runtime/src/gc/census.rs": "388414f9629f196e84673e91bebd04bdcdcabdaa180252d2dfe4b82d1b49ca5a", "crates/perry-runtime/src/gc/cycle.rs": "2e2f5adca2229f74409e01a1cb571e2147cd8a33f58d0976711fce98d4777309", "crates/perry-runtime/src/gc/mod.rs": "7dd42b9506a97e6844fd3225dc53dfd59512631784750f58ff72208d68595481", - "crates/perry-runtime/src/gc/policy.rs": "2c49102ee846cae79bcb96a95a60b8804ac66a86617db8adb24859288e3cda30", + "crates/perry-runtime/src/gc/policy.rs": "71ac3a7651e61f553680eab523662a2c66cbb1bdbc3871a47d822c553a28ad80", "crates/perry-runtime/src/gc/progress.rs": "a5ad3971bbe4047229ca57325234780daa85921dbc778e1c08dff4ad07ccfb96" } } @@ -353,6 +353,12 @@ "verdict": "not_a_gc_pointer", "why": "#9772: releasable block BYTES the last idle selection promised \u2014 a size, not an address. A `Cell` compared against what the collection actually released." }, + { + "file": "crates/perry-runtime/src/gc/policy.rs", + "name": "GC_TINY_PARSE_PRESSURE_BASE_BYTES", + "verdict": "not_a_gc_pointer", + "why": "#9831: `arena_in_use_bytes()` (a sum of block bump offsets) recorded as each collection ends, read back by the tiny-parse pressure guard to price growth since then. A byte COUNT, never an address or a NaN-boxed value: written only from `note_collection_finished_arena_occupancy` and the test seam, read only by `tiny_parse_pressure_due`/`diag_tiny_parse_forced_collection`. Same shape as its frontier siblings `GC_LAST_COLLECTION_POST_IN_USE_BYTES`/`GC_STEP_BYTES`, with the verdict those still owe." + }, { "file": "crates/perry-runtime/src/gc/survival_diag.rs", "name": "MINOR_SEQ", From ec45e0385bba38cfcb1b6b0bfc160e72cf1f0b5b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 6 Sep 2026 04:42:35 +0000 Subject: [PATCH 16/26] docs(changelog): add the #9838 fragment Claude-Session: https://claude.ai/code/session_015kqVkH6rHzfvXskGAj3tRv --- .../9838-tiny-parse-pressure-pricing.md | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 changelog.d/9838-tiny-parse-pressure-pricing.md diff --git a/changelog.d/9838-tiny-parse-pressure-pricing.md b/changelog.d/9838-tiny-parse-pressure-pricing.md new file mode 100644 index 0000000000..dcd96c9b53 --- /dev/null +++ b/changelog.d/9838-tiny-parse-pressure-pricing.md @@ -0,0 +1,37 @@ +**The tiny-parse pressure guard now prices the collections it forces by the +adaptive step's productivity backoff (#9831).** On the compiled claude-code +TUI a 3300-character streamed reply spent 30–41 s of CPU in the base arm and +27.8–29.2 s with the fix (mean −19 %, every interleaved pair a win), with +post-turn and post-idle RSS flat within the base's own spread and peak RSS +unchanged. + +#9831 measured the `ArenaBytes` arm firing 51 times in one 66-delta reply, +each collection freeing a median 131 KB, while the adaptive step sat +saturated at 1 GiB — and located the discarded backoff in the arm's own +ceiling clamp. That clamp was not what re-fired the arm: between two firings +the arena grew a few hundred KB against a trigger armed 16–128 MB above the +post-collection total. What pulled the trigger down was the tiny-parse +pressure guard, which after every `JSON.parse` growing the arena by ≤ 1 MB +tested the absolute `arena_in_use_bytes() >= 48 MB` and, if it held, set the +trigger to "now". That is a quantity no collection can lower below the live +set, so on a heap that sits above it permanently every small parse (one per +SSE delta) forced a minor whose backoff nothing read — #9589's shape one +trigger over. + +The guard now also requires the arena to have grown, since the last +collection of any kind ended, by a headroom priced from the step: the step +rescaled so its power-on value buys the 16 MB headroom floor and each +doubling the arm's clamp discards buys one more doubling, bounded by the +trigger ceiling. A productive collection keeps today's cadence; an +unproductive one earns room. The parse-boundary collector re-prices a +pending request so a collection that already satisfied it is not followed by +a second. `PERRY_GC_DIAG=1` gains a `[gc-tiny-parse] forced collection …` +witness line. The arm's own arithmetic is unchanged and now documents why +(pricing it directly was measured at −10.8 % CPU for +22 % footprint, the +issue's refuted branch). + +Validation: `test_memory_json_churn.ts` (the guard's motivating shape) is +byte-identical in output and RSS in all four GC modes; 48/48 `test_gap_gc_*` +and 8/8 `test_gap_json_*` pass; nine new `gc::tests::tiny_parse_pressure` +tests pin the pricing and the predicate, sabotage-proved against both the +old absolute guard and a raw-step pricing. From 6b4215eb7f4628163e30c062fe9436e96fea74ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 6 Sep 2026 05:52:25 +0000 Subject: [PATCH 17/26] docs(changelog): do not open a line of the #9838 fragment with a bare issue reference --- changelog.d/9838-tiny-parse-pressure-pricing.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog.d/9838-tiny-parse-pressure-pricing.md b/changelog.d/9838-tiny-parse-pressure-pricing.md index dcd96c9b53..1c24003c62 100644 --- a/changelog.d/9838-tiny-parse-pressure-pricing.md +++ b/changelog.d/9838-tiny-parse-pressure-pricing.md @@ -5,7 +5,7 @@ TUI a 3300-character streamed reply spent 30–41 s of CPU in the base arm and post-turn and post-idle RSS flat within the base's own spread and peak RSS unchanged. -#9831 measured the `ArenaBytes` arm firing 51 times in one 66-delta reply, +Issue #9831 measured the `ArenaBytes` arm firing 51 times in one 66-delta reply, each collection freeing a median 131 KB, while the adaptive step sat saturated at 1 GiB — and located the discarded backoff in the arm's own ceiling clamp. That clamp was not what re-fired the arm: between two firings From 3ac293b05c0c22e0622b3337c3cb14caef200c54 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 6 Sep 2026 07:38:37 +0200 Subject: [PATCH 18/26] fix(gc): re-baseline the whole-arena trigger after a malloc-pressure minor (#9840) `GC_NEXT_TRIGGER_BYTES` is documented as "bumped after each `gc_collect_inner` based on collection effectiveness". It was not. `gc_finish_arena_trigger_collection` re-baselined it; the finisher for the SAME nursery collection with the malloc sweep added, `gc_finish_malloc_trigger_collection`, did not. So the whole-arena threshold was measured from the last ARENA-KIND collection rather than from the last collection, and a run of `MallocCount` minors could walk the arena total across a threshold nothing had refreshed. The asymmetry predates the budgeted split (9d3bd2e3b's pre-split `gc_check_trigger` had the same two branches). It is justified in ONE direction only -- an arena minor may legitimately skip the malloc sweep, so it must not move the malloc trigger -- and that direction is unchanged and still pinned by `test_gc_check_trigger_copied_minor_without_malloc_sweep_preserves_malloc_trigger`. A `MallocCount` minor has no such exemption: it swept the arena. The threshold re-baseline is factored out of `gc_finish_arena_trigger_collection` into `gc_rebaseline_arena_trigger_after_collection` and called from both nursery finishers, with `pre_in_use` captured for `MallocCount` cycles on all three paths that reach one (alloc-point direct, moving safepoint, budgeted). The base stays `arena_total_bytes()` -- COMMITTED bytes -- because that is what `next_arena_trigger_base()` is compared against. It is deliberately neither of the two occupancy readings #9831 publishes at `note_collection_finished_arena_occupancy`, whose doc comment now tabulates all three quantities and their units, since two of them share that funnel and a re-baseline from a bump-offset or live-census base would arm this trigger below the arena's own total. `OldReclaim` and the idle reclaim stay out: after a full that released blocks the un-moved trigger sits FURTHER above the new total, which is the conservative direction, and a full's cadence belongs to the old-generation band. A nursery-trigger cycle that `arena_growth_full_escalation_ due()` escalated to a full still finishes here, exactly as the arena arm's escalated fulls already did. Measured on the compiled claude-code TUI (PERRY_GC_DIAG=1, per firing, four 3300-character captures across two independently built binaries): the streaming turn ran a strict 6:1 pattern -- six `MallocCount` minors promoting ~3.2 MB each crossed the stale threshold inside the sixth minor, and at the very next safepoint the `ArenaBytes` arm fired on a nursery of 856 bytes (`promoted_bytes=216 freed_bytes=640`), paying the whole per-collection fixed cost to free 640 bytes. Eight of ~60 collections per 3300-character turn. Length is part of every figure: the shape needs a run of promoting `MallocCount` minors, and the 400-character capture has 48-62 fewer of them than the 3300-character ones -- it has ZERO. So this change is predicted flat at 400 on every counter, and that holds with or without the in-flight change moving `RegExpHeader`s (the arm's only measured input on this program) to the nursery. One coupling is stated because it touches a fix that landed hours earlier: `GC_STEP_BYTES` had exactly one production writer -- the arena finisher -- and #9831 made it an INPUT to the tiny-parse pressure guard's headroom, so scoring a `MallocCount` minor's productivity moves that guard too. That is the same symmetry rather than a side effect (the step is documented as "collection effectiveness", not "arena-kind collection effectiveness"). Estimated over 209 `MallocCount` firings in the same captures, `pct_freed` has a median of 4-5 % and lands <10 % in 194 cases, 10-24 % in 10, 25-84 % in 5 and >84 % in none: 93 % take the "<10 % -> double" band and push the step UP, so on this program the coupling makes #9831's guard MORE conservative, not less. The arm's dueness predicate is byte-identical, so when it is due it fires the same collection. `PERRY_GC_ARENA_REBASELINE_ALL=0` restores the old asymmetry; its OFF state is asserted in CI as the GC knob kill-policy requires, by a test that is simultaneously the sabotage proof for the two ON-state tests -- the OFF branch IS the deleted call, so the proof runs in CI instead of being performed by hand and lost. PERRY_GC_DIAG=1 gains `[gc-arena-rebaseline] arm=... next_trigger=... total=... headroom=... pct=... step=...`, whose field names are disjoint from the reclaim keys `scripts/gc_repsel_matrix.sh` sums; `[gc-step]` stays ArenaBytes-only for that same reason, so the ratchet's reclaim total does not gain an addend from a change that reclaims nothing new. Tests: `direct_malloc_minor_also_rebaselines_the_whole_arena_trigger` (direct synchronous arm), `test_budgeted_malloc_minor_rebaselines_the_whole_arena_trigger` (budgeted arm, the one cc takes), and `direct_malloc_minor_arena_rebaseline_kill_switch_restores_the_stale_threshold` (the OFF state, and the sabotage proof). --- .../9840-arena-trigger-rebaseline-symmetry.md | 76 ++++++ crates/perry-runtime/src/gc/policy.rs | 247 +++++++++++++++++- .../gc/tests/copying/survival_and_malloc.rs | 126 +++++++++ .../perry-runtime/src/gc/tests/debt_pacer.rs | 222 ++++++++++++++++ 4 files changed, 661 insertions(+), 10 deletions(-) create mode 100644 changelog.d/9840-arena-trigger-rebaseline-symmetry.md diff --git a/changelog.d/9840-arena-trigger-rebaseline-symmetry.md b/changelog.d/9840-arena-trigger-rebaseline-symmetry.md new file mode 100644 index 0000000000..1bed389cdf --- /dev/null +++ b/changelog.d/9840-arena-trigger-rebaseline-symmetry.md @@ -0,0 +1,76 @@ +### Fixed + +- **A nursery collection triggered by malloc pressure now re-baselines the + whole-arena GC trigger, as `GC_NEXT_TRIGGER_BYTES`'s own contract already + said it did.** The cell is documented as "bumped after each + `gc_collect_inner` based on collection effectiveness". It was not: + `gc_finish_arena_trigger_collection` re-baselined it, and + `gc_finish_malloc_trigger_collection` — the finisher for the *same nursery + collection* with the malloc sweep added — did not. So the whole-arena + threshold was measured from the last **arena-kind** collection rather than + from the last collection, and a run of `MallocCount` minors could walk the + arena total across a threshold nothing had refreshed. + + The asymmetry predates the budgeted split (`9d3bd2e3b`'s pre-split + `gc_check_trigger` had the same two branches) and is correct in the *other* + direction, which is unchanged and still pinned by + `test_gc_check_trigger_copied_minor_without_malloc_sweep_preserves_malloc_trigger`: + an arena minor that skips the malloc sweep must not move the malloc trigger. + A `MallocCount` minor has no such exemption — it swept the arena. + + Measured on the perry-compiled claude-code TUI (`PERRY_GC_DIAG=1`, per + firing, four 3300-character captures across two independently built + binaries): the streaming turn ran a strict 6:1 pattern in which six + `MallocCount` minors promoting ~3.2 MB each crossed the stale arena + threshold *inside the sixth minor*, and at the very next safepoint the arena + arm fired on a nursery of **856 bytes** (`promoted_bytes=216 + freed_bytes=640`) — one collection in seven, paying the whole + per-collection fixed cost (root scan, side-table prune, dirty-page restore) + to free 640 bytes. + + **The length matters and every figure here states it.** Shape (b) needs a + run of promoting `MallocCount` minors to walk the total across the stale + threshold, so it exists on the long reply only: the 3300-character captures + run 48–62 `MallocCount` firings each, and the 400-character capture runs + **zero** (its collections are all `ArenaBytes` plus a handful of + `OldGenBytes`). At 400 characters this change is therefore expected to be + flat on every counter, and that is a prediction rather than a hope — there is + no producer for the shape at that length, with or without the in-flight + change that moves `RegExpHeader`s (the arm's only measured input on this + program) to the nursery. + + Full collections are deliberately excluded: after a full that released + blocks, the un-moved trigger sits *further* above the new total, which is the + conservative direction, and a full's cadence belongs to the old-generation + band rather than to this arm. + + This is the same symmetry #9831 gave the tiny-parse pressure guard's base + cell, applied to the one pacing quantity still keyed to a single collection + kind. The two cells are in different units on purpose and stay that way: the + guard's base is `arena_in_use_bytes()` (bump offsets, what it reads at each + parse boundary); this trigger's base is `arena_total_bytes()` (committed), + which is what `next_arena_trigger_base()` is compared against. + + One coupling beyond the trigger, stated because it touches a fix that landed + hours earlier: `GC_STEP_BYTES` had exactly one production writer — the arena + finisher — and #9831 made it an *input* to the tiny-parse pressure guard's + headroom. Scoring a `MallocCount` minor's productivity therefore moves that + guard too. That is the same symmetry rather than a side effect (the step is + documented as "collection effectiveness", not "arena-kind collection + effectiveness"), and on the compiled claude-code TUI it moves the guard in + the *conservative* direction. Estimated over 209 `MallocCount` firings in the + four 3300-character captures, `pct_freed` has a median of 4–5 % and lands + `<10 %` in 194 cases, `10–24 %` in 10, `25–84 %` in 5 and `>84 %` in none — + so 93 % of these collections take the "< 10 % → double" band and push the + step up, which raises the guard's headroom. `[gc-arena-rebaseline]` carries + `pct=` and `step=` for both arms so this is read off a capture rather than + estimated from a neighbouring diagnostic, which is all the pre-fix diag + allowed. + + The arm's dueness predicate is byte-identical, so when it is due it still + fires the same collection. `PERRY_GC_ARENA_REBASELINE_ALL=0` restores the old + asymmetry — its OFF state is asserted in CI as the knob kill-policy requires, + by a test that is simultaneously the sabotage proof for the two ON-state + tests (the OFF branch *is* the deleted call) — and `PERRY_GC_DIAG=1` gains a + `[gc-arena-rebaseline] arm=…` line attributing each re-baseline to the + finisher that performed it. diff --git a/crates/perry-runtime/src/gc/policy.rs b/crates/perry-runtime/src/gc/policy.rs index 0117ec509f..fed40c7b31 100644 --- a/crates/perry-runtime/src/gc/policy.rs +++ b/crates/perry-runtime/src/gc/policy.rs @@ -2015,6 +2015,29 @@ pub(super) fn pacing_arena_in_use_bytes() -> usize { /// Called once at the end of every cycle, minor and full alike. The copying /// fast path publishes directly; non-copying cycles publish from /// `GcCycle::publish_reclaim_outcome` after their sweep census. +/// +/// **Three post-collection quantities, three units — do not conflate them.** +/// This funnel is the one place a reader can see all three at once, which is +/// why the list lives here: +/// +/// | cell | unit | read by | +/// |---|---|---| +/// | `GC_LAST_COLLECTION_POST_IN_USE_BYTES` | `pacing_arena_in_use_bytes()` — the LIVE census (`arena_live_allocated_bytes`), test-injectable | `arena_growth_full_escalation_due` | +/// | `GC_TINY_PARSE_PRESSURE_BASE_BYTES` (#9831) | `arena_in_use_bytes()` — BUMP OFFSETS, the same reading the guard takes at each parse boundary | `tiny_parse_pressure_due_with` | +/// | `GC_NEXT_TRIGGER_BYTES` (#9840) | `arena_total_bytes()` — COMMITTED bytes, which is what `next_arena_trigger_base()` is compared against | `gc_budgeted_due_trigger`'s `ArenaBytes` arm | +/// +/// The third is *not* written here, and that is deliberate rather than an +/// omission: re-arming the arena trigger needs the collection's productivity +/// score (`outcome.freed_bytes` and `pre_in_use`, which price `GC_STEP_BYTES`) +/// and the once-consumed `take_promoted_young_capacity_credit()`, neither of +/// which exists at this point in a cycle — and it must skip fulls, which this +/// funnel deliberately does not. It is written by +/// [`gc_rebaseline_arena_trigger_after_collection`], which both nursery- +/// collection finishers call; #9840 is the change that made that "both" +/// true, and it is the same symmetry #9831 applied to the cell above. +/// Mixing the units — re-baselining a committed-bytes trigger from a bump- +/// offset or live-census base — would arm it below the arena's own total and +/// make the arm due the instant it re-armed. pub(super) fn note_collection_finished_arena_occupancy(full: bool) { let bytes = pacing_arena_in_use_bytes(); GC_LAST_COLLECTION_POST_IN_USE_BYTES.with(|cell| cell.set(bytes)); @@ -2198,9 +2221,71 @@ fn gc_rebaseline_malloc_trigger_to_survivors(mstep: usize) { GC_NEXT_MALLOC_TRIGGER.with(|c| c.set(survivors + mstep)); } -fn gc_finish_arena_trigger_collection(pre_in_use: usize, outcome: GcCollectOutcome) -> u64 { +/// Which nursery-collection finisher is re-baselining the whole-arena trigger. +/// Diagnostic attribution only — the arithmetic is identical for both. +#[derive(Clone, Copy)] +enum ArenaRebaselineArm { + ArenaBytes, + MallocCount, +} + +impl ArenaRebaselineArm { + fn label(self) -> &'static str { + match self { + Self::ArenaBytes => "arena", + Self::MallocCount => "malloc", + } + } +} + +/// Re-baseline `GC_NEXT_TRIGGER_BYTES` (and adapt `GC_STEP_BYTES`) from the +/// state a nursery collection leaves behind. +/// +/// Shared by BOTH nursery-collection finishers. `GC_NEXT_TRIGGER_BYTES`'s doc +/// says it is bumped "after each `gc_collect_inner`", and until #9840 that was +/// false: only the `ArenaBytes` finisher moved it, so the whole-arena trigger +/// was measured from the last *arena-kind* collection rather than the last +/// collection. A `MallocCount` minor is the same nursery collection with the +/// malloc sweep added — the arena was swept — so it re-baselines the arena +/// trigger with exactly the arithmetic the arena arm would have used on the +/// same nursery. +/// +/// The asymmetry the split kept, in the OTHER direction, is still correct and +/// still here (see `gc_finish_arena_trigger_collection`): an arena minor that +/// skipped the malloc sweep must not move the malloc trigger. +/// +/// **Unit.** The base is `arena_total_bytes()` — COMMITTED bytes, all +/// generations — because that is the quantity `next_arena_trigger_base()` is +/// compared against in `gc_budgeted_due_trigger`. It is deliberately neither +/// of the two post-collection occupancy readings published at +/// [`note_collection_finished_arena_occupancy`] (a live census, and #9831's +/// bump-offset guard base); that funnel's doc comment tabulates all three. +/// Re-baselining this cell from either of those would arm the trigger below +/// the arena's own total and make the arm due the instant it re-armed. +/// +/// **What is in scope.** The two *nursery-trigger* finishers, whichever +/// collection their arm ended up running — an `ArenaBytes` or `MallocCount` +/// trigger that `arena_growth_full_escalation_due()` escalated to a full still +/// finishes here, exactly as the arena arm's escalated fulls already did before +/// #9840. Out of scope is `BudgetedGcRebaseline::OldReclaim` (and the idle +/// reclaim): those are paced by the old-generation band, and after a full that +/// released blocks the un-moved trigger sits *further* above the new total, +/// which is the conservative direction. +/// +/// Measured on the compiled claude-code TUI before this (`PERRY_GC_DIAG=1`, +/// per firing, four 3300-character captures on two binaries): the streaming +/// turn ran a strict 6:1 pattern — six `MallocCount` minors promoting ~3.2 MB +/// each grew the old generation past the arena trigger *inside the sixth +/// minor*, and at the very next safepoint the arm fired on a nursery of +/// **856 bytes** (`promoted_bytes=216 freed_bytes=640`), paying the whole +/// per-collection fixed cost to free 640 bytes. One in seven of the turn's +/// collections. See `secret-tests/cc-perf-campaign/DESIGN_arena_contract.md`. +fn gc_rebaseline_arena_trigger_after_collection( + pre_in_use: usize, + outcome: &GcCollectOutcome, + arm: ArenaRebaselineArm, +) { let sweep_freed_bytes = outcome.freed_bytes; - let malloc_swept = outcome.malloc_swept; let post_in_use = crate::arena::arena_in_use_bytes(); // Adaptive step: @@ -2265,8 +2350,13 @@ fn gc_finish_arena_trigger_collection(pre_in_use: usize, outcome: GcCollectOutco let freed = std::cmp::max(block_reclaim, sweep_freed_bytes as usize); let mut step = GC_STEP_BYTES.with(|c| c.get()); let old_step = step; + // #9840: reported on `[gc-arena-rebaseline]` for BOTH arms, because the + // step is now scored by both and #9831's tiny-parse guard prices its + // headroom from it — see the note above the `GC_STEP_BYTES` write below. + let mut scored_pct_freed = 0usize; if pre_in_use > 0 { let pct_freed = (freed * 100) / pre_in_use; + scored_pct_freed = pct_freed; // 2026-05-02: widen the "double" band from `>90% || <10%` to // `>=85% || <10%`. ECS perf-comprehensive's two // alloc-heavy benches (10k two-comp, 5k × 3 cmds) sweep @@ -2293,8 +2383,44 @@ fn gc_finish_arena_trigger_collection(pre_in_use: usize, outcome: GcCollectOutco step = (step / 2).max(16 * 1024 * 1024); } // 10-25% freed → keep step unchanged (marginal churn). + // + // #9840, and the one behavioural coupling this change has beyond the + // trigger itself: `GC_STEP_BYTES` had exactly one production writer — + // this line, reached only by the `ArenaBytes` finisher — and #9831 made + // it an INPUT to the tiny-parse pressure guard + // (`tiny_parse_pressure_headroom_bytes`). Scoring a `MallocCount` + // minor's productivity here therefore moves that guard's headroom too. + // That is the intended reading of both cells and not a side effect: + // the step is documented as "collection effectiveness", the guard's + // own base cell was made kind-agnostic by #9831 at + // `note_collection_finished_arena_occupancy`, and a step scored by + // only one of the two nursery arms is the same defect this change + // fixes, one cell over. The direction on a given workload is a + // question for measurement, not for reasoning. Estimated over the four + // 3300-character captures (209 `MallocCount` firings: `freed_bytes` + // from each firing's own `[gc-copy-minor]` line over the nearest + // `[gc-step]`'s post-collection in-use — an ADJACENT-diagnostic + // estimate, because before this change a `MallocCount` minor emitted no + // line carrying its own `pre_in_use`, which is exactly why the new one + // does): + // + // pct_freed median 4-5 % <10 %: 194/209 10-24 %: 10/209 + // 25-84 %: 5/209 >84 %: 0/209 + // + // 93 % of them land in the "<10 % → double" band and none in ">84 %". + // A `MallocCount` minor frees 5-44 MB against an 83-277 MB `pre_in_use` + // — most of which is old generation it cannot touch — so on cc this + // coupling pushes the step UP and makes #9831's guard MORE conservative, + // reinforcing that fix rather than eroding it. `[gc-arena-rebaseline]` + // carries `pct=`/`step=` for both arms so the claim is read off a + // capture instead of estimated from a neighbour. GC_STEP_BYTES.with(|c| c.set(step)); - if crate::gc::gc_diag_enabled() { + // `[gc-step]` stays an ArenaBytes-only line: `scripts/gc_repsel_matrix.sh` + // sums every `sweep_freed=` it can grep, so printing it for the malloc + // arm too would double-count that arm's reclaim (already reported on its + // `[gc-copy-minor]` line) in the ratchet. The malloc arm is attributed + // on `[gc-arena-rebaseline]` below instead. + if matches!(arm, ArenaRebaselineArm::ArenaBytes) && crate::gc::gc_diag_enabled() { eprintln!( "[gc-step] pre_in_use={} post_in_use={} sweep_freed={} block_reclaim={} pct={}% step={}→{}", pre_in_use, post_in_use, sweep_freed_bytes, block_reclaim, pct_freed, old_step, step @@ -2333,19 +2459,107 @@ fn gc_finish_arena_trigger_collection(pre_in_use: usize, outcome: GcCollectOutco std::cmp::max(capped, floor).saturating_add(super::take_promoted_young_capacity_credit()); GC_NEXT_TRIGGER_BYTES.with(|c| c.set(next_trigger)); GC_TRIGGER_ARMED.with(|a| a.set(true)); + if crate::gc::gc_diag_enabled() { + // Field names deliberately disjoint from the `[gc-step]` line above and + // from the `[gc-copy-minor]` line: `scripts/gc_repsel_matrix.sh` sums + // every `sweep_freed=`/`freed_bytes=` it can grep, so a second line + // carrying those keys would double-count reclaim in the ratchet. + eprintln!( + "[gc-arena-rebaseline] arm={} next_trigger={} total={} headroom={} pct={}% step={}→{}", + arm.label(), + next_trigger, + new_total, + next_trigger.saturating_sub(new_total), + scored_pct_freed, + old_step, + step + ); + } +} + +fn gc_finish_arena_trigger_collection(pre_in_use: usize, outcome: GcCollectOutcome) -> u64 { + gc_rebaseline_arena_trigger_after_collection( + pre_in_use, + &outcome, + ArenaRebaselineArm::ArenaBytes, + ); // Rebaseline the malloc-count trigger only if this collection // actually swept malloc objects. Copied-minor arena collections // may skip the malloc sweep while count pressure is still below // its trigger; moving the trigger in that case would postpone // reclamation of already-tracked dead malloc churn. - if malloc_swept { + if outcome.malloc_swept { let mstep = GC_MALLOC_COUNT_STEP.with(|c| c.get()); gc_rebaseline_malloc_trigger_to_survivors(mstep); } outcome.emit_after_current() } -fn gc_finish_malloc_trigger_collection(pre_count: usize, outcome: GcCollectOutcome) -> u64 { +/// `PERRY_GC_ARENA_REBASELINE_ALL=0|off|false` restores the pre-#9840 +/// asymmetry: only the `ArenaBytes` finisher moves the whole-arena trigger. +/// +/// The kill switch, and the positive control — both arms of the measurement +/// live in ONE binary, so no build difference can be confounded with the +/// change. Default ON; only the three explicit off-spellings turn it off, so a +/// typo cannot silently change which behaviour a bisect is measuring +/// (`env_default_on_from_value`'s contract). +/// +/// CLAUDE.md's GC knob kill-policy is binding: this knob's OFF state is +/// asserted, not merely available. `direct_malloc_minor_arena_rebaseline_kill_ +/// switch_restores_the_stale_threshold` in `gc/tests/debt_pacer.rs` runs the +/// same fixture as the ON-state test through the seam below and asserts the +/// defect returns — trigger left at the pre-collection value, and a second +/// minor firing on the nursery the first one emptied. That test is +/// simultaneously the sabotage proof for the ON-state pair: the OFF state IS +/// the deletion of the call, kept live by CI instead of performed by hand. +/// +/// The env read is cached, so a test cannot flip it by poking the process +/// environment (and must not try — the tests run in one process). The +/// `#[cfg(test)]` seam is the supported way in, matching +/// `pacing_arena_in_use_bytes`. +fn arena_rebaseline_all_enabled() -> bool { + #[cfg(test)] + if let Some(enabled) = TEST_ARENA_REBASELINE_ALL.with(|cell| cell.get()) { + return enabled; + } + static ENABLED: std::sync::OnceLock = std::sync::OnceLock::new(); + *ENABLED.get_or_init(|| crate::gc::env_default_on_enabled("PERRY_GC_ARENA_REBASELINE_ALL")) +} + +#[cfg(test)] +thread_local! { + /// Test-only override for [`arena_rebaseline_all_enabled`]. Thread-local, + /// so concurrently-running tests cannot see each other's value. + static TEST_ARENA_REBASELINE_ALL: std::cell::Cell> = + const { std::cell::Cell::new(None) }; +} + +/// Force [`arena_rebaseline_all_enabled`] for the duration of a test, restoring +/// the previous override on drop. `Drop` rather than a bare setter because the +/// tests that use it assert a *defect* is present, and a leaked `false` would +/// silently disarm every later test in the same thread. +#[cfg(test)] +pub(super) struct ArenaRebaselineAllTestGuard(Option); + +#[cfg(test)] +impl ArenaRebaselineAllTestGuard { + pub(super) fn force(enabled: bool) -> Self { + Self(TEST_ARENA_REBASELINE_ALL.with(|cell| cell.replace(Some(enabled)))) + } +} + +#[cfg(test)] +impl Drop for ArenaRebaselineAllTestGuard { + fn drop(&mut self) { + TEST_ARENA_REBASELINE_ALL.with(|cell| cell.set(self.0)); + } +} + +fn gc_finish_malloc_trigger_collection( + pre_count: usize, + pre_in_use: usize, + outcome: GcCollectOutcome, +) -> u64 { debug_assert!( outcome.malloc_swept, "malloc-count trigger must sweep malloc objects" @@ -2382,6 +2596,15 @@ fn gc_finish_malloc_trigger_collection(pre_count: usize, outcome: GcCollectOutco if outcome.malloc_swept { GC_NEXT_MALLOC_TRIGGER.with(|c| c.set(survivors + mstep)); } + // #9840: this collection swept the nursery too, so the whole-arena trigger + // is measured from after it — see `gc_rebaseline_arena_trigger_after_collection`. + if arena_rebaseline_all_enabled() { + gc_rebaseline_arena_trigger_after_collection( + pre_in_use, + &outcome, + ArenaRebaselineArm::MallocCount, + ); + } outcome.emit_after_current() } @@ -2679,7 +2902,7 @@ pub fn gc_check_trigger() { // exactly as the budgeted and full-GC paths do on completion. match kind { GcTriggerKind::MallocCount => { - gc_finish_malloc_trigger_collection(pre_malloc_count, outcome); + gc_finish_malloc_trigger_collection(pre_malloc_count, pre_in_use, outcome); } _ => { gc_finish_arena_trigger_collection(pre_in_use, outcome); @@ -2757,7 +2980,7 @@ pub struct JsGcStepResult { #[derive(Clone, Copy)] enum BudgetedGcRebaseline { ArenaBytes { pre_in_use: usize }, - MallocCount { pre_count: usize }, + MallocCount { pre_count: usize, pre_in_use: usize }, OldReclaim, } @@ -3026,7 +3249,7 @@ pub(crate) fn gc_safepoint_moving_minor() -> bool { let outcome = super::gc_collect_minor_with_trigger(GcTriggerSnapshot::capture(kind)); match kind { GcTriggerKind::MallocCount => { - gc_finish_malloc_trigger_collection(pre_malloc_count, outcome); + gc_finish_malloc_trigger_collection(pre_malloc_count, pre_in_use, outcome); } _ => { gc_finish_arena_trigger_collection(pre_in_use, outcome); @@ -3460,6 +3683,7 @@ fn gc_start_budgeted_cycle_for_pressure(progress_kind: GcProgressKind) -> Option BudgetedGcTrigger::MallocCount => { let rebaseline = BudgetedGcRebaseline::MallocCount { pre_count: malloc_object_count(), + pre_in_use: crate::arena::arena_in_use_bytes(), }; // Major-GC pacing (malloc-count trigger twin of the ArenaBytes branch). if gen_gc_enabled() && !arena_growth_full_escalation_due() { @@ -3554,8 +3778,11 @@ fn gc_finish_budgeted_cycle(mut cycle: BudgetedGcCycle) -> JsGcStepResult { BudgetedGcRebaseline::ArenaBytes { pre_in_use } => { gc_finish_arena_trigger_collection(pre_in_use, outcome); } - BudgetedGcRebaseline::MallocCount { pre_count } => { - gc_finish_malloc_trigger_collection(pre_count, outcome); + BudgetedGcRebaseline::MallocCount { + pre_count, + pre_in_use, + } => { + gc_finish_malloc_trigger_collection(pre_count, pre_in_use, outcome); } BudgetedGcRebaseline::OldReclaim => { let freed = outcome.emit_after_current(); diff --git a/crates/perry-runtime/src/gc/tests/copying/survival_and_malloc.rs b/crates/perry-runtime/src/gc/tests/copying/survival_and_malloc.rs index 83130cf246..d35ad10f9f 100644 --- a/crates/perry-runtime/src/gc/tests/copying/survival_and_malloc.rs +++ b/crates/perry-runtime/src/gc/tests/copying/survival_and_malloc.rs @@ -345,6 +345,132 @@ fn test_gc_check_trigger_copied_minor_malloc_sweep_rebaselines_trigger() { ); } +/// #9840 on the BUDGETED path — the one cc actually takes. Companion to +/// `debt_pacer::direct_malloc_minor_also_rebaselines_the_whole_arena_trigger` +/// (the direct synchronous arm); the moving safepoint +/// (`gc_safepoint_moving_minor`) is the third caller and shares this same +/// finisher. +/// +/// A `MallocCount` minor sweeps the nursery exactly as an `ArenaBytes` minor +/// does, so the whole-arena trigger must be measured from after it. Leaving it +/// where the previous arena-kind collection put it is what let six +/// `MallocCount` minors' promotion walk the arena total across a stale +/// threshold and fire the arena arm on an 856-byte nursery — see the direct +/// test's doc comment for the measurement. +/// +/// Sabotage (delete the `gc_rebaseline_arena_trigger_after_collection` call +/// from `gc_finish_malloc_trigger_collection`): the first assertion sees the +/// pre-collection trigger, and the second sees a whole-arena cycle start on +/// the quiet nursery. +#[test] +fn test_budgeted_malloc_minor_rebaselines_the_whole_arena_trigger() { + let _legacy_pacing = crate::gc::policy::force_legacy_gc_pacing(); + let _guard = CopyingNurseryTestGuard::new(1); + let trigger_guard = GcTriggerThresholdTestGuard::suppress_automatic_triggers(); + let old_in_use = crate::arena::old_gen_in_use_bytes(); + GC_LAST_OLD_RECLAIM_IN_USE_BYTES.with(|bytes| bytes.set(old_in_use)); + GC_OLD_RECLAIM_PENDING.with(|pending| pending.set(false)); + + let live_malloc = gc_malloc( + std::mem::size_of::(), + GC_TYPE_CLOSURE, + ); + unsafe { + init_test_closure(live_malloc); + } + js_shadow_slot_set(0, ptr_bits(live_malloc as usize)); + activate_malloc_registry_for_tests(); + + let churn_headers = allocate_dead_malloc_churn_headers(48); + assert_eq!( + tracked_malloc_headers_matching(&churn_headers), + churn_headers.len(), + "malloc churn should be tracked before gc_check_trigger" + ); + + // Arena arm armed but NOT due (1 MB of headroom); malloc pressure due. + let arena_total_before = crate::arena::arena_total_bytes(); + let stale_trigger = arena_total_before + 1024 * 1024; + GC_NEXT_TRIGGER_BYTES.with(|trigger| trigger.set(stale_trigger)); + trigger_guard.make_malloc_sweep_due(); + + let collections_before = gc_collection_count(); + gc_check_trigger(); + + let mut step_status = JsGcStepResult::default(); + assert_eq!( + js_gc_step_status(&mut step_status), + JS_GC_STEP_STATUS_ACTIVE, + "gc_check_trigger should schedule malloc pressure as bounded assist work" + ); + assert_eq!( + step_status.trigger_kind, + GcTriggerKind::MallocCount.ffi_code(), + "the cycle under test must be the MallocCount one" + ); + + let completed = complete_budgeted_gc_cycle(); + assert_eq!(completed.status, JS_GC_STEP_STATUS_COMPLETED); + assert!( + gc_collection_count() > collections_before, + "draining the budgeted malloc-pressure cycle should collect" + ); + assert_eq!( + tracked_malloc_headers_matching(&churn_headers), + 0, + "the cycle must have swept malloc, or it did not take the arm under test" + ); + + // (1) The budgeted finisher re-baselined the whole-arena trigger too. + let arena_total_after = crate::arena::arena_total_bytes(); + let next_trigger = GC_NEXT_TRIGGER_BYTES.with(|trigger| trigger.get()); + assert!( + next_trigger >= arena_total_after + gc_trigger_headroom_floor_bytes(), + "the budgeted MallocCount finisher must re-baseline the whole-arena \ + trigger above the set it left behind (next_trigger={next_trigger}, \ + arena_total_after={arena_total_after}); leaving it at the \ + pre-collection value ({stale_trigger}) is shape (b)'s stale threshold" + ); + + // (2) ...so a little old-generation growth cannot re-arm a whole-arena + // cycle on the nursery this collection just emptied. + let old_in_use = crate::arena::old_gen_in_use_bytes(); + GC_LAST_OLD_RECLAIM_IN_USE_BYTES.with(|bytes| bytes.set(old_in_use)); + GC_OLD_RECLAIM_PENDING.with(|pending| pending.set(false)); + let mut filler = Vec::new(); + for _ in 0..32 { + filler.push(crate::arena::arena_alloc_gc_old( + 64 * 1024, + 8, + GC_TYPE_STRING, + )); + } + assert!( + crate::arena::arena_total_bytes() > stale_trigger, + "the filler must grow the arena total past the PRE-collection trigger, \ + or the assertion below cannot distinguish the two behaviours" + ); + let old_in_use = crate::arena::old_gen_in_use_bytes(); + GC_LAST_OLD_RECLAIM_IN_USE_BYTES.with(|bytes| bytes.set(old_in_use)); + GC_OLD_RECLAIM_PENDING.with(|pending| pending.set(false)); + + let collections_before_growth = gc_collection_count(); + gc_check_trigger(); + let mut after_growth = JsGcStepResult::default(); + assert_ne!( + js_gc_step_status(&mut after_growth), + JS_GC_STEP_STATUS_ACTIVE, + "2 MB of old-generation growth after a nursery collection must not open \ + a whole-arena cycle on the quiet nursery" + ); + assert_eq!( + gc_collection_count(), + collections_before_growth, + "...nor collect" + ); + drop(filler); +} + #[test] fn test_gc_check_trigger_copied_minor_without_malloc_sweep_preserves_malloc_trigger() { let _legacy_pacing = crate::gc::policy::force_legacy_gc_pacing(); diff --git a/crates/perry-runtime/src/gc/tests/debt_pacer.rs b/crates/perry-runtime/src/gc/tests/debt_pacer.rs index 2e71058dfa..dfb1361181 100644 --- a/crates/perry-runtime/src/gc/tests/debt_pacer.rs +++ b/crates/perry-runtime/src/gc/tests/debt_pacer.rs @@ -374,6 +374,228 @@ fn direct_malloc_minor_rebaselines_trigger_above_survivors() { assert!(next_malloc_trigger > survivors_after); } +/// #9840, the whole-arena half of the same direct `MallocCount` minor: a +/// `MallocCount` minor is an `ArenaBytes` minor with the malloc sweep added — +/// the arena *was* swept — so it must re-baseline `GC_NEXT_TRIGGER_BYTES` too. +/// +/// `GC_NEXT_TRIGGER_BYTES`'s own doc says it is bumped "after each +/// `gc_collect_inner` based on collection effectiveness". Until this test it +/// was not: only `gc_finish_arena_trigger_collection` moved it, so the arm's +/// threshold was measured from the last *arena-kind* collection rather than +/// from the last collection, and a run of `MallocCount` minors walked the +/// arena total across a threshold that nothing had refreshed. (The asymmetry +/// in the OTHER direction is correct and is pinned by +/// `test_gc_check_trigger_copied_minor_without_malloc_sweep_preserves_malloc_trigger`: +/// an arena minor that skipped the malloc sweep must not move the malloc +/// trigger.) +/// +/// Measured on the perry-compiled claude-code TUI before the fix +/// (`PERRY_GC_DIAG=1`, per firing, four 3300-character captures across two +/// binaries): the streaming turn ran a strict 6:1 pattern — six `MallocCount` +/// minors promoting ~3.2 MB each grew the old generation past the stale arena +/// threshold *inside the sixth minor*, and at the very next safepoint the arm +/// fired on a nursery of **856 bytes** (`promoted_bytes=216 freed_bytes=640`), +/// paying the entire per-collection fixed cost — root scan, side-table prunes, +/// dirty-page restore — to free 640 bytes. One collection in seven. +/// +/// Sabotage (delete the `gc_rebaseline_arena_trigger_after_collection` call +/// from `gc_finish_malloc_trigger_collection`): the first assertion sees the +/// trigger still at the value that was armed BEFORE the collection, and the +/// second sees a second minor fire on the quiet nursery after 2 MB of +/// old-generation growth — shape (b), in miniature. +#[test] +fn direct_malloc_minor_also_rebaselines_the_whole_arena_trigger() { + let _legacy_pacing = crate::gc::policy::force_legacy_gc_pacing(); + let _nursery = CopyingNurseryTestGuard::new(1); + let _scanners = ScopedRootScannerRegistryGuard::new(); + gc_register_root_scanner(noop_copy_only_root_scanner); + let trigger_guard = GcTriggerThresholdTestGuard::suppress_automatic_triggers(); + reset_old_reclaim_pressure(); + + let live_malloc = gc_malloc( + std::mem::size_of::(), + GC_TYPE_CLOSURE, + ); + unsafe { + init_test_closure(live_malloc); + } + js_shadow_slot_set(0, ptr_bits(live_malloc as usize)); + let churn_headers = allocate_dead_malloc_churn_headers(128); + assert_eq!( + tracked_malloc_headers_matching(&churn_headers), + churn_headers.len(), + "malloc churn should be tracked before the collection" + ); + + // The arena arm is ARMED BUT NOT DUE — 1 MB of headroom left. This is the + // state the cc captures show at the start of a `MallocCount` run: the arena + // arm re-baselined a while ago and the total has not yet reached it. + let arena_total_before = crate::arena::arena_total_bytes(); + GC_NEXT_TRIGGER_BYTES.with(|trigger| trigger.set(arena_total_before + 1024 * 1024)); + // ...and malloc pressure IS due, so the direct minor takes the MallocCount arm. + trigger_guard.make_malloc_sweep_due(); + + let before = gc_collection_count(); + gc_check_trigger(); + assert!( + gc_collection_count() > before, + "a registered synchronous-only scanner should drive the MallocCount \ + trigger through the direct synchronous minor" + ); + assert_eq!( + tracked_malloc_headers_matching(&churn_headers), + 0, + "the MallocCount minor must have swept the dead malloc churn — without \ + it this test would assert the arena re-baseline of a collection that \ + never took the arm under test" + ); + + // (1) The whole-arena trigger is measured from what THIS collection left + // behind, with the same headroom floor the arena finisher applies. + let arena_total_after = crate::arena::arena_total_bytes(); + let next_trigger = GC_NEXT_TRIGGER_BYTES.with(|trigger| trigger.get()); + assert!( + next_trigger >= arena_total_after + gc_trigger_headroom_floor_bytes(), + "a MallocCount minor must re-baseline the whole-arena trigger above the \ + set it left behind (next_trigger={next_trigger}, \ + arena_total_after={arena_total_after}, floor={}); leaving it at the \ + pre-collection value ({}) is the stale threshold shape (b) rides", + gc_trigger_headroom_floor_bytes(), + arena_total_before + 1024 * 1024 + ); + + // (2) Shape (b) itself, in miniature: a little old-generation growth after + // the collection must NOT re-arm a whole-arena minor on a nursery that + // the collection just emptied. `reset_old_reclaim_pressure` first, so + // the arm under test is the only one that could fire. + reset_old_reclaim_pressure(); + let collections_before_growth = gc_collection_count(); + let mut filler = Vec::new(); + for _ in 0..32 { + filler.push(crate::arena::arena_alloc_gc_old( + 64 * 1024, + 8, + GC_TYPE_STRING, + )); + } + assert!( + crate::arena::arena_total_bytes() > arena_total_before + 1024 * 1024, + "the filler must grow the arena total past the PRE-collection trigger \ + value, or the second assertion cannot distinguish the two behaviours \ + (total={}, pre-collection trigger={})", + crate::arena::arena_total_bytes(), + arena_total_before + 1024 * 1024 + ); + reset_old_reclaim_pressure(); + gc_check_trigger(); + assert_eq!( + gc_collection_count(), + collections_before_growth, + "2 MB of old-generation growth after a nursery collection must not run \ + another whole-arena minor on the quiet nursery: the arena trigger was \ + re-baselined by the collection that just ran, so 2 MB cannot reach it \ + (floor is {} MB of headroom)", + gc_trigger_headroom_floor_bytes() / (1024 * 1024) + ); + drop(filler); +} + +/// The OFF state of `PERRY_GC_ARENA_REBASELINE_ALL`, which CLAUDE.md's GC knob +/// kill-policy requires ("every GC env knob either has a required CI arm +/// exercising its OFF state, or it is deleted"), and which is at the same time +/// the **sabotage proof** for the two ON-state tests: the knob's OFF branch is +/// exactly the deletion of the +/// `gc_rebaseline_arena_trigger_after_collection` call from +/// `gc_finish_malloc_trigger_collection`, so asserting the defect returns under +/// the knob keeps that proof running in CI instead of being performed by hand +/// and then lost. +/// +/// Same fixture as `direct_malloc_minor_also_rebaselines_the_whole_arena_trigger`, +/// opposite expectations at both ends — the trigger is left exactly where it +/// was armed before the collection, and 2 MB of old-generation growth is then +/// enough to fire a whole-arena minor on the nursery that collection just +/// emptied. That second half is shape (b) in miniature and is the reason the +/// first half matters: a stale threshold is only a defect because something +/// crosses it. +#[test] +fn direct_malloc_minor_arena_rebaseline_kill_switch_restores_the_stale_threshold() { + let _legacy_pacing = crate::gc::policy::force_legacy_gc_pacing(); + let _rebaseline_all = crate::gc::policy::ArenaRebaselineAllTestGuard::force(false); + let _nursery = CopyingNurseryTestGuard::new(1); + let _scanners = ScopedRootScannerRegistryGuard::new(); + gc_register_root_scanner(noop_copy_only_root_scanner); + let trigger_guard = GcTriggerThresholdTestGuard::suppress_automatic_triggers(); + reset_old_reclaim_pressure(); + + let live_malloc = gc_malloc( + std::mem::size_of::(), + GC_TYPE_CLOSURE, + ); + unsafe { + init_test_closure(live_malloc); + } + js_shadow_slot_set(0, ptr_bits(live_malloc as usize)); + let churn_headers = allocate_dead_malloc_churn_headers(128); + + let arena_total_before = crate::arena::arena_total_bytes(); + let stale_trigger = arena_total_before + 1024 * 1024; + GC_NEXT_TRIGGER_BYTES.with(|trigger| trigger.set(stale_trigger)); + trigger_guard.make_malloc_sweep_due(); + + let before = gc_collection_count(); + gc_check_trigger(); + assert!( + gc_collection_count() > before, + "the MallocCount minor must still run under the kill switch — the knob \ + gates the re-baseline, not the collection" + ); + assert_eq!( + tracked_malloc_headers_matching(&churn_headers), + 0, + "the collection must have swept malloc, or it did not take the arm \ + whose finisher is under test" + ); + + // (1) The defect: the whole-arena trigger is exactly where it was armed + // BEFORE the collection. Not merely "below the floor" — unmoved. + assert_eq!( + GC_NEXT_TRIGGER_BYTES.with(|trigger| trigger.get()), + stale_trigger, + "with the re-baseline disabled the MallocCount finisher must leave the \ + whole-arena trigger untouched — that is the pre-#9840 behaviour this \ + knob restores" + ); + + // (2) ...and it is a threshold something crosses: 2 MB of old-generation + // growth now fires a whole-arena minor on the emptied nursery. + reset_old_reclaim_pressure(); + let collections_before_growth = gc_collection_count(); + let mut filler = Vec::new(); + for _ in 0..32 { + filler.push(crate::arena::arena_alloc_gc_old( + 64 * 1024, + 8, + GC_TYPE_STRING, + )); + } + assert!( + crate::arena::arena_total_bytes() > stale_trigger, + "the filler must cross the stale threshold (total={}, stale_trigger={})", + crate::arena::arena_total_bytes(), + stale_trigger + ); + reset_old_reclaim_pressure(); + gc_check_trigger(); + assert!( + gc_collection_count() > collections_before_growth, + "shape (b): with the threshold left stale, old-generation growth fires \ + a WHOLE-ARENA minor on a nursery the previous collection emptied — \ + this is the collection #9840 removes, and the assertion that fails \ + when the fix is present" + ); + drop(filler); +} + /// Debt-proportional assist pacing: the per-assist work budget must grow /// linearly with measured debt (and be exactly the base when no debt). #[test] From c9dff4c95ca02ad5a2c3493b5f0f4c01fd483555 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 6 Sep 2026 10:54:48 +0200 Subject: [PATCH 19/26] fix(gates): rebuild the root-holder inventory structurally after #9838's merge --- crates/perry-runtime/src/hot_diag.rs | 8 +++++++- .../perry-runtime/src/object/iterator_prototypes.rs | 12 +++++++----- scripts/gc_runtime_root_holders.json | 2 +- 3 files changed, 15 insertions(+), 7 deletions(-) diff --git a/crates/perry-runtime/src/hot_diag.rs b/crates/perry-runtime/src/hot_diag.rs index 9c7ab13ae7..43ec8fba16 100644 --- a/crates/perry-runtime/src/hot_diag.rs +++ b/crates/perry-runtime/src/hot_diag.rs @@ -1037,7 +1037,13 @@ fn buffer_dump() { let amax = BUF_ADDR_MAX.load(Ordering::Relaxed); let wlo = BUF_WIN_LO.load(Ordering::Relaxed); let whi = BUF_WIN_HI.load(Ordering::Relaxed); - let pct = |a: u64, b: u64| if b == 0 { 0.0 } else { 100.0 * a as f64 / b as f64 }; + let pct = |a: u64, b: u64| { + if b == 0 { + 0.0 + } else { + 100.0 * a as f64 / b as f64 + } + }; let mb = |n: usize| n as f64 / (1024.0 * 1024.0); let win_span = whi.saturating_sub(wlo); let probe_span = amax.saturating_sub(amin); diff --git a/crates/perry-runtime/src/object/iterator_prototypes.rs b/crates/perry-runtime/src/object/iterator_prototypes.rs index 1865aa9fed..8094cc9a18 100644 --- a/crates/perry-runtime/src/object/iterator_prototypes.rs +++ b/crates/perry-runtime/src/object/iterator_prototypes.rs @@ -631,10 +631,11 @@ mod override_probe_allocation_tests { let iter_obj = || js_nanbox_get_pointer(iter_h.get_nanbox_f64()) as *mut ObjectHeader; // Warm once: a first call may lazily build anything it builds. - assert!( - call_overridden_iterator_next(iter_obj(), crate::array::ARRAY_ITERATOR_CLASS_ID) - .is_none() - ); + assert!(call_overridden_iterator_next( + iter_obj(), + crate::array::ARRAY_ITERATOR_CLASS_ID + ) + .is_none()); const N: usize = 1000; let minors_before = crate::gc::instruments::copying_minor_cycles(); let bytes_before = crate::arena::arena_in_use_bytes(); @@ -727,7 +728,8 @@ mod override_probe_allocation_tests { let getter = crate::closure::js_closure_alloc(accessor_getter_thunk as *const u8, 0); crate::closure::js_register_closure_arity(accessor_getter_thunk as *const u8, 0); let getter_h = scope.root_nanbox_f64(js_nanbox_pointer(getter as i64)); - let key = scope.root_string_ptr(crate::string::js_string_from_bytes(b"next".as_ptr(), 4)); + let key = + scope.root_string_ptr(crate::string::js_string_from_bytes(b"next".as_ptr(), 4)); super::super::js_object_define_accessor( js_nanbox_pointer(array_proto() as i64), key.with_const_ptr::(|k| { diff --git a/scripts/gc_runtime_root_holders.json b/scripts/gc_runtime_root_holders.json index 9f1488bd20..39d54ebf42 100644 --- a/scripts/gc_runtime_root_holders.json +++ b/scripts/gc_runtime_root_holders.json @@ -294,7 +294,7 @@ "crates/perry-runtime/src/gc/census.rs": "388414f9629f196e84673e91bebd04bdcdcabdaa180252d2dfe4b82d1b49ca5a", "crates/perry-runtime/src/gc/cycle.rs": "2e2f5adca2229f74409e01a1cb571e2147cd8a33f58d0976711fce98d4777309", "crates/perry-runtime/src/gc/mod.rs": "7dd42b9506a97e6844fd3225dc53dfd59512631784750f58ff72208d68595481", - "crates/perry-runtime/src/gc/policy.rs": "71ac3a7651e61f553680eab523662a2c66cbb1bdbc3871a47d822c553a28ad80", + "crates/perry-runtime/src/gc/policy.rs": "dc9242ed40c0aa9c411d1ec0235c0219c6716dd82d56eb4d46578f7e889825d2", "crates/perry-runtime/src/gc/progress.rs": "a5ad3971bbe4047229ca57325234780daa85921dbc778e1c08dff4ad07ccfb96" } } From 36fd76baabe3abb2634925d6b99dcea3e24790b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 5 Sep 2026 00:36:39 +0200 Subject: [PATCH 20/26] diag(gc): trigger/full/budgeted/charge attribution, per-minor survival origins, allocation-site sampling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three instruments for the cc-perf campaign, all inert unless asked for. `PERRY_GC_DIAG=1` gains the lines that say WHY the collector ran: `[gc-trigger]` (every predicate input at each decision site), `[gc-full]` (the arm behind each synchronous full mark-sweep, counted per site), `[gc-budgeted] start/done` (steps, per-phase step time, root-scan share), `[gc-charge]` (mutator-assist / synchronous-full time per calling site, resolved to JS display names) and `[gc-survival]` (per copying minor, the root that first reached each surviving byte — shadow stack, native stack map, named scanner, remembered set by old-parent type — with transitive reach charged to the originating root through a parallel worklist origin vector). `PERRY_ALLOC_SITE_SAMPLE=` samples the arena allocation sites byte- proportionally across the runtime allocators AND the codegen inline bump path (the mirrored inline block limit is capped at one interval while sampling, so the fast path returns to the runtime once per interval). The survival test is sabotage-checked: disabling the drain propagation charges the 40 elements to `worklist_drain` and the test fails on that row. The knob's OFF state and magnitude parse are pinned next to the other GC knobs. `gc_diag_enabled()` gets the per-thread test override the census already has, so the diag paths are testable without touching the process environment. Claude-Session: https://claude.ai/code/session_01YPfnmWZmSpSWpmnoXvH8z2 --- changelog.d/gc-churn-attribution-diag.md | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 changelog.d/gc-churn-attribution-diag.md diff --git a/changelog.d/gc-churn-attribution-diag.md b/changelog.d/gc-churn-attribution-diag.md new file mode 100644 index 0000000000..f4a6a2785d --- /dev/null +++ b/changelog.d/gc-churn-attribution-diag.md @@ -0,0 +1,22 @@ +### Runtime + +- `PERRY_GC_DIAG=1` now says WHY the collector ran, not only what it did: + `[gc-trigger]` prints every predicate input at each collection decision + (armed arena trigger vs `arena_total`, from-space vs the nursery cap, + old-gen reclaimable pressure vs baseline/band, the malloc pair, the + pending/retaining flags); `[gc-full]` names the arm behind every full + mark-sweep with a per-site count; `[gc-budgeted] start/done` reports each + incremental cycle's steps, per-phase step time and root-scan share; + `[gc-charge]` attributes mutator-assist and synchronous-full time to the + calling site (return-address chain resolved to the JS display name); + `[gc-survival]` gives, per copying minor, which root first reached each + surviving byte — shadow stack, native stack map, a named side-table + scanner, or the remembered set split by the old parent's type — with + transitive reach charged to the originating root. +- `PERRY_ALLOC_SITE_SAMPLE=` (arena/alloc_sample.rs): byte-proportional + allocation-site sampling for the GC arena, covering the runtime allocators + and the codegen inline bump path (the mirrored inline block limit is capped + at one interval while sampling). `[alloc-site]` reports bytes by object type + and the top sites after each copying minor and at exit. Off by default; one + relaxed atomic load per allocation when off; the OFF state and the magnitude + parse are pinned in `gc/tests/env_knob_parse.rs`. From 7a6dc54745debfeae7d0a99d0ad9fea9bdd5ef97 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 5 Sep 2026 07:30:33 +0200 Subject: [PATCH 21/26] perf(runtime): stop minting throwaway strings and per-character descriptors on the primitive-string path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three allocations a JS program can never observe, found with the allocation-site sampler (`PERRY_ALLOC_SITE_SAMPLE`) on the compiled claude-code TUI, where they are the largest attributed source of garbage in both the streaming turn and the render pass that follows it. 1. A one-ASCII-character string is now the canonical per-thread header. `js_string_char_at` minted a fresh 32-byte heap string per character read, and everything that walks a string a character at a time goes through it: `s[i]`, `charAt`, string spread, the String-wrapper index installer. There are 128 possible contents. The table has the same residency contract as the small-integer string table next to it (longlived arena, `refcount = 0` so it is never mutated in place, pinned out of the young generation) and rides that table's existing root scanner rather than registering a 96th one. 2. Runtime-internal constant property names resolve through the intern table. The `globalThis` builtin lookup, `x.constructor`, `toString` resolution and primitive-method dispatch each built a fresh heap string for a literal name on every call; `js_get_global_this_builtin_value` alone accounted for 133 MB of the 990 MB one 3300-character reply allocates. `string::canonical_key` routes them through the content-keyed per-thread table that `js_string_materialize_to_heap` already uses, which is also what the property read/write fast paths require of a key. 3. A `String` wrapper no longer stores a property descriptor per character. ECMA-262 §10.4.3 gives every in-range index of a String exotic object `{ writable: false, enumerable: true, configurable: false }` — a fact of the class and the boxed length, not per-object state — so `get_property_attrs` answers it from the wrapper's payload. Storing it cost, per boxed character, a Rust `String`, a `PROPERTY_DESCRIPTORS` entry only a full collection's dead-owner prune could reclaim, an owner-index entry, and one program-wide `prop_plan_epoch_bump()`. A sloppy method call on a string primitive boxes its receiver, so the TUI paid all of it per rendered line. A real stored descriptor still wins, so `Object.freeze`/`defineProperty` on a wrapper are unchanged. `PERRY_GC_DIAG=1` also gains `[gc-primitive-dispatch]`: which `.prototype.` names reach the primitive-method fallback, how often, and how many wrapper index properties they cost — the counter that says whether a boxing fix ran. Claude-Session: https://claude.ai/code/session_01YPfnmWZmSpSWpmnoXvH8z2 --- changelog.d/gc-churn-attribution-diag.md | 22 --------- crates/perry-runtime/src/object/mod.rs | 2 +- crates/perry-runtime/src/string/tests.rs | 63 ++++++++++++++++++++++++ 3 files changed, 64 insertions(+), 23 deletions(-) delete mode 100644 changelog.d/gc-churn-attribution-diag.md diff --git a/changelog.d/gc-churn-attribution-diag.md b/changelog.d/gc-churn-attribution-diag.md deleted file mode 100644 index f4a6a2785d..0000000000 --- a/changelog.d/gc-churn-attribution-diag.md +++ /dev/null @@ -1,22 +0,0 @@ -### Runtime - -- `PERRY_GC_DIAG=1` now says WHY the collector ran, not only what it did: - `[gc-trigger]` prints every predicate input at each collection decision - (armed arena trigger vs `arena_total`, from-space vs the nursery cap, - old-gen reclaimable pressure vs baseline/band, the malloc pair, the - pending/retaining flags); `[gc-full]` names the arm behind every full - mark-sweep with a per-site count; `[gc-budgeted] start/done` reports each - incremental cycle's steps, per-phase step time and root-scan share; - `[gc-charge]` attributes mutator-assist and synchronous-full time to the - calling site (return-address chain resolved to the JS display name); - `[gc-survival]` gives, per copying minor, which root first reached each - surviving byte — shadow stack, native stack map, a named side-table - scanner, or the remembered set split by the old parent's type — with - transitive reach charged to the originating root. -- `PERRY_ALLOC_SITE_SAMPLE=` (arena/alloc_sample.rs): byte-proportional - allocation-site sampling for the GC arena, covering the runtime allocators - and the codegen inline bump path (the mirrored inline block limit is capped - at one interval while sampling). `[alloc-site]` reports bytes by object type - and the top sites after each copying minor and at exit. Off by default; one - relaxed atomic load per allocation when off; the OFF state and the magnitude - parse are pinned in `gc/tests/env_knob_parse.rs`. diff --git a/crates/perry-runtime/src/object/mod.rs b/crates/perry-runtime/src/object/mod.rs index 87dbfafc47..64c979d936 100644 --- a/crates/perry-runtime/src/object/mod.rs +++ b/crates/perry-runtime/src/object/mod.rs @@ -268,7 +268,7 @@ pub(crate) use descriptor_state::{ class_instance_set_may_intercept, clear_accessor_descriptor, clear_property_attrs, constructor_accessor_ever_installed, descriptors_in_use, disable_class_field_inline_guard, get_accessor_descriptor, get_property_attrs, install_fresh_accessor_property, - json_object_getter_value, mark_all_keys, object_has_descriptors, + json_object_getter_value, mark_all_keys, note_descriptor_target, object_has_descriptors, object_proto_may_intercept_key, owner_has_property_descriptors, owner_may_have_descriptor_entries, plain_data_write_may_intercept, prune_dead_descriptor_owner_entries, prune_dead_descriptor_owner_entries_young, diff --git a/crates/perry-runtime/src/string/tests.rs b/crates/perry-runtime/src/string/tests.rs index 2888700868..68b2f2aae0 100644 --- a/crates/perry-runtime/src/string/tests.rs +++ b/crates/perry-runtime/src/string/tests.rs @@ -1295,6 +1295,69 @@ mod split_empty_delimiter_code_units { }); assert_eq!(crate::array::js_array_length(arr), 3); } + +} + +/// The canonical one-ASCII-character string table (`string::format`). +#[cfg(test)] +mod canonical_char_cache { + use super::*; + + /// A one-ASCII-character string has exactly 128 possible contents, so + /// `js_string_char_at` (and everything that funnels through it: `s[i]`, + /// `charAt`, `[...s]`, the String-wrapper index installer) hands back the + /// canonical per-thread header instead of minting one per read. + /// + /// The identity assertion is the whole point — it is what makes the + /// allocation disappear — and it fails the moment the canonical table is + /// bypassed. The `refcount == 0` assertion is the safety half: a shared + /// header must never be eligible for the in-place append optimisation. + #[test] + fn ascii_char_at_returns_one_canonical_shared_header_per_byte() { + let scope = crate::gc::RuntimeHandleScope::new(); + let s = scope.root_string_ptr(js_string_from_bytes(b"abca".as_ptr(), 4)); + let (a0, b1, a3) = s.with_const_ptr::(|s| { + ( + js_string_char_at(s, 0), + js_string_char_at(s, 1), + js_string_char_at(s, 3), + ) + }); + assert_eq!(a0, a3, "the same character must reuse the canonical header"); + assert_ne!(a0, b1, "different characters are different headers"); + unsafe { + assert_eq!((*a0).byte_len, 1); + assert_eq!((*a0).utf16_len, 1); + let data = (a0 as *const u8).add(std::mem::size_of::()); + assert_eq!(*data, b'a'); + assert_eq!( + (*a0).refcount, + 0, + "a shared header must be ineligible for the in-place append path" + ); + } + // A second string with the same character resolves to the same header: + // the table is keyed by content, not by source string. + let other = scope.root_string_ptr(js_string_from_bytes(b"za".as_ptr(), 2)); + let a_again = other.with_const_ptr::(|o| js_string_char_at(o, 1)); + assert_eq!(a0, a_again); + } + + /// Non-ASCII keeps the minting path (the canonical table is ASCII-only), + /// and the value is still correct — the fast path must not answer for + /// characters it does not represent. + #[test] + fn non_ascii_char_at_is_unaffected_by_the_canonical_table() { + let scope = crate::gc::RuntimeHandleScope::new(); + let s = scope.root_string_ptr(js_string_from_bytes("aé".as_ptr(), 3)); + let (c0, c1) = s.with_const_ptr::(|s| { + (js_string_char_at(s, 0), js_string_char_at(s, 1)) + }); + unsafe { + assert_eq!((*c0).byte_len, 1); + assert_eq!((*c1).byte_len, 2, "é is two UTF-8 bytes"); + } + } } /// The canonical one-ASCII-character string table (`string::format`). From e54a62751cce84e1bab35ac79c9c3f16e859fd4f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 5 Sep 2026 08:19:37 +0200 Subject: [PATCH 22/26] perf(runtime): dispatch String.prototype.codePointAt natively MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `codePointAt` had a `String.prototype` thunk but no arm in the native string-method dispatch, so every call fell through to `call_primitive_builtin_prototype_method`: resolve `globalThis.String.prototype.codePointAt`, clone that closure to rebind `this`, and — the thunk not being registered strict — run `ToObject` on the receiver, minting a `String` wrapper whose own index properties are one per UTF-16 code unit. The new `[gc-primitive-dispatch]` counter says how much that cost: on the compiled claude-code TUI, `codePointAt` is the ONLY method name that reaches the fallback at all, and it reaches it 99,008 times per 400-character streamed reply — 99,008 `globalThis` lookups, 99,008 closure clones and 99,008 String wrappers, because grapheme-aware text measurement calls it once per character. The arm is the sibling of `charCodeAt` one line above it and reads the receiver the same way. The test asserts the WRAPPER COUNT rather than the return value: the fallback computes the same number, so an answer-only test would pass with the arm deleted. A positive control pins that the counter can move. --- .../9795-string-code-point-at-dispatch.md | 12 +++ .../src/object/native_call_method.rs | 2 + .../code_point_at_dispatch_tests.rs | 80 +++++++++++++++++++ .../native_call_method/string_methods.rs | 14 ++++ 4 files changed, 108 insertions(+) create mode 100644 changelog.d/9795-string-code-point-at-dispatch.md create mode 100644 crates/perry-runtime/src/object/native_call_method/code_point_at_dispatch_tests.rs diff --git a/changelog.d/9795-string-code-point-at-dispatch.md b/changelog.d/9795-string-code-point-at-dispatch.md new file mode 100644 index 0000000000..04cde8e0c0 --- /dev/null +++ b/changelog.d/9795-string-code-point-at-dispatch.md @@ -0,0 +1,12 @@ +### Runtime + +- perf(runtime): `String.prototype.codePointAt` is answered by the native + string-method dispatch instead of falling through to the primitive-method + fallback. It had a prototype thunk but no dispatch arm, so every call + resolved `globalThis.String.prototype.codePointAt`, cloned that closure to + rebind `this`, and — the thunk not being registered strict — ran `ToObject` + on the receiver, minting a `String` wrapper with an own index property per + UTF-16 code unit. Grapheme-aware text measurement calls it once per + character: on the compiled claude-code TUI it was the only method name + reaching the fallback, at 99,008 calls and 99,008 wrappers per 400-character + streamed reply (`PERRY_GC_DIAG=1`, `[gc-primitive-dispatch]`). diff --git a/crates/perry-runtime/src/object/native_call_method.rs b/crates/perry-runtime/src/object/native_call_method.rs index f05a2285a6..c7452709a7 100644 --- a/crates/perry-runtime/src/object/native_call_method.rs +++ b/crates/perry-runtime/src/object/native_call_method.rs @@ -18,6 +18,8 @@ mod primitive_methods; mod proto_dispatch; mod string_methods; +#[cfg(test)] +mod code_point_at_dispatch_tests; #[cfg(test)] mod dispatch_arg_coercion_tests; #[cfg(test)] diff --git a/crates/perry-runtime/src/object/native_call_method/code_point_at_dispatch_tests.rs b/crates/perry-runtime/src/object/native_call_method/code_point_at_dispatch_tests.rs new file mode 100644 index 0000000000..e1ce0b8cfc --- /dev/null +++ b/crates/perry-runtime/src/object/native_call_method/code_point_at_dispatch_tests.rs @@ -0,0 +1,80 @@ +//! #9761: `String.prototype.codePointAt` must be answered by the native +//! string-method dispatch, not by the primitive-method FALLBACK. +//! +//! The fallback (`call_primitive_builtin_prototype_method`) resolves +//! `globalThis.String.prototype[]`, clones that closure to rebind `this`, +//! and — because the resolved thunk is not registered strict — runs `ToObject` +//! on the receiver, minting a `String` wrapper with an own index property per +//! UTF-16 code unit. `codePointAt` had a prototype thunk but no dispatch arm, +//! and grapheme-aware text measurement calls it once per character: on the +//! compiled claude-code TUI it was the ONLY name reaching the fallback, at +//! 99,008 calls (and 99,008 wrappers) per 400-character streamed reply. +//! +//! The assertion is the wrapper count, not the return value: a test that only +//! checked the answer would pass with the arm deleted, because the fallback +//! computes the same number — expensively. `BOXED_PRIMITIVE_PAYLOADS` gains one +//! entry per wrapper, so "no new boxed primitives" is exactly "the fallback did +//! not run". + +use crate::value::JSValue; + +unsafe fn call_string_method(receiver: &str, method: &str, args: &[f64]) -> f64 { + let s = crate::string::js_string_from_bytes(receiver.as_ptr(), receiver.len() as u32); + let recv = f64::from_bits(JSValue::string_ptr(s).bits()); + super::js_native_call_method( + recv, + method.as_ptr() as *const i8, + method.len(), + if args.is_empty() { + std::ptr::null() + } else { + args.as_ptr() + }, + args.len(), + ) +} + +#[test] +fn code_point_at_dispatches_natively_and_boxes_no_receiver() { + unsafe { + // Warm anything the first dispatch installs, so the delta below is the + // method call itself and not one-time globalThis population. + let _ = call_string_method("ab", "charCodeAt", &[0.0]); + let before = crate::builtins::test_boxed_primitive_payload_count(); + + let cp = call_string_method("a", "codePointAt", &[0.0]); + assert_eq!(cp, 97.0, "codePointAt(0) of \"a\""); + let astral = call_string_method("\u{1F600}b", "codePointAt", &[0.0]); + assert_eq!(astral, 128512.0, "an astral pair is one code point"); + let past_end = call_string_method("a", "codePointAt", &[5.0]); + assert!( + JSValue::from_bits(past_end.to_bits()).is_undefined(), + "out of range is undefined" + ); + + assert_eq!( + crate::builtins::test_boxed_primitive_payload_count(), + before, + "the native arm must not mint a String wrapper; a non-zero delta \ + means the call fell through to the primitive-method fallback" + ); + } +} + +/// Positive control for the assertion above: the counter must be able to move, +/// or "no new boxed primitives" proves nothing. Minting the wrapper the +/// fallback would have minted is the direct, environment-independent form — +/// a second dispatch through a name without a native arm cannot serve as the +/// control here, because the unit-test thread has no populated `globalThis` +/// and the fallback returns before it boxes. +#[test] +fn the_wrapper_counter_moves_when_a_receiver_is_boxed() { + let before = crate::builtins::test_boxed_primitive_payload_count(); + let s = crate::string::js_string_from_bytes(b"abc".as_ptr(), 3); + let value = f64::from_bits(JSValue::string_ptr(s).bits()); + let _wrapper = crate::builtins::js_boxed_string_new(value, 1); + assert!( + crate::builtins::test_boxed_primitive_payload_count() > before, + "if this cannot move, the codePointAt assertion above is vacuous" + ); +} diff --git a/crates/perry-runtime/src/object/native_call_method/string_methods.rs b/crates/perry-runtime/src/object/native_call_method/string_methods.rs index cae30b6a76..d6213dee14 100644 --- a/crates/perry-runtime/src/object/native_call_method/string_methods.rs +++ b/crates/perry-runtime/src/object/native_call_method/string_methods.rs @@ -136,6 +136,20 @@ pub(super) unsafe fn dispatch_string( "charCodeAt" => { return Some(crate::string::js_string_char_code_at(s_ptr, arg_i32(0))); } + // #9761: `codePointAt` had a `String.prototype` thunk but no + // arm here, so it was the ONE method name the compiled + // claude-code TUI drove into the primitive-method FALLBACK: + // 99,008 calls per 400-character reply, each of which looked + // `globalThis.String` up, cloned the prototype closure to + // rebind `this`, and — because the callee is sloppy — ran + // `ToObject` on the receiver, minting a `String` wrapper with + // its own index property. Grapheme-aware text measurement + // calls it once per character, so a missing arm here is a + // per-character wrapper. It is the sibling of `charCodeAt` + // one line up and reads the same receiver the same way. + "codePointAt" => { + return Some(crate::string::js_string_code_point_at(s_ptr, arg_i32(0))); + } "slice" => { // Coerce args first (`arg_i32` may run user `valueOf` and move // the receiver under GC), then re-fetch the rooted receiver. From b7879e66834a2fa86c76c0befb6db8be109ce9b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 6 Sep 2026 11:12:32 +0200 Subject: [PATCH 23/26] style: cargo fmt --- crates/perry-runtime/src/string/tests.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/perry-runtime/src/string/tests.rs b/crates/perry-runtime/src/string/tests.rs index 68b2f2aae0..3b19dddbdb 100644 --- a/crates/perry-runtime/src/string/tests.rs +++ b/crates/perry-runtime/src/string/tests.rs @@ -1295,7 +1295,6 @@ mod split_empty_delimiter_code_units { }); assert_eq!(crate::array::js_array_length(arr), 3); } - } /// The canonical one-ASCII-character string table (`string::format`). From 3fd258698f972d915788e07897de4705101969e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 6 Sep 2026 11:48:39 +0200 Subject: [PATCH 24/26] fix(train): drop the duplicated canonical_char_cache test module Two byte-identical copies of the module reached the train tree, so `perry-runtime`'s test build failed with E0428. Keep one. --- crates/perry-runtime/src/string/tests.rs | 60 ------------------------ 1 file changed, 60 deletions(-) diff --git a/crates/perry-runtime/src/string/tests.rs b/crates/perry-runtime/src/string/tests.rs index 3b19dddbdb..eb9d6adcf7 100644 --- a/crates/perry-runtime/src/string/tests.rs +++ b/crates/perry-runtime/src/string/tests.rs @@ -1361,66 +1361,6 @@ mod canonical_char_cache { /// The canonical one-ASCII-character string table (`string::format`). #[cfg(test)] -mod canonical_char_cache { - use super::*; - - /// A one-ASCII-character string has exactly 128 possible contents, so - /// `js_string_char_at` (and everything that funnels through it: `s[i]`, - /// `charAt`, `[...s]`, the String-wrapper index installer) hands back the - /// canonical per-thread header instead of minting one per read. - /// - /// The identity assertion is the whole point — it is what makes the - /// allocation disappear — and it fails the moment the canonical table is - /// bypassed. The `refcount == 0` assertion is the safety half: a shared - /// header must never be eligible for the in-place append optimisation. - #[test] - fn ascii_char_at_returns_one_canonical_shared_header_per_byte() { - let scope = crate::gc::RuntimeHandleScope::new(); - let s = scope.root_string_ptr(js_string_from_bytes(b"abca".as_ptr(), 4)); - let (a0, b1, a3) = s.with_const_ptr::(|s| { - ( - js_string_char_at(s, 0), - js_string_char_at(s, 1), - js_string_char_at(s, 3), - ) - }); - assert_eq!(a0, a3, "the same character must reuse the canonical header"); - assert_ne!(a0, b1, "different characters are different headers"); - unsafe { - assert_eq!((*a0).byte_len, 1); - assert_eq!((*a0).utf16_len, 1); - let data = (a0 as *const u8).add(std::mem::size_of::()); - assert_eq!(*data, b'a'); - assert_eq!( - (*a0).refcount, - 0, - "a shared header must be ineligible for the in-place append path" - ); - } - // A second string with the same character resolves to the same header: - // the table is keyed by content, not by source string. - let other = scope.root_string_ptr(js_string_from_bytes(b"za".as_ptr(), 2)); - let a_again = other.with_const_ptr::(|o| js_string_char_at(o, 1)); - assert_eq!(a0, a_again); - } - - /// Non-ASCII keeps the minting path (the canonical table is ASCII-only), - /// and the value is still correct — the fast path must not answer for - /// characters it does not represent. - #[test] - fn non_ascii_char_at_is_unaffected_by_the_canonical_table() { - let scope = crate::gc::RuntimeHandleScope::new(); - let s = scope.root_string_ptr(js_string_from_bytes("aé".as_ptr(), 3)); - let (c0, c1) = s.with_const_ptr::(|s| { - (js_string_char_at(s, 0), js_string_char_at(s, 1)) - }); - unsafe { - assert_eq!((*c0).byte_len, 1); - assert_eq!((*c1).byte_len, 2, "é is two UTF-8 bytes"); - } - } -} - /// `header_str_checked` answers exactly like `from_utf8(..).ok()` — a pure /// ASCII key without the scan, a non-ASCII scalar key by validation, and a /// WTF-8 payload (lone surrogate) as `None`. From e17e86ed02fa24ad0ed8edb7b6f14db39ecedbb5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 6 Sep 2026 11:50:50 +0200 Subject: [PATCH 25/26] fix(train): carry #9838's PASS1_MARKED re-audit for the #9831 policy.rs change The structural JSON merge recomputed the census-window pin from the tree but did not carry the author's written re-audit. A pin whose hash tracks the tree while its justification lags is exactly the gap the pin exists to catch: the gate stays green and nobody has re-argued the window. --- scripts/gc_runtime_root_holders.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/gc_runtime_root_holders.json b/scripts/gc_runtime_root_holders.json index 39d54ebf42..bca48d8f59 100644 --- a/scripts/gc_runtime_root_holders.json +++ b/scripts/gc_runtime_root_holders.json @@ -276,7 +276,7 @@ "file": "crates/perry-runtime/src/gc/census.rs", "name": "PASS1_MARKED", "verdict": "non_moving_snapshot", - "why": "Real GC header addresses, deliberately untraced so the diagnostic does not keep its observed objects alive. Populated only at the end of mark propagation of a synchronous full cycle; consumed at sweep entry in the same run_to_completion invocation. The intervening full-cycle phases do not relocate or run JS callbacks. The Vec is used for membership comparisons and dropped with the census before sweep. Budgeted and minor cycles skip both boundaries. Pin re-audited 2026-09-05 after #9760 touched `gc/mod.rs`: that change is `mod heap_stats;` plus a `pub(crate) use` re-export and alters no mark/sweep control flow. `heap_stats()` is reached only from `js_bun_jsc_heap_stats` (the JS-facing `bun:jsc.heapStats()`), i.e. from mutator code, never inside a cycle, and its own module contract forbids allocation or collection during its walk. The mark-complete \u2192 sweep-entry window is unchanged. Re-audited 2026-09-05 (train125) after #9769 and #9771 touched pinned files. #9769 adds one `reg_scanner!` registration to `gc/mod.rs`; #9771 adds a feature-gated `alloc_census_init()` there and a feature-gated Rust-heap dump inside `take_census`. `alloc-census` is not in the default feature set, and decisively: `census_take_if_armed_at_full_sweep_start` does `PASS1_MARKED.with(|p| p.borrow_mut().take())` BEFORE calling `take_census`, so the snapshot has already left the thread-local by the time #9771's code runs \u2014 it cannot affect the window. Neither change alters mark/sweep control flow. Re-audited 2026-09-05 (train126) after #9755 restructured `gc/cycle.rs`. Its hunks are all root-scan machinery (`RootScanSubphase`, `RootScanCycleState`, the mutable-scanner iteration state), which runs BEFORE mark propagation completes; `gc/mod.rs` gains only a `mod young_log;` declaration. The bracketing is unchanged \u2014 `census_pass1_if_armed` is still inside `step_mark_propagation` and `census_take_if_armed_at_full_sweep_start` inside `step_sweep` \u2014 and a synchronous full mark-sweep still moves nothing between them. Re-audited 2026-09-05 for the retained array-growth verifier fix: the only cycle.rs change passes the existing non-copying evacuation verifier an explicit all-forwarded policy. That call remains in minor finalization, outside the synchronous full-cycle census window; its root/heap reads do not allocate GC objects, move objects, or invoke JS callbacks. The mark-complete and sweep-entry boundaries are unchanged. (A union merge briefly restored an older branch's pins for these files; they are recomputed from the tree here. #9822 does not modify any window file \u2014 its diff against them is empty \u2014 so the audits already recorded above still stand.)", + "why": "Real GC header addresses, deliberately untraced so the diagnostic does not keep its observed objects alive. Populated only at the end of mark propagation of a synchronous full cycle; consumed at sweep entry in the same run_to_completion invocation. The intervening full-cycle phases do not relocate or run JS callbacks. The Vec is used for membership comparisons and dropped with the census before sweep. Budgeted and minor cycles skip both boundaries. Pin re-audited 2026-09-05 after #9760 touched `gc/mod.rs`: that change is `mod heap_stats;` plus a `pub(crate) use` re-export and alters no mark/sweep control flow. `heap_stats()` is reached only from `js_bun_jsc_heap_stats` (the JS-facing `bun:jsc.heapStats()`), i.e. from mutator code, never inside a cycle, and its own module contract forbids allocation or collection during its walk. The mark-complete \u2192 sweep-entry window is unchanged. Re-audited 2026-09-05 (train125) after #9769 and #9771 touched pinned files. #9769 adds one `reg_scanner!` registration to `gc/mod.rs`; #9771 adds a feature-gated `alloc_census_init()` there and a feature-gated Rust-heap dump inside `take_census`. `alloc-census` is not in the default feature set, and decisively: `census_take_if_armed_at_full_sweep_start` does `PASS1_MARKED.with(|p| p.borrow_mut().take())` BEFORE calling `take_census`, so the snapshot has already left the thread-local by the time #9771's code runs \u2014 it cannot affect the window. Neither change alters mark/sweep control flow. Re-audited 2026-09-05 (train126) after #9755 restructured `gc/cycle.rs`. Its hunks are all root-scan machinery (`RootScanSubphase`, `RootScanCycleState`, the mutable-scanner iteration state), which runs BEFORE mark propagation completes; `gc/mod.rs` gains only a `mod young_log;` declaration. The bracketing is unchanged \u2014 `census_pass1_if_armed` is still inside `step_mark_propagation` and `census_take_if_armed_at_full_sweep_start` inside `step_sweep` \u2014 and a synchronous full mark-sweep still moves nothing between them. Re-audited 2026-09-06 after #9831 touched `gc/policy.rs`. Its hunks are (a) the tiny-parse pressure guard's pricing (`tiny_parse_pressure_headroom_bytes`, `tiny_parse_pressure_due*`, a `Cell` byte-count base) consulted from JSON.parse's mutator-side boundaries (`gc_bump_malloc_trigger`, `gc_collect_pending_suppressed_parse`, `gc_schedule_parse_boundary_collection_if_pressure`), none of which is reachable from inside a cycle, and (b) one extra `Cell` store in `note_collection_finished_arena_occupancy`, which runs from `publish_reclaim_outcome` in the Publish subphase \u2014 after `step_sweep` has already consumed the snapshot. Mark/sweep control flow between `census_pass1_if_armed` and `census_take_if_armed_at_full_sweep_start` is untouched.", "window": { "start": { "file": "crates/perry-runtime/src/gc/census.rs", From bb66131d5c8bd6047185210094d6bc42eaaeef8a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 6 Sep 2026 12:06:02 +0200 Subject: [PATCH 26/26] fix(train): drop the unused note_descriptor_target re-export `cargo check --all-targets -D warnings` fails on it: nothing in the workspace consumes `crate::object::note_descriptor_target`, only `descriptor_state.rs`'s own internal callers, which do not go through the re-export. --- crates/perry-runtime/src/object/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/perry-runtime/src/object/mod.rs b/crates/perry-runtime/src/object/mod.rs index 64c979d936..87dbfafc47 100644 --- a/crates/perry-runtime/src/object/mod.rs +++ b/crates/perry-runtime/src/object/mod.rs @@ -268,7 +268,7 @@ pub(crate) use descriptor_state::{ class_instance_set_may_intercept, clear_accessor_descriptor, clear_property_attrs, constructor_accessor_ever_installed, descriptors_in_use, disable_class_field_inline_guard, get_accessor_descriptor, get_property_attrs, install_fresh_accessor_property, - json_object_getter_value, mark_all_keys, note_descriptor_target, object_has_descriptors, + json_object_getter_value, mark_all_keys, object_has_descriptors, object_proto_may_intercept_key, owner_has_property_descriptors, owner_may_have_descriptor_entries, plain_data_write_may_intercept, prune_dead_descriptor_owner_entries, prune_dead_descriptor_owner_entries_young,