From 0d92005429ca0388ff6206eba88bfdf287eeac1b 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 1/3] 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 | 10 +- 4 files changed, 357 insertions(+), 9 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 0977d5e70a..830c6cc70c 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 @@ 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 ac6d08f01e..1dd69f6e99 100644 --- a/crates/perry-runtime/src/gc/tests/mod.rs +++ b/crates/perry-runtime/src/gc/tests/mod.rs @@ -60,6 +60,7 @@ pub(super) mod support; mod teardown; mod telemetry_verifier; mod temp_roots; +mod tiny_parse_pressure; mod triggers; mod typed_layout_intact_residual; mod u8_inline_cache; 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 b52c8629f5..0b2afbf149 100644 --- a/scripts/gc_runtime_root_holders.json +++ b/scripts/gc_runtime_root_holders.json @@ -270,7 +270,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.", + "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", @@ -288,7 +288,7 @@ "crates/perry-runtime/src/gc/census.rs": "8050a9d1ca15f783195ccfa5963089b60bc4a6c31d9ced5755537623e70e7e3c", "crates/perry-runtime/src/gc/cycle.rs": "763d552271b8e983a796b4e9648cd8ee984a0602b2b56aeefdb8713c0049c31f", "crates/perry-runtime/src/gc/mod.rs": "085c3dcde34a172aa2b96ee4500658abae77cd34ee7f0e2dfeee06ae5774a414", - "crates/perry-runtime/src/gc/policy.rs": "319ed42f1a985c88f6362657a08518077283fe5216d6055fc82343b34dec50f9", + "crates/perry-runtime/src/gc/policy.rs": "5626929989c5093109cd9322ad4ff3415bd1585ac33bfb2070b06cc4fa2a0207", "crates/perry-runtime/src/gc/progress.rs": "a5ad3971bbe4047229ca57325234780daa85921dbc778e1c08dff4ad07ccfb96" } } @@ -311,6 +311,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/trace.rs", "name": "FORWARDED_STUB_MEMBERSHIP_RECOVERIES", From 644b9d36247a53150edb2c3b301e2a964ad8b870 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 2/3] 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 bbdf2f983116091c4ac938ff99b26e41c0ab8e5f 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 3/3] 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 830c6cc70c..554a2bf947 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() } @@ -2673,7 +2896,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); @@ -2749,7 +2972,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, } @@ -2947,7 +3170,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); @@ -3381,6 +3604,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() { @@ -3474,8 +3698,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]