diff --git a/changelog.d/9827-gc-trigger-path-hot-tls.md b/changelog.d/9827-gc-trigger-path-hot-tls.md new file mode 100644 index 0000000000..e791ac3b7d --- /dev/null +++ b/changelog.d/9827-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/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/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..5f7c84f3ef 100644 --- a/crates/perry-runtime/src/arena/mod.rs +++ b/crates/perry-runtime/src/arena/mod.rs @@ -46,13 +46,15 @@ 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, reset_gc_trigger_arena_probe, }; pub(crate) use page_meta::{ - 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 bc6984dff3..d461d6f2d5 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,93 @@ 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 +436,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` @@ -319,13 +697,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()); @@ -411,7 +796,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) { @@ -1138,7 +1523,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 @@ -1963,3 +2348,212 @@ 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/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/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); 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 } }