From 2e99865be13ebeb493f65e17e06bb1064b63186a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 5 Sep 2026 21:27:42 +0200 Subject: [PATCH 1/5] perf(gc): route the trigger path and dirty-page barrier through hot TLS MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `gc_check_trigger` runs on every `gc_malloc`, and `gc_budgeted_due_trigger` resolved eleven raw `thread_local!` declarations one `_tlv_get_addr` call at a time. Measured with `sample` on the compiled claude-code TUI streaming a 3300-char reply (14,578 active main-thread samples, callers resolved by an explicit ancestor walk): `_tlv_get_addr` was 380 main-thread leaf samples, 71 of them with `gc_budgeted_due_trigger` as the immediate caller, 36 in `old_page_account_dirty_slots`, 31 in `scan_dirty_object_slots`, 27 in `gc_malloc_header_is_tracked`. Sixty-seven declarations move to `crate::perry_thread_local!`. Why they were still cold is a measurement bug in the gate, not an oversight: `scripts/check_thread_locals.py` ratchets on raw `thread_local!` BLOCKS per file, and a block holds any number of declarations — so `gc/policy.rs` counted as 6 while declaring 28, and adding a `static` to a recorded block passed silently. In the same unit as the hot side, main was 318 hot against 339 cold declarations. The gate now ratchets on declarations (385/272) and `--self-test` gained the direction that catches it. `ARENA_TOTAL_BYTES`, `BLOCK_POOL` and `BLOCK_POOL_BYTES` stay raw and say so: they are read from `Arena::new`, which runs as `tls_hot::fill`'s first provider, so a `HotKey` there re-enters `fill` — which has not yet written the `temp_roots` field it gates on — and re-runs `ARENA`'s initializer without bound. Claude-Session: https://claude.ai/code/session_014UZWia6L37DpA93VLtNK9m --- changelog.d/9830-gc-trigger-path-hot-tls.md | 55 +++++++++++ crates/perry-runtime/src/arena/block.rs | 44 +++++++++ crates/perry-runtime/src/arena/mod.rs | 2 + crates/perry-runtime/src/arena/page_meta.rs | 9 +- crates/perry-runtime/src/gc/barrier/mod.rs | 8 +- crates/perry-runtime/src/gc/malloc.rs | 17 +++- crates/perry-runtime/src/gc/old_free.rs | 10 +- crates/perry-runtime/src/gc/policy.rs | 62 ++++++++++-- crates/perry-runtime/src/gc/tenuring.rs | 6 +- crates/perry-runtime/src/gc/tests/mod.rs | 2 + .../src/gc/tests/tls_fill_reentrancy.rs | 43 +++++++++ .../src/gc/tests/trigger_path_tls.rs | 67 +++++++++++++ crates/perry-runtime/src/gc/trace.rs | 6 +- scripts/check_thread_locals.py | 62 +++++++++--- scripts/gc_runtime_root_holders.json | 4 +- scripts/thread_local_cold_allowlist.json | 94 +++++++++---------- 16 files changed, 408 insertions(+), 83 deletions(-) create mode 100644 changelog.d/9830-gc-trigger-path-hot-tls.md create mode 100644 crates/perry-runtime/src/gc/tests/tls_fill_reentrancy.rs create mode 100644 crates/perry-runtime/src/gc/tests/trigger_path_tls.rs diff --git a/changelog.d/9830-gc-trigger-path-hot-tls.md b/changelog.d/9830-gc-trigger-path-hot-tls.md new file mode 100644 index 0000000000..e791ac3b7d --- /dev/null +++ b/changelog.d/9830-gc-trigger-path-hot-tls.md @@ -0,0 +1,55 @@ +**The GC trigger path and the dirty-page barrier stop paying `_tlv_get_addr` +per read, and the policy gate that let them stop paying it now counts the +thing it is bounding.** + +`gc_check_trigger` runs on every `gc_malloc`, and its predicate +(`gc_budgeted_due_trigger`) resolved eleven raw `thread_local!` declarations +one out-of-line call at a time. Measured with `sample` on the compiled +claude-code TUI streaming a 3300-char reply (14,578 active main-thread +samples, callers resolved by an explicit ancestor walk rather than +nearest-symbol labels): `_tlv_get_addr` was 380 main-thread leaf samples, +**71 of them with `gc_budgeted_due_trigger` as the immediate caller**, 36 in +`old_page_account_dirty_slots`, 31 in `scan_dirty_object_slots`, 27 in +`gc_malloc_header_is_tracked`. `crates/perry-runtime/src/tls_hot.rs` has +existed to abolish exactly this since #7469; the allocation path's *fields* +were covered and the trigger path never was. + +Sixty-seven declarations across `gc/policy.rs`, `gc/malloc.rs`, `gc/old_free.rs`, +`gc/tenuring.rs`, `gc/trace.rs`, `gc/barrier/mod.rs`, `arena/block.rs` and +`arena/page_meta.rs` move to `crate::perry_thread_local!` — same syntax, same +`.with()` at every call site, the address served from this thread's hot cache +instead of a libdyld call. + +**Why they were still cold is a measurement bug in the gate, not an oversight +anyone could have noticed.** `scripts/check_thread_locals.py` ratchets on the +number of raw `thread_local!` **blocks** per file, while `thread_local! { … }` +holds any number of declarations — so `gc/policy.rs` counted as **6** while +declaring **28**, and adding a `static` to an already-recorded block passed +the gate silently. Counted in the same unit as the hot side, `main` was **318 +hot declarations against 339 cold ones** — cold was the majority, reported as +a 2.6:1 minority. The gate now ratchets on declarations (`385 hot / 272 +cold`), and `--self-test` grew a seventh direction that fails when a `static` +is added to a recorded block; restoring the block count makes that case, and +only that case, fail. + +Three declarations stay deliberately raw and say so at their declaration: +`ARENA_TOTAL_BYTES`, `BLOCK_POOL` and `BLOCK_POOL_BYTES` are read from +`Arena::new`, which runs as `tls_hot::fill`'s **first** provider, so a +`HotKey` there re-enters `fill` — which by design has not yet written the +`temp_roots` field it gates on — and re-runs `ARENA`'s initializer without +bound. It is a stack overflow at thread start, not a slow path, and it is the +first documented instance of the rule that a declaration read from inside a +`fill` provider cannot use the macro. `gc::tests::tls_fill_reentrancy` is the +standing guard, and it is sabotage-proved: moving `ARENA_TOTAL_BYTES` alone +into the neighbouring hot block aborts that test with `fatal runtime error: +stack overflow`. + +`gc::tests::trigger_path_tls` is the runtime half of the gate: it drives +`gc_check_trigger` on a fresh thread and asserts every trigger-path +declaration owns a hot slot and that the path publishes slots at all. +Reverting any one of them to a raw `thread_local!` removes `slot_index` and +breaks the build at that declaration's own name. It is a test that can fail +and did: the first run rejected `GC_DEFERRED_REQUEST` with `index 4294967295`, +correctly — `defer_gc_request` reads it only while a root lock is held, so it +is not a fast-path read and never claims a slot. The list is what the fast +path reads, not what the module declares. diff --git a/crates/perry-runtime/src/arena/block.rs b/crates/perry-runtime/src/arena/block.rs index 0e8a09e487..9f19920394 100644 --- a/crates/perry-runtime/src/arena/block.rs +++ b/crates/perry-runtime/src/arena/block.rs @@ -122,6 +122,29 @@ impl Drop for BlockPool { } } +// --------------------------------------------------------------------------- +// These three stay RAW, and not because nobody got to them: they are read from +// inside `Arena::new`, which runs as `ARENA`'s lazy initializer — and `ARENA` +// is the FIRST provider `tls_hot::fill` resolves. Routing them through the hot +// cache closes a cycle that ends in a stack overflow at thread start: +// +// HotKey::get -> hot() -> hot_uncached -> hot_via_tls +// -> (temp_roots still null) fill() +// -> arena_hot_addr() -> ARENA.with(..) -> Arena::new +// -> ARENA_TOTAL_BYTES / BLOCK_POOL{,_BYTES}.with(..) +// -> HotKey::get -> hot() -> ... (temp_roots STILL null) ... +// +// `fill` writes `temp_roots` last precisely so a re-entrant reader cannot see a +// half-filled cache as ready — which makes the nesting re-run `fill`, and +// re-run `ARENA`'s initializer, without bound. It bites on any thread where +// the first `hot()` precedes the first arena touch, i.e. every freshly spawned +// one, and it fails as a crash rather than as a slow path. +// +// The rule this is an instance of: a declaration read from the dynamic extent +// of a `tls_hot::fill` provider cannot use `crate::perry_thread_local!`. Today +// `ARENA` is the only provider whose initializer runs code, so this is the +// whole set. +// --------------------------------------------------------------------------- thread_local! { static BLOCK_POOL: RefCell = const { RefCell::new(BlockPool { blocks: Vec::new(), @@ -981,7 +1004,9 @@ thread_local! { /// alloc into a tombstone slot or the end, and release inside /// `arena_reset_empty_blocks`). pub(crate) static ARENA_TOTAL_BYTES: std::cell::Cell = const { std::cell::Cell::new(0) }; +} +crate::perry_thread_local! { /// Cached running sum of `block.offset` across the old-gen arena — /// the delta-maintained twin of `ARENA_TOTAL_BYTES` above, same /// rationale: `gc_budgeted_due_trigger()` reads the old-gen in-use @@ -1007,8 +1032,18 @@ thread_local! { /// the OldReclaim trigger. pub(crate) static OLD_GEN_IN_USE_BYTES: std::cell::Cell = const { std::cell::Cell::new(0) }; +} + +// `ARENA` and `INLINE_STATE` below stay raw: they are NAMED `HotTls` fields +// (`arena`, `inline_state`) whose addresses `tls_hot::fill` reads through the +// providers further down, and a named field is one dependent load cheaper +// than a claimed slot. Their neighbours in this block were never migrated. +thread_local! { pub(crate) static ARENA: UnsafeCell = UnsafeCell::new(Arena::new(HeapGeneration::Nursery, HeapSpace::NurseryEden)); +} + +crate::perry_thread_local! { /// Segregated long-lived arena (issue #179). Holds objects that are /// intentionally pinned for the lifetime of the program by explicit @@ -1056,6 +1091,9 @@ thread_local! { pub(crate) static OLD_ARENA: UnsafeCell = UnsafeCell::new(Arena::new_lazy(HeapGeneration::Old, HeapSpace::Old)); +} + +thread_local! { /// Inline allocator state — a cache of the current arena block's /// `(data, offset, size)` tuple, exposed via a stable pointer so /// codegen can emit inline bump-allocate IR without going through @@ -1072,6 +1110,12 @@ thread_local! { }) }; } +/// Hot-cache slot claimed by `OLD_GEN_IN_USE_BYTES`. See above. +#[cfg(test)] +pub(crate) fn old_gen_in_use_bytes_slot_index() -> u32 { + OLD_GEN_IN_USE_BYTES.slot_index() +} + // --- #7469 hot-TLS address providers. See `crate::tls_hot`. --- /// Address of this thread's `ARENA`. Resolving it once and caching it is what diff --git a/crates/perry-runtime/src/arena/mod.rs b/crates/perry-runtime/src/arena/mod.rs index 9ba5401b19..4a92a805c2 100644 --- a/crates/perry-runtime/src/arena/mod.rs +++ b/crates/perry-runtime/src/arena/mod.rs @@ -46,6 +46,8 @@ pub(crate) use block::{ /// allocation path uses instead of a per-access `_tlv_get_addr`. pub(crate) use block::{arena_hot_addr, hot_arena, hot_inline_state, inline_state_hot_addr}; #[cfg(test)] +pub(crate) use block::old_gen_in_use_bytes_slot_index; +#[cfg(test)] pub(crate) use block::{ block_pool_bytes_for_test, block_pool_explicit_drained_bytes_for_test, block_pool_put, force_next_block_alloc_failure, gc_trigger_arena_borrow_depth, gc_trigger_arena_calls, diff --git a/crates/perry-runtime/src/arena/page_meta.rs b/crates/perry-runtime/src/arena/page_meta.rs index bc6984dff3..0fe26f3c9b 100644 --- a/crates/perry-runtime/src/arena/page_meta.rs +++ b/crates/perry-runtime/src/arena/page_meta.rs @@ -319,13 +319,20 @@ pub(crate) struct OldArenaSourceBlockSelection { pub(crate) pages: crate::fast_hash::PtrHashSet, } +// `PAGE_GENERATIONS` and `PAGE_GENERATION_CACHE` stay raw because they are +// NAMED fields of `HotTls` (`page_generations`, `page_generation_cache`), whose +// providers below hand `tls_hot::fill` their addresses. A named field is one +// dependent load cheaper than a claimed slot, which is why the closed set +// exists; everything else in this file was simply never migrated. thread_local! { static PAGE_GENERATIONS: RefCell = RefCell::new(crate::fast_hash::new_ptr_hash_map()); static PAGE_GENERATION_CACHE: UnsafeCell = const { UnsafeCell::new(PageGenerationCacheSet::empty()) }; +} +crate::perry_thread_local! { static OLD_GEN_PAGE_OBJECTS: RefCell = RefCell::new(crate::fast_hash::new_ptr_hash_map()); @@ -1138,7 +1145,7 @@ pub(crate) fn register_old_object_pages(header_addr: usize, total_size: usize) { // which fails if a new toucher of either table appears without one. // --------------------------------------------------------------------------- -thread_local! { +crate::perry_thread_local! { /// Old-object page registrations not yet folded into `OLD_GEN_PAGE_OBJECTS`. /// Entries are `(header_addr, total_size)`; nothing here is dereferenced, so /// a deferred entry never keeps an object alive and is not a GC root — and diff --git a/crates/perry-runtime/src/gc/barrier/mod.rs b/crates/perry-runtime/src/gc/barrier/mod.rs index 808a66427c..a4bbd8e407 100644 --- a/crates/perry-runtime/src/gc/barrier/mod.rs +++ b/crates/perry-runtime/src/gc/barrier/mod.rs @@ -697,6 +697,10 @@ pub(super) unsafe fn scan_dirty_object_slots( // retained only as a test fallback for the previous object-level // HashSet behavior. +// The three declarations below stay raw: they are NAMED `HotTls` fields +// (`incremental_mark_valid_ptrs`, `birth_extra_flags`, +// `incremental_mark_minor_only`), one dependent load cheaper than a claimed +// slot. Everything after them was simply never migrated. thread_local! { /// Active incremental mark barrier state (Full AND budgeted Minor /// cycles — a Minor cycle sliced across mutator turns has exactly the @@ -737,7 +741,9 @@ thread_local! { /// later. Old children need no shading in a minor anyway: minors never /// collect live old-gen objects. pub(super) static INCREMENTAL_MARK_BARRIER_MINOR_ONLY: Cell = const { Cell::new(false) }; +} +crate::perry_thread_local! { /// Dirty old-generation pages that have received a YOUNG-gen /// pointer since the last collection. This is Perry's compact /// modbuf: barriers log bounded page regions, and minor GC scans @@ -1761,7 +1767,7 @@ fn mark_dirty_old_page_uncached(page: usize) -> bool { inserted } -thread_local! { +crate::perry_thread_local! { /// PERRY_GC_VERIFY_EVACUATION diagnostic only: every old page EVER marked /// dirty over the process lifetime (never cleared). Lets the verifier's /// missing-edge report say whether the slot's page was recorded at some diff --git a/crates/perry-runtime/src/gc/malloc.rs b/crates/perry-runtime/src/gc/malloc.rs index 7203300abf..df85b12362 100644 --- a/crates/perry-runtime/src/gc/malloc.rs +++ b/crates/perry-runtime/src/gc/malloc.rs @@ -196,7 +196,7 @@ pub(super) const MALLOC_STATE_HEAVY_LEN_THRESHOLD: usize = 64 * 1024; /// 7/8 load factor without further rehashes). pub(super) const MALLOC_STATE_HEAVY_CAPACITY: usize = 256 * 1024; -thread_local! { +crate::perry_thread_local! { pub(crate) static MALLOC_STATE: RefCell = RefCell::new(MallocState { objects: Vec::with_capacity(MALLOC_STATE_INITIAL_CAPACITY), set: crate::fast_hash::PtrHashSet::with_capacity_and_hasher( @@ -209,12 +209,25 @@ thread_local! { heavy_capacity_reserved: false, kind_telemetry: [MallocKindTelemetry::zero(); MALLOC_KIND_BUCKET_COUNT], }); +} +// `ARENA_FREE_LIST` / `ARENA_FREE_LIST_NONEMPTY` stay raw: they are NAMED +// `HotTls` fields (`arena_free_list`, `arena_free_list_nonempty`), one +// dependent load cheaper than a claimed slot. +thread_local! { pub(crate) static ARENA_FREE_LIST: RefCell> = const { RefCell::new(Vec::new()) }; pub(crate) static ARENA_FREE_LIST_NONEMPTY: std::cell::Cell = const { std::cell::Cell::new(false) }; } +/// Hot-cache slot claimed by `MALLOC_STATE`, whose length is the +/// `MallocCount` trigger's basis on every `gc_malloc`. Liveness +/// instrumentation for `gc::tests::trigger_path_tls`. +#[cfg(test)] +pub(crate) fn malloc_state_slot_index() -> u32 { + MALLOC_STATE.slot_index() +} + pub fn gc_malloc(size: usize, obj_type: u8) -> *mut u8 { let total = GC_HEADER_SIZE + size; let layout = Layout::from_size_align(total, 8).unwrap(); @@ -480,7 +493,7 @@ pub(super) fn malloc_sweep_revalidate_header( }) } -thread_local! { +crate::perry_thread_local! { pub(super) static MALLOC_REGISTRY_REBUILD_COUNT: Cell = const { Cell::new(0) }; } diff --git a/crates/perry-runtime/src/gc/old_free.rs b/crates/perry-runtime/src/gc/old_free.rs index 360a75c4c8..de64ba30e2 100644 --- a/crates/perry-runtime/src/gc/old_free.rs +++ b/crates/perry-runtime/src/gc/old_free.rs @@ -47,7 +47,7 @@ use super::*; -thread_local! { +crate::perry_thread_local! { /// total_size -> user_ptrs of swept holes of exactly that size. static OLD_FREE_MAP: RefCell>> = RefCell::new(crate::fast_hash::new_ptr_hash_map()); @@ -60,6 +60,14 @@ pub(crate) fn old_free_bytes() -> usize { OLD_FREE_BYTES.with(Cell::get) } +/// Hot-cache slot claimed by `OLD_FREE_BYTES`, which +/// `gc_budgeted_due_trigger` reads on every `gc_malloc`. Liveness +/// instrumentation for `gc::tests::trigger_path_tls`. +#[cfg(test)] +pub(crate) fn old_free_bytes_slot_index() -> u32 { + OLD_FREE_BYTES.slot_index() +} + fn old_free_push(user_ptr: usize, total_size: usize) { if user_ptr == 0 || total_size < GC_HEADER_SIZE { return; diff --git a/crates/perry-runtime/src/gc/policy.rs b/crates/perry-runtime/src/gc/policy.rs index 0977d5e70a..ef59bef746 100644 --- a/crates/perry-runtime/src/gc/policy.rs +++ b/crates/perry-runtime/src/gc/policy.rs @@ -5,7 +5,7 @@ pub(super) const GC_FLAG_IN_ALLOC: u8 = 0b01; /// Bit 1 of GC_FLAGS — suppression flag (JSON.parse). pub(super) const GC_FLAG_SUPPRESSED: u8 = 0b10; -thread_local! { +crate::perry_thread_local! { pub(super) static GC_FLAGS: Cell = const { Cell::new(0) }; } @@ -292,7 +292,7 @@ thread_local! { static GC_NURSERY_CAP_TEST_SUPPRESSED: Cell = const { Cell::new(false) }; } -thread_local! { +crate::perry_thread_local! { /// Lower bound for the next GC trigger. Bumped after each /// `gc_collect_inner` based on collection effectiveness (see the /// adaptive logic in `gc_check_trigger`). @@ -401,7 +401,7 @@ pub(super) const GC_MALLOC_COUNT_STEP_INITIAL: usize = 100_000; pub(super) const GC_MALLOC_COUNT_STEP_MAX: usize = 2_000_000; pub(super) const GC_MALLOC_COUNT_STEP_MIN: usize = 10_000; -thread_local! { +crate::perry_thread_local! { /// Per-program adaptive malloc-count step. Mirrors `GC_STEP_BYTES` /// behaviour: doubles when mostly-garbage, halves when mostly-live. pub(super) static GC_MALLOC_COUNT_STEP: std::cell::Cell = @@ -431,7 +431,7 @@ thread_local! { /// the Map/Set side-allocation finalizers) instead of leaking. const GC_EXTERNAL_SIDE_ALLOC_STEP: usize = 16 * 1024 * 1024; -thread_local! { +crate::perry_thread_local! { static GC_EXTERNAL_SIDE_ALLOC_PENDING: std::cell::Cell = const { std::cell::Cell::new(0) }; static GC_EXTERNAL_SIDE_LIVE_BYTES: std::cell::Cell = const { std::cell::Cell::new(0) }; } @@ -942,7 +942,7 @@ impl GcTriggerSnapshot { } } -thread_local! { +crate::perry_thread_local! { pub(super) static GC_DEFERRED_REQUEST: Cell = const { Cell::new(DeferredGcRequest::None) }; pub(super) static GC_OLD_RECLAIM_PENDING: Cell = const { Cell::new(false) }; @@ -2655,7 +2655,7 @@ enum BudgetedGcTrigger { MallocCount, } -thread_local! { +crate::perry_thread_local! { static GC_BUDGETED_CYCLE: RefCell> = const { RefCell::new(None) }; static GC_BUDGETED_CYCLE_ACTIVE: Cell = const { Cell::new(false) }; static GC_BUDGETED_STEP_ACTIVE: Cell = const { Cell::new(false) }; @@ -2690,6 +2690,56 @@ pub(super) fn gc_old_reclaim_debt_bytes(old_in_use: usize, baseline: usize) -> u old_in_use.saturating_sub(trigger) as u64 } +/// The thread-locals [`gc_budgeted_due_trigger`] reads on its fast path, with +/// the hot-cache slot each one claimed. +/// +/// Enumerating them in the module that reads them is what makes the coverage +/// test able to fail: reverting any declaration below to a raw +/// `thread_local!` removes `slot_index` and breaks this function's build at +/// the declaration's own name, rather than leaving a silently slow path — the +/// failure mode `tls_hot.rs` was written to abolish and that this path +/// nonetheless kept for three years. +#[cfg(test)] +pub(crate) fn trigger_path_hot_slot_indices() -> Vec<(&'static str, u32)> { + // Touch each one first: a slot is claimed on first read, not at + // declaration, so an unread declaration reports the unassigned sentinel. + // + // `GC_DEFERRED_REQUEST` is deliberately absent: `defer_gc_request` reads it + // only when `GC_ROOT_LOCK_DEPTH` is non-zero, so on the fast path it is + // never touched and never claims a slot. Listing it made this function's + // own test fail with `index 4294967295` on the first run — which is the + // evidence that the test can fail, and the reason the list is "what the + // fast path reads" rather than "what the module declares". + let _ = gc_budgeted_due_trigger(); + vec![ + ("GC_OLD_RECLAIM_PENDING", GC_OLD_RECLAIM_PENDING.slot_index()), + ( + "GC_LAST_OLD_RECLAIM_IN_USE_BYTES", + GC_LAST_OLD_RECLAIM_IN_USE_BYTES.slot_index(), + ), + ("GC_NEXT_MALLOC_TRIGGER", GC_NEXT_MALLOC_TRIGGER.slot_index()), + ("GC_NEXT_TRIGGER_BYTES", GC_NEXT_TRIGGER_BYTES.slot_index()), + ("GC_TRIGGER_ARMED", GC_TRIGGER_ARMED.slot_index()), + ( + "GC_EXTERNAL_SIDE_LIVE_BYTES", + GC_EXTERNAL_SIDE_LIVE_BYTES.slot_index(), + ), + ("GC_MAJOR_PACING_RETAINING", GC_MAJOR_PACING_RETAINING.slot_index()), + ("GC_FLAGS", GC_FLAGS.slot_index()), + ( + "GC_BUDGETED_CYCLE_ACTIVE", + GC_BUDGETED_CYCLE_ACTIVE.slot_index(), + ), + ("GC_BUDGETED_STEP_ACTIVE", GC_BUDGETED_STEP_ACTIVE.slot_index()), + ( + "OLD_GEN_IN_USE_BYTES", + crate::arena::old_gen_in_use_bytes_slot_index(), + ), + ("OLD_FREE_BYTES", super::old_free_bytes_slot_index()), + ("MALLOC_STATE", super::malloc_state_slot_index()), + ] +} + fn gc_budgeted_due_trigger() -> Option { let old_pending = GC_OLD_RECLAIM_PENDING.with(Cell::get); // #6010: external Map/Set side-buffer bytes escalate to OldReclaim too. diff --git a/crates/perry-runtime/src/gc/tenuring.rs b/crates/perry-runtime/src/gc/tenuring.rs index 4fd221261f..1abb9dd99c 100644 --- a/crates/perry-runtime/src/gc/tenuring.rs +++ b/crates/perry-runtime/src/gc/tenuring.rs @@ -152,7 +152,7 @@ const RAISE_DEBOUNCE_CYCLES: u8 = 2; /// adaptive threshold has eliminated the re-copying). const NURSERY_CAP_SCALE_MAX: u8 = 4; -thread_local! { +crate::perry_thread_local! { static TENURING_SURVIVALS: Cell = const { Cell::new(GC_TENURING_SURVIVALS_MAX) }; static RAISE_STREAK: Cell = const { Cell::new(0) }; /// Survival-rate lock: promote-on-first-copy until influx goes quiet. @@ -354,7 +354,7 @@ pub(super) fn note_surviving_object_census(moved_bytes: usize, moved_objects: us return; } OBJECT_CENSUS_SEEDED.with(|seeded| seeded.set(true)); - let previous = MEAN_SURVIVING_OBJECT_BYTES.replace(mean); + let previous = MEAN_SURVIVING_OBJECT_BYTES.with(|c| c.replace(mean)); if previous != mean && crate::gc::gc_diag_enabled() { eprintln!( "[gc-tenuring] nursery cap object denomination: mean_surviving_object_bytes {} -> {} \ @@ -417,7 +417,7 @@ pub(super) fn maybe_seed_object_census_from_allocation(from_space_in_use_bytes: if mean == 0 { return; } - let previous = MEAN_SURVIVING_OBJECT_BYTES.replace(mean); + let previous = MEAN_SURVIVING_OBJECT_BYTES.with(|c| c.replace(mean)); if crate::gc::gc_diag_enabled() { eprintln!( "[gc-tenuring] nursery cap object denomination: allocation census seeded \ diff --git a/crates/perry-runtime/src/gc/tests/mod.rs b/crates/perry-runtime/src/gc/tests/mod.rs index ac6d08f01e..959a63a087 100644 --- a/crates/perry-runtime/src/gc/tests/mod.rs +++ b/crates/perry-runtime/src/gc/tests/mod.rs @@ -58,6 +58,8 @@ mod smoke; mod step_bounds; pub(super) mod support; mod teardown; +mod tls_fill_reentrancy; +mod trigger_path_tls; mod telemetry_verifier; mod temp_roots; mod triggers; diff --git a/crates/perry-runtime/src/gc/tests/tls_fill_reentrancy.rs b/crates/perry-runtime/src/gc/tests/tls_fill_reentrancy.rs new file mode 100644 index 0000000000..da66fb1113 --- /dev/null +++ b/crates/perry-runtime/src/gc/tests/tls_fill_reentrancy.rs @@ -0,0 +1,43 @@ +//! A thread whose FIRST act is resolving the hot-TLS cache must survive it. +//! +//! `tls_hot::fill`'s first provider is `arena_hot_addr()`, which initializes +//! `ARENA` — and `Arena::new` reads `ARENA_TOTAL_BYTES` and `BLOCK_POOL`. If +//! either of those were declared with `crate::perry_thread_local!`, the read +//! would go `HotKey::get -> hot() -> hot_uncached -> hot_via_tls`, find +//! `temp_roots` still null (`fill` writes it LAST, deliberately, so a +//! re-entrant reader cannot mistake a half-filled cache for a ready one), call +//! `fill` again, and re-run `ARENA`'s initializer — without bound. +//! +//! That failure is a stack overflow at thread start, not a slow path, and it +//! only appears on a thread where the first `hot()` precedes the first arena +//! touch. This test is that thread. It is the standing guard on the rule +//! recorded at those three declarations in `arena/block.rs`: a thread-local +//! read from inside the dynamic extent of a `fill` provider cannot use the +//! macro. +//! +//! Sabotage-proved: moving `ARENA_TOTAL_BYTES` alone into the +//! `crate::perry_thread_local!` block below it makes this test abort with +//! `thread '' has overflowed its stack / fatal runtime error: stack +//! overflow` (release, 2026-09-05). It is not a hypothetical. +#[test] +fn a_fresh_thread_may_resolve_the_hot_cache_before_touching_the_arena() { + // A small stack so an unbounded `fill` recursion aborts here rather than + // running long enough to look like a hang. + std::thread::Builder::new() + .stack_size(1 << 20) + .spawn(|| { + // `published_slots` reaches `hot()` and nothing else, so this is + // the first arena-touching call on the thread. + let _ = crate::tls_hot::published_slots(); + // And the arena still works afterwards: `fill` ran to completion + // rather than being abandoned mid-way by a recursion guard. + assert!( + crate::arena::arena_total_bytes() > 0, + "the arena reported no reserved bytes after `fill`, so \ + `ARENA`'s initializer did not complete on this thread" + ); + }) + .unwrap() + .join() + .expect("resolving the hot TLS cache first on a fresh thread panicked"); +} diff --git a/crates/perry-runtime/src/gc/tests/trigger_path_tls.rs b/crates/perry-runtime/src/gc/tests/trigger_path_tls.rs new file mode 100644 index 0000000000..e5791c4e8e --- /dev/null +++ b/crates/perry-runtime/src/gc/tests/trigger_path_tls.rs @@ -0,0 +1,67 @@ +//! The `gc_check_trigger` fast path must resolve its thread-locals through the +//! hot cache, not through `_tlv_get_addr`. +//! +//! `gc_check_trigger` runs on **every** `gc_malloc`. Measured with `sample` on +//! the compiled claude-code TUI streaming a 3300-char reply (14,580 main-thread +//! samples), `_tlv_get_addr` was 380 main-thread leaf samples and **85 of them +//! sat under `gc_budgeted_due_trigger < gc_check_trigger < gc_malloc`** — the +//! trigger predicate resolving `GC_OLD_RECLAIM_PENDING`, +//! `GC_LAST_OLD_RECLAIM_IN_USE_BYTES`, `GC_NEXT_MALLOC_TRIGGER`, +//! `GC_TRIGGER_ARMED`, `GC_NEXT_TRIGGER_BYTES`, `GC_EXTERNAL_SIDE_LIVE_BYTES`, +//! `OLD_GEN_IN_USE_BYTES`, `ARENA_TOTAL_BYTES`, `OLD_FREE_BYTES`, +//! `MALLOC_STATE` and the survivor cells one out-of-line call at a time. +//! +//! `scripts/check_thread_locals.py` cannot express this: it ratchets on the +//! number of raw `thread_local!` *blocks* per file, so `gc/policy.rs` read as +//! "6" while declaring 28 cold thread-locals, and the block count does not move +//! when a declaration is converted alongside a split (`arena/block.rs` went +//! from 11 cold declarations to 0 with its recorded count unchanged at 2). The +//! declaration-denominated ratchet added in the same change closes the *policy* +//! hole; this test is the *runtime* half — it asserts the mechanism is live on +//! the path the profile named. + +use crate::tls_hot::{published_slots, HOT_SLOT_CAPACITY}; + +/// Every trigger-path declaration named above must own a hot slot, and +/// driving `gc_check_trigger` must actually populate slots on the calling +/// thread. +/// +/// Two assertions, because they fail to different sabotage. Reverting one +/// declaration to a raw `thread_local!` removes `slot_index` and breaks the +/// build at the named line; making the slot allocator hand out its overflow +/// sentinel keeps the build green and trips the bound check below. +#[test] +fn gc_check_trigger_resolves_its_thread_locals_through_the_hot_cache() { + // A fresh thread so the "how many slots did this path publish" reading is + // this path's, not the harness's. + std::thread::spawn(|| { + let before = published_slots(); + crate::gc::gc_check_trigger(); + let after = published_slots(); + + for (name, idx) in crate::gc::policy::trigger_path_hot_slot_indices() { + assert!( + (idx as usize) < HOT_SLOT_CAPACITY, + "{name} did not claim a hot TLS slot (index {idx}, capacity \ + {HOT_SLOT_CAPACITY}); it is paying `_tlv_get_addr` per read on \ + every `gc_malloc`", + ); + } + + // The predicate reads at least the ten declarations listed in the + // module docs. A lower bound, deliberately: `gc_check_trigger` reaches + // other hot declarations too, and this must not fail when an unrelated + // one is added or removed. It DOES fail if the trigger path stops + // going through `HotKey` at all, which is the regression #7469 has + // already suffered three times. + let published = after - before; + assert!( + published >= 11, + "gc_check_trigger published only {published} hot TLS slots on a \ + fresh thread ({before} -> {after}); the trigger path's \ + thread-locals are resolving through `_tlv_get_addr` again", + ); + }) + .join() + .expect("trigger-path TLS probe thread panicked"); +} diff --git a/crates/perry-runtime/src/gc/trace.rs b/crates/perry-runtime/src/gc/trace.rs index fb350daa45..0232cc6a60 100644 --- a/crates/perry-runtime/src/gc/trace.rs +++ b/crates/perry-runtime/src/gc/trace.rs @@ -1,6 +1,6 @@ use super::*; -thread_local! { +crate::perry_thread_local! { /// Set by test-only helpers that wipe page metadata for isolation /// (`old_arena_page_index_clear_for_tests`): real objects become /// unclassifiable in that synthetic state, so the differential verifier @@ -85,7 +85,7 @@ pub(super) fn classifier_verify_enabled() -> bool { *CACHED.get_or_init(|| super::env_flag_enabled("PERRY_GC_VERIFY_CLASSIFIER")) } -thread_local! { +crate::perry_thread_local! { pub(super) static MARK_SEEDS: std::cell::UnsafeCell> = const { std::cell::UnsafeCell::new(Vec::new()) }; } @@ -922,7 +922,7 @@ pub(super) fn trace_marked_objects(valid_ptrs: &ValidPointerSet) { /// old-block neighbors as new block-persist candidates. pub(super) const BLOCK_PERSIST_WINDOW: usize = 5; -thread_local! { +crate::perry_thread_local! { /// Objects this thread's block-persistence pass has FORCE-MARKED since /// process start — i.e. kept alive for no reason other than sharing a /// block with something reachable. diff --git a/scripts/check_thread_locals.py b/scripts/check_thread_locals.py index b2db1a1481..bff22628f2 100755 --- a/scripts/check_thread_locals.py +++ b/scripts/check_thread_locals.py @@ -211,11 +211,23 @@ def cfg_test_module_files(root: Path, crates: list[str]) -> set[str]: return test_only -def shipping_raw_blocks(src: str) -> int: - """Raw `thread_local!` blocks that survive into a non-test build. +def shipping_raw_declarations(src: str) -> int: + """Raw `thread_local!` DECLARATIONS that survive into a non-test build. Skips a block carrying `#[cfg(test)]` directly above it, and any block inside an inline `#[cfg(test)] mod … { … }`. + + Declarations, not blocks. `_tlv_get_addr` is paid per *declaration* read, + and `thread_local! { … }` holds any number of them, so a block-denominated + ratchet cannot see the thing it exists to bound. Measured on this + repository at `d36a1af0c`: the 122 recorded cold BLOCKS were **339 cold + declarations**, `gc/policy.rs` alone recorded as 6 while declaring 28 — + and a `static` added to an already-recorded block passed the gate in + silence, which is how the allocation path re-accumulated the cost #7469 + removed. Counting declarations also makes the two halves of the report + commensurable: the hot side was already counted in declarations, so + "318 hot / 122 cold" was comparing unlike units and read as a 2.6:1 + majority for the mechanism when cold was in fact the larger number. """ gated_spans = [brace_span(src, m.start()) for m in CFG_TEST_INLINE_MOD_RE.finditer(src)] count = 0 @@ -226,12 +238,13 @@ def shipping_raw_blocks(src: str) -> int: line = preceding[preceding.rfind("\n") + 1 :].strip() if line == "#[cfg(test)]": continue - count += 1 + open_at, close_at = brace_span(src, m.start()) + count += len(DECL_RE.findall(src[open_at + 1 : close_at])) return count def scan(root: Path, crates: list[str]) -> tuple[dict[str, int], int]: - """Raw `thread_local!` blocks per file, and total hot declarations.""" + """Raw `thread_local!` declarations per file, and total hot declarations.""" raw: dict[str, int] = {} hot_declarations = 0 test_only_files = cfg_test_module_files(root, crates) @@ -249,7 +262,7 @@ def scan(root: Path, crates: list[str]) -> tuple[dict[str, int], int]: ) if rel in EXCLUDED or rel in test_only_files: continue - count = shipping_raw_blocks(src) + count = shipping_raw_declarations(src) if count: raw[rel] = count return raw, hot_declarations @@ -271,7 +284,7 @@ def verify(root: Path, crates: list[str], allowlist_path: Path) -> list[str]: for rel, count in sorted(raw.items()): if rel not in recorded: problems.append( - f"{rel}: {count} raw `thread_local!` block(s), none allowed.\n" + f"{rel}: {count} raw `thread_local!` declaration(s), none allowed.\n" f" Use `crate::perry_thread_local!` — same syntax, same " f"`.with()` at every call site, and the address lands in this " f"thread's hot cache instead of costing a `_tlv_get_addr` call " @@ -281,7 +294,7 @@ def verify(root: Path, crates: list[str], allowlist_path: Path) -> list[str]: elif recorded[rel] != count: direction = "gained" if count > recorded[rel] else "lost" problems.append( - f"{rel}: {direction} raw `thread_local!` blocks " + f"{rel}: {direction} raw `thread_local!` declarations " f"({recorded[rel]} recorded, {count} found). " f"Convert it, or run --update to re-record." ) @@ -290,7 +303,7 @@ def verify(root: Path, crates: list[str], allowlist_path: Path) -> list[str]: if rel not in raw: problems.append( f"{rel}: recorded as having {recorded[rel]} cold " - f"`thread_local!` block(s), but has none. A stale entry is one " + f"`thread_local!` declaration(s), but has none. A stale entry is one " f"nobody has to justify — delete it with --update." ) @@ -311,9 +324,11 @@ def write_allowlist(root: Path, crates: list[str], allowlist_path: Path) -> None json.dumps( { "_comment": ( - "Files still declaring raw `thread_local!`. Every entry is a " - "declaration that pays `_tlv_get_addr` on Darwin; the count is " - "a ratchet, so adding one to an already-listed file fails too. " + "Files still declaring raw `thread_local!`. The count is the " + "number of DECLARATIONS that survive into a shipping build — " + "each one pays `_tlv_get_addr` per read on Darwin — and it is a " + "ratchet, so adding a `static` to an already-listed file fails " + "whether or not it opens a new block. " "New code should use `crate::perry_thread_local!` — see " "crates/perry-runtime/src/tls_hot.rs. Regenerate with " "scripts/check_thread_locals.py --update." @@ -364,13 +379,30 @@ def self_test() -> int: failures.append("a new raw `thread_local!` in an unlisted file passed") (src_dir / "new.rs").unlink() - # 2. A second raw declaration in an already-listed file must fail. + # 2. A second raw declaration in an already-listed file must fail — + # in a NEW block… write_source(src_dir / "cold.rs", "thread_local! { static A: u8 = const { 0 }; }\n" "thread_local! { static D: u8 = const { 0 }; }\n" ) if not verify(root, CRATES, allowlist): - failures.append("a raw `thread_local!` added to a listed file passed") + failures.append("a raw `thread_local!` block added to a listed file passed") + + # 2b. …and, the half a block-denominated ratchet could not see, inside + # the block that is ALREADY recorded. This is how the allocation + # path re-accumulated the cost #7469 removed: `gc/policy.rs` + # declared 28 cold thread-locals while the allowlist read "6", and + # every one after the first was free to add. + write_source(src_dir / "cold.rs", + "thread_local! {\n" + " static A: u8 = const { 0 };\n" + " static D: u8 = const { 0 };\n" + "}\n" + ) + if not verify(root, CRATES, allowlist): + failures.append( + "a raw `static` added to a listed file's EXISTING block passed" + ) # 3. A stale entry must fail. write_source(src_dir / "cold.rs", @@ -429,7 +461,7 @@ def self_test() -> int: print(f"SELF-TEST FAILED: {f}", file=sys.stderr) if failures: return 1 - print("self-test: the checker can fail in all six directions") + print("self-test: the checker can fail in all seven directions") return 0 @@ -455,7 +487,7 @@ def main() -> int: raw, hot_declarations = scan(REPO, CRATES) print( f"thread-local policy OK: {hot_declarations} hot declarations, " - f"{sum(raw.values())} raw blocks in {len(raw)} recorded cold files, " + f"{sum(raw.values())} cold declarations in {len(raw)} recorded files, " f"capacity {hot_slot_capacity(REPO)}" ) return 0 diff --git a/scripts/gc_runtime_root_holders.json b/scripts/gc_runtime_root_holders.json index b52c8629f5..e3b4d1fd22 100644 --- a/scripts/gc_runtime_root_holders.json +++ b/scripts/gc_runtime_root_holders.json @@ -270,7 +270,7 @@ "file": "crates/perry-runtime/src/gc/census.rs", "name": "PASS1_MARKED", "verdict": "non_moving_snapshot", - "why": "Real GC header addresses, deliberately untraced so the diagnostic does not keep its observed objects alive. Populated only at the end of mark propagation of a synchronous full cycle; consumed at sweep entry in the same run_to_completion invocation. The intervening full-cycle phases do not relocate or run JS callbacks. The Vec is used for membership comparisons and dropped with the census before sweep. Budgeted and minor cycles skip both boundaries. Pin re-audited 2026-09-05 after #9760 touched `gc/mod.rs`: that change is `mod heap_stats;` plus a `pub(crate) use` re-export and alters no mark/sweep control flow. `heap_stats()` is reached only from `js_bun_jsc_heap_stats` (the JS-facing `bun:jsc.heapStats()`), i.e. from mutator code, never inside a cycle, and its own module contract forbids allocation or collection during its walk. The mark-complete \u2192 sweep-entry window is unchanged. Re-audited 2026-09-05 (train125) after #9769 and #9771 touched pinned files. #9769 adds one `reg_scanner!` registration to `gc/mod.rs`; #9771 adds a feature-gated `alloc_census_init()` there and a feature-gated Rust-heap dump inside `take_census`. `alloc-census` is not in the default feature set, and decisively: `census_take_if_armed_at_full_sweep_start` does `PASS1_MARKED.with(|p| p.borrow_mut().take())` BEFORE calling `take_census`, so the snapshot has already left the thread-local by the time #9771's code runs \u2014 it cannot affect the window. Neither change alters mark/sweep control flow. Re-audited 2026-09-05 (train126) after #9755 restructured `gc/cycle.rs`. Its hunks are all root-scan machinery (`RootScanSubphase`, `RootScanCycleState`, the mutable-scanner iteration state), which runs BEFORE mark propagation completes; `gc/mod.rs` gains only a `mod young_log;` declaration. The bracketing is unchanged \u2014 `census_pass1_if_armed` is still inside `step_mark_propagation` and `census_take_if_armed_at_full_sweep_start` inside `step_sweep` \u2014 and a synchronous full mark-sweep still moves nothing between them.", + "why": "Real GC header addresses, deliberately untraced so the diagnostic does not keep its observed objects alive. Populated only at the end of mark propagation of a synchronous full cycle; consumed at sweep entry in the same run_to_completion invocation. The intervening full-cycle phases do not relocate or run JS callbacks. The Vec is used for membership comparisons and dropped with the census before sweep. Budgeted and minor cycles skip both boundaries. Pin re-audited 2026-09-05 after #9760 touched `gc/mod.rs`: that change is `mod heap_stats;` plus a `pub(crate) use` re-export and alters no mark/sweep control flow. `heap_stats()` is reached only from `js_bun_jsc_heap_stats` (the JS-facing `bun:jsc.heapStats()`), i.e. from mutator code, never inside a cycle, and its own module contract forbids allocation or collection during its walk. The mark-complete \u2192 sweep-entry window is unchanged. Re-audited 2026-09-05 (train125) after #9769 and #9771 touched pinned files. #9769 adds one `reg_scanner!` registration to `gc/mod.rs`; #9771 adds a feature-gated `alloc_census_init()` there and a feature-gated Rust-heap dump inside `take_census`. `alloc-census` is not in the default feature set, and decisively: `census_take_if_armed_at_full_sweep_start` does `PASS1_MARKED.with(|p| p.borrow_mut().take())` BEFORE calling `take_census`, so the snapshot has already left the thread-local by the time #9771's code runs \u2014 it cannot affect the window. Neither change alters mark/sweep control flow. Re-audited 2026-09-05 (train126) after #9755 restructured `gc/cycle.rs`. Its hunks are all root-scan machinery (`RootScanSubphase`, `RootScanCycleState`, the mutable-scanner iteration state), which runs BEFORE mark propagation completes; `gc/mod.rs` gains only a `mod young_log;` declaration. The bracketing is unchanged \u2014 `census_pass1_if_armed` is still inside `step_mark_propagation` and `census_take_if_armed_at_full_sweep_start` inside `step_sweep` \u2014 and a synchronous full mark-sweep still moves nothing between them. Re-audited 2026-09-05 after #9830 touched `gc/policy.rs`. That change is (a) six `thread_local! {` blocks rewritten as `crate::perry_thread_local! {` and (b) one `#[cfg(test)]` accessor listing the trigger path's hot-slot indices. The macro keeps the same storage, the same `.with()` at every read and write, and the same destructor registration (the teardown guard exists exactly when `needs_drop` holds, which is what `std::thread_local!` already decided); no value, predicate or branch in the file changes, so no mark or sweep control flow does. The one new behaviour is on a declaration's FIRST read: `HotKey::resolve_and_cache` takes a mutex and allocates a key through the GLOBAL allocator. Even if a first read landed inside this window it would be sound \u2014 the window's contract is that nothing relocates and no JS callback runs, and a mimalloc allocation does neither. `census_pass1_if_armed` is still inside `step_mark_propagation` and `census_take_if_armed_at_full_sweep_start` inside `step_sweep`; the bracketing is untouched.", "window": { "start": { "file": "crates/perry-runtime/src/gc/census.rs", @@ -288,7 +288,7 @@ "crates/perry-runtime/src/gc/census.rs": "8050a9d1ca15f783195ccfa5963089b60bc4a6c31d9ced5755537623e70e7e3c", "crates/perry-runtime/src/gc/cycle.rs": "763d552271b8e983a796b4e9648cd8ee984a0602b2b56aeefdb8713c0049c31f", "crates/perry-runtime/src/gc/mod.rs": "085c3dcde34a172aa2b96ee4500658abae77cd34ee7f0e2dfeee06ae5774a414", - "crates/perry-runtime/src/gc/policy.rs": "319ed42f1a985c88f6362657a08518077283fe5216d6055fc82343b34dec50f9", + "crates/perry-runtime/src/gc/policy.rs": "92dbf50a9838a1b040dfb369b88edf1a8bc508c82240ae053d0261a07ed070e4", "crates/perry-runtime/src/gc/progress.rs": "a5ad3971bbe4047229ca57325234780daa85921dbc778e1c08dff4ad07ccfb96" } } diff --git a/scripts/thread_local_cold_allowlist.json b/scripts/thread_local_cold_allowlist.json index 4744f170ac..9686b6e210 100644 --- a/scripts/thread_local_cold_allowlist.json +++ b/scripts/thread_local_cold_allowlist.json @@ -1,83 +1,79 @@ { - "_comment": "Files still declaring raw `thread_local!`. Every entry is a declaration that pays `_tlv_get_addr` on Darwin; the count is a ratchet, so adding one to an already-listed file fails too. New code should use `crate::perry_thread_local!` \u2014 see crates/perry-runtime/src/tls_hot.rs. Regenerate with scripts/check_thread_locals.py --update.", - "_hot_declarations": 287, + "_comment": "Files still declaring raw `thread_local!`. The count is the number of DECLARATIONS that survive into a shipping build \u2014 each one pays `_tlv_get_addr` per read on Darwin \u2014 and it is a ratchet, so adding a `static` to an already-listed file fails whether or not it opens a new block. New code should use `crate::perry_thread_local!` \u2014 see crates/perry-runtime/src/tls_hot.rs. Regenerate with scripts/check_thread_locals.py --update.", + "_hot_declarations": 385, "files": { "crates/perry-runtime/src/agent.rs": 1, - "crates/perry-runtime/src/arena/block.rs": 2, + "crates/perry-runtime/src/arena/block.rs": 5, "crates/perry-runtime/src/arena/page_meta.rs": 2, - "crates/perry-runtime/src/async_context.rs": 2, - "crates/perry-runtime/src/async_hooks.rs": 3, - "crates/perry-runtime/src/builtins/console.rs": 2, - "crates/perry-runtime/src/builtins/formatting.rs": 5, + "crates/perry-runtime/src/async_context.rs": 3, + "crates/perry-runtime/src/async_hooks.rs": 7, + "crates/perry-runtime/src/builtins/console.rs": 4, + "crates/perry-runtime/src/builtins/formatting.rs": 9, "crates/perry-runtime/src/builtins/formatting/boxed_primitives.rs": 1, - "crates/perry-runtime/src/builtins/globals.rs": 2, + "crates/perry-runtime/src/builtins/globals.rs": 4, "crates/perry-runtime/src/bun_ffi/types.rs": 1, "crates/perry-runtime/src/child_process/reactor.rs": 1, - "crates/perry-runtime/src/child_process/v8_serde.rs": 1, + "crates/perry-runtime/src/child_process/v8_serde.rs": 2, "crates/perry-runtime/src/closure/dispatch/errors.rs": 1, - "crates/perry-runtime/src/cluster.rs": 2, + "crates/perry-runtime/src/cluster.rs": 3, "crates/perry-runtime/src/dyn_eval/bridge.rs": 1, "crates/perry-runtime/src/dyn_eval/env.rs": 1, - "crates/perry-runtime/src/dyn_eval/mod.rs": 1, + "crates/perry-runtime/src/dyn_eval/mod.rs": 6, "crates/perry-runtime/src/eh.rs": 1, "crates/perry-runtime/src/eh_walker.rs": 1, - "crates/perry-runtime/src/error.rs": 2, + "crates/perry-runtime/src/error.rs": 3, "crates/perry-runtime/src/event_pump.rs": 1, - "crates/perry-runtime/src/fs/dir_glob_watch/watch.rs": 1, - "crates/perry-runtime/src/fs/filehandle.rs": 1, - "crates/perry-runtime/src/fs/mod.rs": 1, - "crates/perry-runtime/src/fs/stream.rs": 1, - "crates/perry-runtime/src/gc/barrier/mod.rs": 2, - "crates/perry-runtime/src/gc/barrier_arming.rs": 1, + "crates/perry-runtime/src/fs/dir_glob_watch/watch.rs": 7, + "crates/perry-runtime/src/fs/filehandle.rs": 6, + "crates/perry-runtime/src/fs/mod.rs": 6, + "crates/perry-runtime/src/fs/stream.rs": 4, + "crates/perry-runtime/src/gc/barrier/mod.rs": 3, + "crates/perry-runtime/src/gc/barrier_arming.rs": 3, "crates/perry-runtime/src/gc/cycle_malloc_trim.rs": 1, "crates/perry-runtime/src/gc/fromspace_scan.rs": 1, "crates/perry-runtime/src/gc/layout.rs": 1, - "crates/perry-runtime/src/gc/layout_tables.rs": 1, + "crates/perry-runtime/src/gc/layout_tables.rs": 3, "crates/perry-runtime/src/gc/malloc.rs": 2, - "crates/perry-runtime/src/gc/mod.rs": 2, - "crates/perry-runtime/src/gc/old_free.rs": 1, - "crates/perry-runtime/src/gc/policy.rs": 6, - "crates/perry-runtime/src/gc/promote_in_place.rs": 1, - "crates/perry-runtime/src/gc/roots/runtime_handles.rs": 1, + "crates/perry-runtime/src/gc/mod.rs": 3, + "crates/perry-runtime/src/gc/promote_in_place.rs": 11, + "crates/perry-runtime/src/gc/roots/runtime_handles.rs": 2, "crates/perry-runtime/src/gc/roots/scan_mode.rs": 1, "crates/perry-runtime/src/gc/roots/shadow_stack.rs": 2, "crates/perry-runtime/src/gc/roots/temp_roots.rs": 1, - "crates/perry-runtime/src/gc/scan_fallback.rs": 1, + "crates/perry-runtime/src/gc/scan_fallback.rs": 2, "crates/perry-runtime/src/gc/shape_install.rs": 1, - "crates/perry-runtime/src/gc/telemetry.rs": 2, - "crates/perry-runtime/src/gc/tenuring.rs": 1, - "crates/perry-runtime/src/gc/trace.rs": 3, + "crates/perry-runtime/src/gc/telemetry.rs": 3, "crates/perry-runtime/src/intl/number_format.rs": 1, "crates/perry-runtime/src/iter_result.rs": 1, - "crates/perry-runtime/src/json/mod.rs": 1, + "crates/perry-runtime/src/json/mod.rs": 12, "crates/perry-runtime/src/json/raw_json.rs": 1, "crates/perry-runtime/src/json_tape.rs": 2, - "crates/perry-runtime/src/json_tape_store.rs": 1, + "crates/perry-runtime/src/json_tape_store.rs": 3, "crates/perry-runtime/src/media_playback.rs": 1, - "crates/perry-runtime/src/native_arena.rs": 1, + "crates/perry-runtime/src/native_arena.rs": 3, "crates/perry-runtime/src/node_http2_constants.rs": 1, - "crates/perry-runtime/src/node_inspector.rs": 1, + "crates/perry-runtime/src/node_inspector.rs": 2, "crates/perry-runtime/src/node_repl.rs": 1, - "crates/perry-runtime/src/node_stream_constructors.rs": 2, - "crates/perry-runtime/src/node_stream_tests.rs": 1, - "crates/perry-runtime/src/node_submodules/blob.rs": 1, - "crates/perry-runtime/src/node_submodules/diagnostics.rs": 3, - "crates/perry-runtime/src/node_submodules/diagnostics_tail.rs": 1, - "crates/perry-runtime/src/node_submodules/mod.rs": 1, - "crates/perry-runtime/src/node_submodules/test.rs": 1, - "crates/perry-runtime/src/node_submodules/test_once_unit_tests.rs": 1, + "crates/perry-runtime/src/node_stream_constructors.rs": 3, + "crates/perry-runtime/src/node_stream_tests.rs": 23, + "crates/perry-runtime/src/node_submodules/blob.rs": 4, + "crates/perry-runtime/src/node_submodules/diagnostics.rs": 9, + "crates/perry-runtime/src/node_submodules/diagnostics_tail.rs": 2, + "crates/perry-runtime/src/node_submodules/mod.rs": 4, + "crates/perry-runtime/src/node_submodules/test.rs": 11, + "crates/perry-runtime/src/node_submodules/test_once_unit_tests.rs": 2, "crates/perry-runtime/src/node_submodules/test_property.rs": 1, - "crates/perry-runtime/src/node_submodules/trace_events.rs": 1, - "crates/perry-runtime/src/object/native_module/callable_exports.rs": 2, + "crates/perry-runtime/src/node_submodules/trace_events.rs": 8, + "crates/perry-runtime/src/object/native_module/callable_exports.rs": 3, "crates/perry-runtime/src/object/spill.rs": 1, "crates/perry-runtime/src/os/os_process_emitter.rs": 1, - "crates/perry-runtime/src/os_process_streams.rs": 1, - "crates/perry-runtime/src/perf_hooks.rs": 2, - "crates/perry-runtime/src/process.rs": 2, - "crates/perry-runtime/src/process/env_misc.rs": 3, + "crates/perry-runtime/src/os_process_streams.rs": 3, + "crates/perry-runtime/src/perf_hooks.rs": 11, + "crates/perry-runtime/src/process.rs": 12, + "crates/perry-runtime/src/process/env_misc.rs": 4, "crates/perry-runtime/src/process/permission.rs": 1, "crates/perry-runtime/src/process/report.rs": 1, - "crates/perry-runtime/src/proxy.rs": 1, + "crates/perry-runtime/src/proxy.rs": 5, "crates/perry-runtime/src/pty/reactor.rs": 1, "crates/perry-runtime/src/static_plugins.rs": 1, "crates/perry-runtime/src/timer.rs": 1, @@ -87,7 +83,7 @@ "crates/perry-runtime/src/util_promisify.rs": 1, "crates/perry-runtime/src/v8.rs": 2, "crates/perry-runtime/src/wasi.rs": 1, - "crates/perry-runtime/src/weakref.rs": 1, - "crates/perry-runtime/src/web_storage.rs": 1 + "crates/perry-runtime/src/weakref.rs": 2, + "crates/perry-runtime/src/web_storage.rs": 2 } } From 4a58e853e5ff8cd8f4b679a8d7c8339f0af0a251 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 5 Sep 2026 22:39:00 +0200 Subject: [PATCH 2/5] docs(changelog): rename the fragment to its PR number Claude-Session: https://claude.ai/code/session_014UZWia6L37DpA93VLtNK9m --- ...gc-trigger-path-hot-tls.md => 9827-gc-trigger-path-hot-tls.md} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename changelog.d/{9830-gc-trigger-path-hot-tls.md => 9827-gc-trigger-path-hot-tls.md} (100%) diff --git a/changelog.d/9830-gc-trigger-path-hot-tls.md b/changelog.d/9827-gc-trigger-path-hot-tls.md similarity index 100% rename from changelog.d/9830-gc-trigger-path-hot-tls.md rename to changelog.d/9827-gc-trigger-path-hot-tls.md From 165aa78bfe227672eadce8e0708edb0ff1b559bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 6 Sep 2026 07:49:48 +0200 Subject: [PATCH 3/5] perf(gc): direct-indexed page-class table behind PERRY_GC_PAGE_CLASS_TABLE MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit WIP — committed to preserve state while the lane is paused for box load (load 298, 19.8/21.5 GB swap). NOT measured on the rig; do not land as is. Replaces the 4-way round-robin page-generation cache with a direct-indexed table over the arena's 1 MiB address classes. `PageGenerationMap` stays authoritative: every miss falls through to it exactly as before, so this is a cache replacement, not a map replacement. The 4-way set is retained in the same binary behind `PERRY_GC_PAGE_CLASS_TABLE=0` as the positive control. STATE OF THIS COMMIT Applied, complete: * the table itself (`lookup`/`insert`/`rebase_to_cover`/`invalidate`), base taken from the first insert, epoch-stamped entries, O(1) whole-table invalidation; * sizing: `INITIAL_SPAN = 4096`, `BASE_SLACK = SPAN / 2`. The draft's `S = 1024` was mis-tuned — with base `first_key - S` the span covered is `min(S + 1, N - S)`, maximised at `S = N / 2`, so `S = 1024` covered 1,025 classes against a measured span of 1,021 while `S = N / 2` covers 2,048 for the same 160 KB. A `const` assert now fails the build for any pairing covering less than twice the measured span; the old pairing fails it; * out-of-span coverage: an insert outside the table rebases it up to a 16,384-class cap, and past the cap the key is left uncached and falls through to the map — never silently mis-indexed; * the arm is a plain `u8` field in the set's first cache line, not the `OnceLock` env read the draft had on the lookup path. That path runs ~440 M times per turn and an acquire load on each would have been paid by BOTH arms of the A/B while still being charged against main; * `#[repr(C, align(64))]` so "the hot fields share one cache line" is true rather than likely; * counters (`hits`/`misses`/`inserts`/`oos`/`rebases`/`refused`) and the `[gc-page-class]` line, emitted per copying minor under `PERRY_GC_DIAG` because the rig SIGKILLs the process. `oos` is on the miss path only and is what distinguishes a residual miss that is an unregistered address from one that is the table failing. Verified: * all four tests pass on the pristine tree (`cargo test -p perry-runtime --lib page_class_table`, dev profile, 4 passed); * the four sabotages each fail on their own named assertion — base-from-first- registration, out-of-span handling, range containment, and invalidation. Two of them produce a literal misclassified pointer (`left: Old, right: Nursery` and `left: Nursery, right: Old`), which is the failure mode this structure has to be proof against; * every `PAGE_GENERATIONS` mutation site was enumerated (three, plus one read-only census walk) and each ends with an unconditional `invalidate_generation_cache()`. The table holds ~2,000 entries where the 4-way set held 4, so a missing invalidation the old structure survived by luck would be a live misclassification here. NOT done — this is what the lane owes: * the rig. The relink was killed mid-`cargo build` at the coordinator's pause, so there is no candidate binary and NO number in this commit has been measured on cc; * `cargo test -p perry-runtime --release -- gc:: arena::`; * `cargo fmt` (the `arena/mod.rs` re-export is not in sorted order) and clippy; * a changelog fragment. Pre-registered falsifiers, written before any measurement, are in `secret-tests/cc-perf-campaign/RESULT_page_class_table.md`. The headline is that the spec's "miss rate below 2 %" bar is arithmetically unreachable: 22.3 % of today's misses are on addresses in no registered block, which the map cannot answer either, so nothing is cached for them in either arm. The derived floor is ~4.5 %, and the decision turns on misses to REGISTERED classes going to ~0. --- crates/perry-runtime/src/arena/mod.rs | 1 + crates/perry-runtime/src/arena/page_meta.rs | 601 +++++++++++++++++++- crates/perry-runtime/src/gc/copying.rs | 3 + 3 files changed, 591 insertions(+), 14 deletions(-) diff --git a/crates/perry-runtime/src/arena/mod.rs b/crates/perry-runtime/src/arena/mod.rs index 4a92a805c2..b5835e69c5 100644 --- a/crates/perry-runtime/src/arena/mod.rs +++ b/crates/perry-runtime/src/arena/mod.rs @@ -54,6 +54,7 @@ pub(crate) use block::{ reset_gc_trigger_arena_probe, }; pub(crate) use page_meta::{ + page_class_table_report, address_span_overlaps_pages, defer_old_object_page_registration, register_block_space_with_object_starts, register_old_object_pages, unregister_block_generation, unregister_old_block_pages, OLD_GEN_RECLAIM_POOLED_BYTES, diff --git a/crates/perry-runtime/src/arena/page_meta.rs b/crates/perry-runtime/src/arena/page_meta.rs index 0fe26f3c9b..5b5fe48016 100644 --- a/crates/perry-runtime/src/arena/page_meta.rs +++ b/crates/perry-runtime/src/arena/page_meta.rs @@ -138,21 +138,201 @@ impl PageGenerationCache { // that — an 8.6% regression on the same row (0/7 pairs). Keep the scan short. const PAGE_GENERATION_CACHE_WAYS: usize = 4; -/// Small direct-probed cache in front of [`PageGenerationMap`]. +/// One entry of the direct-indexed table: the range last confirmed for this +/// 1 MiB class, stamped with the invalidation epoch it was confirmed under. +#[derive(Clone, Copy)] +struct PageClassEntry { + range: PageGenerationRange, + epoch: u64, +} + +impl PageClassEntry { + /// The filler every unwritten slot holds. `epoch: 0` is the sentinel no + /// live epoch ever takes (`epoch` starts at 1 and [`PageGenerationCacheSet:: + /// invalidate`] steps over 0 on wrap), so a freshly allocated table is + /// entirely dead without a second "valid" flag to keep coherent. + const DEAD: Self = Self { + range: PageGenerationCache::empty().range, + epoch: 0, + }; +} + +/// Initial span of the direct table, in 1 MiB classes, and how far below the +/// first registered key the base is placed. /// -/// Pure accelerator: a miss, a stale way, or a full set all fall through to -/// the authoritative map, so the only thing correctness depends on is that -/// every invalidation clears **all** ways — which is why -/// [`invalidate_generation_cache`] resets the whole set rather than one entry. +/// Measured on the compiled claude-code TUI: the live span is **1,018-1,021 +/// classes** at ~40 % density, with the base moving per process (ASLR). The +/// base is `first_registered_key - SLACK`, and the first registration can fall +/// anywhere in the eventual span, so both ends have to be covered by the two +/// constants alone. Writing `S = SLACK`, `N = SPAN` and `W = 1,021` for the +/// measured width, a table that covers every case without rebasing needs /// -/// Stored behind an `UnsafeCell`, not a `Cell`: `Cell::get` returns a **copy**, -/// and copying ~200 bytes on every classification cost more than the map lookup -/// the cache exists to avoid (measured as a ~2% regression on `retain.ts` -/// before this was switched). Access is single-threaded by construction — the -/// cache is thread-local and no path holds a reference across a call that could -/// re-enter classification. -#[derive(Clone, Copy)] +/// * `S >= W - 1` — otherwise a first registration at the TOP of the span +/// leaves the classes below the base uncovered; and +/// * `N > W - 1 + S` — otherwise a first registration at the BOTTOM leaves the +/// classes above `base + N` uncovered. +/// +/// Both must hold, so the width actually covered is +/// `W <= min(S + 1, N - S)` — **maximised at `S = N / 2`**, where it is `N / 2`. +/// That is the whole of the sizing argument, and it is worth writing down +/// because the obvious pairing gets it wrong: `N = 4096, S = 1024` pays for +/// 4,096 entries and covers a span of only **1,025** — four classes above the +/// measured 1,021, which is not a margin. `S = N / 2` covers **2,048** for the +/// same 4,096 entries: **twice the measured span at identical cost**. +/// +/// So `N = 4096, S = 2048`: 4,096 x 40 B = **160 KB** on each thread that +/// classifies, allocated only on that thread's first insert, covering any span +/// up to 2,048 classes wherever the first registration falls within it. +/// +/// Exceeding it is not a correctness problem — [`PageGenerationCacheSet:: +/// rebase_to_cover`] widens the table and the `rebases` counter says how often +/// that happened — so these are sized to make the rebase rare, not to make it +/// impossible. +const PAGE_CLASS_TABLE_INITIAL_SPAN: usize = 4096; +const PAGE_CLASS_TABLE_BASE_SLACK: usize = PAGE_CLASS_TABLE_INITIAL_SPAN / 2; + +/// The span these two constants actually cover, `min(S + 1, N - S)`, and the +/// compile-time guard that keeps the derivation above load-bearing rather than +/// decorative. The pairing this replaced (`N = 4096, S = 1024`) covers 1,025 — +/// four classes above the measured span — and fails this assert, which is the +/// point: the sizing is not obvious and a plausible-looking edit gets it wrong. +const PAGE_CLASS_TABLE_COVERED_SPAN: usize = { + let below = PAGE_CLASS_TABLE_BASE_SLACK + 1; + let above = PAGE_CLASS_TABLE_INITIAL_SPAN - PAGE_CLASS_TABLE_BASE_SLACK; + if below < above { + below + } else { + above + } +}; +/// Measured live span on the compiled claude-code TUI, worst of two runs. +const PAGE_CLASS_TABLE_MEASURED_SPAN: usize = 1021; +const _: () = assert!( + PAGE_CLASS_TABLE_COVERED_SPAN >= 2 * PAGE_CLASS_TABLE_MEASURED_SPAN, + "the initial table must cover at least twice the measured span, wherever \ + the first registration falls in it — otherwise the common case rebases" +); +/// Above this span the table stops growing and out-of-span keys simply fall +/// through to the authoritative map uncached. 16 GiB of address span is far +/// past any arena this runtime places; the cap exists so a stray registration +/// at a wild address cannot allocate an unbounded table. +const PAGE_CLASS_TABLE_MAX_SPAN: usize = 16 * 1024; + +/// Which arm [`PageGenerationCacheSet`] is running, resolved once per thread on +/// the first insert and then read as a PLAIN FIELD in the hot path. +/// +/// Not `page_class_table_enabled()` on the lookup path, deliberately: that is a +/// `OnceLock` and a `OnceLock` read is an ACQUIRE load. This path runs +/// **440 M times per turn** — an `ldar` plus a branch on every one of them is a +/// cost the table is supposed to be removing, and it would land on BOTH arms, +/// so the A/B would have hidden it while the comparison against main paid it. +/// The field shares the first cache line with `base`/`epoch`/`table`, which a +/// lookup loads anyway, so the arm test is free. +/// +/// `ARM_UNRESOLVED` behaves as the table arm and is CORRECT for both: before +/// the first insert the table is empty and every way is invalid, so either arm +/// answers "miss" for every key. +const ARM_UNRESOLVED: u8 = 0; +const ARM_TABLE: u8 = 1; +const ARM_WAYS: u8 = 2; + +/// The cache in front of [`PageGenerationMap`]: a **direct-indexed table** keyed +/// by `addr >> GENERATION_CLASS_SHIFT`, with the previous 4-way set retained +/// behind `PERRY_GC_PAGE_CLASS_TABLE=0` as the control arm. +/// +/// # Why a table and not a bigger cache +/// The 4-way set was measured (`PERRY_CLASSIFY_DIAG`, 3300-char claude-code +/// reply) at **440 M lookups per turn, 20 % miss, 60 % of those misses on a key +/// evicted within the last 64 evictions** — pure capacity, against a working +/// set of **402-432 registered classes**. All four ways were in use +/// (`ways_distinct_max = 4`), so the shortfall is ~120x, which no associativity +/// reaches; #7469 already measured 16 ways as an 8.6 % regression for 1.5 % +/// fewer misses, and five further associativity changes measured flat. The +/// registered classes sit in a span of **1,018-1,021** at ~40 % density, so a +/// table over the span holds every one of them in ~33 KB and answers a lookup +/// with one bounds compare and one load. The same bounds check rejects the +/// ~8,000 candidate addresses per turn that are in no registered block — the +/// other 22 % of misses — without a separate filter. +/// +/// # What it is not +/// A cache, not the truth. `PageGenerationMap` stays authoritative: every miss +/// falls through to it exactly as before, and every registration, unregistration +/// and retag invalidates the whole table by bumping `epoch` (O(1), and the same +/// "clear everything" contract the 4-way set had, for the same reason: a stale +/// entry is exactly what this guards against). A hit still requires +/// `range.contains(addr)` — a key match at a range boundary is not an address +/// match. +/// +/// # The one place the table is WEAKER than the set it replaces +/// A class can hold more than one range (`PageGenerationSlot::Multiple`). The +/// 4-way set could hold two of them at once, in two ways under the same key, +/// and hit on both; the table has one slot per class, so ranges sharing a +/// class evict each other and alternate accesses miss. This is a real +/// regression in kind, bounded by how many classes are `Multiple` — and it is +/// what the `[gc-page-class]` miss rate would show if the collapse predicted +/// below fails to appear. Registered blocks are `BLOCK_SIZE`-sized and +/// `BLOCK_SIZE == 1 << GENERATION_CLASS_SHIFT`, so one block is exactly one +/// class and the multi-range case is the sub-block registration, not the norm. +/// +/// # The two things measurement did not settle, handled explicitly +/// * **The base moves per process** (observed: `0x43daa2` vs `0x57e3c2` on two +/// runs). It is taken from the first insert, minus slack — never compiled in. +/// * **The span can grow** (observed: 1,018 vs 1,021 on two runs of one +/// binary). An insert outside `[base, base + len)` rebases the table to cover +/// it, up to `PAGE_CLASS_TABLE_MAX_SPAN`; past the cap the key is left +/// uncached and falls through. Both paths are pinned by tests that fail when +/// the fallback is removed, because a wrong answer here is a misclassified +/// pointer — a collector that moves the wrong thing. +/// +/// Stored behind an `UnsafeCell`, not a `Cell`, for the reason recorded on the +/// 4-way set when it was switched: `Cell::get` returns a **copy**, and copying +/// the set on every classification cost more than the map lookup the cache +/// exists to avoid (a ~2 % regression on `retain.ts`). That argument is +/// stronger here, not weaker — the table is far larger than the set was. +/// Access is single-threaded by construction: the cell is thread-local and no +/// path holds a reference across a call that could re-enter classification. +// `repr(C)` for field ORDER, not for FFI: the four fields a lookup touches are +// declared first so they share one cache line. Under `repr(Rust)` the layout is +// unspecified and the 192-byte `ways` array — dead weight in the table arm — +// may be placed in front of them, which would make the spec's "one bounds +// compare and one load" two lines' worth of traffic. `align(64)` is what makes +// that claim true rather than likely: at the struct's natural 8-byte alignment +// the hot group could straddle two lines depending on where the thread-local +// block lands. +#[repr(C, align(64))] struct PageGenerationCacheSet { + // ---- the table: everything `lookup` reads, in one line ---- + /// `ARM_UNRESOLVED` / `ARM_TABLE` / `ARM_WAYS`. See the constants above for + /// why the arm is a field and not the `OnceLock` read. + arm: u8, + /// First class covered. Meaningful only when `table` is non-empty. + base: usize, + /// Bumped on every invalidation; an entry is live only if its `epoch` + /// matches. Starts at 1 so a zeroed entry is never live. + epoch: u64, + /// Entries for classes `base .. base + table.len()`. + table: Vec, + /// Counted unconditionally (a field increment on a `&mut` we already hold) + /// and reported only under `PERRY_GC_DIAG`. This is the falsifier: the + /// table's whole claim is that the miss rate collapses. Both arms count, + /// so the control arm carries the same increment and the comparison is + /// symmetric. + hits: u64, + misses: u64, + // ---- cold: written on the miss path or rarer ---- + /// Misses the authoritative map could answer, i.e. misses that cached + /// something. `misses - inserts` is the population that is in no + /// registered block at all — the 22 % the bounds check is supposed to + /// reject for free. + inserts: u64, + /// Rebases performed and inserts refused past the cap — the two paths the + /// span measurement could not rule out. + rebases: u64, + refused: u64, + /// Lookups that missed because the key was OUTSIDE `[base, base + len)`. + /// See the increment site for why this is the counter that matters. + oos: u64, + // ---- control arm: the 4-way round-robin set, unchanged ---- ways: [PageGenerationCache; PAGE_GENERATION_CACHE_WAYS], /// Round-robin victim for the next insert. next: usize, @@ -161,23 +341,90 @@ struct PageGenerationCacheSet { impl PageGenerationCacheSet { const fn empty() -> Self { Self { + arm: ARM_UNRESOLVED, + base: 0, + epoch: 1, + table: Vec::new(), + hits: 0, + misses: 0, + inserts: 0, + rebases: 0, + refused: 0, + oos: 0, ways: [PageGenerationCache::empty(); PAGE_GENERATION_CACHE_WAYS], next: 0, } } #[inline(always)] - fn lookup(&self, key: usize, addr: usize) -> Option { + fn lookup(&mut self, key: usize, addr: usize) -> Option { + if self.arm != ARM_WAYS { + // `wrapping_sub` folds `key < base` into the same out-of-range + // check as `key >= base + len`: a key below the base wraps to a + // huge index and fails `< len`. + let idx = key.wrapping_sub(self.base); + if idx < self.table.len() { + let e = &self.table[idx]; + if e.epoch == self.epoch && e.range.contains(addr) { + self.hits += 1; + return Some(e.range); + } + } else { + // Miss-path only, so it costs nothing on a hit — and it is the + // counter that decides between the two explanations for a + // residual miss rate. Out of span: the key is a candidate + // address in no registered block (the population the table was + // never able to hold, since the map has no answer to cache + // either). In span: the table itself failed — a class holding + // more than one range, or invalidation churn. + self.oos += 1; + } + self.misses += 1; + return None; + } for way in self.ways.iter() { if way.valid && way.key == key && way.range.contains(addr) { + self.hits += 1; return Some(way.range); } } + self.misses += 1; None } #[inline] fn insert(&mut self, key: usize, range: PageGenerationRange) { + if self.arm == ARM_UNRESOLVED { + // The one env read, on the cold path, once per thread. + self.arm = if page_class_table_enabled() { + ARM_TABLE + } else { + ARM_WAYS + }; + } + if self.arm == ARM_TABLE { + if self.table.is_empty() { + // The base is taken from the FIRST insert, minus slack. Never a + // constant: the arena's placement moves with ASLR. + self.base = key.saturating_sub(PAGE_CLASS_TABLE_BASE_SLACK); + self.table = vec![PageClassEntry::DEAD; PAGE_CLASS_TABLE_INITIAL_SPAN]; + } + let mut idx = key.wrapping_sub(self.base); + if idx >= self.table.len() { + if !self.rebase_to_cover(key) { + // Past the cap: leave it uncached. The caller already has + // the authoritative answer and returns it; only the + // acceleration is forgone. + self.refused += 1; + return; + } + idx = key - self.base; + } + self.table[idx] = PageClassEntry { range, epoch: self.epoch }; + self.inserts += 1; + return; + } + self.inserts += 1; let slot = self.next % PAGE_GENERATION_CACHE_WAYS; self.ways[slot] = PageGenerationCache { key, @@ -186,6 +433,134 @@ impl PageGenerationCacheSet { }; self.next = slot.wrapping_add(1); } + + /// Grow the table so that `key` is inside it, keeping every class it + /// already covered. Returns false — and changes nothing — if the resulting + /// span would exceed the cap. + #[cold] + #[inline(never)] + fn rebase_to_cover(&mut self, key: usize) -> bool { + let old_lo = self.base; + let old_hi = self.base + self.table.len(); // exclusive + let new_lo = old_lo.min(key.saturating_sub(PAGE_CLASS_TABLE_BASE_SLACK)); + let new_hi = old_hi.max(key.saturating_add(1 + PAGE_CLASS_TABLE_BASE_SLACK)); + let span = new_hi - new_lo; + if span > PAGE_CLASS_TABLE_MAX_SPAN { + return false; + } + // Entries are a cache; dropping them is always correct. Rebasing by + // bumping the epoch rather than copying keeps this simple and it is + // rare — measured span growth was 1,018 -> 1,021 over two whole runs. + self.epoch = self.epoch.wrapping_add(1); + self.table = vec![PageClassEntry::DEAD; span]; + self.base = new_lo; + self.rebases += 1; + true + } + + /// Invalidate everything, both arms. O(1) for the table: an epoch bump + /// makes every entry stale at once, which is the same contract the 4-way + /// set met by being reset wholesale — and the reason the table can meet it + /// without touching ~2,000 entries. + /// + /// The bump is the whole of the table's correctness. Without it a retagged + /// block keeps answering with its previous generation, which is a + /// misclassified pointer: the collector treats an old object as young, or + /// declines to trace a young one. `a_registration_change_invalidates_every_entry` + /// is the standing guard. + #[inline] + fn invalidate(&mut self) { + self.ways = [PageGenerationCache::empty(); PAGE_GENERATION_CACHE_WAYS]; + self.next = 0; + self.epoch = self.epoch.wrapping_add(1); + if self.epoch == 0 { + // Wrapped: 0 is the "never live" sentinel a zeroed entry carries, + // so step past it. Reaching this needs 2^64 invalidations; the + // branch is here so the sentinel cannot be forged rather than + // because the wrap is expected. + self.epoch = 1; + } + } + + /// `(hits, misses, inserts, span, rebases, refused)` for the diagnostic + /// line and for the tests. + fn stats(&self) -> PageClassStats { + PageClassStats { + arm: self.arm, + hits: self.hits, + misses: self.misses, + inserts: self.inserts, + span: self.table.len(), + rebases: self.rebases, + refused: self.refused, + oos: self.oos, + } + } +} + +/// What [`PageGenerationCacheSet::stats`] reports. A named struct rather than a +/// tuple because the report and four tests read different fields of it and a +/// six-tuple's positions are not self-describing at the call site. +#[derive(Clone, Copy)] +struct PageClassStats { + arm: u8, + hits: u64, + misses: u64, + inserts: u64, + span: usize, + rebases: u64, + refused: u64, + oos: u64, +} + +/// `PERRY_GC_PAGE_CLASS_TABLE=0` restores the 4-way set. The kill switch, and +/// the positive control: both arms live in ONE binary so no build difference +/// can be confounded with the change. +#[inline(always)] +fn page_class_table_enabled() -> bool { + static ENABLED: std::sync::OnceLock = std::sync::OnceLock::new(); + *ENABLED.get_or_init(|| crate::gc::env_default_on_enabled("PERRY_GC_PAGE_CLASS_TABLE")) +} + +/// One line under `PERRY_GC_DIAG=1`, emitted per copying minor from the +/// collector (never at exit — the rig SIGKILLs the process). +pub(crate) fn page_class_table_report() { + if !crate::gc::gc_diag_enabled() { + return; + } + // SAFETY: thread-local, single-threaded, shared borrow ends here. + let st = unsafe { (*hot_page_generation_cache()).stats() }; + let tot = st.hits + st.misses; + if tot == 0 { + return; + } + // `misses - inserts` is the population in no registered block at all: the + // map had no answer either, so nothing was cached. Reported apart because + // the two halves are removed by different properties of the table — the + // first by capacity, the second by the bounds check. + let unregistered = st.misses.saturating_sub(st.inserts); + let arm_name = match st.arm { + ARM_WAYS => "4way", + ARM_TABLE => "table", + // Never inserted, so never resolved: report what it WOULD pick. + _ if page_class_table_enabled() => "table(unresolved)", + _ => "4way(unresolved)", + }; + eprintln!( + "[gc-page-class] arm={} lookups={tot} hit={} ({:.3}%) miss={} ({:.3}%) \ +miss_registered={} miss_unregistered={} miss_out_of_span={} span={} rebases={} refused={}", + arm_name, + st.hits, + 100.0 * st.hits as f64 / tot as f64, + st.misses, + 100.0 * st.misses as f64 / tot as f64, + st.inserts, + unregistered, + st.oos, + st.span, + st.rebases, + st.refused, + ); } /// #7187: this map used to carry a bespoke identity hasher (`write_usize` @@ -418,7 +793,7 @@ pub(crate) fn generation_page_base(page: usize) -> usize { fn invalidate_generation_cache() { // Every way, not one — a stale way is exactly what this guards against. // SAFETY: thread-local, single-threaded. - PAGE_GENERATION_CACHE.with(|cache| unsafe { *cache.get() = PageGenerationCacheSet::empty() }); + PAGE_GENERATION_CACHE.with(|cache| unsafe { (*cache.get()).invalidate() }); } fn register_old_block_pages(base: usize, size: usize) { @@ -1970,3 +2345,201 @@ mod block_range_tests { assert_eq!(old_arena_block_range_index(&[], 0x1000_0000), None); } } + +#[cfg(test)] +mod page_class_table_tests { + //! The direct-indexed page-class table, pinned at the two points the span + //! measurement could not settle. A wrong answer from this structure is a + //! misclassified pointer — a collector that moves the wrong thing — so + //! each path has a test that fails when its fallback is removed. + use super::*; + + fn fresh(f: impl FnOnce() -> T + Send + 'static) -> T { + // Thread-local table, thread-local map: a fresh thread is a fresh world. + std::thread::spawn(f).join().expect("page-class table test panicked") + } + + fn table_stats() -> PageClassStats { + // SAFETY: thread-local, single-threaded, borrow ends here. + unsafe { (*hot_page_generation_cache()).stats() } + } + + const MB: usize = 1 << GENERATION_CLASS_SHIFT; + + /// The base is taken from the FIRST registration, wherever it is — not + /// from a constant. An arena that starts at a high address (ASLR moved the + /// base by 0x142920 classes between two measured runs) must hit the table, + /// not fall through to the map forever. + /// + /// Sabotage: hard-wire `self.base = 0` in `insert` — the classification + /// still returns the right generation (the map is authoritative) but every + /// lookup misses, and this test fails on the hit counter. + #[test] + fn base_is_taken_from_the_first_registration_not_a_constant() { + if !page_class_table_enabled() { + return; + } + fresh(|| { + // Far from zero, and not 1 MiB-aligned so the key math is exercised. + let base = 0x5f0_0000_0000usize + 0x3_8000; + register_block_space(base, MB, HeapGeneration::Old, HeapSpace::Old); + let inside = base + 0x1234; + // First classification: a miss that fills the entry. + assert_eq!(classify_heap_generation(inside), HeapGeneration::Old); + let before = table_stats(); + assert!(before.span > 0, "the first insert must allocate the table"); + assert_eq!( + (before.rebases, before.refused), + (0, 0), + "a base derived from the first registration must cover that \ + registration in the initial table — no rebase, no refusal" + ); + // Second: MUST be a table hit. + assert_eq!(classify_heap_generation(inside), HeapGeneration::Old); + let after = table_stats(); + assert_eq!( + after.hits, + before.hits + 1, + "a re-classification of a registered address must hit the table; \ + a base that is not derived from the first registration leaves \ + every key out of span and the table permanently cold" + ); + }); + } + + /// A key OUTSIDE the current span must still classify correctly, through + /// the authoritative map — either by rebasing the table to cover it or, past + /// the cap, by falling through uncached. Both are exercised. + /// + /// Sabotage: in `insert`, replace the out-of-span branch with an unchecked + /// `self.table[idx]` — the first assertion below panics on the bounds + /// check, and a release build without bounds checks would write past the + /// allocation. Or make `lookup` return the entry without `idx < len` — the + /// far address then reads a garbage entry and this test's generation + /// assertion fails. + #[test] + fn a_key_outside_the_span_still_classifies_correctly() { + if !page_class_table_enabled() { + return; + } + fresh(|| { + let near = 0x6a0_0000_0000usize; + register_block_space(near, MB, HeapGeneration::Old, HeapSpace::Old); + assert_eq!(classify_heap_generation(near + 8), HeapGeneration::Old); + let s0 = table_stats(); + assert_eq!(s0.span, PAGE_CLASS_TABLE_INITIAL_SPAN); + + // 1. Within the cap: a block 4,000 classes away. Must rebase and + // then hit. + let far = near + 4_000 * MB; + register_block_space(far, MB, HeapGeneration::Nursery, HeapSpace::NurseryEden); + assert_eq!( + classify_heap_generation(far + 8), + HeapGeneration::Nursery, + "an out-of-span key must classify through the map" + ); + let s1 = table_stats(); + assert_eq!( + s1.rebases, + s0.rebases + 1, + "a key inside the cap must rebase the table" + ); + assert!(s1.span > s0.span, "rebasing must widen the span"); + assert_eq!(s1.refused, 0); + // And the ORIGINAL block is still answered correctly after rebase. + assert_eq!(classify_heap_generation(near + 8), HeapGeneration::Old); + let h_before = table_stats().hits; + assert_eq!(classify_heap_generation(far + 8), HeapGeneration::Nursery); + assert_eq!( + table_stats().hits, + h_before + 1, + "after rebase the far key must hit" + ); + + // 2. Past the cap: 40,000 classes away. Must NOT rebase (the cap + // bounds the allocation) and must STILL classify correctly, + // uncached. + let wild = near + 40_000 * MB; + register_block_space(wild, MB, HeapGeneration::Longlived, HeapSpace::Old); + assert_eq!( + classify_heap_generation(wild + 8), + HeapGeneration::Longlived, + "a key past the cap must fall through to the map, not be dropped" + ); + let s2 = table_stats(); + assert_eq!( + s2.rebases, s1.rebases, + "a key past the cap must not grow the table" + ); + assert_eq!(s2.span, s1.span); + assert!(s2.refused >= 1, "the refusal must be counted, not silent"); + // Classify it again: still correct, still uncached. + assert_eq!(classify_heap_generation(wild + 8), HeapGeneration::Longlived); + }); + } + + /// A key match is NOT an address match. Two ranges can share a 1 MiB class + /// (`PageGenerationSlot::Multiple`); an entry confirmed for one must not + /// answer for an address in the other. + /// + /// Sabotage: drop `e.range.contains(addr)` from `lookup` — the second + /// classification returns the first range's generation for an address that + /// is not in it. + /// + /// Deliberately NOT gated on the arm: it asserts only on classification + /// results, which must hold whichever structure answers, so a run with + /// `PERRY_GC_PAGE_CLASS_TABLE=0` exercises the 4-way control arm through + /// this test. (The 4-way set can hold both ranges at once, in two ways + /// under one key; the table holds the last-confirmed one and misses to the + /// map for the other. Both are correct, which is what is pinned here.) + #[test] + fn a_hit_requires_range_containment_not_just_key_equality() { + fresh(|| { + // Two half-class ranges in the SAME class, different generations. + let class_base = 0x7b0_0000_0000usize; + let half = MB / 2; + register_block_space(class_base, half, HeapGeneration::Old, HeapSpace::Old); + register_block_space( + class_base + half, + half, + HeapGeneration::Nursery, + HeapSpace::NurseryEden, + ); + assert_eq!(classify_heap_generation(class_base + 8), HeapGeneration::Old); + // Same key, other half: the cached entry (Old) must NOT answer. + assert_eq!( + classify_heap_generation(class_base + half + 8), + HeapGeneration::Nursery, + "an entry for another range in the same class answered for this address" + ); + assert_eq!(classify_heap_generation(class_base + 8), HeapGeneration::Old); + }); + } + + /// Registration invalidates: a retagged block must never be answered from + /// a stale entry. This is the 4-way set's original contract carried over. + /// + /// Sabotage: make `invalidate` a no-op for the table — the second + /// classification returns the pre-retag generation. + /// + /// Also ungated on the arm: "a retag is never answered from a stale entry" + /// is the contract of BOTH structures, and running it under + /// `PERRY_GC_PAGE_CLASS_TABLE=0` is what keeps the control arm from + /// rotting untested while the table is the default. + #[test] + fn a_registration_change_invalidates_every_entry() { + fresh(|| { + let base = 0x8c0_0000_0000usize; + register_block_space(base, MB, HeapGeneration::Nursery, HeapSpace::NurseryEden); + assert_eq!(classify_heap_generation(base + 8), HeapGeneration::Nursery); + assert_eq!(classify_heap_generation(base + 8), HeapGeneration::Nursery); // cached + unregister_block_generation(base, MB); + register_block_space(base, MB, HeapGeneration::Old, HeapSpace::Old); + assert_eq!( + classify_heap_generation(base + 8), + HeapGeneration::Old, + "a stale table entry answered after the block was retagged" + ); + }); + } +} diff --git a/crates/perry-runtime/src/gc/copying.rs b/crates/perry-runtime/src/gc/copying.rs index be5dfdab47..71c85a308e 100644 --- a/crates/perry-runtime/src/gc/copying.rs +++ b/crates/perry-runtime/src/gc/copying.rs @@ -1616,6 +1616,9 @@ pub(super) fn run_copied_minor_attempt( collector.sticky.restore(); if !collector.skip_remembering { restore_surviving_dirty_coverage(&snapshot, &dirty_scan_covered, "copying_minor"); + // Per minor, not at exit: the rig SIGKILLs cc. Cumulative counters, so + // the last line before the kill is the answer. + crate::arena::page_class_table_report(); } let malloc_freed_bytes = if malloc_sweep_due { let phase_start = trace_phase_start(trace); From 93ffee5de2a5e2dcf392ae6c31a3ac91e633770c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 6 Sep 2026 08:00:46 +0200 Subject: [PATCH 4/5] docs(changelog): fragment for the page-class table, and sort the re-export Formatting and documentation only; no behaviour change. `cargo fmt` on the touched files, restricted to the lines this branch added. Note for whoever runs the fmt gate: `arena/mod.rs` is ALREADY not rustfmt-clean on main at an unrelated `#[cfg(test)]` re-export, and reformatting it would have put that pre-existing churn in this diff, so it is deliberately left alone. --- .../9845-gc-page-class-direct-table.md | 63 +++++++++++++++++++ crates/perry-runtime/src/arena/mod.rs | 3 +- crates/perry-runtime/src/arena/page_meta.rs | 24 +++++-- 3 files changed, 83 insertions(+), 7 deletions(-) create mode 100644 changelog.d/9845-gc-page-class-direct-table.md diff --git a/changelog.d/9845-gc-page-class-direct-table.md b/changelog.d/9845-gc-page-class-direct-table.md new file mode 100644 index 0000000000..ddd333451d --- /dev/null +++ b/changelog.d/9845-gc-page-class-direct-table.md @@ -0,0 +1,63 @@ +**The page-generation cache becomes a direct-indexed table over the arena's +1 MiB address classes, so a classification is a bounds compare and one load +instead of a four-way probe that missed one call in five.** + +`classify_heap_generation` and `classify_heap_space_in_range` sit under three +callers with no cheaper predicate of their own. The write barrier's +`remembered_child_needs_tracking` runs **35,871,391 times per turn** on the +compiled claude-code TUI and **95.23 %** of those take its cheapest arm — one +cached classification and a compare — so there is no barrier predicate left to +fix: what remains after the predicate is already optimal is the classification +itself. `mark_addr` (233 of 760 `classify*` leaf samples) and the side-table +prunes pay the same cost. + +The structure in front of the authoritative `PageGenerationMap` was a **4-way +round-robin set**. Measured with a dedicated counter on a 3300-char streaming +reply: **440 M lookups per turn at 20.0–21.6 % miss**, with **59.7–61.8 % of +misses on a key evicted within the last 64 evictions** — capacity, not conflict — +against a working set of **402–432 registered classes**. `ways_distinct_max` was +4, so every way was already in use and the shortfall is ~120x. + +**Widening it was not an option, and the reason is on the record.** #7469 +measured 16 ways as an **8.6 % regression** on the same row (0/7 pairs) for 1.5 % +fewer misses, and five further associativity changes measured flat. The rule +those produced — *associativity pays only when a miss is expensive* — says that a +miss which is just a hash lookup wants the cache to become **unnecessary**, not +larger. + +It can be. The registered classes occupy a span of **1,018–1,021 classes at +~40 % density**, so a table over that span holds every one of them in **160 KB** +and answers with one bounds compare and one load. `PageGenerationMap` stays +authoritative and every miss falls through to it exactly as before; the change is +confined to `PageGenerationCacheSet` and its two callers. + +Four things the measurement did not settle, each handled explicitly and each +pinned by a test that fails when its guard is removed — a wrong answer here is a +misclassified pointer, so none of them is left to inference: + +* **The base moves per process** (`0x43daa2` vs `0x57e3c2` on two runs — ASLR). + It is taken from the first insert, never compiled in. +* **The span can grow** (1,018 → 1,021 across two runs of one binary). An insert + outside the table rebases it, up to a 16,384-class cap; past the cap the key is + left uncached and falls through to the map rather than being mis-indexed. +* **The sizing is not obvious.** With base `first_key - S` and a table of `N`, + the span covered is `min(S + 1, N - S)`, maximised at `S = N / 2`. The natural + pairing `N = 4096, S = 1024` covers **1,025** classes — four above the measured + span — while `S = N / 2` covers **2,048** for the same memory. A `const` assert + now fails the build for any pairing covering less than twice the measured span. +* **A key match is not an address match.** A class can hold more than one range, + so a hit still requires `range.contains(addr)`. + +Invalidation is an epoch bump: O(1), and the same "clear everything" contract the +4-way set met by being reset wholesale. That contract matters more here, because +the table holds ~2,000 entries where the set held 4 — a missing invalidation the +old structure survived by luck would be a live misclassification — so all three +`PageGenerationMap` mutation sites were enumerated and each ends with an +unconditional `invalidate_generation_cache()`. + +The arm is a plain `u8` field in the set's first cache line rather than the env +`OnceLock`: this path runs 440 M times per turn, and an acquire load on each +would have been charged to both arms of the A/B — hiding it in the comparison +that was meant to isolate it — while still being paid against main. +`PERRY_GC_PAGE_CLASS_TABLE=0` restores the 4-way set in the same binary, which is +how the numbers above and below were taken. diff --git a/crates/perry-runtime/src/arena/mod.rs b/crates/perry-runtime/src/arena/mod.rs index b5835e69c5..5f7c84f3ef 100644 --- a/crates/perry-runtime/src/arena/mod.rs +++ b/crates/perry-runtime/src/arena/mod.rs @@ -54,8 +54,7 @@ pub(crate) use block::{ reset_gc_trigger_arena_probe, }; pub(crate) use page_meta::{ - page_class_table_report, - address_span_overlaps_pages, defer_old_object_page_registration, + address_span_overlaps_pages, defer_old_object_page_registration, page_class_table_report, register_block_space_with_object_starts, register_old_object_pages, unregister_block_generation, unregister_old_block_pages, OLD_GEN_RECLAIM_POOLED_BYTES, OLD_GEN_RECLAIM_RETURNED_BYTES, OLD_GEN_RECLAIM_REUSABLE_BYTES, diff --git a/crates/perry-runtime/src/arena/page_meta.rs b/crates/perry-runtime/src/arena/page_meta.rs index 5b5fe48016..d461d6f2d5 100644 --- a/crates/perry-runtime/src/arena/page_meta.rs +++ b/crates/perry-runtime/src/arena/page_meta.rs @@ -420,7 +420,10 @@ impl PageGenerationCacheSet { } idx = key - self.base; } - self.table[idx] = PageClassEntry { range, epoch: self.epoch }; + self.table[idx] = PageClassEntry { + range, + epoch: self.epoch, + }; self.inserts += 1; return; } @@ -2356,7 +2359,9 @@ mod page_class_table_tests { fn fresh(f: impl FnOnce() -> T + Send + 'static) -> T { // Thread-local table, thread-local map: a fresh thread is a fresh world. - std::thread::spawn(f).join().expect("page-class table test panicked") + std::thread::spawn(f) + .join() + .expect("page-class table test panicked") } fn table_stats() -> PageClassStats { @@ -2474,7 +2479,10 @@ mod page_class_table_tests { assert_eq!(s2.span, s1.span); assert!(s2.refused >= 1, "the refusal must be counted, not silent"); // Classify it again: still correct, still uncached. - assert_eq!(classify_heap_generation(wild + 8), HeapGeneration::Longlived); + assert_eq!( + classify_heap_generation(wild + 8), + HeapGeneration::Longlived + ); }); } @@ -2505,14 +2513,20 @@ mod page_class_table_tests { HeapGeneration::Nursery, HeapSpace::NurseryEden, ); - assert_eq!(classify_heap_generation(class_base + 8), HeapGeneration::Old); + assert_eq!( + classify_heap_generation(class_base + 8), + HeapGeneration::Old + ); // Same key, other half: the cached entry (Old) must NOT answer. assert_eq!( classify_heap_generation(class_base + half + 8), HeapGeneration::Nursery, "an entry for another range in the same class answered for this address" ); - assert_eq!(classify_heap_generation(class_base + 8), HeapGeneration::Old); + assert_eq!( + classify_heap_generation(class_base + 8), + HeapGeneration::Old + ); }); } From a529223465ef8c0db72219ee91e1e14c6f8570f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 6 Sep 2026 13:34:28 +0200 Subject: [PATCH 5/5] perf(gc): negative-cache the classes the page map does not hold After #9853's direct-indexed table took classification misses from 18.5% to ~6% of lookups, what remains is dominated by addresses the authoritative PageGenerationMap cannot answer either: conservative-scan candidates, interior pointers and non-heap words that every classification path re-asks about millions of times per turn. Nothing was ever cached for them, so each one paid a full failing hash lookup. COUNTER FIRST, and it corrects the issue it closes. #9852 states that the "unregistered" miss population is 90.7% of every remaining miss and that all of it is negative-cacheable. Only the first half is close. The uncached population collapses two cases that `pages.get(&key).and_then(|s| s.find(addr))` had never separated, so this change adds the two counters that split them before it adds the cache: * pages.get(&key) is None -- no registered range covers this 1 MiB class. Sound to remember at class granularity. * the class EXISTS but no range in it contains addr. Remembering the class as absent would answer wrongly for the addresses it really covers. Measured on the compiled claude-code TUI, table arm, three 3300-char runs and two at 400 (neg_absent + neg_present == miss_unregistered exactly in all five): 3300 absent/miss 52.8 / 55.6 / 62.1 % present/miss 27.4 / 16.3 / 21.4 % 400 absent/miss 41.5 / 66.2 % present/miss 28.7 / 15.9 % So the reach is 52.8-62.1% of remaining misses at 3300, not 90.7%: 19-34% of the population #9852 counted is the unsound case. The share is quoted as a range because it varies that much between runs of one binary. MECHANISM. A negative is an ordinary table entry carrying the current epoch, with the sentinel range base=usize::MAX end=0 -- contains() is false for every address, so the hit arm is byte-for-byte what it was and the marker test is reached only after contains() has already failed, i.e. on the miss path. lookup() becomes a tri-state; KnownAbsent returns exactly what the fall-through would have returned (HeapGeneration::Unknown / None) without asking the map. INVALIDATION NEEDS NO NEW RULE. All three PageGenerationMap mutation sites already end with invalidate_generation_cache(), which bumps the epoch. A registration into this class is precisely the event that would make a negative wrong, and precisely the event that kills it. A stale negative is the worst answer this structure can give -- the collector declines to trace a live young object -- so that equivalence is the whole correctness argument. Two further preconditions, each one a way this could be WRONG rather than merely slow: the table arm only (the 4-way set has no negative concept and its lookup does not test for one), and never the insert that allocates the table (the base must come from a REGISTERED key; an unregistered candidate address can be anywhere at all). PERRY_GC_PAGE_CLASS_NEGATIVE=0 restores the previous behaviour in the same binary, mirroring PERRY_GC_PAGE_CLASS_TABLE, so the A/B has no build difference to confound it. Four tests, each with its sabotage named at the test, including the one whose sabotage is caching the negative unconditionally. The first asserts on neg_hits rather than on the returned value, because the value is Unknown whether or not the negative was consulted -- a test that checked only the answer would pass with the feature deleted. cargo test --release -p perry-runtime --lib -- gc:: arena:: --test-threads=1: 1,142 passed, 0 failed. Refs #9852. Claude-Session: https://claude.ai/code/session_014knX724SYDogwzsXybCGxp --- changelog.d/9852-gc-page-class-negative.md | 23 + crates/perry-runtime/src/arena/page_meta.rs | 445 ++++++++++++++++++-- 2 files changed, 436 insertions(+), 32 deletions(-) create mode 100644 changelog.d/9852-gc-page-class-negative.md diff --git a/changelog.d/9852-gc-page-class-negative.md b/changelog.d/9852-gc-page-class-negative.md new file mode 100644 index 0000000000..ac00dd00d4 --- /dev/null +++ b/changelog.d/9852-gc-page-class-negative.md @@ -0,0 +1,23 @@ +### Fixed + +- **gc:** the page-class table now remembers that a 1 MiB class holds no + registered range, so a candidate address in no GC block is answered from the + table instead of a failing hash lookup in the authoritative + `PageGenerationMap` (#9852). + + After the direct-indexed table (#9853) took classification misses from + 18.5 % to ~6 % of lookups, what remained was dominated by addresses the map + cannot answer either: conservative-scan candidates, interior pointers and + non-heap words that every classification path re-asks about millions of + times per turn. Counted on the compiled claude-code TUI, 3300-char reply, + three runs: **52.8-62.1 % of every remaining miss** is a class the map does + not hold at all, at 41.5-66.2 % over two 400-char runs. + + A negative entry is an ordinary table entry carrying the current epoch, so + the registration that would make it wrong is the same event that already + invalidates every positive entry — no new invalidation obligation. It is + written **only** where `pages.get(&key)` is itself `None`; a class that holds + a range the address happens to miss is a different case (16-29 % of the same + population) and caching "absent" for it would answer wrongly for the + addresses the class really covers. `PERRY_GC_PAGE_CLASS_NEGATIVE=0` restores + the previous behaviour in the same binary. diff --git a/crates/perry-runtime/src/arena/page_meta.rs b/crates/perry-runtime/src/arena/page_meta.rs index d461d6f2d5..aae1996f2f 100644 --- a/crates/perry-runtime/src/arena/page_meta.rs +++ b/crates/perry-runtime/src/arena/page_meta.rs @@ -62,6 +62,41 @@ impl PageGenerationRange { fn contains(self, addr: usize) -> bool { addr >= self.base && addr < self.end } + + /// The marker a NEGATIVE table entry carries: "no registered range covers + /// any address in this 1 MiB class" (#9852). + /// + /// `base = usize::MAX, end = 0` makes [`contains`] false for every address + /// without a branch of its own, so the hit arm is byte-for-byte the code it + /// was before negatives existed and the marker test is reached only after + /// `contains` has already failed — i.e. on the miss path only. + /// + /// [`contains`]: PageGenerationRange::contains + const ABSENT: Self = Self { + base: usize::MAX, + end: 0, + generation: HeapGeneration::Unknown, + space: HeapSpace::Unknown, + object_starts: std::ptr::null_mut(), + }; + + #[inline] + fn is_absent(self) -> bool { + self.base == usize::MAX && self.end == 0 + } +} + +/// What a table probe concluded. Three states, not two, because "the table +/// knows there is nothing here" and "the table does not know" lead to +/// different code: the first answers the caller, the second falls through to +/// the authoritative map. +#[derive(Clone, Copy)] +enum ClassLookup { + Hit(PageGenerationRange), + /// A live NEGATIVE entry: no registered range covers this class, so the + /// map has no answer either and does not need to be asked (#9852). + KnownAbsent, + Unknown, } #[derive(Clone, Debug)] @@ -236,6 +271,14 @@ const ARM_UNRESOLVED: u8 = 0; const ARM_TABLE: u8 = 1; const ARM_WAYS: u8 = 2; +/// The negative cache's arm (#9852), resolved separately from `arm` because it +/// is a separate kill switch: the table can be on with negatives off, which is +/// the control arm for this change. `NEG_UNRESOLVED` stores nothing, which is +/// correct for both settings. +const NEG_UNRESOLVED: u8 = 0; +const NEG_ON: u8 = 1; +const NEG_OFF: u8 = 2; + /// The cache in front of [`PageGenerationMap`]: a **direct-indexed table** keyed /// by `addr >> GENERATION_CLASS_SHIFT`, with the previous 4-way set retained /// behind `PERRY_GC_PAGE_CLASS_TABLE=0` as the control arm. @@ -332,6 +375,29 @@ struct PageGenerationCacheSet { /// Lookups that missed because the key was OUTSIDE `[base, base + len)`. /// See the increment site for why this is the counter that matters. oos: u64, + /// `NEG_UNRESOLVED` / `NEG_ON` / `NEG_OFF` — the negative cache's own kill + /// switch (`PERRY_GC_PAGE_CLASS_NEGATIVE`), resolved once per thread on the + /// cold store path for the same reason `arm` is a field and not an + /// `OnceLock` read: this must not put an acquire load on a path that runs + /// hundreds of millions of times per turn. + neg_arm: u8, + /// Lookups answered `KnownAbsent` from a live negative entry — map lookups + /// that did not happen. This is the counter the change is judged on. + neg_hits: u64, + /// Negative entries written. + neg_inserts: u64, + /// MEASUREMENT ONLY (#9852). Splits the miss population the map could not + /// answer into the two cases `pages.get(&key).and_then(|s| s.find(addr))` + /// currently collapses: + /// * `neg_class_absent` — `pages.get(&key)` is itself `None`: no + /// registered range covers this 1 MiB class. This is the ONLY sound + /// negative to cache at class granularity. + /// * `neg_class_present` — the class exists in the map but no range in it + /// contains `addr`. Caching "absent" for the class would be WRONG for + /// the other addresses in it, so this population is not cacheable by the + /// proposed design. + neg_class_absent: u64, + neg_class_present: u64, // ---- control arm: the 4-way round-robin set, unchanged ---- ways: [PageGenerationCache; PAGE_GENERATION_CACHE_WAYS], /// Round-robin victim for the next insert. @@ -351,13 +417,18 @@ impl PageGenerationCacheSet { rebases: 0, refused: 0, oos: 0, + neg_arm: NEG_UNRESOLVED, + neg_hits: 0, + neg_inserts: 0, + neg_class_absent: 0, + neg_class_present: 0, ways: [PageGenerationCache::empty(); PAGE_GENERATION_CACHE_WAYS], next: 0, } } #[inline(always)] - fn lookup(&mut self, key: usize, addr: usize) -> Option { + fn lookup(&mut self, key: usize, addr: usize) -> ClassLookup { if self.arm != ARM_WAYS { // `wrapping_sub` folds `key < base` into the same out-of-range // check as `key >= base + len`: a key below the base wraps to a @@ -365,9 +436,20 @@ impl PageGenerationCacheSet { let idx = key.wrapping_sub(self.base); if idx < self.table.len() { let e = &self.table[idx]; - if e.epoch == self.epoch && e.range.contains(addr) { - self.hits += 1; - return Some(e.range); + if e.epoch == self.epoch { + if e.range.contains(addr) { + self.hits += 1; + return ClassLookup::Hit(e.range); + } + // Reached only after `contains` has failed, so the hit arm + // is unchanged and this costs nothing on a hit (#9852). + // A negative entry is stamped with the same epoch as a + // positive one, so the registration that would make it + // wrong invalidates it by the same bump. + if e.range.is_absent() { + self.neg_hits += 1; + return ClassLookup::KnownAbsent; + } } } else { // Miss-path only, so it costs nothing on a hit — and it is the @@ -380,16 +462,16 @@ impl PageGenerationCacheSet { self.oos += 1; } self.misses += 1; - return None; + return ClassLookup::Unknown; } for way in self.ways.iter() { if way.valid && way.key == key && way.range.contains(addr) { self.hits += 1; - return Some(way.range); + return ClassLookup::Hit(way.range); } } self.misses += 1; - None + ClassLookup::Unknown } #[inline] @@ -409,22 +491,9 @@ impl PageGenerationCacheSet { self.base = key.saturating_sub(PAGE_CLASS_TABLE_BASE_SLACK); self.table = vec![PageClassEntry::DEAD; PAGE_CLASS_TABLE_INITIAL_SPAN]; } - let mut idx = key.wrapping_sub(self.base); - if idx >= self.table.len() { - if !self.rebase_to_cover(key) { - // Past the cap: leave it uncached. The caller already has - // the authoritative answer and returns it; only the - // acceleration is forgone. - self.refused += 1; - return; - } - idx = key - self.base; + if self.place(key, range) { + self.inserts += 1; } - self.table[idx] = PageClassEntry { - range, - epoch: self.epoch, - }; - self.inserts += 1; return; } self.inserts += 1; @@ -437,6 +506,79 @@ impl PageGenerationCacheSet { self.next = slot.wrapping_add(1); } + /// Write `range` into `key`'s slot, rebasing if the key is outside the + /// current span. Returns whether it was stored; `false` means the span cap + /// refused the key and the caller must fall through to the map, which it + /// already can — only the acceleration is forgone. **Requires a non-empty + /// table**: allocating one here would take the base from whatever key + /// arrived first, and only a REGISTERED key is guaranteed to sit inside the + /// arena's eventual span. + #[inline] + fn place(&mut self, key: usize, range: PageGenerationRange) -> bool { + debug_assert!(!self.table.is_empty(), "place on an unallocated table"); + let mut idx = key.wrapping_sub(self.base); + if idx >= self.table.len() { + if !self.rebase_to_cover(key) { + self.refused += 1; + return false; + } + idx = key - self.base; + } + self.table[idx] = PageClassEntry { + range, + epoch: self.epoch, + }; + true + } + + /// Remember that NO registered range covers `key`'s 1 MiB class (#9852). + /// + /// Called only where the caller has just read `pages.get(&key)` as `None`. + /// That is the whole soundness argument and it has to stay exact: a class + /// that holds a range but does not contain this particular address is a + /// DIFFERENT case, and caching "absent" for it would answer wrongly for + /// every other address in the class. Measured on the compiled claude-code + /// TUI, that second case is 16-29 % of the population, so this is not a + /// theoretical distinction. + /// + /// Three preconditions, each one a way this could be wrong rather than + /// merely slow: + /// * **the table arm only** — the 4-way set has no negative concept and its + /// `lookup` does not test for one; + /// * **a table that already exists** — see [`place`]: a negative key must + /// never be the one that fixes the base; + /// * **the kill switch** — `PERRY_GC_PAGE_CLASS_NEGATIVE=0` stores nothing, + /// which turns the feature off completely because `lookup` can only + /// answer `KnownAbsent` from an entry this function wrote. + /// + /// Invalidation needs no new rule: the entry carries the current epoch, and + /// all three `PageGenerationMap` mutation sites already end with + /// `invalidate_generation_cache()`, which bumps it. A registration into + /// this class is exactly the event that would make the entry wrong, and it + /// is exactly the event that kills it. + /// + /// [`place`]: PageGenerationCacheSet::place + #[inline] + fn insert_negative(&mut self, key: usize) { + if self.arm != ARM_TABLE || self.table.is_empty() { + return; + } + if self.neg_arm == NEG_UNRESOLVED { + // The one env read, on the cold path, once per thread. + self.neg_arm = if page_class_negative_enabled() { + NEG_ON + } else { + NEG_OFF + }; + } + if self.neg_arm != NEG_ON { + return; + } + if self.place(key, PageGenerationRange::ABSENT) { + self.neg_inserts += 1; + } + } + /// Grow the table so that `key` is inside it, keeping every class it /// already covered. Returns false — and changes nothing — if the resulting /// span would exceed the cap. @@ -485,6 +627,17 @@ impl PageGenerationCacheSet { } } + /// MEASUREMENT ONLY (#9852). Called from the two `_uncached` arms when the + /// authoritative map had no answer, with whether the CLASS was absent. + #[inline] + fn note_negative(&mut self, class_absent: bool) { + if class_absent { + self.neg_class_absent += 1; + } else { + self.neg_class_present += 1; + } + } + /// `(hits, misses, inserts, span, rebases, refused)` for the diagnostic /// line and for the tests. fn stats(&self) -> PageClassStats { @@ -497,6 +650,10 @@ impl PageGenerationCacheSet { rebases: self.rebases, refused: self.refused, oos: self.oos, + neg_hits: self.neg_hits, + neg_inserts: self.neg_inserts, + neg_class_absent: self.neg_class_absent, + neg_class_present: self.neg_class_present, } } } @@ -514,6 +671,10 @@ struct PageClassStats { rebases: u64, refused: u64, oos: u64, + neg_hits: u64, + neg_inserts: u64, + neg_class_absent: u64, + neg_class_present: u64, } /// `PERRY_GC_PAGE_CLASS_TABLE=0` restores the 4-way set. The kill switch, and @@ -525,6 +686,16 @@ fn page_class_table_enabled() -> bool { *ENABLED.get_or_init(|| crate::gc::env_default_on_enabled("PERRY_GC_PAGE_CLASS_TABLE")) } +/// `PERRY_GC_PAGE_CLASS_NEGATIVE=0` stops the table remembering that a class +/// holds nothing (#9852), leaving every such lookup to the authoritative map +/// exactly as before. Mirrors `PERRY_GC_PAGE_CLASS_TABLE` deliberately: both +/// arms live in ONE binary, so the A/B has no build difference to confound it. +#[inline(always)] +fn page_class_negative_enabled() -> bool { + static ENABLED: std::sync::OnceLock = std::sync::OnceLock::new(); + *ENABLED.get_or_init(|| crate::gc::env_default_on_enabled("PERRY_GC_PAGE_CLASS_NEGATIVE")) +} + /// One line under `PERRY_GC_DIAG=1`, emitted per copying minor from the /// collector (never at exit — the rig SIGKILLs the process). pub(crate) fn page_class_table_report() { @@ -533,7 +704,10 @@ pub(crate) fn page_class_table_report() { } // SAFETY: thread-local, single-threaded, shared borrow ends here. let st = unsafe { (*hot_page_generation_cache()).stats() }; - let tot = st.hits + st.misses; + // `neg_hits` are lookups too: they are answered without touching the map, + // so counting them anywhere but the denominator would make the miss RATE + // improve by shrinking what it is a rate of. + let tot = st.hits + st.neg_hits + st.misses; if tot == 0 { return; } @@ -551,7 +725,8 @@ pub(crate) fn page_class_table_report() { }; eprintln!( "[gc-page-class] arm={} lookups={tot} hit={} ({:.3}%) miss={} ({:.3}%) \ -miss_registered={} miss_unregistered={} miss_out_of_span={} span={} rebases={} refused={}", +miss_registered={} miss_unregistered={} miss_out_of_span={} span={} rebases={} refused={} \ +neg_hit={} ({:.3}%) neg_stored={} neg_class_absent={} neg_class_present={}", arm_name, st.hits, 100.0 * st.hits as f64 / tot as f64, @@ -563,6 +738,11 @@ miss_registered={} miss_unregistered={} miss_out_of_span={} span={} rebases={} r st.span, st.rebases, st.refused, + st.neg_hits, + 100.0 * st.neg_hits as f64 / tot as f64, + st.neg_inserts, + st.neg_class_absent, + st.neg_class_present, ); } @@ -1305,8 +1485,13 @@ pub(crate) fn classify_heap_generation(addr: usize) -> HeapGeneration { // attributed samples) precisely because it resolved two distinct // thread-locals per call. // SAFETY: thread-local, single-threaded, and the borrow ends here. - if let Some(range) = unsafe { (*hot_page_generation_cache()).lookup(key, addr) } { - return range.generation; + match unsafe { (*hot_page_generation_cache()).lookup(key, addr) } { + ClassLookup::Hit(range) => return range.generation, + // The map's answer for a class with no registered range IS `Unknown` + // (`pages.get(&key)` is `None` -> `HeapGeneration::Unknown`), so this + // returns what the fall-through would have returned, without it. + ClassLookup::KnownAbsent => return HeapGeneration::Unknown, + ClassLookup::Unknown => {} } classify_heap_generation_uncached(addr, key) } @@ -1315,10 +1500,26 @@ pub(crate) fn classify_heap_generation(addr: usize) -> HeapGeneration { /// re-prime the one-entry cache. #[inline(never)] fn classify_heap_generation_uncached(addr: usize, key: usize) -> HeapGeneration { - let found = { + // MEASUREMENT ONLY (#9852): the `class_absent` flag splits the two cases + // `pages.get(&key).and_then(...)` collapses. Computed inside the borrow, + // counted after it drops. + let (found, class_absent) = { let pages = hot_page_generations().borrow(); - pages.get(&key).and_then(|slot| slot.find(addr)) + match pages.get(&key) { + None => (None, true), + Some(slot) => (slot.find(addr), false), + } }; + if found.is_none() { + // SAFETY: thread-local, single-threaded, and the borrow ends here. + unsafe { + let set = &mut *hot_page_generation_cache(); + set.note_negative(class_absent); + if class_absent { + set.insert_negative(key); + } + } + } if let Some(range) = found { // SAFETY: as above. unsafe { (*hot_page_generation_cache()).insert(key, range) }; @@ -1357,8 +1558,12 @@ pub(crate) fn classify_heap_space_in_range(addr: usize) -> Option<(HeapSpace, us } let key = generation_class_key_for_addr(addr); // SAFETY: thread-local, single-threaded, and the borrow ends here. - if let Some(range) = unsafe { (*hot_page_generation_cache()).lookup(key, addr) } { - return Some((range.space, range.base, range.object_starts)); + match unsafe { (*hot_page_generation_cache()).lookup(key, addr) } { + ClassLookup::Hit(range) => return Some((range.space, range.base, range.object_starts)), + // As above: `None` is what the map returns for a class it does not + // hold, so the negative answers with the same value. + ClassLookup::KnownAbsent => return None, + ClassLookup::Unknown => {} } classify_heap_space_in_range_uncached(addr, key) } @@ -1369,10 +1574,24 @@ fn classify_heap_space_in_range_uncached( addr: usize, key: usize, ) -> Option<(HeapSpace, usize, *mut u64)> { - let found = { + // MEASUREMENT ONLY (#9852) — see `classify_heap_generation_uncached`. + let (found, class_absent) = { let pages = hot_page_generations().borrow(); - pages.get(&key).and_then(|slot| slot.find(addr)) + match pages.get(&key) { + None => (None, true), + Some(slot) => (slot.find(addr), false), + } }; + if found.is_none() { + // SAFETY: thread-local, single-threaded, and the borrow ends here. + unsafe { + let set = &mut *hot_page_generation_cache(); + set.note_negative(class_absent); + if class_absent { + set.insert_negative(key); + } + } + } let range = found?; // SAFETY: thread-local, single-threaded, and the borrow ends here. unsafe { (*hot_page_generation_cache()).insert(key, range) }; @@ -2530,6 +2749,168 @@ mod page_class_table_tests { }); } + /// #9852. A repeated classification of an address whose 1 MiB class holds + /// no registered range must be answered from the table, not from the map. + /// + /// Asserted on `neg_hits`, not on the returned value: the value is + /// `Unknown` whether or not the negative was consulted, so a test that + /// checked only the answer would pass with the whole feature deleted. + /// + /// Sabotage: drop the `set.insert_negative(key)` call in + /// `classify_heap_generation_uncached` — the second classification is a + /// miss again and `neg_hits` stays 0, failing here. + #[test] + fn a_class_with_no_registered_range_is_answered_from_the_negative_entry() { + if !page_class_table_enabled() || !page_class_negative_enabled() { + return; + } + fresh(|| { + // A registration first: the base must come from a REGISTERED key. + let base = 0x9d0_0000_0000usize; + register_block_space(base, MB, HeapGeneration::Old, HeapSpace::Old); + assert_eq!(classify_heap_generation(base + 8), HeapGeneration::Old); + // An address in a neighbouring class that holds nothing at all. + let absent = base + 4 * MB + 0x40; + assert_eq!(classify_heap_generation(absent), HeapGeneration::Unknown); + let after_first = table_stats(); + assert_eq!( + after_first.neg_inserts, 1, + "a class the map does not hold must leave a negative entry" + ); + let before = after_first.neg_hits; + assert_eq!(classify_heap_generation(absent), HeapGeneration::Unknown); + assert_eq!( + table_stats().neg_hits, + before + 1, + "the second classification of an absent class went to the map; \ + the negative entry was not consulted" + ); + // And the same class answers `classify_heap_space_in_range` too. + assert_eq!(classify_heap_space_in_range(absent + 8), None); + assert_eq!( + table_stats().neg_hits, + before + 2, + "the space classifier did not consult the negative entry" + ); + }); + } + + /// #9852, the unsound case. A class that DOES hold a range, probed at an + /// address outside it, must not be remembered as absent — the negative is + /// class-granular and would then answer wrongly for the addresses the + /// class really covers. Measured on cc, this case is 16-29 % of the + /// population, so it is the common case and not a corner. + /// + /// Sabotage: call `insert_negative` unconditionally instead of under + /// `if class_absent` — the final assertion reads `Unknown` for an address + /// in a registered half-block. + #[test] + fn a_partially_registered_class_is_never_remembered_as_absent() { + if !page_class_table_enabled() || !page_class_negative_enabled() { + return; + } + fresh(|| { + let class_base = 0xae0_0000_0000usize; + let half = MB / 2; + // Only the FIRST half of the class is registered. + register_block_space(class_base, half, HeapGeneration::Old, HeapSpace::Old); + // Probe the unregistered half: the class exists, the address is + // not in its range. + assert_eq!( + classify_heap_generation(class_base + half + 8), + HeapGeneration::Unknown + ); + let st = table_stats(); + assert_eq!( + st.neg_class_present, 1, + "the probe must be counted as class-present, not class-absent" + ); + assert_eq!( + st.neg_inserts, 0, + "a class that holds a range must never get a negative entry" + ); + // The registered half must still classify correctly. + assert_eq!( + classify_heap_generation(class_base + 8), + HeapGeneration::Old, + "a negative cached for a partially registered class answered \ + for an address the class really covers" + ); + }); + } + + /// #9852. A negative is invalidated by the registration that makes it + /// wrong, through the epoch bump every mutation site already performs. A + /// stale negative is the worst failure this structure can produce: the + /// collector declines to trace a live young object. + /// + /// Sabotage: stamp `epoch: 0` on the negative entry and it can never be + /// live (this test still passes but the previous one fails); stamp a + /// constant epoch instead of `self.epoch` and this one fails. + #[test] + fn a_negative_entry_is_killed_by_a_later_registration_in_that_class() { + if !page_class_table_enabled() || !page_class_negative_enabled() { + return; + } + fresh(|| { + let base = 0xbf0_0000_0000usize; + register_block_space(base, MB, HeapGeneration::Old, HeapSpace::Old); + assert_eq!(classify_heap_generation(base + 8), HeapGeneration::Old); + let later = base + 8 * MB; + // Absent now, and remembered as absent. + assert_eq!(classify_heap_generation(later + 8), HeapGeneration::Unknown); + assert_eq!(table_stats().neg_inserts, 1); + // Now that class really is registered. + register_block_space(later, MB, HeapGeneration::Nursery, HeapSpace::NurseryEden); + assert_eq!( + classify_heap_generation(later + 8), + HeapGeneration::Nursery, + "a stale negative answered after the class was registered — \ + the collector would decline to trace a live young object" + ); + }); + } + + /// #9852. A negative must never be the insert that allocates the table: + /// the base is derived from the first key stored, and only a REGISTERED + /// key is guaranteed to sit inside the arena's eventual span. An + /// unregistered candidate address can be anywhere at all. + /// + /// Sabotage: drop the `self.table.is_empty()` guard in `insert_negative` — + /// the first assertion fails (a table exists with a base taken from a + /// garbage address) and the registered address that follows is out of span. + #[test] + fn a_negative_never_fixes_the_tables_base() { + if !page_class_table_enabled() || !page_class_negative_enabled() { + return; + } + fresh(|| { + // A wild candidate address, classified before anything is + // registered — exactly what a conservative scan produces. + let wild = 0x1_0000_0000_0000usize; + assert_eq!(classify_heap_generation(wild), HeapGeneration::Unknown); + let st = table_stats(); + assert_eq!( + (st.span, st.neg_inserts), + (0, 0), + "a negative allocated the table and fixed its base from an \ + unregistered address" + ); + // The real arena, far away, must still be covered without a rebase. + let base = 0xc10_0000_0000usize; + register_block_space(base, MB, HeapGeneration::Old, HeapSpace::Old); + assert_eq!(classify_heap_generation(base + 8), HeapGeneration::Old); + assert_eq!(classify_heap_generation(base + 8), HeapGeneration::Old); + let st = table_stats(); + assert!(st.span > 0, "the first registered insert must allocate"); + assert_eq!( + (st.rebases, st.refused), + (0, 0), + "the base came from somewhere other than the first registration" + ); + }); + } + /// Registration invalidates: a retagged block must never be answered from /// a stale entry. This is the 4-way set's original contract carried over. ///