From 8668e0436fa6d7794a20995d930720d8d645436b 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 1/4] 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 ++ .../perry-runtime/src/arena/alloc_sample.rs | 269 +++++++++++++ crates/perry-runtime/src/arena/allocators.rs | 10 +- crates/perry-runtime/src/arena/block.rs | 2 +- crates/perry-runtime/src/arena/inline.rs | 15 +- crates/perry-runtime/src/arena/mod.rs | 1 + crates/perry-runtime/src/arena/promote.rs | 2 +- crates/perry-runtime/src/arena/quarantine.rs | 2 +- crates/perry-runtime/src/arena/reset.rs | 9 +- crates/perry-runtime/src/error.rs | 3 +- .../perry-runtime/src/error_stack_frames.rs | 54 +++ crates/perry-runtime/src/gc/copying.rs | 34 ++ crates/perry-runtime/src/gc/diag_sites.rs | 364 ++++++++++++++++++ crates/perry-runtime/src/gc/mod.rs | 6 + crates/perry-runtime/src/gc/policy.rs | 40 +- crates/perry-runtime/src/gc/survival_diag.rs | 240 ++++++++++++ crates/perry-runtime/src/gc/telemetry.rs | 34 ++ .../src/gc/tests/env_knob_parse.rs | 37 ++ crates/perry-runtime/src/gc/tests/mod.rs | 1 + .../src/gc/tests/survival_diag.rs | 143 +++++++ docs/src/internals/garbage-collector.md | 3 +- 21 files changed, 1268 insertions(+), 23 deletions(-) create mode 100644 changelog.d/gc-churn-attribution-diag.md create mode 100644 crates/perry-runtime/src/arena/alloc_sample.rs create mode 100644 crates/perry-runtime/src/gc/diag_sites.rs create mode 100644 crates/perry-runtime/src/gc/survival_diag.rs create mode 100644 crates/perry-runtime/src/gc/tests/survival_diag.rs 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`. diff --git a/crates/perry-runtime/src/arena/alloc_sample.rs b/crates/perry-runtime/src/arena/alloc_sample.rs new file mode 100644 index 0000000000..36d69ca059 --- /dev/null +++ b/crates/perry-runtime/src/arena/alloc_sample.rs @@ -0,0 +1,269 @@ +//! `PERRY_ALLOC_SITE_SAMPLE=`: byte-proportional allocation-site +//! sampling for the GC arena — WHAT allocates, by call chain and object type. +//! +//! The question it answers: a 3,300-character streamed reply in the compiled +//! claude-code TUI pushes ~890 MB through the nursery (86 copying minors) +//! while node allocates a small fraction of that for the same interaction. +//! Nothing in the runtime could say which JS operation, or which runtime +//! helper under it, produced the volume. This does, the way V8's sampling +//! heap profiler does: every `` of arena allocation, capture the +//! native return-address chain of the allocation that crossed the boundary. +//! Each sample stands for `` of allocation, so a site's share of the +//! samples is its share of the bytes, independent of its object size mix. +//! +//! Coverage: +//! +//! * every runtime allocation path — [`super::arena_alloc_gc`], +//! `arena_alloc_gc_no_collect`, the old-gen births, the longlived arena — +//! decrements a per-thread countdown (one relaxed atomic load when the +//! sampler is off, the only cost the default build pays); +//! * the codegen inline bump allocator never enters the runtime, so while +//! sampling is on the mirrored `InlineArenaState.size` is capped at +//! `offset + ` ([`inline_limit`]), and +//! every site that writes the inline offset back to its block charges the +//! inline bytes allocated since the last sync to the SAME countdown +//! ([`note_inline_sync`]). One countdown for both paths is what makes the +//! weighting exact: capping at `offset + interval` on every resync (the +//! first cut) let a loop that interleaves runtime and inline allocations +//! push the cap ahead forever — 29 samples for 29 MB of inline objects. +//! Inert when off — the cap is the real block size. +//! +//! Report: `[alloc-site] …` lines after each copying minor and at process +//! exit — totals, bytes by object type, and the top sites as an +//! innermost-first chain resolved to JS display names where a frame is +//! compiled user code (`crate::error::describe_chain`), else the linker +//! symbol. Cumulative since process start. + +use std::cell::{Cell, RefCell}; +use std::collections::HashMap; +use std::sync::atomic::{AtomicUsize, Ordering}; + +/// Sampling interval in bytes; 0 = off. Written once at `gc_init`. +static INTERVAL: AtomicUsize = AtomicUsize::new(0); + +/// The interval a bare `=1`/`on` selects; pinned by the knob test. +pub(crate) const DEFAULT_INTERVAL_BYTES: usize = 64 * 1024; +const DEPTH: usize = 6; +const TYPE_SLOTS: usize = 32; + +#[derive(Default, Clone)] +struct Site { + samples: u64, + sampled_bytes: u64, + by_type: [u32; TYPE_SLOTS], +} + +#[derive(Default)] +struct Table { + sites: HashMap<[usize; DEPTH], Site>, + samples: u64, + sampled_bytes: u64, + by_type: [u64; TYPE_SLOTS], + inline_trips: u64, +} + +crate::perry_thread_local! { + static UNTIL: Cell = const { Cell::new(0) }; + static TABLE: RefCell = RefCell::new(Table::default()); +} + +/// Read `PERRY_ALLOC_SITE_SAMPLE` once (from `gc_init`). A bare `1` or an +/// unparsable value selects the default interval. +pub(crate) fn init_from_env() { + let raw = std::env::var("PERRY_ALLOC_SITE_SAMPLE").ok(); + let interval = parse_interval(raw.as_deref()); + if interval == 0 { + return; + } + INTERVAL.store(interval, Ordering::Relaxed); + eprintln!("[alloc-site] sampling every {interval} bytes of arena allocation"); +} + +/// The knob's value semantics, as a pure function so the OFF state can be +/// pinned by `gc/tests/env_knob_parse.rs` without touching the process +/// environment (the shared GC-knob vocabulary, #7991): the boolean spellings +/// read through [`crate::gc::env_flag_from_value`] — `1`/`on`/`true`/`yes` +/// select [`DEFAULT_INTERVAL_BYTES`], every OFF spelling and every typo read +/// as OFF; an integer ≥ 2 is the interval in bytes, floored at +/// [`MIN_INTERVAL_BYTES`] so a stray small value cannot turn every allocation +/// into a stack walk. +pub(crate) fn parse_interval(raw: Option<&str>) -> usize { + if crate::gc::env_flag_from_value(raw) { + return DEFAULT_INTERVAL_BYTES; + } + raw.and_then(|r| r.trim().parse::().ok()) + .filter(|&v| v >= 2) + .map_or(0, |v| v.max(MIN_INTERVAL_BYTES)) +} + +/// Smallest interval an explicit integer can select. +pub(crate) const MIN_INTERVAL_BYTES: usize = 256; + +/// A runtime-path allocation of `total` bytes (header included) of +/// `obj_type` is about to happen. +#[inline(always)] +pub(crate) fn note(total: usize, obj_type: u8) { + let interval = INTERVAL.load(Ordering::Relaxed); + if interval == 0 { + return; + } + note_slow(total, obj_type, interval); +} + +/// Charge `bytes` to the countdown; true when a sample is due (the countdown +/// is then re-armed with a full interval). +#[inline] +fn countdown(bytes: usize, interval: usize) -> bool { + UNTIL.with(|u| { + let left = u.get(); + if left > bytes { + u.set(left - bytes); + false + } else { + u.set(interval); + true + } + }) +} + +#[cold] +#[inline(never)] +fn note_slow(total: usize, obj_type: u8, interval: usize) { + if countdown(total, interval) { + sample(total, obj_type, false); + } +} + +/// A site is writing the inline bump offset back to its arena block: +/// `inline_offset - block_offset` bytes were allocated by the compiled fast +/// path since the last sync. Charge them to the shared countdown. +#[inline(always)] +pub(crate) fn note_inline_sync(block_offset: usize, inline_offset: usize) { + let interval = INTERVAL.load(Ordering::Relaxed); + if interval == 0 || inline_offset <= block_offset { + return; + } + note_inline_slow(inline_offset - block_offset, interval); +} + +#[cold] +#[inline(never)] +fn note_inline_slow(bytes: usize, interval: usize) { + if countdown(bytes, interval) { + // The inline allocator only births class instances (`GC_TYPE_OBJECT` + // with a per-site header image); the size charged is the whole burst. + sample(bytes, crate::gc::GC_TYPE_OBJECT, true); + } +} + +/// While sampling, cap the mirrored inline block limit at the bytes left +/// before the next sample, so the compiled fast path returns to the runtime +/// (`js_inline_arena_slow_alloc`, whose write-back charges the burst) exactly +/// when a sample is due. Identity when off. +#[inline(always)] +pub(crate) fn inline_limit(offset: usize, block_size: usize) -> usize { + let interval = INTERVAL.load(Ordering::Relaxed); + if interval == 0 { + return block_size; + } + let left = UNTIL.with(Cell::get).max(1); + block_size.min(offset.saturating_add(left)) +} + +fn sample(total: usize, obj_type: u8, inline_trip: bool) { + let mut pcs = [0usize; crate::error::MAX_CAPTURED_FRAMES]; + let n = crate::error::capture_ips(&mut pcs); + // Frame 0 is the return into this sampler; frame 1 is the allocation + // helper (or, on the inline path, the write-back site), and the chain + // walks out to the compiled JS function that owns the allocation. + let mut key = [0usize; DEPTH]; + for (slot, pc) in key.iter_mut().zip(&pcs[1.min(n)..n]) { + *slot = *pc; + } + let t = (obj_type as usize).min(TYPE_SLOTS - 1); + TABLE.with(|table| { + let Ok(mut table) = table.try_borrow_mut() else { + return; + }; + table.samples += 1; + table.sampled_bytes += total as u64; + table.by_type[t] += 1; + if inline_trip { + table.inline_trips += 1; + } + let site = table.sites.entry(key).or_default(); + site.samples += 1; + site.sampled_bytes += total as u64; + site.by_type[t] += 1; + }); +} + +fn type_name(t: usize) -> &'static str { + crate::gc::gc_type_info(t as u8).map_or("?", |i| i.name) +} + +/// Print the cumulative histogram. `label` names the occasion. +pub(crate) fn report(label: &str) { + let interval = INTERVAL.load(Ordering::Relaxed); + if interval == 0 { + return; + } + TABLE.with(|table| { + let Ok(table) = table.try_borrow() else { + return; + }; + if table.samples == 0 { + return; + } + let est_total = table.samples * interval as u64; + eprintln!( + "[alloc-site] {label}: interval={interval} samples={} est_bytes={est_total} inline_trips={} sites={}", + table.samples, + table.inline_trips, + table.sites.len() + ); + let mut types: Vec<(usize, u64)> = table + .by_type + .iter() + .enumerate() + .filter(|(_, &c)| c > 0) + .map(|(t, &c)| (t, c)) + .collect(); + types.sort_by_key(|&(_, c)| std::cmp::Reverse(c)); + let mut line = String::from("[alloc-site] by-type:"); + for (t, c) in types { + line.push_str(&format!( + " {}={}MB", + type_name(t), + c * interval as u64 / (1024 * 1024) + )); + } + eprintln!("{line}"); + let mut sites: Vec<(&[usize; DEPTH], &Site)> = table.sites.iter().collect(); + sites.sort_by_key(|(_, s)| std::cmp::Reverse(s.samples)); + for (key, s) in sites.iter().take(30) { + let n = key.iter().position(|&p| p == 0).unwrap_or(DEPTH); + let mut top_types: Vec<(usize, u32)> = s + .by_type + .iter() + .enumerate() + .filter(|(_, &c)| c > 0) + .map(|(t, &c)| (t, c)) + .collect(); + top_types.sort_by_key(|&(_, c)| std::cmp::Reverse(c)); + let types: Vec = top_types + .iter() + .take(3) + .map(|(t, c)| format!("{}:{}%", type_name(*t), *c as u64 * 100 / s.samples)) + .collect(); + eprintln!( + "[alloc-site] est_bytes={} samples={} mean_obj={} types={} site={}", + s.samples * interval as u64, + s.samples, + s.sampled_bytes / s.samples, + types.join(","), + crate::error::describe_chain(&key[..n], 5) + ); + } + }); +} diff --git a/crates/perry-runtime/src/arena/allocators.rs b/crates/perry-runtime/src/arena/allocators.rs index a99232f485..70c4a2979c 100644 --- a/crates/perry-runtime/src/arena/allocators.rs +++ b/crates/perry-runtime/src/arena/allocators.rs @@ -28,6 +28,7 @@ pub fn arena_alloc(size: usize, align: usize) -> *mut u8 { let offset = (*inline_ptr).offset; let arena = &mut *arena_ptr; let current = arena.current; + super::alloc_sample::note_inline_sync(arena.blocks[current].offset, offset); arena.blocks[current].offset = offset; } let ptr = crate::arena::arena_cell_alloc(arena_ptr, size, align); @@ -41,7 +42,7 @@ pub fn arena_alloc(size: usize, align: usize) -> *mut u8 { let inline = &mut *inline_ptr; inline.data = data; inline.offset = offset; - inline.size = block_size; + inline.size = super::alloc_sample::inline_limit(offset, block_size); } ptr } @@ -78,6 +79,7 @@ pub(crate) fn arena_alloc_gc_no_collect(size: usize, align: usize, obj_type: u8) use crate::gc::{GcHeader, GC_FLAG_ARENA, GC_HEADER_SIZE}; let total = gc_padded_total_size(size, align); + super::alloc_sample::note(total, obj_type); // Old-gen birth walks page lists and can reserve — outside the contract. if crate::gc::is_large_object_total_size_for_type(total, obj_type) { return std::ptr::null_mut(); @@ -123,6 +125,7 @@ fn arena_alloc_no_collect(size: usize, align: usize) -> *mut u8 { let offset = (*inline_ptr).offset; let arena = &mut *arena_ptr; let current = arena.current; + super::alloc_sample::note_inline_sync(arena.blocks[current].offset, offset); arena.blocks[current].offset = offset; } let Some(ptr) = crate::arena::arena_cell_try_alloc_current(arena_ptr, size, align) else { @@ -137,7 +140,7 @@ fn arena_alloc_no_collect(size: usize, align: usize) -> *mut u8 { let inline = &mut *inline_ptr; inline.data = data; inline.offset = offset; - inline.size = block_size; + inline.size = super::alloc_sample::inline_limit(offset, block_size); } ptr } @@ -170,6 +173,7 @@ pub fn arena_alloc_gc_longlived(size: usize, align: usize, obj_type: u8) -> *mut // assumes this invariant. let pad = align.max(8); let total = (GC_HEADER_SIZE + size + pad - 1) & !(pad - 1); + super::alloc_sample::note(total, obj_type); let raw = arena_alloc_longlived(total, align); unsafe { @@ -275,6 +279,7 @@ pub fn arena_alloc_gc_old(size: usize, align: usize, obj_type: u8) -> *mut u8 { pub(crate) fn arena_alloc_gc_old_born_tenured(size: usize, align: usize, obj_type: u8) -> *mut u8 { use crate::gc::{GcHeader, GC_FLAG_TENURED, GC_HEADER_SIZE}; + super::alloc_sample::note(gc_padded_total_size(size, align), obj_type); let user_ptr = arena_alloc_gc_old(size, align, obj_type); unsafe { let header = user_ptr.sub(GC_HEADER_SIZE) as *mut GcHeader; @@ -422,6 +427,7 @@ pub fn arena_alloc_gc(size: usize, align: usize, obj_type: u8) -> *mut u8 { // (`shapes.ts` sat 16 bytes over the flat 16 KB line and re-marked 118 006 // slots per minor because of it). let total = gc_padded_total_size(size, align); + super::alloc_sample::note(total, obj_type); if crate::gc::is_large_object_total_size_for_type(total, obj_type) { let user_ptr = arena_alloc_gc_old(size, align, obj_type); unsafe { diff --git a/crates/perry-runtime/src/arena/block.rs b/crates/perry-runtime/src/arena/block.rs index 0e8a09e487..6908be38da 100644 --- a/crates/perry-runtime/src/arena/block.rs +++ b/crates/perry-runtime/src/arena/block.rs @@ -623,7 +623,7 @@ impl Arena { let block = &self.blocks[self.current]; inline.data = block.data; inline.offset = block.offset; - inline.size = block.size; + inline.size = super::alloc_sample::inline_limit(block.offset, block.size); } }); } diff --git a/crates/perry-runtime/src/arena/inline.rs b/crates/perry-runtime/src/arena/inline.rs index 09071378c8..c186516788 100644 --- a/crates/perry-runtime/src/arena/inline.rs +++ b/crates/perry-runtime/src/arena/inline.rs @@ -43,7 +43,7 @@ pub extern "C" fn js_inline_arena_state() -> *mut InlineArenaState { let block = &arena.blocks[arena.current]; state.data = block.data; state.offset = block.offset; - state.size = block.size; + state.size = super::alloc_sample::inline_limit(block.offset, block.size); } state as *mut InlineArenaState } @@ -82,6 +82,7 @@ pub extern "C" fn js_inline_arena_slow_alloc( { let arena = &mut *arena_ptr; let current = arena.current; + super::alloc_sample::note_inline_sync(arena.blocks[current].offset, offset); arena.blocks[current].offset = offset; } // Allocate via existing path (may push a new block + run GC). @@ -95,7 +96,7 @@ pub extern "C" fn js_inline_arena_slow_alloc( let state_ref = &mut *state; state_ref.data = data; state_ref.offset = block_offset; - state_ref.size = block_size; + state_ref.size = super::alloc_sample::inline_limit(block_offset, block_size); ptr }) } @@ -113,7 +114,9 @@ pub fn sync_inline_arena_state() { if !state.data.is_null() { ARENA.with(|a| { let arena = &mut *(*a).get(); - arena.blocks[arena.current].offset = state.offset; + let current = arena.current; + super::alloc_sample::note_inline_sync(arena.blocks[current].offset, state.offset); + arena.blocks[current].offset = state.offset; }); } }); @@ -135,7 +138,9 @@ pub fn arena_start_fresh_general_block() { ARENA.with(|a| { let arena = &mut *(*a).get(); if !inline.data.is_null() { - arena.blocks[arena.current].offset = inline.offset; + let current = arena.current; + super::alloc_sample::note_inline_sync(arena.blocks[current].offset, inline.offset); + arena.blocks[current].offset = inline.offset; } if arena.blocks[arena.current].offset < FRESH_GENERAL_BLOCK_MIN_USED_BYTES { return; @@ -145,7 +150,7 @@ pub fn arena_start_fresh_general_block() { let block = &arena.blocks[arena.current]; inline.data = block.data; inline.offset = block.offset; - inline.size = block.size; + inline.size = super::alloc_sample::inline_limit(block.offset, block.size); } }); }); diff --git a/crates/perry-runtime/src/arena/mod.rs b/crates/perry-runtime/src/arena/mod.rs index 9ba5401b19..59ffb4cac6 100644 --- a/crates/perry-runtime/src/arena/mod.rs +++ b/crates/perry-runtime/src/arena/mod.rs @@ -8,6 +8,7 @@ pub(crate) use std::alloc::{alloc, Layout}; pub(crate) use std::cell::{Cell, RefCell, UnsafeCell}; pub(crate) use std::collections::hash_map::Entry; +pub(crate) mod alloc_sample; mod allocators; mod block; mod inline; diff --git a/crates/perry-runtime/src/arena/promote.rs b/crates/perry-runtime/src/arena/promote.rs index 9976125ba9..12debf1b57 100644 --- a/crates/perry-runtime/src/arena/promote.rs +++ b/crates/perry-runtime/src/arena/promote.rs @@ -594,7 +594,7 @@ fn reset_young_after_promotion() { let block = &arena.blocks[arena.current]; inline.data = block.data; inline.offset = block.offset; - inline.size = block.size; + inline.size = super::alloc_sample::inline_limit(block.offset, block.size); } }); }); diff --git a/crates/perry-runtime/src/arena/quarantine.rs b/crates/perry-runtime/src/arena/quarantine.rs index 3db7280834..37986e7e06 100644 --- a/crates/perry-runtime/src/arena/quarantine.rs +++ b/crates/perry-runtime/src/arena/quarantine.rs @@ -574,7 +574,7 @@ pub(crate) fn copying_quarantine_from_spaces_and_flip() -> ArenaResetStats { let block = &arena.blocks[arena.current]; inline.data = block.data; inline.offset = block.offset; - inline.size = block.size; + inline.size = super::alloc_sample::inline_limit(block.offset, block.size); } }); }); diff --git a/crates/perry-runtime/src/arena/reset.rs b/crates/perry-runtime/src/arena/reset.rs index a5ba37fa86..e53918b18d 100644 --- a/crates/perry-runtime/src/arena/reset.rs +++ b/crates/perry-runtime/src/arena/reset.rs @@ -30,7 +30,7 @@ pub fn arena_reset_all_blocks_to_zero() { let block = &arena.blocks[0]; inline.data = block.data; inline.offset = 0; - inline.size = block.size; + inline.size = super::alloc_sample::inline_limit(0, block.size); } }); }); @@ -221,7 +221,7 @@ pub(crate) fn copying_reset_from_spaces_and_flip() -> ArenaResetStats { let block = &arena.blocks[arena.current]; inline.data = block.data; inline.offset = block.offset; - inline.size = block.size; + inline.size = super::alloc_sample::inline_limit(block.offset, block.size); } }); }); @@ -496,7 +496,7 @@ pub fn arena_reset_empty_blocks(block_has_live: &[bool]) -> ArenaResetStats { if !block.data.is_null() { inline.data = block.data; inline.offset = block.offset; - inline.size = block.size; + inline.size = super::alloc_sample::inline_limit(block.offset, block.size); } } }); @@ -808,7 +808,8 @@ impl ArenaResetEmptyBlocksState { if !block.data.is_null() { inline.data = block.data; inline.offset = block.offset; - inline.size = block.size; + inline.size = + super::alloc_sample::inline_limit(block.offset, block.size); } } } diff --git a/crates/perry-runtime/src/error.rs b/crates/perry-runtime/src/error.rs index e6a55d645f..8542c6b3dd 100644 --- a/crates/perry-runtime/src/error.rs +++ b/crates/perry-runtime/src/error.rs @@ -1936,7 +1936,8 @@ static KEEP_ERROR_IS_ERROR: extern "C" fn(f64) -> f64 = js_error_is_error; #[path = "error_stack_frames.rs"] mod stack_frames; pub(crate) use stack_frames::{ - capture_frames_payload, frames_payload_to_lines, materialize_error_stack, + capture_frames_payload, capture_ips, describe_chain, frames_payload_to_lines, + materialize_error_stack, MAX_CAPTURED_FRAMES, }; #[path = "error_subclass_stack.rs"] diff --git a/crates/perry-runtime/src/error_stack_frames.rs b/crates/perry-runtime/src/error_stack_frames.rs index 1f062cb14e..ba70ccc5f5 100644 --- a/crates/perry-runtime/src/error_stack_frames.rs +++ b/crates/perry-runtime/src/error_stack_frames.rs @@ -321,6 +321,60 @@ pub(crate) fn capture_encoded() -> ([u8; MAX_CAPTURED_FRAMES * PC_CHARS], usize) (blob, len) } +/// Raw return addresses of the current native stack, innermost first, for +/// the GC's site-attribution diagnostics (`gc/diag_sites.rs`, the +/// `PERRY_ALLOC_SITE_SAMPLE` sampler). Same walk as `capture_encoded`, no +/// encoding. +pub(crate) fn capture_ips(out: &mut [usize; MAX_CAPTURED_FRAMES]) -> usize { + walk::capture(out) +} + +/// Best-effort one-line description of a code address for diagnostics: the +/// registered JS display name when `ip` is inside a compiled user function, +/// else the nearest linker symbol (`dladdr`), else the bare address. Never +/// called on a hot path — the JS-name index takes a lock and may rebuild. +pub(crate) fn describe_ip(ip: usize) -> String { + let js = with_index(|index| { + name_for_ip(index, ip.saturating_sub(1)) + .and_then(|n| std::str::from_utf8(n).ok().map(|s| s.to_string())) + }) + .flatten(); + if let Some(name) = js.filter(|n| !n.is_empty()) { + return format!("js:{name}"); + } + #[cfg(unix)] + { + let mut info: libc::Dl_info = unsafe { std::mem::zeroed() }; + // SAFETY: `dladdr` only reads the address and fills `info`. + if unsafe { libc::dladdr(ip as *const libc::c_void, &mut info) } != 0 + && !info.dli_sname.is_null() + { + let name = unsafe { std::ffi::CStr::from_ptr(info.dli_sname) }.to_string_lossy(); + let off = ip.saturating_sub(info.dli_saddr as usize); + let mut n = name.into_owned(); + if n.len() > 72 { + n.truncate(72); + } + return format!("{n}+{off:#x}"); + } + } + format!("{ip:#x}") +} + +/// `describe_ip` for a chain, innermost first, skipping frames inside `skip` +/// (a set of symbol-name substrings the caller considers plumbing). Returns +/// up to `max` descriptions joined by ` < `. +pub(crate) fn describe_chain(pcs: &[usize], max: usize) -> String { + let mut out = Vec::with_capacity(max); + for &pc in pcs { + if out.len() >= max { + break; + } + out.push(describe_ip(pc)); + } + out.join(" < ") +} + // --------------------------------------------------------------------------- // Resolution: address -> JS display name. // --------------------------------------------------------------------------- diff --git a/crates/perry-runtime/src/gc/copying.rs b/crates/perry-runtime/src/gc/copying.rs index 6c53d857e8..1be599b0ce 100644 --- a/crates/perry-runtime/src/gc/copying.rs +++ b/crates/perry-runtime/src/gc/copying.rs @@ -170,6 +170,8 @@ pub(super) struct CopyingNurseryCollector { /// skipped. `debug_assert_no_remembering_possible` re-derives the premise at /// runtime in debug builds. pub(super) skip_remembering: bool, + /// `PERRY_GC_DIAG=1`: per-minor survival attribution (gc/survival_diag.rs). + pub(super) survival: Option>, /// Weak target slots (WeakRef referent / WeakMap-WeakSet entry key / /// FinalizationRegistry record target) seen during the copy scan. The /// scan must NOT evacuate through them (that would strengthen the weak @@ -242,12 +244,22 @@ impl CopyingNurseryCollector { live_from_bytes: 0, tenuring_survivals, skip_remembering: false, + survival: crate::gc::gc_diag_enabled() + .then(|| Box::new(super::survival_diag::SurvivalDiag::new())), weak_slots: Vec::new(), memo_addr: 0, memo_result: 0, } } + /// Mirror a `worklist.push` into the survival diag's origin vector. + #[inline] + fn survival_push(&mut self) { + if let Some(d) = self.survival.as_mut() { + d.note_worklist_push(); + } + } + pub(super) unsafe fn record_large_excluded(&mut self, header: *mut GcHeader) { if header.is_null() { return; @@ -390,6 +402,7 @@ impl CopyingNurseryCollector { if flags & (GC_FLAG_MARKED | GC_FLAG_PINNED) == 0 { (*ptr.header).gc_flags = flags | GC_FLAG_MARKED; self.worklist.push(ptr.header); + self.survival_push(); self.marked_headers.push(ptr.header); } } @@ -432,6 +445,10 @@ impl CopyingNurseryCollector { (*header).gc_flags = flags | GC_FLAG_MARKED; let total = (*header).size as usize; self.worklist.push(header); + self.survival_push(); + if let Some(d) = self.survival.as_mut() { + d.record((*header).obj_type, total, true); + } self.moved_headers.push(header); self.stats.promoted_objects += 1; self.stats.promoted_bytes += total; @@ -550,6 +567,10 @@ impl CopyingNurseryCollector { gc_type_after_payload_move((*header).obj_type, old_user as usize, new_user as usize); self.worklist.push(new_header); + self.survival_push(); + if let Some(d) = self.survival.as_mut() { + d.record((*new_header).obj_type, total, promote); + } self.moved_headers.push(new_header); self.live_from_bytes += total; if promote { @@ -633,11 +654,17 @@ impl CopyingNurseryCollector { } let header = self.worklist[i]; i += 1; + if let Some(d) = self.survival.as_mut() { + d.begin_drain_entry(i - 1); + } if (*header).gc_flags & GC_FLAG_FORWARDED != 0 { continue; } self.scan_object_fields(header); } + if let Some(d) = self.survival.as_mut() { + d.end_drain(); + } } /// Second pass over the weak target slots collected during the scan: @@ -1348,6 +1375,9 @@ pub(super) fn run_copied_minor_attempt( let remembered_stats = scan_remembered_dirty_slots_copying( &snapshot, |slot, header, external, stats| unsafe { + if let Some(d) = collector.survival.as_mut() { + d.remembered_parent_type = (*header).obj_type; + } let before = *slot; collector.visit_slot_with_parent(slot, header, external); if *slot != before { @@ -1788,6 +1818,10 @@ pub(super) fn run_copied_minor_attempt( super::policy::GC_AT_DECLARED_SAFEPOINT.with(std::cell::Cell::get) ); } + if let Some(d) = collector.survival.as_ref() { + d.report(super::survival_diag::next_minor_seq()); + } + crate::arena::alloc_sample::report("minor"); report_forwarding_refusals("copying_minor"); super::scanner_profile::report_and_reset("copying_minor"); CopiedMinorAttempt::Done(Some(CopiedMinorFastPathOutcome { diff --git a/crates/perry-runtime/src/gc/diag_sites.rs b/crates/perry-runtime/src/gc/diag_sites.rs new file mode 100644 index 0000000000..57a1771641 --- /dev/null +++ b/crates/perry-runtime/src/gc/diag_sites.rs @@ -0,0 +1,364 @@ +//! `PERRY_GC_DIAG=1`: WHY a collection was decided, WHICH arm ran a full +//! mark-sweep, and WHAT the budgeted collector charged the mutator per cycle +//! and per charge site. +//! +//! The per-cycle diag lines (`[gc-copy-minor]`, `[gc-step]`, `[gc]`) report +//! what a collection did; none of them says why it was scheduled. On the +//! compiled claude-code TUI a 400-character streamed reply cost 42 copying +//! minors — most of them over a nearly empty Eden — plus ten back-to-back +//! synchronous full mark-sweeps from the allocation-point old-reclaim arm, +//! and the only way to tell which predicate fired was to re-derive every input +//! by hand. These lines print the inputs at the decision: +//! +//! * `[gc-trigger] site=… kind=…` — every predicate input the trigger policy +//! reads (`arena_total` vs the armed base trigger, from-space occupancy vs +//! the nursery cap, old-gen reclaimable pressure vs its baseline and band, +//! the malloc-count pair, the pending/retaining flags), emitted at each +//! site that decides to collect. +//! * `[gc-full] site=… trigger=…` — one line per full mark-sweep, naming the +//! arm that started it (`alloc_point_old_reclaim`, `safepoint_old_reclaim`, +//! `budgeted`, `manual`, …) with a running per-site count. +//! * `[gc-budgeted] start|done …` — one pair per budgeted (incremental) cycle: +//! trigger, full/minor, how many steps drove it, the wall time of those +//! steps split by cycle phase, and the root-scan share of the total. +//! * `[gc-charge] …` — the mutator-assist and synchronous-full work charged +//! to each calling site (return-address chain, resolved to the JS display +//! name where the frame is compiled user code), so "which JS operation is +//! paying for the collector" is a counter rather than a profile guess. +//! +//! Everything here is gated on [`gc_diag_enabled`] and costs one cached-bool +//! read when off. + +use super::*; +use std::collections::HashMap; +use std::time::Instant; + +/// Print the predicate inputs behind a collection decision. +pub(super) fn trigger_decision(site: &'static str, kind: &'static str) { + if !gc_diag_enabled() { + return; + } + let arena_total = crate::arena::arena_total_bytes(); + let next_base = policy::next_arena_trigger_base(); + let armed = policy::GC_TRIGGER_ARMED.with(Cell::get); + let from_space = crate::arena::copying_from_space_in_use_bytes(); + let nursery_cap = tenuring::scavenge_nursery_cap_effective_bytes(); + let old_reclaimable = policy::old_gen_reclaimable_pressure_bytes(); + let external = policy::external_side_live_bytes(); + let old_baseline = policy::GC_LAST_OLD_RECLAIM_IN_USE_BYTES.with(Cell::get); + let old_band = policy::gc_old_reclaim_growth_band_bytes(old_baseline); + let old_threshold = gc_old_gen_reclaim_threshold_dyn_bytes(); + let old_pending = policy::GC_OLD_RECLAIM_PENDING.with(Cell::get); + let retaining = policy::GC_MAJOR_PACING_RETAINING.with(Cell::get); + let malloc = malloc_object_count(); + let next_malloc = policy::GC_NEXT_MALLOC_TRIGGER.with(Cell::get); + let old_in_use = crate::arena::old_gen_in_use_bytes(); + let old_free = old_free_bytes(); + eprintln!( + "[gc-trigger] site={site} kind={kind} arena_total={arena_total} next_base={next_base} armed={armed} \ + from_space={from_space} nursery_cap={nursery_cap} old_in_use={old_in_use} old_free={old_free} \ + old_reclaimable={old_reclaimable} external_side={external} old_baseline={old_baseline} \ + old_band={old_band} old_threshold={old_threshold} old_pending={old_pending} retaining={retaining} \ + malloc={malloc} next_malloc={next_malloc}" + ); +} + +crate::perry_thread_local! { + /// Label the arm that is about to run a synchronous full leaves for the + /// chokepoint (`gc_collect_full_mark_sweep_with_trigger`) to consume. + static FULL_SITE: Cell> = const { Cell::new(None) }; +} + +/// Name the arm behind the next synchronous full mark-sweep. +pub(super) fn set_full_site(site: &'static str) { + FULL_SITE.with(|s| s.set(Some(site))); +} + +/// Consume the pending arm label; `sync` when none was set (manual `gc()`, +/// emergency, escalation). +pub(super) fn take_full_site() -> &'static str { + FULL_SITE.with(|s| s.take()).unwrap_or("sync") +} + +crate::perry_thread_local! { + static FULL_SITE_COUNTS: RefCell> = const { RefCell::new(Vec::new()) }; + static BUDGETED: RefCell> = const { RefCell::new(None) }; + static CHARGES: RefCell> = RefCell::new(HashMap::new()); +} + +/// Test-only: how many synchronous fulls `full_started` counted at `site`. +#[cfg(test)] +pub(super) fn test_full_site_count(site: &str) -> u32 { + FULL_SITE_COUNTS.with(|c| { + c.borrow() + .iter() + .find(|(s, _)| *s == site) + .map_or(0, |(_, n)| *n) + }) +} + +/// Test-only: `(calls, units, us, fulls, minors)` of every charge row. +#[cfg(test)] +pub(super) fn test_charge_rows() -> Vec<(u64, u64, u64, u64, u64)> { + CHARGES.with(|c| { + c.borrow() + .values() + .map(|r| (r.calls, r.units, r.us, r.fulls, r.minors)) + .collect() + }) +} + +/// Test-only: `(steps, step_us, units, root_scan_us)` of the last completed +/// budgeted cycle's accounting. +#[cfg(test)] +pub(super) fn test_last_budgeted() -> Option<(u64, u64, u64, u64)> { + LAST_BUDGETED.with(Cell::get) +} + +#[cfg(test)] +crate::perry_thread_local! { + static LAST_BUDGETED: Cell> = const { Cell::new(None) }; +} + +/// One full mark-sweep is starting from `site`. +pub(super) fn full_started(site: &'static str, trigger: GcTriggerKind) { + if !gc_diag_enabled() { + return; + } + let count = FULL_SITE_COUNTS.with(|c| { + let mut c = c.borrow_mut(); + if let Some(entry) = c.iter_mut().find(|(s, _)| *s == site) { + entry.1 += 1; + entry.1 + } else { + c.push((site, 1)); + 1 + } + }); + eprintln!( + "[gc-full] site={site} trigger={trigger:?} count_at_site={count} old_reclaimable={} old_baseline={}", + policy::old_gen_reclaimable_pressure_bytes(), + policy::GC_LAST_OLD_RECLAIM_IN_USE_BYTES.with(Cell::get) + ); +} + +/// `GcCyclePhase::ffi_code()` runs 1..=8; index by it directly. +const PHASE_SLOTS: usize = 9; + +struct BudgetedCycleDiag { + trigger: GcTriggerKind, + collection: &'static str, + progress: &'static str, + started: Instant, + steps: u64, + step_us: u64, + units: u64, + phase_us: [u64; PHASE_SLOTS], + phase_steps: [u64; PHASE_SLOTS], +} + +/// A budgeted cycle was just installed as the active cycle. +pub(super) fn budgeted_started( + trigger: GcTriggerKind, + collection: GcCollectionKind, + progress: GcProgressKind, +) { + if !gc_diag_enabled() { + return; + } + let collection = match collection { + GcCollectionKind::Full => "full", + GcCollectionKind::Minor => "minor", + }; + eprintln!( + "[gc-budgeted] start trigger={trigger:?} kind={collection} progress={} old_reclaimable={} old_baseline={} arena_total={}", + progress.as_str(), + policy::old_gen_reclaimable_pressure_bytes(), + policy::GC_LAST_OLD_RECLAIM_IN_USE_BYTES.with(Cell::get), + crate::arena::arena_total_bytes() + ); + BUDGETED.with(|b| { + *b.borrow_mut() = Some(BudgetedCycleDiag { + trigger, + collection, + progress: progress.as_str(), + started: Instant::now(), + steps: 0, + step_us: 0, + units: 0, + phase_us: [0; PHASE_SLOTS], + phase_steps: [0; PHASE_SLOTS], + }); + }); +} + +/// One budgeted step ran: `phase_before` is the phase it started in. +pub(super) fn budgeted_step_done(phase_code: u32, elapsed_us: u64, units: usize) { + if !gc_diag_enabled() { + return; + } + BUDGETED.with(|b| { + if let Some(d) = b.borrow_mut().as_mut() { + d.steps += 1; + d.step_us += elapsed_us; + d.units = d.units.saturating_add(units as u64); + let slot = (phase_code as usize).min(PHASE_SLOTS - 1); + d.phase_us[slot] += elapsed_us; + d.phase_steps[slot] += 1; + } + }); +} + +/// The active budgeted cycle completed and was rebaselined. +pub(super) fn budgeted_completed(freed_bytes: u64) { + if !gc_diag_enabled() { + return; + } + let Some(d) = BUDGETED.with(|b| b.borrow_mut().take()) else { + return; + }; + #[cfg(test)] + LAST_BUDGETED.with(|c| c.set(Some((d.steps, d.step_us, d.units, d.phase_us[2])))); + const NAMES: [&str; PHASE_SLOTS] = [ + "?", + "build_valid_ptrs", + "root_scan", + "mark", + "block_persist", + "atomic_finalize", + "sweep", + "reclaim", + "complete", + ]; + let mut phases = String::new(); + for (name, (us, steps)) in NAMES + .iter() + .zip(d.phase_us.iter().zip(d.phase_steps.iter())) + .skip(1) + { + if *steps == 0 { + continue; + } + phases.push_str(&format!(" {name}={us}us/{steps}steps")); + } + let root_share = (d.phase_us[2] * 1000).checked_div(d.step_us).unwrap_or(0); + eprintln!( + "[gc-budgeted] done trigger={:?} kind={} progress={} steps={} step_us={} units={} wall_us={} freed={} root_scan_permille={} phases:{}", + d.trigger, + d.collection, + d.progress, + d.steps, + d.step_us, + d.units, + d.started.elapsed().as_micros(), + freed_bytes, + root_share, + phases + ); + report_charges("budgeted-done"); +} + +/// Return-address chain depth kept per charge site. Frame 0 is the caller of +/// `gc_check_trigger` (the allocator or `js_json_parse`); the next ones walk +/// out to the compiled JS function that issued the allocation. +const CHARGE_DEPTH: usize = 6; + +#[derive(Default, Clone, Copy)] +struct Charge { + calls: u64, + units: u64, + us: u64, + minors: u64, + fulls: u64, +} + +/// Wraps one `gc_check_trigger` arm: captures the caller chain on `begin` +/// (only under the diag), and on `end` charges the elapsed time and the work +/// units it drove to that chain. +pub(super) struct ChargeProbe { + pcs: [usize; crate::error::MAX_CAPTURED_FRAMES], + n: usize, + started: Option, +} + +impl ChargeProbe { + #[inline] + pub(super) fn begin() -> Self { + let mut probe = Self { + pcs: [0; crate::error::MAX_CAPTURED_FRAMES], + n: 0, + started: None, + }; + if gc_diag_enabled() { + probe.n = crate::error::capture_ips(&mut probe.pcs); + probe.started = Some(Instant::now()); + } + probe + } + + /// `kind`: what the arm did — `assist` (budgeted step), `sync_full`, + /// `direct_minor`. + pub(super) fn end(self, units: usize, kind: ChargeKind) { + let Some(started) = self.started else { + return; + }; + let us = started.elapsed().as_micros().min(u128::from(u64::MAX)) as u64; + let mut key = [0usize; CHARGE_DEPTH]; + // Skip frame 0: it is the return into `gc_check_trigger` itself. + let chain = &self.pcs[1.min(self.n)..self.n]; + for (slot, pc) in key.iter_mut().zip(chain) { + *slot = *pc; + } + CHARGES.with(|c| { + let mut c = c.borrow_mut(); + let e = c.entry(key).or_default(); + e.calls += 1; + e.units = e.units.saturating_add(units as u64); + e.us += us; + match kind { + ChargeKind::Assist => {} + ChargeKind::SyncFull => e.fulls += 1, + ChargeKind::DirectMinor => e.minors += 1, + } + }); + } +} + +#[derive(Clone, Copy)] +pub(super) enum ChargeKind { + Assist, + SyncFull, + DirectMinor, +} + +/// Print the heaviest charge sites since the last report, then reset. +pub(super) fn report_charges(label: &str) { + if !gc_diag_enabled() { + return; + } + let rows: Vec<([usize; CHARGE_DEPTH], Charge)> = + CHARGES.with(|c| c.borrow_mut().drain().collect()); + if rows.is_empty() { + return; + } + let total_us: u64 = rows.iter().map(|(_, r)| r.us).sum(); + let total_calls: u64 = rows.iter().map(|(_, r)| r.calls).sum(); + let mut rows = rows; + rows.sort_by_key(|(_, r)| std::cmp::Reverse(r.us)); + eprintln!( + "[gc-charge] {label}: sites={} calls={total_calls} total_us={total_us}", + rows.len() + ); + for (key, r) in rows.iter().take(12) { + let n = key.iter().position(|&p| p == 0).unwrap_or(CHARGE_DEPTH); + eprintln!( + "[gc-charge] us={} calls={} units={} fulls={} minors={} site={}", + r.us, + r.calls, + r.units, + r.fulls, + r.minors, + crate::error::describe_chain(&key[..n], 5) + ); + } +} diff --git a/crates/perry-runtime/src/gc/mod.rs b/crates/perry-runtime/src/gc/mod.rs index 6a7424daca..c5fceacae0 100644 --- a/crates/perry-runtime/src/gc/mod.rs +++ b/crates/perry-runtime/src/gc/mod.rs @@ -158,11 +158,13 @@ mod prefetch; mod copying; mod copying_first_cycle; mod copying_pointer_set; +mod diag_sites; /// #8174: shared validation for the TARGET of a forwarding pointer. mod forwarding; /// Per-scanner root attribution for the copied-minor root scan (#7915). mod scanner_profile; mod sticky_remembered; +mod survival_diag; use copying::*; use copying_first_cycle::*; // Named rather than glob-imported: a glob does not propagate through the @@ -794,6 +796,7 @@ fn gc_collect_full_mark_sweep_with_trigger(trigger: GcTriggerSnapshot) -> GcColl let _contract_heal = policy::contract_scan_heal_guard(); gc_drain_active_budgeted_cycle(); GC_TRIGGER_BUMPED.with(|c| c.set(false)); + diag_sites::full_started(diag_sites::take_full_site(), trigger.kind); GcCycleState::new_full(trigger).run_to_completion() } @@ -943,6 +946,7 @@ pub fn gc_init() { census::census_on_gc_init(); #[cfg(feature = "alloc-census")] crate::alloc_census::alloc_census_init(); + crate::arena::alloc_sample::init_from_env(); reg_budgeted_scanner!( scan_runtime_handle_roots_mut, scan_runtime_handle_roots_mut_step, @@ -1331,6 +1335,8 @@ pub extern "C" fn js_gc_release_current_thread_collection_side_allocations() { // once-only when the mode is off. schedule::report_exit_summary(); crate::r#box::report_box_stats_at_exit(); + crate::arena::alloc_sample::report("exit"); + diag_sites::report_charges("exit"); emit_incremental_liveness_diag(); emit_schedule_liveness_verdict(); } diff --git a/crates/perry-runtime/src/gc/policy.rs b/crates/perry-runtime/src/gc/policy.rs index 0977d5e70a..7088e39b65 100644 --- a/crates/perry-runtime/src/gc/policy.rs +++ b/crates/perry-runtime/src/gc/policy.rs @@ -2350,6 +2350,9 @@ pub fn gc_check_trigger() { { let _reentry = OldReclaimReentryGuard::enter(); GC_OLD_RECLAIM_PENDING.with(|pending| pending.set(false)); + super::diag_sites::trigger_decision("alloc_point", "OldReclaim"); + super::diag_sites::set_full_site("alloc_point_old_reclaim"); + let probe = super::diag_sites::ChargeProbe::begin(); let _scan = super::roots::ManualGcScanGuard::force_full_scan( super::ConservativeScanSite::OldReclaimAllocPoint, ); @@ -2357,6 +2360,7 @@ pub fn gc_check_trigger() { GcTriggerKind::OldGenBytes, )) .emit_after_current(); + probe.end(0, super::diag_sites::ChargeKind::SyncFull); return; } @@ -2476,6 +2480,8 @@ pub fn gc_check_trigger() { } let pre_in_use = crate::arena::arena_in_use_bytes(); let pre_malloc_count = malloc_object_count(); + super::diag_sites::trigger_decision("alloc_point_slack", "nursery"); + let probe = super::diag_sites::ChargeProbe::begin(); // THE ALLOC POINT IS REGISTER-IMPRECISE, SO THIS MINOR MUST NOT // MOVE. Unconditional, and the unconditionality is the fix for // #7682. @@ -2557,6 +2563,7 @@ pub fn gc_check_trigger() { gc_finish_arena_trigger_collection(pre_in_use, outcome); } } + probe.end(0, super::diag_sites::ChargeKind::DirectMinor); return; } } @@ -2565,10 +2572,11 @@ pub fn gc_check_trigger() { return; } - let _ = gc_mutator_assist_step_work_units_inner_with_progress( - gc_mutator_assist_scaled_work_units(), - GcProgressKind::MutatorAssist, - ); + let units = gc_mutator_assist_scaled_work_units(); + let probe = super::diag_sites::ChargeProbe::begin(); + let _ = + gc_mutator_assist_step_work_units_inner_with_progress(units, GcProgressKind::MutatorAssist); + probe.end(units, super::diag_sites::ChargeKind::Assist); } /// Debt-proportional assist pacing (#6180 Stage 2, measured 2026-07-10). @@ -2795,6 +2803,8 @@ pub(crate) fn gc_safepoint_moving_minor() -> bool { } let _reentry = OldReclaimReentryGuard::enter(); GC_OLD_RECLAIM_PENDING.with(|pending| pending.set(false)); + super::diag_sites::trigger_decision("safepoint", "OldReclaim"); + super::diag_sites::set_full_site("safepoint_old_reclaim"); // No `force_full_scan`: roots are precise at this safepoint. gc_collect_full_mark_sweep_with_trigger(GcTriggerSnapshot::capture( GcTriggerKind::OldGenBytes, @@ -2821,6 +2831,13 @@ pub(crate) fn gc_safepoint_moving_minor() -> bool { }; let pre_in_use = crate::arena::arena_in_use_bytes(); let pre_malloc_count = malloc_object_count(); + super::diag_sites::trigger_decision( + "safepoint", + match kind { + GcTriggerKind::MallocCount => "MallocCount", + _ => "ArenaBytes", + }, + ); // No `force_full_scan`: roots are precise at this safepoint. let outcome = super::gc_collect_minor_with_trigger(GcTriggerSnapshot::capture(kind)); match kind { @@ -3348,6 +3365,7 @@ fn gc_finish_budgeted_cycle(mut cycle: BudgetedGcCycle) -> JsGcStepResult { .state .take_outcome() .expect("completed budgeted GC cycle must produce an outcome"); + let freed_for_diag = outcome.freed_bytes; match cycle.rebaseline { BudgetedGcRebaseline::ArenaBytes { pre_in_use } => { gc_finish_arena_trigger_collection(pre_in_use, outcome); @@ -3363,6 +3381,7 @@ fn gc_finish_budgeted_cycle(mut cycle: BudgetedGcCycle) -> JsGcStepResult { } } GC_BUDGETED_CYCLE_ACTIVE.with(|active| active.set(false)); + super::diag_sites::budgeted_completed(freed_for_diag); gc_step_result( JS_GC_STEP_STATUS_COMPLETED, GcCyclePhase::Complete.ffi_code(), @@ -3543,8 +3562,14 @@ fn gc_budgeted_step_work_units_inner_with_progress( ); return gc_budgeted_skipped_result(); } + super::diag_sites::trigger_decision("budgeted_start", "due"); let cycle = gc_start_budgeted_cycle_for_pressure(start_progress_kind) .expect("budgeted GC pressure was observed before starting cycle"); + super::diag_sites::budgeted_started( + cycle.trigger_kind, + cycle.collection_kind, + start_progress_kind, + ); GC_BUDGETED_CYCLE.with(|slot| { *slot.borrow_mut() = Some(cycle); }); @@ -3570,10 +3595,11 @@ fn gc_budgeted_step_work_units_inner_with_progress( // for. `js_gc_step_us` can only consult its clock BETWEEN units, so the // only honest statement about pause is a measured maximum. let step_started = std::time::Instant::now(); + let phase_code = cycle.state.phase().ffi_code(); let step = cycle.state.step(GcWorkBudget::bounded(work_units)); - super::instruments::note_budgeted_step_duration( - step_started.elapsed().as_micros().min(u128::from(u64::MAX)) as u64, - ); + let step_us = step_started.elapsed().as_micros().min(u128::from(u64::MAX)) as u64; + super::instruments::note_budgeted_step_duration(step_us); + super::diag_sites::budgeted_step_done(phase_code, step_us, work_units); super::instruments::note_incremental_step(); if step.completed { super::instruments::note_incremental_completion(); diff --git a/crates/perry-runtime/src/gc/survival_diag.rs b/crates/perry-runtime/src/gc/survival_diag.rs new file mode 100644 index 0000000000..af7f8fd64e --- /dev/null +++ b/crates/perry-runtime/src/gc/survival_diag.rs @@ -0,0 +1,240 @@ +//! `PERRY_GC_DIAG=1`: per copying minor, WHY each surviving byte survived. +//! +//! `[gc-copy-minor]` reports how much survived; it cannot say what kept it +//! alive. On the compiled claude-code TUI the streaming turn scavenges at +//! 57–99 % survival while a heap census puts the true live set at ~45 MB, so +//! at scavenge time something references almost the whole Eden that stops +//! referencing it soon after. The candidates differ in their fix — nepotism +//! through the remembered set (a dead-but-unswept old object whose dirty page +//! still points at young objects), a stack-map root, a registered side-table +//! scanner, a legitimately live render tree — and only attribution tells +//! them apart. +//! +//! Every object the copying minor moves or promotes is charged to the ORIGIN +//! that first reached it: +//! +//! * a direct root: the walk phase the collector is in +//! (`pin::copying_walk_phase()` — `mutable_root_slots/shadow_stack`, +//! `mutable_root_slots/native_stack`, `mutable_root_slots/global_root`, +//! or the registered scanner's name), and for the remembered set the OLD +//! PARENT'S type (`remembered_set/array`, `remembered_set/object`, …); +//! * a transitive reach: the origin of the worklist entry whose field scan +//! found it. The worklist carries a parallel origin vector so the drain +//! propagates it — the collector's own worklist is untouched. +//! +//! Output, after each minor's `[gc-copy-minor]` line: the top rows by bytes +//! as `[gc-survival] minor=N origin= type= objects= bytes= +//! promoted_bytes=`, then per-origin and per-type totals. Allocation is Rust +//! heap only (no JS-heap allocation inside the collector), and the whole +//! structure exists only while the diag is on — `CopyingNurseryCollector` +//! carries it as `Option>` and every hook is one null check. + +use super::*; +use std::collections::HashMap; + +#[derive(Default, Clone, Copy)] +struct Row { + objects: u64, + bytes: u64, + promoted_bytes: u64, +} + +pub(super) struct SurvivalDiag { + /// Interned origin names; the index is the origin id. + names: Vec, + /// `&'static str` identity → origin id, so a phase name is interned once. + by_ptr: HashMap, + /// Origin ids for `remembered_set/`, by parent `obj_type`. + remembered_ids: Vec, + /// The old parent's type while the remembered-set scan visits its slots. + pub(super) remembered_parent_type: u8, + /// Origin of the worklist entry currently being drained, if draining. + drain_origin: Option, + /// Parallel to `CopyingNurseryCollector::worklist`. + worklist_origin: Vec, + /// `(origin id << 8) | obj_type` → row. + rows: HashMap, +} + +impl SurvivalDiag { + pub(super) fn new() -> Self { + let mut d = Self { + names: Vec::new(), + by_ptr: HashMap::new(), + remembered_ids: Vec::new(), + remembered_parent_type: 0, + drain_origin: None, + worklist_origin: Vec::new(), + rows: HashMap::new(), + }; + for t in 0..=GC_TYPE_MAX as usize { + let name = gc_type_info(t as u8).map_or("?", |i| i.name); + let id = d.intern_owned(format!("remembered_set/{name}")); + d.remembered_ids.push(id); + } + d + } + + fn intern_owned(&mut self, name: String) -> u16 { + if let Some(i) = self.names.iter().position(|n| *n == name) { + return i as u16; + } + self.names.push(name); + (self.names.len() - 1) as u16 + } + + fn intern_static(&mut self, name: &'static str) -> u16 { + let key = name.as_ptr() as usize; + if let Some(&id) = self.by_ptr.get(&key) { + return id; + } + let id = self.intern_owned(name.to_string()); + self.by_ptr.insert(key, id); + id + } + + /// The origin a newly reached object is charged to right now. + fn current_origin(&mut self) -> u16 { + if let Some(o) = self.drain_origin { + return o; + } + let phase = super::pin::copying_walk_phase().unwrap_or("unknown"); + if phase == "remembered_set" { + let t = (self.remembered_parent_type as usize).min(self.remembered_ids.len() - 1); + return self.remembered_ids[t]; + } + self.intern_static(phase) + } + + /// Mirror of `collector.worklist.push(..)`. + #[inline] + pub(super) fn note_worklist_push(&mut self) { + let o = self.current_origin(); + self.worklist_origin.push(o); + } + + /// The drain is about to scan worklist entry `i`. + #[inline] + pub(super) fn begin_drain_entry(&mut self, i: usize) { + self.drain_origin = self.worklist_origin.get(i).copied(); + } + + pub(super) fn end_drain(&mut self) { + self.drain_origin = None; + } + + /// One object of `obj_type` and `bytes` was copied (or promoted). + #[inline] + pub(super) fn record(&mut self, obj_type: u8, bytes: usize, promoted: bool) { + let origin = self.current_origin(); + let key = (u32::from(origin) << 8) | u32::from(obj_type); + let row = self.rows.entry(key).or_default(); + row.objects += 1; + row.bytes += bytes as u64; + if promoted { + row.promoted_bytes += bytes as u64; + } + } + + pub(super) fn report(&self, seq: u64) { + #[cfg(test)] + LAST_REPORT.with(|r| { + *r.borrow_mut() = self + .rows + .iter() + .map(|(k, row)| { + ( + self.names[(k >> 8) as usize].clone(), + (k & 0xff) as u8, + row.objects, + row.bytes, + row.promoted_bytes, + ) + }) + .collect(); + }); + if self.rows.is_empty() { + return; + } + let mut rows: Vec<(u32, Row)> = self.rows.iter().map(|(k, r)| (*k, *r)).collect(); + rows.sort_by_key(|(_, r)| std::cmp::Reverse(r.bytes)); + let total_bytes: u64 = rows.iter().map(|(_, r)| r.bytes).sum(); + let total_objects: u64 = rows.iter().map(|(_, r)| r.objects).sum(); + eprintln!( + "[gc-survival] minor={seq} rows={} objects={total_objects} bytes={total_bytes}", + rows.len() + ); + for (key, r) in rows.iter().take(24) { + let origin = &self.names[(key >> 8) as usize]; + let t = (key & 0xff) as u8; + let tname = gc_type_info(t).map_or("?", |i| i.name); + eprintln!( + "[gc-survival] minor={seq} origin={origin} type={tname} objects={} bytes={} promoted_bytes={}", + r.objects, r.bytes, r.promoted_bytes + ); + } + let mut by_origin: HashMap = HashMap::new(); + let mut by_type: HashMap = HashMap::new(); + for (key, r) in &rows { + let o = by_origin.entry((key >> 8) as u16).or_default(); + o.objects += r.objects; + o.bytes += r.bytes; + o.promoted_bytes += r.promoted_bytes; + let t = by_type.entry((key & 0xff) as u8).or_default(); + t.objects += r.objects; + t.bytes += r.bytes; + t.promoted_bytes += r.promoted_bytes; + } + let mut by_origin: Vec<_> = by_origin.into_iter().collect(); + by_origin.sort_by_key(|(_, r)| std::cmp::Reverse(r.bytes)); + for (o, r) in by_origin.iter().take(12) { + eprintln!( + "[gc-survival] minor={seq} origin-total={} objects={} bytes={} permille={}", + self.names[*o as usize], + r.objects, + r.bytes, + if total_bytes > 0 { + r.bytes * 1000 / total_bytes + } else { + 0 + } + ); + } + let mut by_type: Vec<_> = by_type.into_iter().collect(); + by_type.sort_by_key(|(_, r)| std::cmp::Reverse(r.bytes)); + for (t, r) in by_type.iter().take(8) { + eprintln!( + "[gc-survival] minor={seq} type-total={} objects={} bytes={}", + gc_type_info(*t).map_or("?", |i| i.name), + r.objects, + r.bytes + ); + } + } +} + +crate::perry_thread_local! { + static MINOR_SEQ: Cell = const { Cell::new(0) }; +} + +#[cfg(test)] +crate::perry_thread_local! { + /// Test-only snapshot of the last report's rows: + /// `(origin, obj_type, objects, bytes, promoted_bytes)`. + static LAST_REPORT: RefCell> = const { RefCell::new(Vec::new()) }; +} + +/// Test-only: the rows of the most recent `report` on this thread. +#[cfg(test)] +pub(super) fn test_last_report() -> Vec<(String, u8, u64, u64, u64)> { + LAST_REPORT.with(|r| r.borrow().clone()) +} + +/// Sequence number for the next copying minor's report. +pub(super) fn next_minor_seq() -> u64 { + MINOR_SEQ.with(|c| { + let v = c.get() + 1; + c.set(v); + v + }) +} diff --git a/crates/perry-runtime/src/gc/telemetry.rs b/crates/perry-runtime/src/gc/telemetry.rs index 06d01002ac..ac62c3b605 100644 --- a/crates/perry-runtime/src/gc/telemetry.rs +++ b/crates/perry-runtime/src/gc/telemetry.rs @@ -15,10 +15,44 @@ pub const GC_RECENT_PAUSE_WINDOW: usize = 32; /// The value semantics are #5093's, shared with every other GC knob via /// [`super::env_flag_from_value`]. pub fn gc_diag_enabled() -> bool { + #[cfg(test)] + if GC_DIAG_TEST_FORCED.with(std::cell::Cell::get) { + return true; + } static ENABLED: std::sync::OnceLock = std::sync::OnceLock::new(); *ENABLED.get_or_init(|| env_flag_enabled("PERRY_GC_DIAG")) } +#[cfg(test)] +thread_local! { + /// Test-only per-thread override of `PERRY_GC_DIAG`: the live reader is a + /// process-wide `OnceLock`, and `std::env::set_var` is shared by every + /// libtest thread (see `env_knob_parse.rs`), so a test that needs the + /// diagnostic paths live arms them here instead. + static GC_DIAG_TEST_FORCED: std::cell::Cell = const { std::cell::Cell::new(false) }; +} + +/// Test-only RAII: force `gc_diag_enabled()` ON for this thread. +#[cfg(test)] +pub(crate) struct GcDiagTestGuard { + previous: bool, +} + +#[cfg(test)] +impl GcDiagTestGuard { + pub(crate) fn force_on() -> Self { + let previous = GC_DIAG_TEST_FORCED.with(|c| c.replace(true)); + Self { previous } + } +} + +#[cfg(test)] +impl Drop for GcDiagTestGuard { + fn drop(&mut self) { + GC_DIAG_TEST_FORCED.with(|c| c.set(self.previous)); + } +} + /// Is `PERRY_GC_VERIFY_MARK` ON? Cached for the same reason as /// [`gc_diag_enabled`], and value-parsed for the same reason (#7991): the three /// mark-verifier call sites were presence-only, so `=0` armed a verifier that diff --git a/crates/perry-runtime/src/gc/tests/env_knob_parse.rs b/crates/perry-runtime/src/gc/tests/env_knob_parse.rs index 7f24d9ebcd..8a0bf964f4 100644 --- a/crates/perry-runtime/src/gc/tests/env_knob_parse.rs +++ b/crates/perry-runtime/src/gc/tests/env_knob_parse.rs @@ -50,6 +50,43 @@ const ON_SPELLINGS: &[&str] = &["1", "true", "on", "yes", "TRUE", "On", " 1 ", " /// default-OFF instrument OFF, not arm it. const UNRECOGNISED: &[&str] = &["banana", "2", "-1", "onn", "ye", "enabled", "0x1"]; +/// `PERRY_ALLOC_SITE_SAMPLE` (arena/alloc_sample.rs) is a MAGNITUDE knob with +/// the shared boolean vocabulary layered on top: every OFF spelling and every +/// typo reads as OFF, the ON spellings select the default interval, and an +/// explicit integer is the interval in bytes, floored. +#[test] +fn alloc_site_sample_interval_is_off_by_value_and_a_floored_magnitude_when_on() { + use crate::arena::alloc_sample::{parse_interval, DEFAULT_INTERVAL_BYTES, MIN_INTERVAL_BYTES}; + for raw in OFF_SPELLINGS { + assert_eq!(parse_interval(*raw), 0, "{raw:?} must read as OFF"); + } + for raw in ON_SPELLINGS { + assert_eq!( + parse_interval(Some(raw)), + DEFAULT_INTERVAL_BYTES, + "{raw:?} is the boolean ON spelling and selects the default interval" + ); + } + for raw in UNRECOGNISED { + if raw.trim().parse::().is_ok_and(|v| v >= 2) { + continue; // an integer is a magnitude for this knob, pinned below + } + assert_eq!( + parse_interval(Some(raw)), + 0, + "{raw:?} is a typo and must leave the sampler OFF" + ); + } + assert_eq!(parse_interval(Some("65536")), 65536); + assert_eq!(parse_interval(Some(" 4096 ")), 4096); + assert_eq!( + parse_interval(Some("2")), + MIN_INTERVAL_BYTES, + "a tiny explicit interval is floored, not honoured" + ); + assert!(DEFAULT_INTERVAL_BYTES >= MIN_INTERVAL_BYTES); +} + #[test] fn default_off_knobs_are_parsed_by_value_not_presence() { for raw in OFF_SPELLINGS { diff --git a/crates/perry-runtime/src/gc/tests/mod.rs b/crates/perry-runtime/src/gc/tests/mod.rs index ad0ae26bcc..69c9b0c20c 100644 --- a/crates/perry-runtime/src/gc/tests/mod.rs +++ b/crates/perry-runtime/src/gc/tests/mod.rs @@ -57,6 +57,7 @@ mod shape_keys_descriptor_edge; mod smoke; mod step_bounds; pub(super) mod support; +mod survival_diag; mod teardown; mod telemetry_verifier; mod temp_roots; diff --git a/crates/perry-runtime/src/gc/tests/survival_diag.rs b/crates/perry-runtime/src/gc/tests/survival_diag.rs new file mode 100644 index 0000000000..7467984450 --- /dev/null +++ b/crates/perry-runtime/src/gc/tests/survival_diag.rs @@ -0,0 +1,143 @@ +//! `[gc-survival]` / `[gc-trigger]` / `[gc-full]` / `[gc-budgeted]` / +//! `[gc-charge]` (gc/survival_diag.rs, gc/diag_sites.rs): the attribution +//! instruments are validated against heaps and cycles of KNOWN shape before +//! they are pointed at anything real. Every assertion here can fail on the +//! instrument: a lost drain propagation charges elements to the drain phase, +//! a missed worklist mirror misaligns the origin vector, an unconsumed site +//! label mislabels the next full, an uncounted step leaves `steps` short. + +use super::super::*; +use super::support::*; + +/// A young array holding `N` young strings, rooted from ONE shadow-stack slot. +/// The elements are reachable only through the array, so their origin is the +/// array's — which is exactly the claim the parallel origin vector makes. +#[test] +fn survival_rows_charge_transitive_reach_to_the_originating_root() { + let _diag = crate::gc::telemetry::GcDiagTestGuard::force_on(); + let _guard = CopyingNurseryTestGuard::new(1); + let _trigger_guard = GcTriggerThresholdTestGuard::suppress_automatic_triggers(); + const N: usize = 40; + let mut arr = crate::array::js_array_alloc(N as u32); + for _ in 0..N { + let child = young_leaf(); + arr = crate::array::js_array_push_f64(arr, f64::from_bits(string_bits(child))); + } + js_shadow_slot_set(0, ptr_bits(arr as usize)); + + let trace = collect_minor_trace(GcTriggerKind::Direct); + assert_copied_minor_trace(&trace, true, CopiedMinorFallbackReason::None, false); + let moved = + trace.copying_nursery.copied_objects as u64 + trace.copying_nursery.promoted_objects as u64; + assert!( + moved > N as u64, + "subject must be live: the minor moved {moved} objects, expected at least {}", + N + 1 + ); + + let rows = super::super::survival_diag::test_last_report(); + assert!( + !rows.is_empty(), + "the diag was forced on, so the minor must have reported rows" + ); + let attributed: u64 = rows.iter().map(|r| r.2).sum(); + assert_eq!( + attributed, moved, + "every moved object is attributed exactly once (origin vector aligned with the worklist)" + ); + const SHADOW: &str = "mutable_root_slots/shadow_stack"; + let strings_via_shadow: u64 = rows + .iter() + .filter(|(o, t, ..)| o == SHADOW && *t == GC_TYPE_STRING) + .map(|r| r.2) + .sum(); + let arrays_via_shadow: u64 = rows + .iter() + .filter(|(o, t, ..)| o == SHADOW && *t == GC_TYPE_ARRAY) + .map(|r| r.2) + .sum(); + assert!( + arrays_via_shadow >= 1, + "the rooted array is charged to the shadow-stack root: rows={rows:?}" + ); + assert!( + strings_via_shadow >= N as u64, + "the {N} elements reach the collector only through the array, so they are charged to \ + the array's origin, not to the drain: rows={rows:?}" + ); + assert!( + rows.iter().all(|(o, ..)| !o.contains("worklist_drain")), + "transitive reach must never be charged to the drain phase: rows={rows:?}" + ); +} + +#[test] +fn full_site_label_is_consumed_once_and_counted_per_site() { + use super::super::diag_sites::*; + let _diag = crate::gc::telemetry::GcDiagTestGuard::force_on(); + set_full_site("survival_diag_test_a"); + assert_eq!(take_full_site(), "survival_diag_test_a"); + assert_eq!( + take_full_site(), + "sync", + "a label is consumed by the first full after it; the next full must not inherit it" + ); + let before = test_full_site_count("survival_diag_test_b"); + full_started("survival_diag_test_b", GcTriggerKind::Manual); + full_started("survival_diag_test_b", GcTriggerKind::OldGenBytes); + assert_eq!(test_full_site_count("survival_diag_test_b"), before + 2); + assert_eq!(test_full_site_count("survival_diag_test_never"), 0); +} + +#[test] +fn budgeted_accounting_counts_steps_and_root_scan_time() { + use super::super::diag_sites::*; + let _diag = crate::gc::telemetry::GcDiagTestGuard::force_on(); + budgeted_started( + GcTriggerKind::OldGenBytes, + GcCollectionKind::Full, + GcProgressKind::MutatorAssist, + ); + // Phase codes follow `GcCyclePhase::ffi_code`: 2 = root scan, 6 = sweep. + budgeted_step_done(2, 300, 16); + budgeted_step_done(2, 200, 16); + budgeted_step_done(6, 50, 16); + budgeted_completed(4096); + let (steps, step_us, units, root_us) = + test_last_budgeted().expect("a completed cycle publishes its accounting"); + assert_eq!(steps, 3); + assert_eq!(step_us, 550); + assert_eq!(units, 48); + assert_eq!( + root_us, 500, + "root-scan time is the sum of the steps taken in phase 2" + ); +} + +#[test] +fn charge_probe_attributes_only_under_the_diag() { + use super::super::diag_sites::*; + { + // Diag OFF: a probe is inert and records nothing. + let probe = ChargeProbe::begin(); + probe.end(7, ChargeKind::Assist); + } + let _diag = crate::gc::telemetry::GcDiagTestGuard::force_on(); + report_charges("survival_diag_test_reset"); + assert!( + test_charge_rows().is_empty(), + "report_charges drains the table" + ); + let probe = ChargeProbe::begin(); + probe.end(7, ChargeKind::SyncFull); + let probe = ChargeProbe::begin(); + probe.end(3, ChargeKind::Assist); + let rows = test_charge_rows(); + let calls: u64 = rows.iter().map(|r| r.0).sum(); + let units: u64 = rows.iter().map(|r| r.1).sum(); + let fulls: u64 = rows.iter().map(|r| r.3).sum(); + assert_eq!(calls, 2, "two probes ended under the diag: rows={rows:?}"); + assert_eq!(units, 10); + assert_eq!(fulls, 1); + report_charges("survival_diag_test_done"); +} diff --git a/docs/src/internals/garbage-collector.md b/docs/src/internals/garbage-collector.md index a8a2865dcd..670d013a95 100644 --- a/docs/src/internals/garbage-collector.md +++ b/docs/src/internals/garbage-collector.md @@ -255,7 +255,8 @@ These are the operational controls most useful outside collector development: | `PERRY_RS4GC=0` | select shadow roots on a native-root-capable target | | `PERRY_CONSERVATIVE_STACK_SCAN=full` | diagnostic full native-stack scan; disables copying | | `PERRY_GC_TRACE=1` | emit structured per-cycle trace records | -| `PERRY_GC_DIAG=1` | emit human-readable collector diagnostics | +| `PERRY_GC_DIAG=1` | emit human-readable collector diagnostics (per cycle, plus `[gc-trigger]`/`[gc-full]`/`[gc-budgeted]`/`[gc-charge]` decision and charge attribution and the per-minor `[gc-survival]` root attribution) | +| `PERRY_ALLOC_SITE_SAMPLE=N` | sample the arena allocation-site histogram every N bytes (`[alloc-site]`); `1`/`on` selects the default interval | Rooting stress uses `PERRY_GC_SCHEDULE_SEED`, `PERRY_GC_SCHEDULE_RATE`, `PERRY_GC_SCHEDULE_ALLOC_KB`, From 5cd7d323bc169f0fe51050ceb5d4322ee38ec9d3 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 2/4] 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 --- .../9794-alloc-primitive-string-path.md | 29 ++++ ...g.md => 9794-gc-churn-attribution-diag.md} | 0 .../perry-runtime/src/builtins/formatting.rs | 2 +- .../builtins/formatting/boxed_primitives.rs | 59 ++++++- crates/perry-runtime/src/builtins/mod.rs | 3 +- crates/perry-runtime/src/gc/copying.rs | 1 + crates/perry-runtime/src/gc/diag_sites.rs | 78 +++++++++ crates/perry-runtime/src/gc/mod.rs | 3 + .../src/object/descriptor_state.rs | 160 +++++++++++++++++- crates/perry-runtime/src/object/mod.rs | 2 +- .../src/object/native_call_method.rs | 24 ++- .../src/object/object_ops/prototype.rs | 10 +- .../src/object/prototype_helpers.rs | 2 +- crates/perry-runtime/src/string/char_ops.rs | 15 +- crates/perry-runtime/src/string/format.rs | 59 +++++++ crates/perry-runtime/src/string/mod.rs | 26 +++ crates/perry-runtime/src/string/tests.rs | 63 +++++++ crates/perry-runtime/src/value/to_string.rs | 8 +- 18 files changed, 514 insertions(+), 30 deletions(-) create mode 100644 changelog.d/9794-alloc-primitive-string-path.md rename changelog.d/{gc-churn-attribution-diag.md => 9794-gc-churn-attribution-diag.md} (100%) diff --git a/changelog.d/9794-alloc-primitive-string-path.md b/changelog.d/9794-alloc-primitive-string-path.md new file mode 100644 index 0000000000..d5b377ae50 --- /dev/null +++ b/changelog.d/9794-alloc-primitive-string-path.md @@ -0,0 +1,29 @@ +### Runtime + +- perf(string): a one-ASCII-character string is now the canonical per-thread + header instead of a fresh 32-byte allocation. `js_string_char_at` — and + everything that funnels through it (`s[i]`, `charAt`, string spread, the + String-wrapper index installer) — used to mint one string per character read, + which on a text-measuring workload is the single largest source of garbage. + Same residency contract as the existing small-integer string table + (longlived arena, `refcount = 0`, pinned, scanned by the same root scanner — + no new scanner is registered). + +- perf(runtime): `String`/`Number`/`Boolean`/`BigInt` wrapper dispatch, + `x.constructor`, `toString` resolution and the `globalThis` builtin lookup + resolve their constant property names through the intern table instead of + minting a heap string per lookup. `js_get_global_this_builtin_value` alone + allocated 133 MB during a 3300-character claude-code reply, all of it the + same handful of literals. Interned keys also make the property-read and + property-write fast paths eligible, which a freshly minted key never was. + +- perf(runtime): a `String` wrapper no longer stores one 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 hash-map entry only a full + collection 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 compiled claude-code TUI paid that for every rendered + line. diff --git a/changelog.d/gc-churn-attribution-diag.md b/changelog.d/9794-gc-churn-attribution-diag.md similarity index 100% rename from changelog.d/gc-churn-attribution-diag.md rename to changelog.d/9794-gc-churn-attribution-diag.md diff --git a/crates/perry-runtime/src/builtins/formatting.rs b/crates/perry-runtime/src/builtins/formatting.rs index 3743f7dea7..a76ed50948 100644 --- a/crates/perry-runtime/src/builtins/formatting.rs +++ b/crates/perry-runtime/src/builtins/formatting.rs @@ -18,7 +18,7 @@ mod collection_equality; mod errors; pub(crate) use boxed_primitives::{ boxed_primitive_json_value, boxed_primitive_payload, boxed_primitive_to_string_tag, - prune_dead_boxed_primitive_payload_owners, + boxed_string_wrapper_utf16_len, prune_dead_boxed_primitive_payload_owners, }; pub use boxed_primitives::{ js_boxed_bigint_new, js_boxed_boolean_new, js_boxed_number_new, js_boxed_string_new, diff --git a/crates/perry-runtime/src/builtins/formatting/boxed_primitives.rs b/crates/perry-runtime/src/builtins/formatting/boxed_primitives.rs index 08c9a151c4..e3e7441306 100644 --- a/crates/perry-runtime/src/builtins/formatting/boxed_primitives.rs +++ b/crates/perry-runtime/src/builtins/formatting/boxed_primitives.rs @@ -172,6 +172,29 @@ fn install_string_wrapper_length( ); } +/// UTF-16 length of the primitive a `String` wrapper boxes, or `None` when +/// `addr` is not one. Two header reads and a side-table probe: no content +/// copy, no allocation, so a descriptor lookup can afford to ask. +pub(crate) fn boxed_string_wrapper_utf16_len(addr: usize) -> Option { + unsafe { + let header = crate::value::addr_class::try_read_gc_header(addr)?; + if header.obj_type != crate::gc::GC_TYPE_OBJECT { + return None; + } + let obj_ptr = addr as *const crate::object::ObjectHeader; + let (class_id, payload) = boxed_primitive_payload_for_object(obj_ptr)?; + if class_id != CLASS_ID_BOXED_STRING { + return None; + } + let str_ptr = crate::value::js_get_string_pointer_unified(payload) + as *const crate::string::StringHeader; + if str_ptr.is_null() { + return None; + } + Some(crate::string::js_string_length(str_ptr)) + } +} + /// String exotic objects (ECMA-262 §10.4.3) expose each UTF-16 code unit as an /// integer-indexed own property `"0".."len-1"` with the descriptor /// `{ value: , writable: false, enumerable: true, configurable: false }`. @@ -187,20 +210,42 @@ fn install_string_wrapper_indices( return; } let len = crate::string::js_string_length(string_ptr); + if len == 0 { + return; + } + // The index descriptors are a fact of the CLASS, not of this object: + // every in-range index of every String wrapper is + // `{ writable: false, enumerable: true, configurable: false }`. They are + // therefore answered by `descriptor_state::string_wrapper_index_attrs` + // from the wrapper's own payload rather than stored, and the loop below + // installs only the key/value pair. + // + // What that removes, per boxed character: a `String` (Rust heap), a + // `(usize, String)` hash entry in `PROPERTY_DESCRIPTORS`, an owner-index + // entry, a meta-descriptor key bit — and a `prop_plan_epoch_bump()`, a + // PROGRAM-WIDE property-plan invalidation, once per character. A sloppy + // method call on a string primitive boxes its receiver + // (`call_primitive_closure_value` -> `js_object_coerce`), so on the + // compiled claude-code TUI that ran over every rendered line: measured at + // 295 MB of the 990 MB a 3300-character reply allocates. + // + // `note_descriptor_target` still marks the wrapper, so every caller that + // gates on "does this object have non-default descriptors?" before + // consulting `get_property_attrs` keeps reaching the synthesized answer. + crate::object::note_descriptor_target(obj as usize); + crate::gc::diag_string_wrapper_materialized(len as u64); for i in 0..len { let ch = crate::string::js_string_char_at(string_ptr, i as i32); if ch.is_null() { continue; } - let name = i.to_string(); - let key = crate::string::js_string_from_bytes(name.as_ptr(), name.len() as u32); + // Canonical (cached, longlived) decimal key for 0..=255 and the + // canonical one-character string for ASCII: both come out of the + // string caches, so a boxed ASCII string of up to 256 characters + // installs its indices without allocating a single string. + let key = crate::string::js_number_to_string(i as f64); let ch_value = f64::from_bits(crate::value::JSValue::string_ptr(ch).bits()); crate::object::js_object_set_field_by_name(obj, key, ch_value); - crate::object::set_builtin_property_attrs( - obj as usize, - name, - crate::object::PropertyAttrs::new(false, true, false), - ); } } diff --git a/crates/perry-runtime/src/builtins/mod.rs b/crates/perry-runtime/src/builtins/mod.rs index 3d21c1c681..3fa8626df8 100644 --- a/crates/perry-runtime/src/builtins/mod.rs +++ b/crates/perry-runtime/src/builtins/mod.rs @@ -163,7 +163,8 @@ pub use formatting::{ pub(crate) use formatting::{ boxed_primitive_json_value, boxed_primitive_payload, boxed_primitive_to_string_tag, - format_finite_number_js, format_jsvalue, int32_or_class_repr, is_array_hole, is_negative_zero, + boxed_string_wrapper_utf16_len, format_finite_number_js, format_jsvalue, int32_or_class_repr, + is_array_hole, is_negative_zero, jsvalue_string_content, prune_dead_boxed_primitive_payload_owners, InspectCompactGuard, InspectCustomInspectGuard, InspectDepthLimitGuard, InspectGettersGuard, InspectShowHiddenGuard, InspectSortedGuard, INT_EXACT_FASTPATH_LIMIT, diff --git a/crates/perry-runtime/src/gc/copying.rs b/crates/perry-runtime/src/gc/copying.rs index 1be599b0ce..308af06d42 100644 --- a/crates/perry-runtime/src/gc/copying.rs +++ b/crates/perry-runtime/src/gc/copying.rs @@ -1822,6 +1822,7 @@ pub(super) fn run_copied_minor_attempt( d.report(super::survival_diag::next_minor_seq()); } crate::arena::alloc_sample::report("minor"); + super::diag_sites::report_primitive_dispatch("minor"); report_forwarding_refusals("copying_minor"); super::scanner_profile::report_and_reset("copying_minor"); CopiedMinorAttempt::Done(Some(CopiedMinorFastPathOutcome { diff --git a/crates/perry-runtime/src/gc/diag_sites.rs b/crates/perry-runtime/src/gc/diag_sites.rs index 57a1771641..622d3d3a6f 100644 --- a/crates/perry-runtime/src/gc/diag_sites.rs +++ b/crates/perry-runtime/src/gc/diag_sites.rs @@ -362,3 +362,81 @@ pub(super) fn report_charges(label: &str) { ); } } + +// --- primitive-method dispatch tower ------------------------------------- +// +// A method call whose receiver is a string/number/boolean/bigint primitive and +// whose method the native dispatch tower does not recognise falls through to +// `native_call_method::call_primitive_builtin_prototype_method`: it resolves +// `globalThis..prototype[]` and, for a SLOPPY callee, boxes +// the receiver with `ToObject`. For a string that wrapper materialises one own +// property per UTF-16 code unit. So a single unrecognised method name on a hot +// render path turns into O(length) allocations per call, and the only way to +// tell WHICH names those are is to count them at the fork. + +thread_local! { + /// `".prototype." -> (calls, receiver_utf16_chars)`. + static PRIMITIVE_DISPATCH: RefCell> = + RefCell::new(HashMap::new()); + /// String wrappers actually materialised: (wrappers, index properties). + static STRING_WRAPPERS: Cell<(u64, u64)> = const { Cell::new((0, 0)) }; +} + +/// Record one trip through the primitive-method fallback. `recv_chars` is the +/// receiver's UTF-16 length (0 when the receiver is not a string) — the number +/// of own index properties a sloppy callee's `ToObject` wrapper costs. +pub(crate) fn primitive_dispatch(builtin: &[u8], method: &str, recv_chars: u64) { + if !gc_diag_enabled() { + return; + } + let name = format!("{}.prototype.{method}", String::from_utf8_lossy(builtin)); + PRIMITIVE_DISPATCH.with(|m| { + let mut m = m.borrow_mut(); + let entry = m.entry(name).or_insert((0, 0)); + entry.0 += 1; + entry.1 += recv_chars; + }); +} + +/// Record one `String` wrapper materialisation and how many index properties +/// it installed. This is the counter that proves the wrapper fix: the wrapper +/// count must NOT move (semantics unchanged) while the bytes those wrappers +/// allocate collapse. +pub(crate) fn string_wrapper_materialized(indices: u64) { + if !gc_diag_enabled() { + return; + } + STRING_WRAPPERS.with(|c| { + let (w, i) = c.get(); + c.set((w + 1, i + indices)); + }); +} + +/// Print the fallback histogram, hottest first. +pub(super) fn report_primitive_dispatch(label: &str) { + if !gc_diag_enabled() { + return; + } + let (wrappers, indices) = STRING_WRAPPERS.with(Cell::get); + if wrappers > 0 { + eprintln!( + "[gc-primitive-dispatch] {label}: string_wrappers={wrappers} index_properties={indices}" + ); + } + let rows: Vec<(String, (u64, u64))> = + PRIMITIVE_DISPATCH.with(|m| m.borrow().iter().map(|(k, v)| (k.clone(), *v)).collect()); + if rows.is_empty() { + return; + } + let calls: u64 = rows.iter().map(|(_, v)| v.0).sum(); + let chars: u64 = rows.iter().map(|(_, v)| v.1).sum(); + let mut rows = rows; + rows.sort_by_key(|(_, v)| std::cmp::Reverse(v.0)); + eprintln!( + "[gc-primitive-dispatch] {label}: names={} calls={calls} receiver_chars={chars}", + rows.len() + ); + for (name, (n, ch)) in rows.iter().take(20) { + eprintln!("[gc-primitive-dispatch] calls={n} receiver_chars={ch} {name}"); + } +} diff --git a/crates/perry-runtime/src/gc/mod.rs b/crates/perry-runtime/src/gc/mod.rs index c5fceacae0..3ecee0b079 100644 --- a/crates/perry-runtime/src/gc/mod.rs +++ b/crates/perry-runtime/src/gc/mod.rs @@ -159,6 +159,8 @@ mod copying; mod copying_first_cycle; mod copying_pointer_set; mod diag_sites; +pub(crate) use diag_sites::primitive_dispatch as diag_primitive_dispatch; +pub(crate) use diag_sites::string_wrapper_materialized as diag_string_wrapper_materialized; /// #8174: shared validation for the TARGET of a forwarding pointer. mod forwarding; /// Per-scanner root attribution for the copied-minor root scan (#7915). @@ -1337,6 +1339,7 @@ pub extern "C" fn js_gc_release_current_thread_collection_side_allocations() { crate::r#box::report_box_stats_at_exit(); crate::arena::alloc_sample::report("exit"); diag_sites::report_charges("exit"); + diag_sites::report_primitive_dispatch("exit"); emit_incremental_liveness_diag(); emit_schedule_liveness_verdict(); } diff --git a/crates/perry-runtime/src/object/descriptor_state.rs b/crates/perry-runtime/src/object/descriptor_state.rs index 909002a9aa..e0138f22da 100644 --- a/crates/perry-runtime/src/object/descriptor_state.rs +++ b/crates/perry-runtime/src/object/descriptor_state.rs @@ -531,15 +531,73 @@ pub(crate) fn get_property_attrs(obj: usize, key: &str) -> Option // #6759 Phase C2: the meta-record summary proves most misses without // the `String` build + table probe (and shields a fresh object at a // recycled address from a dead owner's not-yet-pruned entries). - if !may_have_descriptor_entry(obj, key, false) { + if may_have_descriptor_entry(obj, key, false) { + if let Some(attrs) = state() + .descriptors + .property_descriptors + .borrow() + .get(&(obj, key.to_string())) + .copied() + { + return Some(attrs); + } + } + string_wrapper_index_attrs(obj, key) +} + +/// ECMA-262 §10.4.3: every in-range integer index of a `String` exotic object +/// (`new String("abc")`, and the wrapper `ToObject` mints for a sloppy method +/// call on a string primitive) has the descriptor +/// `{ writable: false, enumerable: true, configurable: false }`. That is a +/// property of the CLASS and of the boxed length — never of the individual +/// object — so it is answered from the wrapper's own payload instead of being +/// stored once per character in `PROPERTY_DESCRIPTORS`. +/// +/// Storing it cost, per boxed character: a `String` key on the Rust heap, a +/// hash-map entry that only a full collection's dead-owner prune can reclaim, +/// an owner-index entry, a meta-descriptor key bit, and one program-wide +/// `prop_plan_epoch_bump()`. On the compiled claude-code TUI, whose render +/// path boxes a receiver per string method call, those entries were the +/// unbounded half of the process's resident growth during a turn. +/// +/// A REAL entry still wins (the probe above runs first): `Object.freeze` or +/// an explicit `defineProperty` on a wrapper installs one and is observed. +/// +/// The first byte is checked before anything else: an index key starts with an +/// ASCII digit, so every ordinary property name leaves through one compare. +#[inline] +fn string_wrapper_index_attrs(obj: usize, key: &str) -> Option { + let bytes = key.as_bytes(); + if !bytes.first().is_some_and(u8::is_ascii_digit) { return None; } - state() - .descriptors - .property_descriptors - .borrow() - .get(&(obj, key.to_string())) - .copied() + let index = canonical_index_key(bytes)?; + let len = crate::builtins::boxed_string_wrapper_utf16_len(obj)?; + (index < len).then(|| PropertyAttrs::new(false, true, false)) +} + +/// `CanonicalNumericIndexString` for the digits-only case: the key must be the +/// exact `ToString` of the integer it names, so `"0"` is an index but `"01"`, +/// `"1.0"` and `""` are not (mirrors `string::canonical_string_index`). +#[inline] +fn canonical_index_key(bytes: &[u8]) -> Option { + if bytes.is_empty() || bytes.len() > 10 { + return None; + } + if bytes[0] == b'0' { + return (bytes.len() == 1).then_some(0); + } + let mut value: u64 = 0; + for &b in bytes { + if !b.is_ascii_digit() { + return None; + } + value = value * 10 + (b - b'0') as u64; + if value > u32::MAX as u64 { + return None; + } + } + u32::try_from(value).ok() } /// Whether this specific object has ever had a property descriptor installed on @@ -1813,3 +1871,91 @@ mod c5a_tests { test_reset_class_field_inline_guard(); } } + +#[cfg(test)] +pub(crate) fn test_property_descriptor_entry_count(obj: usize) -> usize { + state() + .descriptors + .property_descriptors + .borrow() + .keys() + .filter(|(owner, _)| *owner == obj) + .count() +} + +#[cfg(test)] +mod string_wrapper_index_attrs_tests { + use super::*; + + fn boxed(text: &str) -> usize { + let s = crate::string::js_string_from_bytes(text.as_ptr(), text.len() as u32); + let value = f64::from_bits(crate::value::JSValue::string_ptr(s).bits()); + let boxed = crate::builtins::js_boxed_string_new(value, 1); + crate::value::js_nanbox_get_pointer(boxed) as usize + } + + /// The index descriptors of a `String` exotic object are answered from the + /// wrapper's payload, not from `PROPERTY_DESCRIPTORS`. Both halves matter: + /// the ANSWER must still be the spec's + /// `{ writable: false, enumerable: true, configurable: false }` (delete + /// this synthesis and `str[0] = "x"` starts mutating the wrapper), and the + /// STORAGE must be one entry — `length` — however long the string is + /// (that is the allocation this exists to remove). + #[test] + fn in_range_indices_are_synthesized_and_not_stored() { + let obj = boxed("hello world"); + for index in ["0", "1", "10"] { + let attrs = get_property_attrs(obj, index) + .unwrap_or_else(|| panic!("index {index} must have a descriptor")); + assert!(!attrs.writable(), "index {index} is not writable"); + assert!(attrs.enumerable(), "index {index} is enumerable"); + assert!(!attrs.configurable(), "index {index} is not configurable"); + } + assert_eq!( + test_property_descriptor_entry_count(obj), + 1, + "only `length` is stored; the 11 index descriptors are synthesized" + ); + } + + /// Out of range, non-canonical, and non-index keys get the ordinary + /// answer, so the synthesis cannot invent properties the object does not + /// have. `"01"` and `"1.0"` are NOT canonical index strings. + #[test] + fn only_canonical_in_range_indices_are_synthesized() { + let obj = boxed("abc"); + assert!(get_property_attrs(obj, "3").is_none(), "past the end"); + assert!(get_property_attrs(obj, "01").is_none(), "not canonical"); + assert!(get_property_attrs(obj, "1.0").is_none(), "not canonical"); + assert!(get_property_attrs(obj, "").is_none()); + assert!(get_property_attrs(obj, "toString").is_none()); + assert!( + get_property_attrs(obj, "0").is_some(), + "the positive control: the same call answers for a real index" + ); + } + + /// Nothing but a String wrapper answers. A plain object with an index-named + /// property keeps the JS default (writable, enumerable, configurable), which + /// is what `None` means to every caller. + #[test] + fn a_plain_object_is_never_treated_as_a_string_wrapper() { + let obj = crate::object::js_object_alloc(0, 1) as usize; + let key = crate::string::js_string_from_bytes(b"0".as_ptr(), 1); + crate::object::js_object_set_field_by_name(obj as *mut _, key, 1.0); + assert!(get_property_attrs(obj, "0").is_none()); + assert!(get_property_attrs(0, "0").is_none(), "null address"); + } + + /// A REAL entry still wins: `Object.defineProperty` / `Object.freeze` on a + /// wrapper installs one, and the synthesized default must not shadow it. + #[test] + fn a_stored_descriptor_overrides_the_synthesized_one() { + let obj = boxed("xy"); + set_property_attrs(obj, "1".to_string(), PropertyAttrs::new(true, false, true)); + let attrs = get_property_attrs(obj, "1").expect("stored entry"); + assert!(attrs.writable() && !attrs.enumerable() && attrs.configurable()); + let other = get_property_attrs(obj, "0").expect("synthesized entry"); + assert!(!other.writable() && other.enumerable() && !other.configurable()); + } +} diff --git a/crates/perry-runtime/src/object/mod.rs b/crates/perry-runtime/src/object/mod.rs index f598712f9b..5fa1620532 100644 --- a/crates/perry-runtime/src/object/mod.rs +++ b/crates/perry-runtime/src/object/mod.rs @@ -253,7 +253,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, reflect_getter_closure_bits, set_accessor_descriptor, diff --git a/crates/perry-runtime/src/object/native_call_method.rs b/crates/perry-runtime/src/object/native_call_method.rs index 05d32b1a39..79cf199420 100644 --- a/crates/perry-runtime/src/object/native_call_method.rs +++ b/crates/perry-runtime/src/object/native_call_method.rs @@ -335,6 +335,20 @@ unsafe fn call_primitive_closure_value( Some(result) } +/// UTF-16 length of a string receiver, 0 for every other primitive — the +/// number of own index properties its `ToObject` wrapper would materialise. +unsafe fn primitive_receiver_utf16_len(receiver: f64) -> u64 { + let jsval = JSValue::from_bits(receiver.to_bits()); + if !jsval.is_any_string() { + return 0; + } + let ptr = crate::value::js_get_string_pointer_unified(receiver) as *const crate::StringHeader; + if ptr.is_null() { + return 0; + } + crate::string::js_string_length(ptr) as u64 +} + unsafe fn call_primitive_builtin_prototype_method( receiver: f64, builtin_name: &[u8], @@ -342,6 +356,12 @@ unsafe fn call_primitive_builtin_prototype_method( args_ptr: *const f64, args_len: usize, ) -> Option { + // #9761 attribution: this is the fork where an unrecognised primitive + // method name turns into a `globalThis` lookup plus, for a sloppy callee, + // a `ToObject` wrapper whose own index properties are O(receiver length). + crate::gc::diag_primitive_dispatch(builtin_name, method_name, unsafe { + primitive_receiver_utf16_len(receiver) + }); let ctor = crate::object::js_get_global_this_builtin_value(builtin_name.as_ptr(), builtin_name.len()); let ctor_value = JSValue::from_bits(ctor.to_bits()); @@ -374,7 +394,9 @@ unsafe fn call_primitive_builtin_prototype_method( if let Some(value) = builtin_proto_accessor_method(proto_ptr, method_name, receiver) { return call_primitive_closure_value(receiver, value, args_ptr, args_len); } - let key = crate::string::js_string_from_bytes(method_name.as_ptr(), method_name.len() as u32); + // A method name is a literal at the call site; the canonical interned + // header is allocated once per thread instead of once per dispatch. + let key = crate::string::canonical_key(method_name.as_bytes()); let value = js_object_get_field_by_name(proto_ptr, key); call_primitive_closure_value(receiver, value, args_ptr, args_len) } diff --git a/crates/perry-runtime/src/object/object_ops/prototype.rs b/crates/perry-runtime/src/object/object_ops/prototype.rs index 2dc1109e76..a4f8c418d3 100644 --- a/crates/perry-runtime/src/object/object_ops/prototype.rs +++ b/crates/perry-runtime/src/object/object_ops/prototype.rs @@ -37,8 +37,14 @@ pub extern "C" fn js_get_global_this_builtin_value(name_ptr: *const u8, name_len // one of them straddles the collection. let scope = crate::gc::RuntimeHandleScope::new(); let global_handle = scope.root_nanbox_f64(js_get_global_this()); - let (key, global_this_f64) = global_handle - .across_nanbox(|| crate::string::js_string_from_bytes(name.as_ptr(), name.len() as u32)); + // #9761: this lookup used to MINT the name string on every call — the + // comment below still records why that allocation is a collection point. + // It is now the canonical interned header, so the allocation happens once + // per thread per name instead of once per lookup: on the compiled cc TUI + // this single site was 133 MB of the 990 MB a 3300-character reply + // allocates (every primitive method call asks for `globalThis.String`). + let (key, global_this_f64) = + global_handle.across_nanbox(|| crate::string::canonical_key(name.as_bytes())); let global_obj = crate::value::js_nanbox_get_pointer(global_this_f64) as *const ObjectHeader; if global_obj.is_null() { return f64::from_bits(crate::value::TAG_UNDEFINED); diff --git a/crates/perry-runtime/src/object/prototype_helpers.rs b/crates/perry-runtime/src/object/prototype_helpers.rs index 169bab42a9..2e6e7e9771 100644 --- a/crates/perry-runtime/src/object/prototype_helpers.rs +++ b/crates/perry-runtime/src/object/prototype_helpers.rs @@ -5,7 +5,7 @@ pub(crate) fn constructor_dynamic_prototype(obj: *const ObjectHeader) -> Option< return None; } let key = - crate::string::js_string_from_bytes(b"constructor".as_ptr(), b"constructor".len() as u32); + crate::string::canonical_key(b"constructor"); let constructor = js_object_get_field_by_name_f64(obj, key); let bits = constructor.to_bits(); let top16 = bits >> 48; diff --git a/crates/perry-runtime/src/string/char_ops.rs b/crates/perry-runtime/src/string/char_ops.rs index 9bdf5cbba0..6ef47c734e 100644 --- a/crates/perry-runtime/src/string/char_ops.rs +++ b/crates/perry-runtime/src/string/char_ops.rs @@ -253,12 +253,16 @@ pub extern "C" fn js_string_char_at(s: *const StringHeader, index: i32) -> *mut return js_string_from_bytes(std::ptr::null(), 0); } - // ASCII fast path: skip utf16_len scan + // ASCII fast path: skip utf16_len scan. The result is one of exactly 128 + // possible strings, so it comes from the canonical per-thread table + // (`ascii_char_string`) instead of being minted: `s[i]` / `charAt` / + // `[...s]` / every runtime character walk stops allocating. The table's + // entries are `refcount = 0` (shared, never mutated in place), which is + // what makes returning the same pointer to every caller sound. if is_ascii_string(s) { unsafe { let data = string_data(s); - let char_ptr = data.add(index as usize); - return js_string_from_ascii_bytes(char_ptr, 1); + return crate::string::ascii_char_string(*data.add(index as usize)); } } @@ -369,8 +373,9 @@ fn encode_3byte_wtf8(unit: u16) -> [u8; 3] { /// for the old `char::from_u32(..).unwrap_or('\u{FFFD}')` lossy path. pub(crate) fn string_from_code_unit(unit: u16) -> *mut StringHeader { if unit < 0x80 { - let byte = unit as u8; - return js_string_from_bytes(&byte as *const u8, 1); + // Canonical table (see `ascii_char_string`): a one-ASCII-character + // string has 128 possible contents and is never mutated in place. + return crate::string::ascii_char_string(unit as u8); } if (0xD800..=0xDFFF).contains(&unit) { let buf = encode_3byte_wtf8(unit); diff --git a/crates/perry-runtime/src/string/format.rs b/crates/perry-runtime/src/string/format.rs index 2e46dc83e4..0d106aaab9 100644 --- a/crates/perry-runtime/src/string/format.rs +++ b/crates/perry-runtime/src/string/format.rs @@ -17,6 +17,52 @@ crate::perry_thread_local! { const { std::cell::UnsafeCell::new([std::ptr::null_mut(); SMALL_INT_CACHE_SIZE]) }; } +/// Cached single-ASCII-character string table (`"\0"`..`"\x7f"`), the exact +/// analogue of [`SMALL_INT_CACHE`] one dimension over: every `s[i]`, +/// `s.charAt(i)`, `[...s]` and every runtime consumer of +/// [`js_string_char_at`](super::js_string_char_at) used to MINT a fresh +/// 32-byte heap string per character read. On the compiled claude-code TUI — +/// which measures, wraps and ANSI-scans every rendered line — that is one of +/// the largest single contributors to allocation volume, and the bytes are +/// pure garbage: a one-character ASCII string has exactly 128 possible +/// contents. +/// +/// Same residency contract as `SMALL_INT_CACHE`, and for the same reasons: +/// per-thread (arena pointers are not shareable), longlived-arena (so the +/// entry never anchors a nursery block), `refcount = 0` (shared — never +/// mutated in place, which is what makes handing the SAME pointer to every +/// caller sound), pinned out of the young generation, and scanned by +/// [`scan_small_int_cache_roots_mut`] so the collector rewrites the slot if +/// the longlived object is ever relocated. +const ASCII_CHAR_CACHE_SIZE: usize = 128; +crate::perry_thread_local! { + static ASCII_CHAR_CACHE: std::cell::UnsafeCell<[*mut StringHeader; ASCII_CHAR_CACHE_SIZE]> = + const { std::cell::UnsafeCell::new([std::ptr::null_mut(); ASCII_CHAR_CACHE_SIZE]) }; +} + +/// The canonical one-character string for an ASCII byte. Allocates at most +/// once per byte value per thread; every later call is a load. +pub(crate) fn ascii_char_string(byte: u8) -> *mut StringHeader { + debug_assert!(byte < 0x80); + let idx = (byte & 0x7f) as usize; + let cached = ASCII_CHAR_CACHE.with(|c| unsafe { (*c.get())[idx] }); + if !cached.is_null() { + return cached; + } + let ptr = js_string_from_bytes_longlived(&byte as *const u8, 1); + unsafe { + (*ptr).refcount = 0; + let gc_header = + (ptr as *const u8).sub(crate::gc::GC_HEADER_SIZE) as *mut crate::gc::GcHeader; + crate::gc::pin_object_non_young(gc_header); + } + ASCII_CHAR_CACHE.with(|c| unsafe { + // GC_STORE_AUDIT(ROOT): ASCII_CHAR_CACHE is scanned by scan_small_int_cache_roots_mut. + crate::gc::runtime_store_root_raw_mut_ptr_slot(&raw mut (*c.get())[idx], ptr); + }); + ptr +} + /// Normalize a `Number.prototype` format-method receiver to its underlying /// `f64`. Codegen lowers `x.toFixed(n)` / `.toExponential(n)` / `.toPrecision(n)` /// to a direct runtime call that passes the receiver's bits as the first `f64` @@ -162,6 +208,19 @@ pub fn scan_small_int_cache_roots_mut(visitor: &mut crate::gc::RuntimeRootVisito } } }); + // The single-character table rides the same scanner rather than + // registering a 96th root scanner: both are per-thread arrays of + // canonical `StringHeader*` with identical residency rules, and the + // per-collection cost of every additional registered scanner is the + // thing the collector is trying to shed. + ASCII_CHAR_CACHE.with(|c| unsafe { + for slot in (*c.get()).iter_mut() { + let mut addr = *slot as usize; + if visitor.visit_tagged_usize_slot(&mut addr, crate::value::STRING_TAG) { + *slot = addr as *mut StringHeader; + } + } + }); } fn is_undefined_arg(value: f64) -> bool { diff --git a/crates/perry-runtime/src/string/mod.rs b/crates/perry-runtime/src/string/mod.rs index f673d45d22..3d1a6c7e88 100644 --- a/crates/perry-runtime/src/string/mod.rs +++ b/crates/perry-runtime/src/string/mod.rs @@ -167,6 +167,32 @@ pub use concat::{ pub use concat_site::{js_string_concat_site_value, CONCAT_SITE_SLOTS}; pub(crate) use format::fix_exponent_format; pub(crate) use format::js_format_f64; +pub(crate) use format::ascii_char_string; + +/// The canonical `StringHeader` for a runtime-internal constant property name. +/// +/// Perry's runtime resolves fixed names — `"constructor"`, `"prototype"`, +/// `"toString"`, the `globalThis` builtin a primitive method call dispatches +/// through — by MINTING a fresh heap string for the literal on every lookup +/// and throwing it away one call later. On the compiled claude-code TUI that +/// is measured in hundreds of megabytes per reply +/// (`js_get_global_this_builtin_value` alone: 133 MB of the 990 MB a +/// 3300-character reply allocates), all of it identical bytes. +/// +/// This routes those literals through the intern table that already exists for +/// exactly this purpose (`js_string_materialize_to_heap` uses it for computed +/// property names): content-keyed, per-thread, allocated once, address-stable, +/// `refcount = 0` and `GC_FLAG_INTERNED` so nothing mutates it in place — and +/// already covered by the intern-table root scanner, so it adds no new root +/// surface. A key that is interned also makes the property-read and +/// property-write fast paths eligible, which the freshly minted copy never was. +/// +/// Use it only for names the runtime itself spells as a literal. A key built +/// from user data belongs on the ordinary allocation path. +#[inline] +pub(crate) fn canonical_key(name: &[u8]) -> *mut StringHeader { + intern::intern_dispatch_bytes(0, name.as_ptr(), name.len(), 0, false) as *mut StringHeader +} pub use format::{ js_number_to_exponential, js_number_to_fixed, js_number_to_precision, js_number_to_string, scan_small_int_cache_roots, scan_small_int_cache_roots_mut, diff --git a/crates/perry-runtime/src/string/tests.rs b/crates/perry-runtime/src/string/tests.rs index c001ba15d9..0dcd02cbc1 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"); + } + } } /// `header_str_checked` answers exactly like `from_utf8(..).ok()` — a pure diff --git a/crates/perry-runtime/src/value/to_string.rs b/crates/perry-runtime/src/value/to_string.rs index b6a2b7b8d9..f87a92cc03 100644 --- a/crates/perry-runtime/src/value/to_string.rs +++ b/crates/perry-runtime/src/value/to_string.rs @@ -624,7 +624,7 @@ unsafe fn array_prototype_to_string_override(value: f64) -> ArrayToStringOutcome let scope = crate::gc::RuntimeHandleScope::new(); let value_handle = scope.root_nanbox_f64(value); let key_handle = - scope.root_string_ptr(crate::string::js_string_from_bytes(b"toString".as_ptr(), 8)); + scope.root_string_ptr(crate::string::canonical_key(b"toString")); let proto = crate::object::builtin_prototype_value("Array"); let proto_handle = scope.root_nanbox_f64(proto); let proto_bits = proto_handle.get_nanbox_f64().to_bits(); @@ -691,7 +691,7 @@ pub(crate) fn call_array_prototype_to_string_method( let scope = crate::gc::RuntimeHandleScope::new(); let receiver_handle = scope.root_nanbox_f64(value); let key_handle = - scope.root_string_ptr(crate::string::js_string_from_bytes(b"toString".as_ptr(), 8)); + scope.root_string_ptr(crate::string::canonical_key(b"toString")); let prototype_handle = scope.root_nanbox_f64(crate::object::builtin_prototype_value("Array")); let prototype_bits = prototype_handle.get_nanbox_f64().to_bits(); @@ -790,7 +790,7 @@ unsafe fn call_method_for_primitive( if obj_ptr.is_null() || (obj_ptr as usize) < 0x10000 { return MethodOutcome::Absent; } - let key = crate::string::js_string_from_bytes(method_name.as_ptr(), method_name.len() as u32); + let key = crate::string::canonical_key(method_name); let key_handle = scope.root_string_ptr(key); // Presence is independent from the value returned by Get. In particular, // an inherited accessor may exist yet return undefined/null; that is a @@ -870,7 +870,7 @@ unsafe fn call_function_method( return FunctionMethodOutcome::Absent; } - let key = crate::string::js_string_from_bytes(method_name.as_ptr(), method_name.len() as u32); + let key = crate::string::canonical_key(method_name); let key_handle = scope.root_string_ptr(key); let key_ptr = key_handle.get_raw_const_ptr::(); let method = function_method_value(closure_ptr, key_ptr, method_name); From 18450679fb3c8a1ee1c3f96297f8285b90f10df7 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 3/4] 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 79cf199420..d584b6870b 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 32f150b22349f9da6886f7180ec13da0a7e4d671 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 5 Sep 2026 10:09:53 +0200 Subject: [PATCH 4/4] fix(runtime): a built-in callee receives the primitive, not a ToObject wrapper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ECMA-262 §10.3.1: a BUILT-IN function's [[Call]] does not run OrdinaryCallBindThis. It receives `thisArg` unchanged and performs whatever coercion it needs itself — which every thunk in `primitive_proto_thunks` already does, accepting the raw primitive BEFORE it looks for a wrapper payload (`string_receiver_or_throw`, `number_receiver_or_throw`, ...). `call_primitive_closure_value` boxed for them anyway, and for a string receiver the `ToObject` wrapper materialises an own index property per UTF-16 code unit. The `codePointAt` arm (#9761) showed what one method name reaching this path costs: 99,008 wrappers per 400-character claude-code reply. This closes the class rather than the instance — the next builtin with a prototype thunk but no native dispatch arm costs a lookup, not a wrapper per character. Only a sloppy USER callee still gets the wrapper. `builtin_closure_length` is the registry that separates the two, and the test pins both directions: a `String.prototype` method closure reads as a built-in, a closure the runtime merely allocated does not — without the negative case the predicate could be "always true" and still pass. --- .../9800-builtin-callee-no-toobject.md | 11 ++++++ .../src/object/native_call_method.rs | 18 +++++++++- .../code_point_at_dispatch_tests.rs | 35 +++++++++++++++++++ 3 files changed, 63 insertions(+), 1 deletion(-) create mode 100644 changelog.d/9800-builtin-callee-no-toobject.md diff --git a/changelog.d/9800-builtin-callee-no-toobject.md b/changelog.d/9800-builtin-callee-no-toobject.md new file mode 100644 index 0000000000..59bc42edb9 --- /dev/null +++ b/changelog.d/9800-builtin-callee-no-toobject.md @@ -0,0 +1,11 @@ +### Runtime + +- fix(runtime): a method call on a primitive whose callee is a BUILT-IN no + longer boxes the receiver. ECMA-262 §10.3.1: a built-in function's `[[Call]]` + does not run `OrdinaryCallBindThis` — it receives `thisArg` unchanged and + performs its own coercion, which every `String`/`Number`/`Boolean`/`BigInt` + prototype thunk already does (they accept the raw primitive before looking + for a wrapper payload). Perry boxed for them anyway, and for a string + receiver that `ToObject` wrapper materialises an own index property per + UTF-16 code unit. Only a sloppy USER callee is owed the wrapper now, which is + the distinction the spec draws. diff --git a/crates/perry-runtime/src/object/native_call_method.rs b/crates/perry-runtime/src/object/native_call_method.rs index d584b6870b..5a43c93ee6 100644 --- a/crates/perry-runtime/src/object/native_call_method.rs +++ b/crates/perry-runtime/src/object/native_call_method.rs @@ -319,7 +319,23 @@ unsafe fn call_primitive_closure_value( let func_ptr = crate::closure::get_valid_func_ptr(ptr as *const crate::closure::ClosureHeader); let strict_callee = !func_ptr.is_null() && crate::closure::is_registered_strict_function(func_ptr); - let this_receiver = if strict_callee { + // ECMA-262 §10.3.1: a BUILT-IN function's [[Call]] does not run + // OrdinaryCallBindThis. It receives `thisArg` unchanged and performs its + // own coercion if it needs one — which every thunk in + // `primitive_proto_thunks` already does, accepting the raw primitive + // BEFORE it looks for a wrapper payload (`string_receiver_or_throw`, + // `number_receiver_or_throw`, …). Perry boxed for them anyway, and for a + // string receiver that wrapper materialises an own index property per + // UTF-16 code unit. `codePointAt` (#9761) showed the price of ONE method + // name reaching this path: 99,008 wrappers per 400-character claude-code + // reply. This closes the class instead of the instance — the next builtin + // that has a prototype thunk but no native dispatch arm costs a lookup, + // not a wrapper per character. + // + // Only a SLOPPY USER callee is still owed the wrapper, which is the + // distinction the spec actually draws. + let builtin_callee = crate::object::builtin_closure_length(ptr).is_some(); + let this_receiver = if strict_callee || builtin_callee { receiver_h.get_nanbox_f64() } else { crate::object::js_object_coerce(receiver_h.get_nanbox_f64()) 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 index e1ce0b8cfc..ae1b8b10e5 100644 --- 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 @@ -78,3 +78,38 @@ fn the_wrapper_counter_moves_when_a_receiver_is_boxed() { "if this cannot move, the codePointAt assertion above is vacuous" ); } + +/// #9761 follow-up: the discriminator `call_primitive_closure_value` uses to +/// decide whether a callee is owed a `ToObject` wrapper. +/// +/// ECMA-262 §10.3.1 gives a built-in's `[[Call]]` the `thisArg` unchanged, so +/// only a sloppy USER callee gets the wrapper. `builtin_closure_length` is the +/// registry that separates them, and this pins both directions — a +/// `String.prototype` method closure is recognised, a closure the runtime +/// merely allocated is not. Without the negative case the predicate could be +/// "always true" and still pass. +#[test] +fn builtin_prototype_method_closures_are_distinguishable_from_ordinary_ones() { + let proto = crate::object::global_this::builtin_prototype_value("String"); + let proto_ptr = crate::value::js_nanbox_get_pointer(proto) as *mut crate::object::ObjectHeader; + assert!(!proto_ptr.is_null(), "String.prototype must exist"); + + let key = crate::string::js_string_from_bytes(b"codePointAt".as_ptr(), 11); + let method = crate::object::js_object_get_field_by_name(proto_ptr, key); + let method_ptr = crate::value::js_nanbox_get_pointer(f64::from_bits(method.bits())) as usize; + assert!(method_ptr != 0, "String.prototype.codePointAt must resolve"); + assert!( + crate::object::builtin_closure_length(method_ptr).is_some(), + "a String.prototype method closure must read as a built-in" + ); + + extern "C" fn not_a_builtin(_c: *const crate::closure::ClosureHeader) -> f64 { + 0.0 + } + let ordinary = crate::closure::js_closure_alloc(not_a_builtin as *const u8, 0); + assert!( + crate::object::builtin_closure_length(ordinary as usize).is_none(), + "an ordinary closure must NOT read as a built-in, or every sloppy user \ + callee would silently lose its ToObject wrapper" + ); +}