diff --git a/cc-perf-campaign/codex/REPORT_minor_phases_and_logs.md b/cc-perf-campaign/codex/REPORT_minor_phases_and_logs.md new file mode 100644 index 0000000000..80af5e11b4 --- /dev/null +++ b/cc-perf-campaign/codex/REPORT_minor_phases_and_logs.md @@ -0,0 +1,232 @@ +# Copying-minor phases and remaining scanner young logs + +Phase-instrument commit: `09846784c` + +Scanner-log implementation commit: `dae519296` + +Branch: `perf/minor-phases-and-logs`, based on +`8b7dc3342`. + +## Copying-minor phase instrument + +- `crates/perry-runtime/src/gc/copying_phase.rs:26` is the diagnostic-only + accumulator. It uses the same `Instant` clock as `pause_us` and records + non-overlapping spans for `root_scan`, `copy_evacuation`, + `remembered_set_young_logs`, `promotion`, + `dead_owner_side_table_pruning`, `from_space_finalization`, + `forwarding_fixups`, and `block_reset_flip`. `other` is the exact residual + between those named spans and the whole pause, and `phase_sum_us` is formed + from the nanosecond partition before conversion, so it equals `pause_us` + apart from the shared sub-microsecond truncation (well inside 2%). +- `crates/perry-runtime/src/gc/copying.rs:1220-1749` starts and records the + counters in the functions whose work they price. The two registered-root + passes accumulate in `root_scan`; the transitive worklist drain is + `copy_evacuation`; remembered snapshot/dirty scan and post-cycle restore are + accumulated together; promotion covers retag plus finish; forwarding covers + promoted-edge rebuild and verification/fixup work; reset covers to-space + preparation plus the final reset/flip. +- `crates/perry-runtime/src/gc/copying_phase.rs:84` renders counts where the + collector already owns them: copied/promoted objects and bytes, remembered + entries and dirty slots, and finalized map/set/error/regexp owners. The + dead-owner fan-out at `crates/perry-runtime/src/gc/dead_owner.rs:261` clocks + every registry table separately and appends those table names and + microseconds to the same field. Those prune callbacks expose no removed-row + count, so no invented count is printed. +- `crates/perry-runtime/src/gc/copying.rs:1962` appends `phases:` to every + completed `[gc-copy-minor] ran` line. `PERRY_GC_DIAG` off creates no phase + accumulator, takes no phase clocks, and builds no detail strings. +- The sabotage unit + `copied_minor_phase_residual_makes_the_partition_exact` removes a named + bucket from the expected arithmetic if the partition is widened or omitted. + +## Scanner map and young-entry logs + +### `scan_descriptor_roots_mut` + +This walks string-keyed property-attribute and accessor tables plus their two +owner indexes. Owner addresses are metadata-only and need a minor visit only +while movable/reclaimable; accessor get/set NaN-boxes are strong roots and may +require tracing through Longlived values. A #9754 owner log already existed. +The write funnel at `object/descriptor_state.rs:148` is present at all five +publication/transfer sites (`:985`, `:1208`, `:1277`, `:1394`, `:1428`). This +change narrows the metadata-key half from `addr_is_minor_relevant` to +`addr_is_minor_collectible`; the re-derivation and post-visit keep predicate +use the same rule at `object/descriptor_state/young.rs:42` and +`object/descriptor_state/gc_scan.rs:17`. The full walk is unchanged and +rebuilds the log. + +### `scan_closure_dynamic_props_roots_mut` + +This walks `CLOSURE_PROPS` values, `CLOSURE_STATIC_PROTOTYPES` values, and the +metadata-only owners of those tables and `CLOSURE_DELETED_KEYS`. A #9754 owner +log already existed. The enforced write funnels are +`closure/dynamic_props.rs:117`, `:186`, `:241`, `:388`, and `:1068`. Owner +retention is now collectible-only; property/prototype values keep the broader +transitive predicate. The minor path is `:533`; the full path remains whole +table and rebuilds the log. + +### `scan_builtin_closure_metadata_roots_mut` + +This walks two owner-keyed, pointer-free metadata tables: closure arity and the +non-constructable set. Only the closure address can move or die. There was no +partial log. The tables and their complete setters were extracted to +`object/native_module/callable_exports/builtin_closure_metadata.rs`; `:18` +arms the owner log before either setter publishes, `:95` drains only logged +collectible owners on a minor, and the unchanged full walk visits all owners +and rebuilds the log. + +### `scan_template_raw_roots_mut` + +This scanner owns three small tables: call-site to cooked/raw template arrays, +cooked to raw template arrays, and array named properties. The attempted young +logs were reverted after MP measurement showed 2.76 ms for the keyed path +against 1.83 ms for the original full walk. The scanner again walks the three +authoritative tables directly, with no insert-side log upkeep. + +### `scan_symbol_side_table_roots_mut` + +This walks six slot shapes: `SYMBOL_PROPERTIES` owner metadata and strong +symbol/value pairs, `SYMBOL_PROPERTY_ATTRS` owner metadata and strong symbol +keys, symbol accessors plus get/set roots, class-static symbol/value pairs, +and metadata-only `SYMBOL_POINTERS`. The attempted typed-slot young log was +reverted after MP measurement showed 2.47 ms for the keyed path against +1.74 ms for the original full walk. Direct scans again iterate the +authoritative tables, and budgeted scans again use the pre-existing full slot +snapshot; none of the symbol writers pays young-log upkeep. + +The retained descriptor, closure-dynamic-property, built-in-closure-metadata, +and shape-cache young logs continue to emit `[gc-young-log]` accounting with +logged/visited/kept/table size. + +## MP measurement and the two reverted/fixed logs + +Perrymaster measured MP-stage medians over 16 steady 3300-character minor +collections, comparing app-m6mp (the five scanner changes) with app-m6ms +(without them): + +| scanner | m6ms full walk | m6mp young log | delta | +|---|---:|---:|---:| +| descriptor_roots | 4.12 ms | 4.02 ms | -0.1 ms | +| closure_dynamic_props | 3.75 ms | 3.06 ms | **-0.7 ms** | +| builtin_closure_metadata | 1.42 ms | 0.93 ms | **-0.5 ms** | +| shape_cache | 0.68 ms | 0.27 ms | **-0.4 ms** | +| **template_raw_roots** | 1.83 ms | **2.76 ms** | **+0.9 ms** | +| **symbol_side_table** | 1.74 ms | **2.47 ms** | **+0.7 ms** | +| transition_cache / intern / class_side / singleton_closure / box | flat | flat | flat | +| **total** | **15.5 ms** | **15.3 ms** | **-0.2 ms** | + +Both regressions are case (b): the logged path made each visited entry more +expensive than the dense full walk. They are not duplicate-log failures: +`YoungLog::take_sorted` sorts and globally deduplicates every batch, and each +writer tests the young/relevant predicate before noting a key. + +- `template_raw_roots`: the full scanner streams each map directly and only + removes/reinserts keys that actually move. The young path sorted its keys, + performed a hash lookup for every cache entry, and unconditionally removed + and reinserted every logged raw-map and named-property owner even when the + owner did not move. A pointer or index cannot safely retain the full walk's + per-entry cost because insertion and rekeying can relocate these `HashMap` + entries. The three logs, their publication hooks, and their two rederivation + tests were therefore removed. +- `symbol_side_table`: the full scanner streams the maps and their property + vectors. The typed-slot path sorted its keys, then recovered every property + entry through an owner hash lookup plus a linear search of that owner's + vector; the other slot shapes also paid keyed table lookups. Hash-map + rekeying and vector growth make raw entry pointers or indices unstable, so a + safe O(young) path with the full walk's per-entry cost would require a + structural table redesign. The typed log, all writer hooks, and its + rederivation test were therefore removed. + +Re-measurement falsifier: on perrymaster, `template_raw_roots` must be at most +**1.83 ms** and `symbol_side_table` at most **1.74 ms** at the median, the +three improved scanners must remain unchanged, and total scanner time must be +at most **14 ms**. + +## Sabotage tests + +- `descriptor_log_rederivation_rejects_a_suppressed_setter`: suppresses the + real property-attrs funnel; re-derivation must report the missing owner. +- `closure_log_rederivation_rejects_a_suppressed_setter`: suppresses the real + closure dynamic-property funnel; re-derivation must report the missing + owner. +- `builtin_closure_log_rederivation_rejects_a_suppressed_writer`: suppresses + the arity setter; re-derivation must report the missing closure. +- `template_raw_log_rederivation_rejects_a_suppressed_writer`, + `array_named_log_rederivation_rejects_a_suppressed_setter`, and + `symbol_log_rederivation_rejects_a_suppressed_property_writer` were removed + with the two reverted logs; their enforced-writer invariant no longer + exists. + +Each completeness check is compiled under `debug_assertions` and `test`. In +the release lib run below, every named sabotage test passed. + +## Shape residual + +The residual is real young work, not another whole-table leak. The exact keep +predicate is `object/shapes.rs:2154`: + +- Nursery Eden, either survivor half, and `PromotedYoung` keys arrays stay + logged because their table keys must be rewritten if they move. +- Malloc-GC keys arrays stay only when an old/cache carrier makes the family a + root and the allocation remains minor-collectible. +- Longlived keys arrays stay only when an old/cache carrier roots the family + **and** at least one property-key leaf in the array is collectible. Longlived + non-carriers and carriers whose leaves are all old/Longlived drop out. +- Old keys arrays always drop out. + +There is one intentional transient duplicate at `object/shapes.rs:2244`: the +mark pass may move a family before the metadata-only slot index is repaired in +the rewrite pass, so both the post-copy address and stale index address must +survive between the passes. Tightening any of these remaining cases would +skip relocation, collection of malloc keys, a strong carrier edge, or the +between-pass index repair. This explains why shape time appears only on the +steady minors that create/grow a burst of genuinely young shape-key arrays; +there is no sound additional predicate tightening in this change. + +## Validation + +- `git diff --check`: PASS. +- `scripts/check_file_size.sh`: PASS (all Rust files at most 2,000 lines). +- `cargo fmt --all -- --check`: PASS. +- `scripts/check_thread_locals.py --self-test`: PASS in all seven directions. +- `scripts/check_thread_locals.py`: PASS, 411 hot declarations and 273 cold + declarations in 84 recorded files, below the 768-slot hot capacity. +- `scripts/gc_rekeyed_key_tables.py`: PASS, 42 sites and 25 registered prunes + classified with zero gaps. The split child now owns the `visit_owner` + inventory entry. +- `cargo test -p perry-runtime --release --lib -j4 -- --test-threads=1` via + `measure_lock.sh --build`: PASS, 3,271 passed, 0 failed, 4 ignored. The three + rederivation tests tied to the reverted logs were explicitly removed; the + retained sabotage tests passed. +- `cargo build --release -p perry-runtime --features wasm-host -j4` via + `measure_lock.sh --build`: PASS. + +## Predictions and exact perrymaster request + +Prediction after the MP follow-up: the two reverted scanners return to their +measured full-walk medians or better, the retained closure, built-in closure, +and shape-cache improvements remain, and steady scanner total is at most +**14 ms**. The phase table, not an estimate, must name the next non-scanner +lever. RSS should fall slightly because the two reverted logs and their +retained buffers are gone. + +Exact perrymaster request: fetch pushed branch `perf/minor-phases-and-logs` and +relink this runtime-only change on main's cache. Run the three required gates +through +`/Users/amlug/projects/perry/secret-tests/cc-perf-campaign/measure_lock.sh --build` +detached, using exactly: + +1. `cargo test -p perry-runtime --release --lib -j4 -- --test-threads=1` +2. `cargo build --release -p perry-runtime --features wasm-host -j4` +3. `cargo build --release -p perry -j4` + +Then run one +graceful four-turn 3300-character cc workload and one 400-character workload +with `PERRY_GC_DIAG=1`, printing and preserving **every complete** +`[gc-copy-minor] ran` line. The phase table for a steady minor is the +deliverable that names the next lever. Confirm `template_raw_roots` is at most +1.83 ms, `symbol_side_table` is at most 1.74 ms, the three improved scanner +medians are unchanged, and total scanner time is at most 14 ms. Finally run +paired **5x3300 + 3x400** against both main and #9950's runtime, +reporting cc turn CPU and peak RSS; target node/bun CPU parity, allowing only ++1-10% RSS. diff --git a/cc-perf-campaign/codex/REPORT_minor_scanner_young_logs.md b/cc-perf-campaign/codex/REPORT_minor_scanner_young_logs.md new file mode 100644 index 0000000000..fe8570d4a8 --- /dev/null +++ b/cc-perf-campaign/codex/REPORT_minor_scanner_young_logs.md @@ -0,0 +1,107 @@ +# Minor scanner young logs + +Implementation SHA: `d399c39ddb638a92b2735a6bacc2aef13def944a` + +## Map and mechanism + +- `object/shapes.rs:1972` scans two address-keyed structures. `families` maps a + keys-array address to every descriptor id whose slab record carries that + address; the descriptor record is the authoritative rewritable `keys` edge. + A family is a strong minor root only when an old receiver or an optimization + cache carries one of its descriptors. `indices` is a weak key-to-slot + accelerator keyed by the same keys-array address and needs only relocation + repair. Shape property-key payloads are strings/symbol headers, both GC + leaves. Nursery keys arrays can move; old arrays cannot; Longlived arrays do + not move or die but can temporarily contain a collectible key leaf. +- Shapes already had #9755's `young_keys` address log and the four + `shapes.indices` arm sites. Its keep predicate was + `addr_is_minor_relevant`, so every Longlived keys array stayed in the log + forever. `object/shapes.rs:2154` now re-derives actual minor work: nursery + addresses remain for relocation, malloc roots remain while carrier-owned, + and a Longlived carrier remains only while its property-key payload contains + a collectible leaf. `object/shapes.rs:271` receives old/cache carrier notes + without recursively borrowing the shape table; `object/shapes.rs:801` is the + enforced structural-publication funnel that re-arms a same-address mutation. + Scanner-internal rekeys do not enqueue a duplicate visit. +- `box.rs:954` previously walked every address in `BOX_REGISTRY`. These are + malloc-allocated mutable-capture/async state cells; the registry address is + not a GC pointer. Only the `Box::value` NaN-box can point into the nursery. + `I32Box` and `BoolBox` registries contain no GC edge and were never part of + this scanner. There was no partial box log. +- `box.rs:123` adds the box remembered set. Both allocation arms and both + mutation ABIs arm it before publishing a minor-relevant payload + (`box.rs:133`, `box.rs:705`, `box.rs:725`, `box.rs:1318`, `box.rs:1357`). + The trusted setter is included because generated boxed-local stores use it; + omitting that silent path would violate the enforced-funnel rule. Release + paths only clear/de-register cells, and scanner rewrites compact their own + entries. `box.rs:1040` owns the priced `visited` counter. +- Both minor walks sort/deduplicate their logged addresses, drop stale keys, + and keep only post-visit non-old entries. Full/major scans still enumerate + the authoritative whole tables and rebuild the logs. Under + `debug_assertions` and in lib tests, each minor scan re-derives the relevant + set from the whole table and asserts that the log is complete. +- `gc/copying.rs:1889` now emits `pause_us=` and `scan_us=` together on every + completed `[gc-copy-minor] ran` line. `pause_us` is sampled as the final + action before the copied-minor returns to the mutator; `scan_us` is the + already-profiled scanner total returned by `gc/scanner_profile.rs:131`. + Timing remains behind the existing cached `PERRY_GC_DIAG` gate. + +## Tests and sabotages + +- `shape_table_minor_walk_visits_exactly_k_young_entries`: N old families and + k young families produce `visited == k`. Sabotage: remove + `note_young_keys`; the completeness re-derivation panics. +- `shape_table_rederivation_rejects_a_suppressed_logging_site`: a test-only + suppression skips the production family arm and the scan must panic. +- `shape_mutation_to_new_young_key_rearms_minor_log`: a Longlived carrier that + gains a new nursery key at the same address must move that key. Sabotage: + remove the re-arm in `stamp_object_shape_id_with_carrier_note`. +- `box_roots_minor_walk_visits_exactly_k_young_entries`: N old payloads and k + young payloads produce `visited == k`. Sabotage: remove either allocator arm. +- `box_root_rederivation_rejects_a_suppressed_mutation_hook`: a test-only + suppression skips `js_box_set_bits` logging and the authoritative registry + walk must panic. +- `box_mutation_to_new_young_object_is_visited`: an old box changed to a new + nursery object is visited. Sabotage: remove the setter hook. +- `promoted_shape_entry_leaves_young_log_and_remains_in_major_walk` and + `promoted_box_root_leaves_log_and_is_found_by_full_walk`: promotion makes + `kept == 0`, while the next authoritative full walk still visits the entry. + Sabotage: retain the pre-visit/from-space classification or scope the full + walk to the log. +- Existing scanner-completeness and moving-witness suites are unchanged and + remain part of the requested runtime-lib gate. + +## Validation + +- `scripts/check_file_size.sh`: PASS. +- `git diff --check`: PASS. +- Cargo gates: NOT RUN. `df -g /` immediately before the first possible Cargo + invocation reported `0` GB available, below the binding 12 GB floor. Per the + task rule, no Cargo command was started and no wait for disk was attempted. +- Not run for the same reason: + `cargo test -p perry-runtime --release --lib -- --test-threads=1`; + `cargo build --release -p perry-runtime --features wasm-host`; + `cargo build --release -p perry`. + +## Predictions and exact perrymaster request + +Predictions: on a zero-live steady minor, +`object::shapes::scan_shape_table_rekey_mut` and +`r#box::scan_box_roots_mut` each fall from about 2 ms to at most 0.2 ms; +steady-minor scanner total falls from 7–8 ms to at most 3 ms; every completed +minor reports `pause_us` and `scan_us`. CPU bound is about -3% at 3300 chars +and larger at 400 chars, where minors are a larger share. RSS should be +unchanged (small retained log capacities only, within the allowed 1–10%). + +Perrymaster request, from pushed SHA: relink on the I7-view tree +(runtime-only), then run the three gates through +`/Users/amlug/projects/perry/secret-tests/cc-perf-campaign/measure_lock.sh --build` +at `-j4` using detached `nohup`: (1) +`cargo test -p perry-runtime --release --lib -- --test-threads=1`, (2) +`cargo build --release -p perry-runtime --features wasm-host`, and (3) +`cargo build --release -p perry`. Because this is GC-adjacent, the coordinator +must apply `run-extended-tests`. After green gates, do one graceful four-turn +3300-char run and one 400-char run with `PERRY_GC_DIAG=1`, preserving complete +`[gc-copy-minor] ran pause_us=... scan_us=...` and +`[gc-scanner-profile] copying_minor` lines. Then run paired 5x3300 + 3x400 +against I7-view for CPU and RSS. diff --git a/changelog.d/minor-scanner-young-logs.md b/changelog.d/minor-scanner-young-logs.md new file mode 100644 index 0000000000..cf4834a894 --- /dev/null +++ b/changelog.d/minor-scanner-young-logs.md @@ -0,0 +1,5 @@ +Copying-minor scans of shape descriptors and captured-variable boxes now walk +only entries that can still expose non-old GC pointers. This removes the two +largest table-size-dependent root-scan costs, while full collections retain +their authoritative whole-table walks. `PERRY_GC_DIAG=1` also reports the +whole copying-minor pause and its scanner share on each completed-minor line. diff --git a/crates/perry-runtime/src/box.rs b/crates/perry-runtime/src/box.rs index dc65e4f791..44079b511d 100644 --- a/crates/perry-runtime/src/box.rs +++ b/crates/perry-runtime/src/box.rs @@ -117,6 +117,28 @@ crate::perry_thread_local! { 16 * 1024, crate::fast_hash::PtrHasher, )); + /// Box addresses whose JSValue payload may matter to a minor collection. + /// The registry itself is the authoritative full/major root set; this is + /// only its minor remembered set. + static BOX_YOUNG_ROOTS: std::cell::RefCell> = + const { std::cell::RefCell::new(crate::gc::young_log::YoungLog::new()) }; + #[cfg(test)] + static BOX_YOUNG_LOG_SUPPRESSED: std::cell::Cell = const { std::cell::Cell::new(false) }; +} + +const BOX_YOUNG_LOG_NAME: &str = "box.roots"; + +/// Arm the box minor-root log before publishing a young payload. +#[inline] +fn note_box_young_root(addr: usize, bits: u64) { + if !crate::gc::young_log::bits_are_minor_relevant(bits) { + return; + } + #[cfg(test)] + if BOX_YOUNG_LOG_SUPPRESSED.with(std::cell::Cell::get) { + return; + } + BOX_YOUNG_ROOTS.with(|log| log.borrow_mut().note(addr)); } /// Number of slots in each registry's direct-mapped positive cache. Eight @@ -680,6 +702,7 @@ pub extern "C" fn js_box_alloc_bits(initial_bits: i64) -> *mut Box { unsafe { (*ptr).value = initial_bits as u64; } + note_box_young_root(addr, initial_bits as u64); BOX_REGISTRY.with(|r| { r.borrow_mut().insert(addr); }); @@ -699,6 +722,7 @@ pub extern "C" fn js_box_alloc_bits(initial_bits: i64) -> *mut Box { return std::ptr::null_mut(); } (*ptr).value = initial_bits as u64; + note_box_young_root(ptr as usize, initial_bits as u64); BOX_REGISTRY.with(|r| { r.borrow_mut().insert(ptr as usize); }); @@ -928,7 +952,14 @@ pub fn scan_box_roots(mark: &mut dyn FnMut(f64)) { } pub fn scan_box_roots_mut(visitor: &mut crate::gc::RuntimeRootVisitor<'_>) { + if visitor.young_scope() { + scan_box_young_roots_mut(visitor); + return; + } let full_trace = crate::gc::full_trace_active(); + let mut visited = 0u64; + let table_len = BOX_REGISTRY.with(|registry| registry.borrow().len()) as u64; + let mut kept = Vec::new(); ASYNC_PENDING_RELEASES.with(|pending| { let pending = pending.borrow(); BOX_REGISTRY.with(|r| { @@ -957,11 +988,103 @@ pub fn scan_box_roots_mut(visitor: &mut crate::gc::RuntimeRootVisitor<'_>) { if addr >= 0x1000 && (addr as u64) < 0x0001_0000_0000_0000 && addr % 8 == 0 { unsafe { visitor.visit_nanbox_u64_raw_slot(&raw mut (*ptr).value); + if crate::gc::young_log::bits_are_minor_relevant((*ptr).value) { + kept.push(addr); + } } + visited += 1; } } }); }); + let kept_len = kept.len() as u64; + BOX_YOUNG_ROOTS.with(|log| { + let mut log = log.borrow_mut(); + let _ = log.take_sorted(); + log.extend(kept); + }); + crate::gc::young_log::note_walk( + BOX_YOUNG_LOG_NAME, + crate::gc::young_log::YoungLogWalk { + partial: false, + logged: visited, + visited, + kept: kept_len, + table_len, + }, + ); +} + +/// Every live box whose current payload a minor can move, mark through, or +/// sweep. This is the authoritative debug re-derivation of the remembered set. +fn relevant_box_roots() -> Vec { + let mut relevant = BOX_REGISTRY.with(|registry| { + registry + .borrow() + .iter() + .copied() + .filter(|&addr| { + let ptr = addr as *mut Box; + is_plausible_box_ptr(ptr) + && unsafe { crate::gc::young_log::bits_are_minor_relevant((*ptr).value) } + }) + .collect::>() + }); + relevant.sort_unstable(); + relevant +} + +/// Minor root scan: price only the logged boxes, and compact the log from the +/// post-visit payloads. The visit counter lives here because this is the work +/// whose fixed cost the counter measures. +fn scan_box_young_roots_mut(visitor: &mut crate::gc::RuntimeRootVisitor<'_>) { + let table_len = BOX_REGISTRY.with(|registry| registry.borrow().len()) as u64; + #[cfg(any(debug_assertions, test))] + BOX_YOUNG_ROOTS.with(|log| { + let relevant = relevant_box_roots(); + log.borrow() + .debug_assert_logged(BOX_YOUNG_LOG_NAME, &relevant); + }); + + let mut logged = 0u64; + let mut visited = 0u64; + let mut kept = BOX_YOUNG_ROOTS.with(|log| log.borrow_mut().take_spare()); + loop { + let batch = BOX_YOUNG_ROOTS.with(|log| log.borrow_mut().take_sorted()); + if batch.is_empty() { + break; + } + logged += batch.len() as u64; + for addr in batch { + let registered = BOX_REGISTRY.with(|registry| registry.borrow().contains(&addr)); + if !registered { + continue; + } + let ptr = addr as *mut Box; + if !is_plausible_box_ptr(ptr) { + continue; + } + visited += 1; + unsafe { + visitor.visit_nanbox_u64_raw_slot(&raw mut (*ptr).value); + if crate::gc::young_log::bits_are_minor_relevant((*ptr).value) { + kept.push(addr); + } + } + } + } + let kept_len = kept.len() as u64; + BOX_YOUNG_ROOTS.with(|log| log.borrow_mut().extend(kept)); + crate::gc::young_log::note_walk( + BOX_YOUNG_LOG_NAME, + crate::gc::young_log::YoungLogWalk { + partial: true, + logged, + visited, + kept: kept_len, + table_len, + }, + ); } /// Get the raw JSValue bit pattern from a box. @@ -1213,6 +1336,7 @@ pub extern "C" fn js_box_set_bits(ptr: *mut Box, value_bits: i64) { return; } let bits = value_bits as u64; + note_box_young_root(ptr as usize, bits); (*ptr).value = bits; crate::gc::runtime_write_barrier_root_nanbox(bits); } @@ -1232,6 +1356,7 @@ pub extern "C" fn js_box_set_bits(ptr: *mut Box, value_bits: i64) { #[no_mangle] pub unsafe extern "C" fn js_box_set_bits_trusted_no_barrier(ptr: *mut Box, value_bits: i64) { unsafe { + note_box_young_root(ptr as usize, value_bits as u64); (*ptr).value = value_bits as u64; } } @@ -1479,6 +1604,7 @@ pub(crate) fn test_clear_box_registry() { BOX_REGISTRY.with(|r| r.borrow_mut().clear()); I32_BOX_REGISTRY.with(|r| r.borrow_mut().clear()); BOOL_BOX_REGISTRY.with(|r| r.borrow_mut().clear()); + BOX_YOUNG_ROOTS.with(|log| log.borrow_mut().clear()); BOX_FREE_HEAD.with(|h| h.set(0)); I32_BOX_FREE_HEAD.with(|h| h.set(0)); BOOL_BOX_FREE_HEAD.with(|h| h.set(0)); @@ -1501,6 +1627,25 @@ pub(crate) fn test_clear_box_registry() { } } +/// Test-only sabotage of the box write-side arming hook. The production +/// scanner's re-derivation must reject the missing log entry. +#[cfg(test)] +pub(crate) struct TestBoxYoungLogSuppression(bool); + +#[cfg(test)] +impl TestBoxYoungLogSuppression { + pub(crate) fn new() -> Self { + Self(BOX_YOUNG_LOG_SUPPRESSED.with(|cell| cell.replace(true))) + } +} + +#[cfg(test)] +impl Drop for TestBoxYoungLogSuppression { + fn drop(&mut self) { + BOX_YOUNG_LOG_SUPPRESSED.with(|cell| cell.set(self.0)); + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/crates/perry-runtime/src/closure/dynamic_props.rs b/crates/perry-runtime/src/closure/dynamic_props.rs index d3ea542188..06e760bd7c 100644 --- a/crates/perry-runtime/src/closure/dynamic_props.rs +++ b/crates/perry-runtime/src/closure/dynamic_props.rs @@ -104,6 +104,9 @@ crate::perry_thread_local! { /// thread's minors can move or free them. See `gc/young_log.rs`. static CLOSURE_YOUNG_OWNERS: std::cell::RefCell> = const { std::cell::RefCell::new(crate::gc::young_log::YoungLog::new()) }; + #[cfg(test)] + static TEST_SUPPRESS_CLOSURE_YOUNG_NOTE: std::cell::Cell = + const { std::cell::Cell::new(false) }; } const CLOSURE_YOUNG_LOG_NAME: &str = "closure.dynamic_props"; @@ -112,13 +115,38 @@ const CLOSURE_YOUNG_LOG_NAME: &str = "closure.dynamic_props"; /// when the owner or the value being stored can matter to a minor. #[inline] fn note_young_closure_owner(owner: usize, value_bits: u64) { - if crate::gc::young_log::addr_is_minor_relevant(owner) + if crate::gc::young_log::addr_is_minor_collectible(owner) || crate::gc::young_log::bits_are_minor_relevant(value_bits) { + #[cfg(test)] + if TEST_SUPPRESS_CLOSURE_YOUNG_NOTE.with(std::cell::Cell::get) { + return; + } CLOSURE_YOUNG_OWNERS.with(|log| log.borrow_mut().note(owner)); } } +#[cfg(test)] +mod young_log_sabotage_tests { + use super::*; + + #[test] + fn closure_log_rederivation_rejects_a_suppressed_setter() { + let _lock = crate::gc::global_side_table_test_lock(); + test_clear_closure_side_tables(); + let owner = crate::closure::js_closure_alloc(std::ptr::null(), 0) as usize; + TEST_SUPPRESS_CLOSURE_YOUNG_NOTE.with(|flag| flag.set(true)); + closure_set_dynamic_prop(owner, "sabotage", 7.0); + TEST_SUPPRESS_CLOSURE_YOUNG_NOTE.with(|flag| flag.set(false)); + let missed = std::panic::catch_unwind(debug_assert_closure_young_log_complete); + test_clear_closure_side_tables(); + assert!( + missed.is_err(), + "sabotage: suppressing closure_set_dynamic_prop's note must trip completeness" + ); + } +} + /// A re-keyed entry keeps whatever values it had, so the new owner is logged /// unconditionally; the next minor-scoped walk drops it if nothing in it is /// relevant any more. @@ -515,7 +543,7 @@ fn scan_closure_side_tables_young(visitor: &mut crate::gc::RuntimeRootVisitor<'_ .unwrap_or(0); (props + prototypes + deleted) as u64 }; - #[cfg(debug_assertions)] + #[cfg(any(debug_assertions, test))] debug_assert_closure_young_log_complete(); let mut logged = 0u64; let mut visited = 0u64; @@ -550,13 +578,13 @@ fn scan_closure_side_tables_young(visitor: &mut crate::gc::RuntimeRootVisitor<'_ /// Rule 2 of `gc/young_log.rs`: re-derive the relevant owners from the three /// tables and require the log to name each one. -#[cfg(debug_assertions)] +#[cfg(any(debug_assertions, test))] fn debug_assert_closure_young_log_complete() { - use crate::gc::young_log::{addr_is_minor_relevant, bits_are_minor_relevant}; + use crate::gc::young_log::{addr_is_minor_collectible, bits_are_minor_relevant}; let mut relevant = Vec::new(); if let Ok(props) = get_closure_props().lock() { for (&owner, entry) in props.iter() { - if addr_is_minor_relevant(owner) + if addr_is_minor_collectible(owner) || entry .values .values() @@ -568,14 +596,14 @@ fn debug_assert_closure_young_log_complete() { } if let Ok(prototypes) = get_closure_prototypes().lock() { for (&owner, &proto_bits) in prototypes.iter() { - if addr_is_minor_relevant(owner) || bits_are_minor_relevant(proto_bits) { + if addr_is_minor_collectible(owner) || bits_are_minor_relevant(proto_bits) { relevant.push(owner); } } } if let Ok(deleted) = get_closure_deleted_keys().lock() { for &owner in deleted.keys() { - if addr_is_minor_relevant(owner) { + if addr_is_minor_collectible(owner) { relevant.push(owner); } } @@ -593,7 +621,7 @@ fn scan_closure_owner( visitor: &mut crate::gc::RuntimeRootVisitor<'_>, owner: usize, ) -> (usize, bool) { - use crate::gc::young_log::{addr_is_minor_relevant, bits_are_minor_relevant}; + use crate::gc::young_log::{addr_is_minor_collectible, bits_are_minor_relevant}; let mut relevant = false; let mut current_owner = owner; @@ -657,7 +685,7 @@ fn scan_closure_owner( } } - relevant |= addr_is_minor_relevant(current_owner); + relevant |= addr_is_minor_collectible(current_owner); (current_owner, relevant) } diff --git a/crates/perry-runtime/src/gc/copying.rs b/crates/perry-runtime/src/gc/copying.rs index 2eb134d64f..0e6d48a23a 100644 --- a/crates/perry-runtime/src/gc/copying.rs +++ b/crates/perry-runtime/src/gc/copying.rs @@ -1,3 +1,7 @@ +use super::copying_phase::{ + finalize_dead_copied_minor_from_space_side_allocations, CopyingMinorPhase as Phase, + CopyingMinorPhaseDiag as PhaseDiag, +}; use super::*; /// Largest object `move_young` will relocate. See its use site for the @@ -1213,6 +1217,7 @@ pub(super) fn run_copied_minor_attempt( let ptrs = eligibility .ptrs .expect("eligible copied-minor decision must carry pointer classifier"); + let mut phase_diag = PhaseDiag::enabled(); let phase_start = trace_phase_start(trace); let from_space_bytes = crate::arena::copying_from_space_in_use_bytes(); @@ -1236,11 +1241,15 @@ pub(super) fn run_copied_minor_attempt( && ptrs.malloc_registry_empty_at_start && untraced_promotion_instrument_veto().is_none() && super::should_attempt_first_cycle_promotion(); + let promotion_phase_start = PhaseDiag::start(&phase_diag); let promotion = if super::should_promote_young_in_place() || speculate_first_cycle { crate::arena::retag_young_for_in_place_promotion(speculate_first_cycle) } else { crate::arena::InPlacePromotion::default() }; + if let Some(diag) = phase_diag.as_mut() { + diag.record(Phase::Promotion, promotion_phase_start); + } // An empty plan (nothing in use to promote) falls back to the ordinary // path, so the from-space reset still runs. let promoting_in_place = !promotion.is_empty(); @@ -1319,8 +1328,13 @@ pub(super) fn run_copied_minor_attempt( "policy (should_promote_young_untraced)" }) }); + let reset_phase_start = PhaseDiag::start(&phase_diag); collector.stats.reset_blocks += crate::arena::copying_prepare_to_space(); + if let Some(diag) = phase_diag.as_mut() { + diag.record(Phase::BlockResetFlip, reset_phase_start); + } + let root_scan_phase_start = PhaseDiag::start(&phase_diag); let native_stack_walk = if untraced { Default::default() } else { @@ -1399,6 +1413,9 @@ pub(super) fn run_copied_minor_attempt( } visit_ffi_mutable_registered_roots_with_sources(&mut visitor, root_sources); } + if let Some(diag) = phase_diag.as_mut() { + diag.record(Phase::RootScan, root_scan_phase_start); + } // On an untraced promotion the dirty SCAN is where the whole per-object // mark pass lived: `retain`'s array store has a young child in every page @@ -1411,6 +1428,7 @@ pub(super) fn run_copied_minor_attempt( // read path for the remembered set, which is where #7187's lazy barrier // arming happens. Skipping it would leave the barrier unarmed for the next // cycle — a missing-edge bug one collection later. + let remembered_phase_start = PhaseDiag::start(&phase_diag); let snapshot = remembered_dirty_snapshot(); // #9754: objects whose every slot the dirty scan visited in-body — the // post-cycle coverage restore skips them (see `scan_dirty_object_slots`). @@ -1428,6 +1446,8 @@ pub(super) fn run_copied_minor_attempt( // reserved bytes, and under-estimating falls back to ordinary growth. let mut dirty_scan_covered = crate::fast_hash::new_ptr_hash_set_with_capacity(previous_dirty_covered_estimate()); + let mut remembered_entries = 0usize; + let mut remembered_slots = 0usize; if !untraced { let _phase = super::pin::CopyingWalkPhaseGuard::enter("remembered_set"); let remembered_stats = scan_remembered_dirty_slots_copying( @@ -1447,11 +1467,21 @@ pub(super) fn run_copied_minor_attempt( if let Some(trace) = trace.as_mut() { trace.remembered_set = remembered_stats; } + remembered_entries = remembered_stats.entries_scanned; + remembered_slots = remembered_stats.dirty_slots_scanned; + } + if let Some(diag) = phase_diag.as_mut() { + diag.record(Phase::RememberedSetYoungLogs, remembered_phase_start); } + let copy_phase_start = PhaseDiag::start(&phase_diag); unsafe { let _phase = super::pin::CopyingWalkPhaseGuard::enter("worklist_drain"); collector.drain(); } + if let Some(diag) = phase_diag.as_mut() { + diag.record(Phase::CopyEvacuation, copy_phase_start); + } + let rewrite_root_scan_phase_start = PhaseDiag::start(&phase_diag); { let scanners: Vec = if untraced { Vec::new() @@ -1483,6 +1513,9 @@ pub(super) fn run_copied_minor_attempt( } visit_ffi_mutable_registered_roots_with_sources(&mut visitor, root_sources); } + if let Some(diag) = phase_diag.as_mut() { + diag.record(Phase::RootScan, rewrite_root_scan_phase_start); + } // #7803 THE FIX: rebuild the promoted-object remembered set AFTER the last // phase that can move an object, not before the drain. // @@ -1507,6 +1540,7 @@ pub(super) fn run_copied_minor_attempt( // the rebuild performs is exact rather than a from-space // over-approximation. Headers still carry GC_FLAG_MARKED (clear_marks // runs later), which the per-object gate requires. + let forwarding_phase_start = PhaseDiag::start(&phase_diag); if !collector.skip_remembering { let promoted_sticky = rebuild_evacuated_old_to_young_remembered_set(&collector.moved_headers); @@ -1528,6 +1562,9 @@ pub(super) fn run_copied_minor_attempt( super::roots::stack_maps_native_slot_verify(untraced, &|addr| { format!("{:?}", collector.ptrs.classify(addr).map(|ptr| ptr.kind)) }); + if let Some(diag) = phase_diag.as_mut() { + diag.record(Phase::ForwardingFixups, forwarding_phase_start); + } trace_phase_record(trace, "copying_nursery", phase_start); // #7937: the attempt's own trace has finished, so the ratio it was missing @@ -1630,8 +1667,16 @@ pub(super) fn run_copied_minor_attempt( // run here, same window as fromspace_scan (after rewrite, before reset). super::native_stack_scan::run_native_stack_scan(); - crate::promise::cleanup_copied_minor_promise_contexts_for_gc(); - finalize_dead_copied_minor_from_space_side_allocations(); + let finalization = finalize_dead_copied_minor_from_space_side_allocations(); + if let Some(diag) = phase_diag.as_mut() { + diag.add_nanos(Phase::DeadOwnerSideTablePruning, finalization.dead_owner_ns); + diag.add_nanos( + Phase::FromSpaceFinalization, + finalization + .total_ns + .saturating_sub(finalization.dead_owner_ns), + ); + } // #7742: on a promoting cycle the young blocks are handed to old-gen // instead of being reset. This MUST stay before `clear_marks` — the finish // walk reads `GC_FLAG_MARKED` to decide which objects to index — and it @@ -1639,6 +1684,7 @@ pub(super) fn run_copied_minor_attempt( // blocks the reset would recycle are the blocks this keeps. let (reset, promotion_stats) = if promoting_in_place { let phase_start = trace_phase_start(trace); + let promotion_phase_start = PhaseDiag::start(&phase_diag); super::note_promoted_young_capacity(promotion.reserved_bytes()); let promotion_stats = crate::arena::finish_in_place_promotion( promotion, @@ -1648,6 +1694,9 @@ pub(super) fn run_copied_minor_attempt( crate::arena::PromotionLiveness::Marked }, ); + if let Some(diag) = phase_diag.as_mut() { + diag.record(Phase::Promotion, promotion_phase_start); + } trace_phase_record(trace, "in_place_promotion", phase_start); ( crate::arena::ArenaResetStats { @@ -1658,10 +1707,12 @@ pub(super) fn run_copied_minor_attempt( promotion_stats, ) } else { - ( - crate::arena::copying_reset_from_spaces_and_flip(), - crate::arena::InPlacePromotionStats::default(), - ) + let reset_phase_start = PhaseDiag::start(&phase_diag); + let reset = crate::arena::copying_reset_from_spaces_and_flip(); + if let Some(diag) = phase_diag.as_mut() { + diag.record(Phase::BlockResetFlip, reset_phase_start); + } + (reset, crate::arena::InPlacePromotionStats::default()) }; collector.stats.reset_blocks += reset.reset_blocks; if untraced { @@ -1686,6 +1737,7 @@ pub(super) fn run_copied_minor_attempt( if let Some(trace) = trace.as_mut() { trace.old_pages = crate::arena::old_page_summary(); } + let remembered_restore_phase_start = PhaseDiag::start(&phase_diag); remembered_set_clear(); collector.sticky.restore(); if !collector.skip_remembering { @@ -1694,6 +1746,12 @@ pub(super) fn run_copied_minor_attempt( // the last line before the kill is the answer. crate::arena::page_class_table_report(); } + if let Some(diag) = phase_diag.as_mut() { + diag.record( + Phase::RememberedSetYoungLogs, + remembered_restore_phase_start, + ); + } // The mechanism, counted rather than assumed: with the pre-size working, // `capacity` is already >= `len` on entry and hashbrown never grows the // table, so `reserve_rehash` disappears from this path. A capacity that @@ -1872,9 +1930,39 @@ pub(super) fn run_copied_minor_attempt( collector.stats.copied_bytes, collector.stats.survivor_live_bytes, ); + if let Some(d) = collector.survival.as_ref() { + d.report(super::survival_diag::next_minor_seq()); + } + crate::arena::alloc_sample::report("minor"); + super::diag_sites::report_primitive_dispatch("minor"); + crate::object::shapes::id_list_report(); + report_forwarding_refusals("copying_minor"); + let scan_us = super::scanner_profile::report_and_reset("copying_minor"); if crate::gc::gc_diag_enabled() { + // This is intentionally the last diagnostic action before returning to + // the mutator: `pause_us` prices the whole copied-minor path, including + // finalization, pruning, policy feedback and the diagnostic work above. + let pause_ns = start.elapsed().as_nanos() as u64; + let pause_us = pause_ns / 1000; + let phases = phase_diag + .as_ref() + .expect("PERRY_GC_DIAG phase accounting must be enabled") + .render( + pause_ns, + scan_us, + collector.stats.copied_objects, + collector.stats.copied_bytes, + collector.stats.promoted_objects, + collector.stats.promoted_bytes, + remembered_entries, + remembered_slots, + &finalization, + ); eprintln!( - "[gc-copy-minor] ran in_place={} untraced={} untraced_cycles={} untraced_objects={} in_place_blocks={} in_place_dead_bytes={} sparse_blocks={} survival_permille={} copied_objects={} copied_bytes={} promoted_objects={} promoted_bytes={} freed_bytes={} tenuring_survivals={} eden_live_bytes={} trigger={:?} declared_safepoint={}", + "[gc-copy-minor] ran pause_us={} scan_us={} phases: {} in_place={} untraced={} untraced_cycles={} untraced_objects={} in_place_blocks={} in_place_dead_bytes={} sparse_blocks={} survival_permille={} copied_objects={} copied_bytes={} promoted_objects={} promoted_bytes={} freed_bytes={} tenuring_survivals={} eden_live_bytes={} trigger={:?} declared_safepoint={}", + pause_us, + scan_us, + phases, collector.stats.in_place_promotion, untraced, super::untraced_promotion_cycles(), @@ -1894,27 +1982,8 @@ pub(super) fn run_copied_minor_attempt( super::policy::GC_AT_DECLARED_SAFEPOINT.with(std::cell::Cell::get) ); } - if let Some(d) = collector.survival.as_ref() { - d.report(super::survival_diag::next_minor_seq()); - } - crate::arena::alloc_sample::report("minor"); - super::diag_sites::report_primitive_dispatch("minor"); - crate::object::shapes::id_list_report(); - report_forwarding_refusals("copying_minor"); - super::scanner_profile::report_and_reset("copying_minor"); CopiedMinorAttempt::Done(Some(CopiedMinorFastPathOutcome { freed_bytes, malloc_swept: malloc_sweep_due, })) } - -fn finalize_dead_copied_minor_from_space_side_allocations() { - crate::map::finalize_dead_copied_minor_from_space_maps(); - crate::set::finalize_dead_copied_minor_from_space_sets(); - crate::node_submodules::diagnostics_gc::finalize_dead_copied_minor_from_space_errors(); - crate::regex::finalize_dead_copied_minor_from_space_regexps(); - // 2026-07-09 GC audit wave 2: the from-space flip runs no per-object - // finalize hooks, so entries keyed by dead from-space owners in the - // object-address-keyed side tables are pruned here (headers still intact). - super::dead_owner::prune_dead_owner_side_tables_copied_minor(); -} diff --git a/crates/perry-runtime/src/gc/copying_phase.rs b/crates/perry-runtime/src/gc/copying_phase.rs new file mode 100644 index 0000000000..5c693f7005 --- /dev/null +++ b/crates/perry-runtime/src/gc/copying_phase.rs @@ -0,0 +1,204 @@ +//! Diagnostic-only phase accounting for the copying minor. +//! +//! The collector's phases are not all contiguous: registered roots are walked +//! once to evacuate and once to repair forwarding addresses, and in-place +//! promotion has an early retag plus a late finish. The accumulator therefore +//! records non-overlapping spans into semantic buckets. Anything outside a +//! priced span is reported as `other`; top-level buckets plus `other` are an +//! exact partition of the same `Instant` interval used for `pause_us`. + +use std::fmt::Write; +use std::time::Instant; + +#[derive(Clone, Copy)] +pub(super) enum CopyingMinorPhase { + RootScan, + CopyEvacuation, + RememberedSetYoungLogs, + Promotion, + DeadOwnerSideTablePruning, + FromSpaceFinalization, + ForwardingFixups, + BlockResetFlip, +} + +#[derive(Default)] +pub(super) struct CopyingMinorPhaseDiag { + root_scan_ns: u64, + copy_evacuation_ns: u64, + remembered_set_young_logs_ns: u64, + promotion_ns: u64, + dead_owner_side_table_pruning_ns: u64, + from_space_finalization_ns: u64, + forwarding_fixups_ns: u64, + block_reset_flip_ns: u64, +} + +impl CopyingMinorPhaseDiag { + #[inline] + pub(super) fn enabled() -> Option { + super::gc_diag_enabled().then(Self::default) + } + + #[inline] + pub(super) fn start(diag: &Option) -> Option { + diag.as_ref().map(|_| Instant::now()) + } + + #[inline] + pub(super) fn record(&mut self, phase: CopyingMinorPhase, start: Option) { + let Some(start) = start else { + return; + }; + self.add_nanos(phase, start.elapsed().as_nanos() as u64); + } + + #[inline] + pub(super) fn add_nanos(&mut self, phase: CopyingMinorPhase, nanos: u64) { + let slot = match phase { + CopyingMinorPhase::RootScan => &mut self.root_scan_ns, + CopyingMinorPhase::CopyEvacuation => &mut self.copy_evacuation_ns, + CopyingMinorPhase::RememberedSetYoungLogs => &mut self.remembered_set_young_logs_ns, + CopyingMinorPhase::Promotion => &mut self.promotion_ns, + CopyingMinorPhase::DeadOwnerSideTablePruning => { + &mut self.dead_owner_side_table_pruning_ns + } + CopyingMinorPhase::FromSpaceFinalization => &mut self.from_space_finalization_ns, + CopyingMinorPhase::ForwardingFixups => &mut self.forwarding_fixups_ns, + CopyingMinorPhase::BlockResetFlip => &mut self.block_reset_flip_ns, + }; + *slot = slot.saturating_add(nanos); + } + + fn named_nanos(&self) -> u64 { + self.root_scan_ns + .saturating_add(self.copy_evacuation_ns) + .saturating_add(self.remembered_set_young_logs_ns) + .saturating_add(self.promotion_ns) + .saturating_add(self.dead_owner_side_table_pruning_ns) + .saturating_add(self.from_space_finalization_ns) + .saturating_add(self.forwarding_fixups_ns) + .saturating_add(self.block_reset_flip_ns) + } + + pub(super) fn render( + &self, + pause_ns: u64, + scan_us: u64, + copied_objects: usize, + copied_bytes: usize, + promoted_objects: usize, + promoted_bytes: usize, + remembered_entries: usize, + remembered_slots: usize, + finalization: &CopiedMinorFinalizationDiag, + ) -> String { + let named_ns = self.named_nanos(); + let other_ns = pause_ns.saturating_sub(named_ns); + let phase_sum_ns = named_ns.saturating_add(other_ns); + let mut out = String::new(); + write!( + out, + "root_scan={}/{} copy_evacuation={}/{}/{} remembered_set_young_logs={}/{}/{} promotion={}/{}/{} dead_owner_side_table_pruning={}{} from_space_finalization={}/map:{}/{}+set:{}/{}+errors:{}/{}+regex:{}/{} forwarding_fixups={} block_reset_flip={} other={} phase_sum_us={}", + self.root_scan_ns / 1000, + scan_us, + self.copy_evacuation_ns / 1000, + copied_objects, + copied_bytes, + self.remembered_set_young_logs_ns / 1000, + remembered_entries, + remembered_slots, + self.promotion_ns / 1000, + promoted_objects, + promoted_bytes, + self.dead_owner_side_table_pruning_ns / 1000, + finalization.dead_owner_detail, + self.from_space_finalization_ns / 1000, + finalization.map_ns / 1000, + finalization.maps, + finalization.set_ns / 1000, + finalization.sets, + finalization.errors_ns / 1000, + finalization.errors, + finalization.regex_ns / 1000, + finalization.regexps, + self.forwarding_fixups_ns / 1000, + self.block_reset_flip_ns / 1000, + other_ns / 1000, + phase_sum_ns / 1000, + ) + .expect("writing phase diagnostics to a String cannot fail"); + out + } +} + +#[derive(Default)] +pub(super) struct CopiedMinorFinalizationDiag { + pub(super) total_ns: u64, + pub(super) map_ns: u64, + pub(super) maps: usize, + pub(super) set_ns: u64, + pub(super) sets: usize, + pub(super) errors_ns: u64, + pub(super) errors: usize, + pub(super) regex_ns: u64, + pub(super) regexps: usize, + pub(super) dead_owner_ns: u64, + pub(super) dead_owner_detail: String, +} + +/// Finalize the side allocations whose from-space owners just died. The +/// clocks live here, beside the calls they price; without `PERRY_GC_DIAG` this +/// performs the original calls without reading the clock or building strings. +pub(super) fn finalize_dead_copied_minor_from_space_side_allocations() -> CopiedMinorFinalizationDiag +{ + let diag = super::gc_diag_enabled(); + let total_start = diag.then(Instant::now); + let mut out = CopiedMinorFinalizationDiag::default(); + + crate::promise::cleanup_copied_minor_promise_contexts_for_gc(); + + let start = diag.then(Instant::now); + out.maps = crate::map::finalize_dead_copied_minor_from_space_maps(); + out.map_ns = start.map_or(0, |start| start.elapsed().as_nanos() as u64); + + let start = diag.then(Instant::now); + out.sets = crate::set::finalize_dead_copied_minor_from_space_sets(); + out.set_ns = start.map_or(0, |start| start.elapsed().as_nanos() as u64); + + let start = diag.then(Instant::now); + out.errors = + crate::node_submodules::diagnostics_gc::finalize_dead_copied_minor_from_space_errors(); + out.errors_ns = start.map_or(0, |start| start.elapsed().as_nanos() as u64); + + let start = diag.then(Instant::now); + out.regexps = crate::regex::finalize_dead_copied_minor_from_space_regexps(); + out.regex_ns = start.map_or(0, |start| start.elapsed().as_nanos() as u64); + + let start = diag.then(Instant::now); + out.dead_owner_detail = super::dead_owner::prune_dead_owner_side_tables_copied_minor(); + out.dead_owner_ns = start.map_or(0, |start| start.elapsed().as_nanos() as u64); + out.total_ns = total_start.map_or(0, |start| start.elapsed().as_nanos() as u64); + out +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn copied_minor_phase_residual_makes_the_partition_exact() { + let mut diag = CopyingMinorPhaseDiag::default(); + diag.add_nanos(CopyingMinorPhase::RootScan, 11_000); + diag.add_nanos(CopyingMinorPhase::CopyEvacuation, 7_000); + diag.add_nanos(CopyingMinorPhase::BlockResetFlip, 3_000); + + let pause_ns: u64 = 29_000; + let other_ns = pause_ns.saturating_sub(diag.named_nanos()); + assert_eq!(diag.named_nanos() + other_ns, pause_ns); + assert_eq!(other_ns, 8_000); + // Sabotage: remove one named bucket from `named_nanos`; this exact + // residual assertion changes and the test fails rather than merely + // checking that phase reporting did not panic. + } +} diff --git a/crates/perry-runtime/src/gc/dead_owner.rs b/crates/perry-runtime/src/gc/dead_owner.rs index 33e2884256..2eeff26bbe 100644 --- a/crates/perry-runtime/src/gc/dead_owner.rs +++ b/crates/perry-runtime/src/gc/dead_owner.rs @@ -241,6 +241,7 @@ pub(super) fn prune_dead_owner_side_tables_post_trace( &|addr| probe.owner_is_dead(addr, Some(GC_TYPE_CLOSURE)), &|addr| probe.owner_is_dead(addr, Some(GC_TYPE_STRING)), /* young_only = */ !full_trace, + None, ); // #6182: drop dead weak-target HOLDERS (WeakRef / FinalizationRegistry / // WeakMap-WeakSet entry — all GC_TYPE_OBJECT) from the registry so the @@ -257,13 +258,23 @@ pub(super) fn prune_dead_owner_side_tables_post_trace( /// Copied-minor fan-out: prune entries owned by dead from-space objects /// before the flip destroys their headers. Nursery-only by construction, so /// the tenured/malloc caveat cannot mis-fire here. -pub(super) fn prune_dead_owner_side_tables_copied_minor() { +pub(super) fn prune_dead_owner_side_tables_copied_minor() -> String { + let mut detail = String::new(); + let diag = super::gc_diag_enabled(); + if diag { + detail.push('['); + } fan_out( &|addr| owner_is_dead_copied_minor_from_space(addr, None), &|addr| owner_is_dead_copied_minor_from_space(addr, Some(GC_TYPE_CLOSURE)), &|addr| owner_is_dead_copied_minor_from_space(addr, Some(GC_TYPE_STRING)), /* young_only = */ true, + diag.then_some(&mut detail), ); + if diag { + detail.push(']'); + } + detail } /// Which of the pass's three deadness predicates a registered prune is handed. @@ -502,6 +513,7 @@ fn fan_out( is_dead_closure: &dyn Fn(usize) -> bool, is_dead_symbol: &dyn Fn(usize) -> bool, young_only: bool, + mut diag: Option<&mut String>, ) { // Interned key pointers cached in the store-plan cache may die in this // collection — flush every cached verdict. Pointer identity only: the @@ -515,9 +527,15 @@ fn fan_out( DeadKeyOwner::Closure => is_dead_closure, DeadKeyOwner::Symbol => is_dead_symbol, }; + let start = diag.as_ref().map(|_| std::time::Instant::now()); match entry.young_prune { Some(young_prune) if young_only => young_prune(is_dead), _ => (entry.prune)(is_dead), } + if let (Some(detail), Some(start)) = (diag.as_deref_mut(), start) { + use std::fmt::Write; + write!(detail, " {:?}:{}", entry.table, start.elapsed().as_micros()) + .expect("writing dead-owner diagnostics to a String cannot fail"); + } } } diff --git a/crates/perry-runtime/src/gc/mod.rs b/crates/perry-runtime/src/gc/mod.rs index 64409430cb..47f60769ee 100644 --- a/crates/perry-runtime/src/gc/mod.rs +++ b/crates/perry-runtime/src/gc/mod.rs @@ -157,6 +157,7 @@ mod prefetch; mod copying; mod copying_first_cycle; +mod copying_phase; mod copying_pointer_set; mod diag_sites; pub(crate) use diag_sites::primitive_dispatch as diag_primitive_dispatch; diff --git a/crates/perry-runtime/src/gc/scanner_profile.rs b/crates/perry-runtime/src/gc/scanner_profile.rs index 2e95310269..b86871070f 100644 --- a/crates/perry-runtime/src/gc/scanner_profile.rs +++ b/crates/perry-runtime/src/gc/scanner_profile.rs @@ -128,14 +128,14 @@ pub(super) fn note_scanner( /// Print the per-scanner breakdown accumulated since the last report, then /// clear it. Called once per copied minor from the `[gc-copy-minor]` diag site. -pub(super) fn report_and_reset(cycle_label: &str) { +pub(super) fn report_and_reset(cycle_label: &str) -> u64 { if !scanner_profile_enabled() { - return; + return 0; } super::young_log::report_and_reset(cycle_label); let mut rows = SCANNER_PROFILE.with(|rows| std::mem::take(&mut *rows.borrow_mut())); if rows.is_empty() { - return; + return 0; } rows.sort_by(|a, b| b.1.nanos.cmp(&a.1.nanos)); let total_ns: u64 = rows.iter().map(|(_, row)| row.nanos).sum(); @@ -159,4 +159,5 @@ pub(super) fn report_and_reset(cycle_label: &str) { row.rewrites ); } + total_ns / 1000 } diff --git a/crates/perry-runtime/src/gc/tests/young_log_tests.rs b/crates/perry-runtime/src/gc/tests/young_log_tests.rs index e6cf25887d..53eaa899f3 100644 --- a/crates/perry-runtime/src/gc/tests/young_log_tests.rs +++ b/crates/perry-runtime/src/gc/tests/young_log_tests.rs @@ -38,6 +38,10 @@ fn old_closure() -> usize { ptr as usize } +fn old_leaf() -> usize { + crate::arena::arena_alloc_gc_old(32, 8, GC_TYPE_STRING) as usize +} + unsafe fn young_keys_array() -> *mut crate::array::ArrayHeader { let arr = crate::arena::arena_alloc_gc( std::mem::size_of::(), @@ -602,3 +606,241 @@ fn installing_an_external_shape_id_arms_the_family_log() { "the family must have followed the keys array" ); } + +// ------------------------------------------------------ fixed-cost scanners + +/// N old shape families plus k young ones must price exactly k entries in the +/// minor-scoped scanner. Sabotage: make `note_young_keys` a no-op; the +/// re-derivation fails before this count can be observed. +#[test] +fn shape_table_minor_walk_visits_exactly_k_young_entries() { + const N: usize = 96; + const K: usize = 3; + let _guard = CopyingNurseryTestGuard::new(K as u32); + gc_register_mutable_root_scanner(crate::object::shapes::scan_shape_table_rekey_mut); + crate::object::shapes::test_clear_shape_table(); + + for _ in 0..N { + let keys = crate::arena::arena_alloc_gc_old( + std::mem::size_of::(), + std::mem::align_of::(), + GC_TYPE_ARRAY, + ) as *mut crate::array::ArrayHeader; + unsafe { + (*keys).length = 0; + (*keys).capacity = 0; + } + crate::object::shapes::shape_descriptor_ensure(keys, 0, 0).expect("old shape"); + } + for slot in 0..K { + let keys = unsafe { young_keys_array() }; + js_shadow_slot_set(slot as u32, ptr_bits(keys as usize)); + crate::object::shapes::shape_descriptor_ensure(keys, 0, 0).expect("young shape"); + } + + let _ = gc_collect_minor(); + let row = walk("shapes.families+indices"); + assert!(row.partial, "{row:?}"); + assert_eq!( + row.visited, K as u64, + "minor work must be young-sized: {row:?}" + ); + assert!( + row.table_len >= (N + K) as u64, + "fixture did not build N+k: {row:?}" + ); +} + +/// The debug/test authoritative walk is the proof that every shape writer +/// arms the log. This deliberately suppresses the production family funnel; +/// deleting the assertion makes the sabotage go green. +#[test] +fn shape_table_rederivation_rejects_a_suppressed_logging_site() { + let _guard = CopyingNurseryTestGuard::new(0); + crate::object::shapes::test_clear_shape_table(); + let keys = unsafe { young_keys_array() }; + { + let _sabotage = crate::object::shapes::TestShapeYoungLogSuppression::new(); + crate::object::shapes::shape_descriptor_ensure(keys, 0, 0).expect("shape"); + } + let valid = build_valid_pointer_set(); + let mut visitor = RuntimeRootVisitor::for_mark_scoped(&valid, true); + let rejected = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { + crate::object::shapes::scan_shape_table_rekey_mut(&mut visitor); + })); + assert!( + rejected.is_err(), + "a missing shape log note must be detected" + ); +} + +/// Promotion removes a shape address from the minor log, without removing the +/// descriptor from the authoritative table used by the next full/major walk. +/// Sabotage: change the post-visit keep predicate back to +/// `addr_is_minor_relevant(from_space)`; `kept` never reaches zero. +#[test] +fn promoted_shape_entry_leaves_young_log_and_remains_in_major_walk() { + let _guard = CopyingNurseryTestGuard::new(1); + gc_register_mutable_root_scanner(crate::object::shapes::scan_shape_table_rekey_mut); + crate::object::shapes::test_clear_shape_table(); + let keys = unsafe { young_keys_array() }; + js_shadow_slot_set(0, ptr_bits(keys as usize)); + let id = crate::object::shapes::shape_descriptor_ensure(keys, 0, 0).expect("shape"); + + for _ in 0..4 { + let _ = gc_collect_minor(); + } + let promoted = (js_shadow_slot_get(0) & POINTER_MASK) as usize; + assert!( + !crate::arena::pointer_in_nursery(promoted), + "fixture must promote" + ); + assert_eq!(walk("shapes.families+indices").kept, 0); + + let valid = build_valid_pointer_set(); + let mut visitor = RuntimeRootVisitor::for_rewrite(&valid); + crate::object::shapes::scan_shape_table_rekey_mut(&mut visitor); + let row = walk("shapes.families+indices"); + assert!( + !row.partial, + "major/full walk must remain authoritative: {row:?}" + ); + assert!( + row.visited >= 1, + "major/full walk must still see the descriptor" + ); + assert_eq!( + crate::object::shapes::shape_descriptor_by_id(id).map(|d| d.keys), + Some(promoted as u64) + ); +} + +/// An already-Longlived keys array can gain a new nursery key at the same +/// address. Re-stamping the old receiver is the structural publication +/// chokepoint that must re-arm it. +#[test] +fn shape_mutation_to_new_young_key_rearms_minor_log() { + let _guard = CopyingNurseryTestGuard::new(0); + gc_register_mutable_root_scanner(crate::object::shapes::scan_shape_table_rekey_mut); + crate::object::shapes::test_clear_shape_table(); + unsafe { + let bytes = std::mem::size_of::() + 8; + let keys = crate::arena::arena_alloc_gc_longlived(bytes, 8, GC_TYPE_ARRAY) + as *mut crate::array::ArrayHeader; + (*keys).length = 1; + (*keys).capacity = 1; + let slot = + (keys as *mut u8).add(std::mem::size_of::()) as *mut f64; + *slot = f64::from_bits(string_bits(old_leaf())); + let id = crate::object::shapes::shape_descriptor_ensure(keys, 1, 0).expect("shape"); + let (owner, _) = alloc_old_test_object(0); + crate::object::shapes::stamp_object_shape_id_with_carrier_note(owner, id); + let _ = gc_collect_minor(); + assert_eq!(walk("shapes.families+indices").kept, 0); + + let young = young_leaf(); + *slot = f64::from_bits(string_bits(young)); + crate::object::shapes::stamp_object_shape_id_with_carrier_note(owner, id); + let _ = gc_collect_minor(); + let moved = ((*slot).to_bits() & POINTER_MASK) as usize; + assert_ne!( + moved, young, + "the mutation hook must make the new key visible" + ); + assert!(walk("shapes.families+indices").visited >= 1); + } +} + +/// N old box payloads plus k young payloads must price exactly k registry +/// entries. The counter is recorded inside `scan_box_young_roots_mut`. +#[test] +fn box_roots_minor_walk_visits_exactly_k_young_entries() { + const N: usize = 128; + const K: usize = 4; + let _guard = CopyingNurseryTestGuard::new(0); + gc_register_mutable_root_scanner(crate::r#box::scan_box_roots_mut); + for _ in 0..N { + crate::r#box::js_box_alloc_bits(string_bits(old_leaf()) as i64); + } + for _ in 0..K { + crate::r#box::js_box_alloc_bits(string_bits(young_leaf()) as i64); + } + + let _ = gc_collect_minor(); + let row = walk("box.roots"); + assert!(row.partial, "{row:?}"); + assert_eq!( + row.visited, K as u64, + "minor work must be young-sized: {row:?}" + ); + assert_eq!( + row.table_len, + (N + K) as u64, + "fixture registry mismatch: {row:?}" + ); +} + +/// Suppress the real `js_box_set_bits` arming site and prove the full-registry +/// re-derivation catches the omission. +#[test] +fn box_root_rederivation_rejects_a_suppressed_mutation_hook() { + let _guard = CopyingNurseryTestGuard::new(0); + let cell = crate::r#box::js_box_alloc_bits(string_bits(old_leaf()) as i64); + let young = young_leaf(); + { + let _sabotage = crate::r#box::TestBoxYoungLogSuppression::new(); + crate::r#box::js_box_set_bits(cell, string_bits(young) as i64); + } + let valid = build_valid_pointer_set(); + let mut visitor = RuntimeRootVisitor::for_mark_scoped(&valid, true); + let rejected = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { + crate::r#box::scan_box_roots_mut(&mut visitor); + })); + assert!( + rejected.is_err(), + "a missing box mutation note must be detected" + ); +} + +#[test] +fn box_mutation_to_new_young_object_is_visited() { + let _guard = CopyingNurseryTestGuard::new(0); + gc_register_mutable_root_scanner(crate::r#box::scan_box_roots_mut); + let cell = crate::r#box::js_box_alloc_bits(string_bits(old_leaf()) as i64); + let young = young_leaf(); + crate::r#box::js_box_set_bits(cell, string_bits(young) as i64); + + let _ = gc_collect_minor(); + let moved = (crate::r#box::js_box_get_bits(cell) as u64 & POINTER_MASK) as usize; + assert_ne!(moved, young, "setter must re-arm a previously old box"); + assert_eq!(walk("box.roots").visited, 1); +} + +#[test] +fn promoted_box_root_leaves_log_and_is_found_by_full_walk() { + let _guard = CopyingNurseryTestGuard::new(0); + gc_register_mutable_root_scanner(crate::r#box::scan_box_roots_mut); + let cell = crate::r#box::js_box_alloc_bits(string_bits(young_leaf()) as i64); + for _ in 0..4 { + let _ = gc_collect_minor(); + } + let promoted_bits = crate::r#box::js_box_get_bits(cell) as u64; + let promoted = (promoted_bits & POINTER_MASK) as usize; + assert!( + !crate::arena::pointer_in_nursery(promoted), + "fixture must promote" + ); + assert_eq!(walk("box.roots").kept, 0); + + let mut seen = false; + crate::r#box::scan_box_roots(&mut |value| { + if value.to_bits() == promoted_bits { + seen = true; + } + }); + assert!( + seen, + "the unchanged full walk must still enumerate promoted roots" + ); + assert!(!walk("box.roots").partial); +} diff --git a/crates/perry-runtime/src/gc/young_log.rs b/crates/perry-runtime/src/gc/young_log.rs index 9f6d43b22f..d83d9bb36c 100644 --- a/crates/perry-runtime/src/gc/young_log.rs +++ b/crates/perry-runtime/src/gc/young_log.rs @@ -155,7 +155,7 @@ impl YoungLog { /// Rule 2: the log must name every key in `relevant`. `relevant` is the /// set the caller re-derived from the authoritative table under /// `debug_assertions`; a miss is a writer that publishes without noting. - #[cfg(debug_assertions)] + #[cfg(any(debug_assertions, test))] pub(crate) fn debug_assert_logged(&self, table: &'static str, relevant: &[K]) where K: std::fmt::Debug, @@ -210,6 +210,44 @@ pub(crate) fn addr_is_minor_relevant(addr: usize) -> bool { } } +/// Can a minor move or reclaim the object at `addr`? +/// +/// This is narrower than [`addr_is_minor_relevant`]: `Longlived` objects must +/// sometimes be traced *through*, but they are never themselves moved or +/// swept. Side tables whose entries name known GC leaves (shape property +/// keys are strings/symbol headers) use this predicate so an immortal leaf +/// does not pin its entry in a young log forever. +#[inline] +pub(crate) fn addr_is_minor_collectible(addr: usize) -> bool { + if addr == 0 { + return false; + } + match crate::arena::classify_heap_space(addr) { + HeapSpace::NurseryEden + | HeapSpace::Survivor0 + | HeapSpace::Survivor1 + | HeapSpace::PromotedYoung => true, + HeapSpace::Old | HeapSpace::Longlived => false, + HeapSpace::Unknown => { + addr > GC_HEADER_SIZE + && super::malloc::gc_malloc_header_is_tracked( + (addr - GC_HEADER_SIZE) as *const super::GcHeader, + ) + } + } +} + +/// [`addr_is_minor_collectible`] for a NaN-boxed value. +#[inline] +pub(crate) fn bits_are_minor_collectible(bits: u64) -> bool { + let tag = bits & TAG_MASK; + if tag == POINTER_TAG || tag == STRING_TAG || tag == BIGINT_TAG { + addr_is_minor_collectible((bits & POINTER_MASK) as usize) + } else { + false + } +} + /// [`addr_is_minor_relevant`] for a NaN-boxed value: only the three /// pointer-carrying tags decode to an address; numbers, booleans, short /// strings and `undefined` are never relevant. diff --git a/crates/perry-runtime/src/node_submodules/diagnostics_gc.rs b/crates/perry-runtime/src/node_submodules/diagnostics_gc.rs index 8b68e16e92..511a1999ca 100644 --- a/crates/perry-runtime/src/node_submodules/diagnostics_gc.rs +++ b/crates/perry-runtime/src/node_submodules/diagnostics_gc.rs @@ -46,7 +46,7 @@ pub(crate) fn error_side_tables_clear_dead(user_ptr: usize) { /// key is a dead from-space error — unmarked, unforwarded, nursery-space, /// still typed `GC_TYPE_ERROR`. Mirrors /// `finalize_dead_copied_minor_from_space_maps`. -pub(crate) fn finalize_dead_copied_minor_from_space_errors() { +pub(crate) fn finalize_dead_copied_minor_from_space_errors() -> usize { fn is_dead_from_space_error(addr: usize) -> bool { let space = crate::arena::classify_heap_space(addr); if !matches!(space, crate::arena::HeapSpace::NurseryEden) @@ -73,7 +73,9 @@ pub(crate) fn finalize_dead_copied_minor_from_space_errors() { .filter(|addr| is_dead_from_space_error(*addr)) .collect() }); + let count = dead.len(); for addr in dead { error_side_tables_clear_dead(addr); } + count } diff --git a/crates/perry-runtime/src/object/descriptor_state.rs b/crates/perry-runtime/src/object/descriptor_state.rs index 31d0a54ce4..21966ee59e 100644 --- a/crates/perry-runtime/src/object/descriptor_state.rs +++ b/crates/perry-runtime/src/object/descriptor_state.rs @@ -130,6 +130,11 @@ impl DescriptorTables { const DESCRIPTOR_YOUNG_LOG_NAME: &str = "object.descriptors"; +#[cfg(test)] +thread_local! { + static TEST_SUPPRESS_DESCRIPTOR_YOUNG_NOTE: Cell = const { Cell::new(false) }; +} + mod gc_scan; mod young; pub(crate) use gc_scan::{scan_descriptor_owner, scan_descriptor_roots_mut}; @@ -145,15 +150,54 @@ fn note_young_descriptor_owner( owner: usize, acc: Option<&AccessorDescriptor>, ) { - use crate::gc::young_log::{addr_is_minor_relevant, bits_are_minor_relevant}; - if addr_is_minor_relevant(owner) + use crate::gc::young_log::{addr_is_minor_collectible, bits_are_minor_relevant}; + if addr_is_minor_collectible(owner) || acc .is_some_and(|acc| bits_are_minor_relevant(acc.get) || bits_are_minor_relevant(acc.set)) { + #[cfg(test)] + if TEST_SUPPRESS_DESCRIPTOR_YOUNG_NOTE.with(Cell::get) { + return; + } st.descriptors.young_owners.borrow_mut().note(owner); } } +#[cfg(test)] +mod young_log_sabotage_tests { + use super::*; + + #[test] + fn descriptor_log_rederivation_rejects_a_suppressed_setter() { + let _lock = crate::gc::global_side_table_test_lock(); + let owner = crate::object::js_object_alloc(0, 0) as usize; + state().descriptors.young_owners.borrow_mut().clear(); + TEST_SUPPRESS_DESCRIPTOR_YOUNG_NOTE.with(|flag| flag.set(true)); + set_property_attrs( + owner, + "sabotage".to_string(), + PropertyAttrs::new(true, true, true), + ); + TEST_SUPPRESS_DESCRIPTOR_YOUNG_NOTE.with(|flag| flag.set(false)); + let missed = std::panic::catch_unwind(|| { + state() + .descriptors + .young_owners + .borrow() + .debug_assert_logged( + DESCRIPTOR_YOUNG_LOG_NAME, + &relevant_descriptor_owners(state()), + ); + }); + clear_property_attrs(owner, "sabotage"); + state().descriptors.young_owners.borrow_mut().clear(); + assert!( + missed.is_err(), + "sabotage: suppressing set_property_attrs' note must trip completeness" + ); + } +} + /// Record `key` as owned by `owner` in an owner index. Idempotent: a /// `defineProperty` that overwrites an existing descriptor must not push a /// duplicate, or the key would be reported twice by `Object.keys`. diff --git a/crates/perry-runtime/src/object/descriptor_state/gc_scan.rs b/crates/perry-runtime/src/object/descriptor_state/gc_scan.rs index fe0f22ca36..48471ca672 100644 --- a/crates/perry-runtime/src/object/descriptor_state/gc_scan.rs +++ b/crates/perry-runtime/src/object/descriptor_state/gc_scan.rs @@ -137,7 +137,7 @@ pub(crate) fn scan_descriptor_owner( st: &crate::state::RuntimeState, owner: usize, ) -> (usize, bool) { - use crate::gc::young_log::{addr_is_minor_relevant, bits_are_minor_relevant}; + use crate::gc::young_log::{addr_is_minor_collectible, bits_are_minor_relevant}; let new_owner = rewrite_descriptor_owner(visitor, owner); let mut relevant = false; let accessor_keys = st @@ -187,7 +187,7 @@ pub(crate) fn scan_descriptor_owner( owner_index_transfer(&st.descriptors.attr_keys_by_owner, owner, new_owner); owner_index_transfer(&st.descriptors.accessor_keys_by_owner, owner, new_owner); } - relevant |= addr_is_minor_relevant(new_owner); + relevant |= addr_is_minor_collectible(new_owner); (new_owner, relevant) } diff --git a/crates/perry-runtime/src/object/descriptor_state/young.rs b/crates/perry-runtime/src/object/descriptor_state/young.rs index 0133fc7549..631abbc548 100644 --- a/crates/perry-runtime/src/object/descriptor_state/young.rs +++ b/crates/perry-runtime/src/object/descriptor_state/young.rs @@ -13,15 +13,15 @@ use super::*; /// authoritative tables: a non-old owner, or an accessor whose getter or /// setter is non-old. pub(super) fn relevant_descriptor_owners(st: &crate::state::RuntimeState) -> Vec { - use crate::gc::young_log::{addr_is_minor_relevant, bits_are_minor_relevant}; + use crate::gc::young_log::{addr_is_minor_collectible, bits_are_minor_relevant}; let mut relevant = Vec::new(); for &owner in st.descriptors.attr_keys_by_owner.borrow().keys() { - if addr_is_minor_relevant(owner) { + if addr_is_minor_collectible(owner) { relevant.push(owner); } } for &owner in st.descriptors.accessor_keys_by_owner.borrow().keys() { - if addr_is_minor_relevant(owner) { + if addr_is_minor_collectible(owner) { relevant.push(owner); } } @@ -45,7 +45,7 @@ pub(super) fn scan_descriptor_roots_young( ) { let table_len = st.descriptors.attr_keys_by_owner.borrow().len() as u64 + st.descriptors.accessor_keys_by_owner.borrow().len() as u64; - #[cfg(debug_assertions)] + #[cfg(any(debug_assertions, test))] { let relevant = relevant_descriptor_owners(st); st.descriptors diff --git a/crates/perry-runtime/src/object/native_module/callable_exports.rs b/crates/perry-runtime/src/object/native_module/callable_exports.rs index ea47d3ea81..431382981a 100644 --- a/crates/perry-runtime/src/object/native_module/callable_exports.rs +++ b/crates/perry-runtime/src/object/native_module/callable_exports.rs @@ -1,6 +1,8 @@ use super::callable_export_arity_table::native_callable_export_arity; use super::*; +mod builtin_closure_metadata; mod module_cjs; +pub(crate) use builtin_closure_metadata::*; use module_cjs::attach_module_cjs_constructor_statics; pub(crate) use module_cjs::{ module_builtin_modules_value, module_cjs_cache_value, module_cjs_extensions_value, @@ -1483,107 +1485,6 @@ pub(crate) fn set_bound_native_closure_name( ); } -thread_local! { - /// Per-closure spec `.length` for built-in *prototype methods*. Those - /// methods all share one no-op closure thunk - /// (`global_this_builtin_noop_thunk`), so the func-ptr-keyed - /// the closure body registry can't give `Array.prototype.map.length === 1` - /// while `Array.prototype.slice.length === 2` — the last install would - /// win for every method. Recording the length per *closure instance* here - /// (keyed by the closure pointer, like the user-facing dynamic-prop table - /// but isolated from it so a user `fn.length = x` write can't perturb it) - /// lets the `.length` value-read and `getOwnPropertyDescriptor` agree with - /// the spec count. #3143. - static BUILTIN_CLOSURE_LENGTH: std::cell::RefCell> = - std::cell::RefCell::new(crate::fast_hash::new_ptr_hash_map()); - - /// Built-in method closures are callable but lack ECMAScript - /// `[[Construct]]`. Track the installed closure values so the dynamic - /// `new` / `Reflect.construct` paths can reject them without changing - /// ordinary user closures or global constructor closures. - static BUILTIN_CLOSURE_NON_CONSTRUCTABLE: std::cell::RefCell> = - std::cell::RefCell::new(crate::fast_hash::new_ptr_hash_set()); -} - -/// Record the spec `.length` for a built-in prototype-method closure. See -/// [`BUILTIN_CLOSURE_LENGTH`]. -pub(crate) fn set_builtin_closure_length(closure: usize, length: u32) { - BUILTIN_CLOSURE_LENGTH.with(|m| { - m.borrow_mut().insert(closure, length); - }); -} - -/// Look up the recorded spec `.length` for a built-in prototype-method -/// closure, or `None` if this closure isn't one. See [`BUILTIN_CLOSURE_LENGTH`]. -pub(crate) fn builtin_closure_length(closure: usize) -> Option { - BUILTIN_CLOSURE_LENGTH.with(|m| m.borrow().get(&closure).copied()) -} - -pub(crate) fn set_builtin_closure_non_constructable(closure: usize) { - BUILTIN_CLOSURE_NON_CONSTRUCTABLE.with(|m| { - m.borrow_mut().insert(closure); - }); -} - -pub(crate) fn builtin_closure_is_non_constructable(closure: usize) -> bool { - BUILTIN_CLOSURE_NON_CONSTRUCTABLE.with(|m| m.borrow().contains(&closure)) -} - -/// Rekey per-instance built-in closure metadata after a moving collection. -/// -/// The keys are identities, not roots: prototype/global objects keep live -/// built-in closures reachable, while dead closures must remain collectable. -/// `visit_metadata_usize_slot` therefore only follows forwarding records. -pub(crate) fn scan_builtin_closure_metadata_roots_mut( - visitor: &mut crate::gc::RuntimeRootVisitor<'_>, -) { - BUILTIN_CLOSURE_LENGTH.with(|lengths| { - let mut lengths = lengths.borrow_mut(); - let mut moved = Vec::new(); - for old_owner in lengths.keys().copied() { - let mut new_owner = old_owner; - if visitor.visit_metadata_usize_slot(&mut new_owner) && new_owner != old_owner { - moved.push((old_owner, new_owner)); - } - } - for (old_owner, new_owner) in moved { - if let Some(length) = lengths.remove(&old_owner) { - lengths.insert(new_owner, length); - } - } - }); - - BUILTIN_CLOSURE_NON_CONSTRUCTABLE.with(|non_constructable| { - let mut non_constructable = non_constructable.borrow_mut(); - let mut moved = Vec::new(); - for old_owner in non_constructable.iter().copied() { - let mut new_owner = old_owner; - if visitor.visit_metadata_usize_slot(&mut new_owner) && new_owner != old_owner { - moved.push((old_owner, new_owner)); - } - } - for (old_owner, new_owner) in moved { - non_constructable.remove(&old_owner); - non_constructable.insert(new_owner); - } - }); -} - -/// Drop metadata for closures proved dead by the collector before their arena -/// addresses can be recycled for unrelated objects. -pub(crate) fn prune_dead_builtin_closure_metadata_owners(is_dead_owner: &dyn Fn(usize) -> bool) { - BUILTIN_CLOSURE_LENGTH.with(|lengths| { - lengths - .borrow_mut() - .retain(|owner, _| !is_dead_owner(*owner)); - }); - BUILTIN_CLOSURE_NON_CONSTRUCTABLE.with(|non_constructable| { - non_constructable - .borrow_mut() - .retain(|owner| !is_dead_owner(*owner)); - }); -} - pub(crate) fn builtin_closure_is_non_constructable_value(value: f64) -> bool { let jv = JSValue::from_bits(value.to_bits()); if !jv.is_pointer() { diff --git a/crates/perry-runtime/src/object/native_module/callable_exports/builtin_closure_metadata.rs b/crates/perry-runtime/src/object/native_module/callable_exports/builtin_closure_metadata.rs new file mode 100644 index 0000000000..a3d1ec5ee3 --- /dev/null +++ b/crates/perry-runtime/src/object/native_module/callable_exports/builtin_closure_metadata.rs @@ -0,0 +1,207 @@ +//! Young-scoped GC maintenance for per-instance built-in closure metadata. + +thread_local! { + static BUILTIN_CLOSURE_LENGTH: std::cell::RefCell> = + std::cell::RefCell::new(crate::fast_hash::new_ptr_hash_map()); + static BUILTIN_CLOSURE_NON_CONSTRUCTABLE: std::cell::RefCell> = + std::cell::RefCell::new(crate::fast_hash::new_ptr_hash_set()); +} + +crate::perry_thread_local! { + static BUILTIN_CLOSURE_YOUNG: std::cell::RefCell> = + const { std::cell::RefCell::new(crate::gc::young_log::YoungLog::new()) }; +} + +#[cfg(test)] +thread_local! { + static TEST_SUPPRESS_BUILTIN_CLOSURE_YOUNG_NOTE: std::cell::Cell = + const { std::cell::Cell::new(false) }; +} + +const LOG_NAME: &str = "object.builtin_closure_metadata"; + +#[inline] +fn note(closure: usize) { + if !crate::gc::young_log::addr_is_minor_collectible(closure) { + return; + } + #[cfg(test)] + if TEST_SUPPRESS_BUILTIN_CLOSURE_YOUNG_NOTE.with(std::cell::Cell::get) { + return; + } + BUILTIN_CLOSURE_YOUNG.with(|log| log.borrow_mut().note(closure)); +} + +pub(crate) fn set_builtin_closure_length(closure: usize, length: u32) { + note(closure); + BUILTIN_CLOSURE_LENGTH.with(|m| { + m.borrow_mut().insert(closure, length); + }); +} + +pub(crate) fn builtin_closure_length(closure: usize) -> Option { + BUILTIN_CLOSURE_LENGTH.with(|m| m.borrow().get(&closure).copied()) +} + +pub(crate) fn set_builtin_closure_non_constructable(closure: usize) { + note(closure); + BUILTIN_CLOSURE_NON_CONSTRUCTABLE.with(|m| { + m.borrow_mut().insert(closure); + }); +} + +pub(crate) fn builtin_closure_is_non_constructable(closure: usize) -> bool { + BUILTIN_CLOSURE_NON_CONSTRUCTABLE.with(|m| m.borrow().contains(&closure)) +} + +#[cfg(any(debug_assertions, test))] +fn relevant_owners() -> Vec { + let mut owners = Vec::new(); + BUILTIN_CLOSURE_LENGTH.with(|m| { + owners.extend( + m.borrow() + .keys() + .copied() + .filter(|owner| crate::gc::young_log::addr_is_minor_collectible(*owner)), + ); + }); + BUILTIN_CLOSURE_NON_CONSTRUCTABLE.with(|m| { + owners.extend( + m.borrow() + .iter() + .copied() + .filter(|owner| crate::gc::young_log::addr_is_minor_collectible(*owner)), + ); + }); + owners.sort_unstable(); + owners.dedup(); + owners +} + +fn visit_owner(visitor: &mut crate::gc::RuntimeRootVisitor<'_>, owner: usize) -> Option { + let mut new_owner = owner; + visitor.visit_metadata_usize_slot(&mut new_owner); + BUILTIN_CLOSURE_LENGTH.with(|lengths| { + let mut lengths = lengths.borrow_mut(); + if new_owner != owner { + if let Some(length) = lengths.remove(&owner) { + lengths.insert(new_owner, length); + } + } + }); + BUILTIN_CLOSURE_NON_CONSTRUCTABLE.with(|set| { + let mut set = set.borrow_mut(); + if new_owner != owner && set.remove(&owner) { + set.insert(new_owner); + } + }); + crate::gc::young_log::addr_is_minor_collectible(new_owner).then_some(new_owner) +} + +pub(crate) fn scan_builtin_closure_metadata_roots_mut( + visitor: &mut crate::gc::RuntimeRootVisitor<'_>, +) { + let table_len = BUILTIN_CLOSURE_LENGTH.with(|m| m.borrow().len()) + + BUILTIN_CLOSURE_NON_CONSTRUCTABLE.with(|m| m.borrow().len()); + if visitor.young_scope() { + #[cfg(any(debug_assertions, test))] + BUILTIN_CLOSURE_YOUNG.with(|log| { + log.borrow() + .debug_assert_logged(LOG_NAME, &relevant_owners()) + }); + let mut logged = 0u64; + let mut visited = 0u64; + let mut kept = BUILTIN_CLOSURE_YOUNG.with(|log| log.borrow_mut().take_spare()); + loop { + let batch = BUILTIN_CLOSURE_YOUNG.with(|log| log.borrow_mut().take_sorted()); + if batch.is_empty() { + break; + } + logged += batch.len() as u64; + for owner in batch { + visited += 1; + if let Some(owner) = visit_owner(visitor, owner) { + kept.push(owner); + } + } + } + let kept_len = kept.len() as u64; + BUILTIN_CLOSURE_YOUNG.with(|log| log.borrow_mut().extend(kept)); + crate::gc::young_log::note_walk( + LOG_NAME, + crate::gc::young_log::YoungLogWalk { + partial: true, + logged, + visited, + kept: kept_len, + table_len: table_len as u64, + }, + ); + return; + } + + let mut owners = Vec::new(); + BUILTIN_CLOSURE_LENGTH.with(|m| owners.extend(m.borrow().keys().copied())); + BUILTIN_CLOSURE_NON_CONSTRUCTABLE.with(|m| owners.extend(m.borrow().iter().copied())); + owners.sort_unstable(); + owners.dedup(); + let visited = owners.len() as u64; + let _ = BUILTIN_CLOSURE_YOUNG.with(|log| log.borrow_mut().take_sorted()); + let mut kept = BUILTIN_CLOSURE_YOUNG.with(|log| log.borrow_mut().take_spare()); + for owner in owners { + if let Some(owner) = visit_owner(visitor, owner) { + kept.push(owner); + } + } + let kept_len = kept.len() as u64; + BUILTIN_CLOSURE_YOUNG.with(|log| log.borrow_mut().extend(kept)); + crate::gc::young_log::note_walk( + LOG_NAME, + crate::gc::young_log::YoungLogWalk { + partial: false, + logged: visited, + visited, + kept: kept_len, + table_len: table_len as u64, + }, + ); +} + +pub(crate) fn prune_dead_builtin_closure_metadata_owners(is_dead_owner: &dyn Fn(usize) -> bool) { + BUILTIN_CLOSURE_LENGTH.with(|lengths| { + lengths + .borrow_mut() + .retain(|owner, _| !is_dead_owner(*owner)); + }); + BUILTIN_CLOSURE_NON_CONSTRUCTABLE.with(|non_constructable| { + non_constructable + .borrow_mut() + .retain(|owner| !is_dead_owner(*owner)); + }); +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn builtin_closure_log_rederivation_rejects_a_suppressed_writer() { + let _lock = crate::gc::global_side_table_test_lock(); + let closure = crate::closure::js_closure_alloc(std::ptr::null(), 0) as usize; + TEST_SUPPRESS_BUILTIN_CLOSURE_YOUNG_NOTE.with(|flag| flag.set(true)); + set_builtin_closure_length(closure, 3); + TEST_SUPPRESS_BUILTIN_CLOSURE_YOUNG_NOTE.with(|flag| flag.set(false)); + let missed = std::panic::catch_unwind(|| { + BUILTIN_CLOSURE_YOUNG.with(|log| { + log.borrow() + .debug_assert_logged(LOG_NAME, &relevant_owners()) + }); + }); + BUILTIN_CLOSURE_LENGTH.with(|m| m.borrow_mut().remove(&closure)); + BUILTIN_CLOSURE_YOUNG.with(|log| log.borrow_mut().clear()); + assert!( + missed.is_err(), + "sabotage: suppressing the setter's note must trip completeness" + ); + } +} diff --git a/crates/perry-runtime/src/object/shapes.rs b/crates/perry-runtime/src/object/shapes.rs index b0d47aac53..51724f362b 100644 --- a/crates/perry-runtime/src/object/shapes.rs +++ b/crates/perry-runtime/src/object/shapes.rs @@ -264,6 +264,28 @@ struct ShapeTableInner { const SHAPE_YOUNG_LOG_NAME: &str = "shapes.families+indices"; +crate::perry_thread_local! { + /// Carrier notes can be produced while a GC walk already borrows the shape + /// table. Keep that write-side stream separate and merge it at the next + /// scanner entry rather than re-borrowing `ShapeTableInner` recursively. + static SHAPE_CARRIER_YOUNG_KEYS: RefCell> = + const { RefCell::new(crate::gc::young_log::YoungLog::new()) }; + #[cfg(test)] + static SHAPE_YOUNG_LOG_SUPPRESSED: std::cell::Cell = const { std::cell::Cell::new(false) }; +} + +#[inline] +fn note_shape_carrier_candidate(keys: u64) { + if !crate::gc::young_log::addr_is_minor_relevant(keys as usize) { + return; + } + #[cfg(test)] + if SHAPE_YOUNG_LOG_SUPPRESSED.with(std::cell::Cell::get) { + return; + } + SHAPE_CARRIER_YOUNG_KEYS.with(|log| log.borrow_mut().note(keys)); +} + /// Re-export of the id-list operation counters' report, so the collector does /// not have to name a private sibling module. One `[gc-idlist]` line per /// copying minor under `PERRY_GC_DIAG=1`; `elems_moved` is the falsifier for @@ -281,7 +303,11 @@ impl ShapeTableInner { /// call this themselves. #[inline] fn note_young_keys(&mut self, keys: u64) { - if crate::gc::young_log::addr_is_minor_relevant(keys as usize) { + #[cfg(test)] + if SHAPE_YOUNG_LOG_SUPPRESSED.with(std::cell::Cell::get) { + return; + } + if crate::gc::young_log::addr_is_minor_collectible(keys as usize) { self.young_keys.note(keys); } } @@ -692,8 +718,12 @@ pub(crate) unsafe fn note_old_generation_carrier(descriptor: Option) { return; } let record = descriptor.record as *mut ShapeRecord; + let newly_armed = !(*record).has(RECORD_FLAG_CACHE_CARRIER); // GC_STORE_AUDIT(POINTER_FREE): liveness bookkeeping bit, never a heap reference. (*record).set(RECORD_FLAG_CACHE_CARRIER, true); + if newly_armed { + note_shape_carrier_candidate(descriptor.keys); + } } /// The post-birth publication point for a ShapeId into a receiver's header @@ -770,7 +804,15 @@ pub(crate) unsafe fn stamp_object_shape_id_with_carrier_note( ) { (*obj).parent_class_id = id; if !crate::arena::pointer_in_nursery(obj as usize) { - note_old_generation_carrier(shape_descriptor_by_id(id)); + let descriptor = shape_descriptor_by_id(id); + note_old_generation_carrier(descriptor); + // This stamp is the structural-mutation publication funnel. Re-arm + // even when the descriptor was already an old carrier: an owned + // Longlived keys array may have just gained a nursery key at the same + // address, and its carrier flag alone cannot express that transition. + if let Some(descriptor) = descriptor { + note_shape_carrier_candidate(descriptor.keys); + } } } @@ -1930,6 +1972,8 @@ pub(crate) fn prune_dead_shape_keys_young(is_dead_owner: &dyn Fn(usize) -> bool) pub(crate) fn scan_shape_table_rekey_mut(visitor: &mut crate::gc::RuntimeRootVisitor<'_>) { let table = &crate::state::state().shapes; let mut inner = table.inner.borrow_mut(); + let carrier_notes = SHAPE_CARRIER_YOUNG_KEYS.with(|log| log.borrow_mut().take_sorted()); + inner.young_keys.extend(carrier_notes); let rewrite_phase = visitor.is_metadata_rewrite_phase(); // #9754: a minor-scoped pass visits only the young-logged keys addresses; // the full walk below rebuilds the log from what it finds. @@ -2045,7 +2089,7 @@ pub(crate) fn scan_shape_table_rekey_mut(visitor: &mut crate::gc::RuntimeRootVis } // A full walk is authoritative: rebuild the young log from the tables. - let kept = relevant_shape_keys(&inner); + let kept = relevant_shape_keys(table, &inner); let kept_len = kept.len() as u64; let _ = inner.young_keys.take_sorted(); inner.young_keys.extend(kept); @@ -2080,33 +2124,82 @@ fn move_shape_family(table: &ShapeTable, inner: &mut ShapeTableInner, old: u64, inner.facts_remove(record.facts_key_with_keys(old), id); inner.facts_push_back(record.facts_key_with_keys(new), id); } - inner.family_push_back(new, id); + // Scanner-internal rekey: the caller keeps `new` from its post-visit + // relevance result (or the full walk rebuilds the log). Re-entering + // the writer funnel here would enqueue the same family mid-walk and + // price it twice in one minor. + inner.families.entry(new).or_default().push_back(id); } } /// Every keys address a minor can act on, re-derived from the authoritative /// tables (families and slot indices whose keys array is not old). -fn relevant_shape_keys(inner: &ShapeTableInner) -> Vec { - use crate::gc::young_log::addr_is_minor_relevant; - let mut relevant: Vec = inner - .families - .keys() - .copied() - .filter(|&keys| keys != 0 && addr_is_minor_relevant(keys as usize)) - .collect(); - relevant.extend( - inner - .indices - .keys() - .copied() - .filter(|&keys| addr_is_minor_relevant(keys)) - .map(|keys| keys as u64), - ); +fn relevant_shape_keys(table: &ShapeTable, inner: &ShapeTableInner) -> Vec { + let mut relevant: Vec = inner.families.keys().copied().collect(); + relevant.extend(inner.indices.keys().copied().map(|keys| keys as u64)); relevant.sort_unstable(); relevant.dedup(); + relevant.retain(|&keys| shape_keys_entry_is_minor_relevant(table, inner, keys)); relevant } +/// Exact minor-work predicate for one shape-table key. +/// +/// Nursery addresses must be rekeyed even for weak metadata entries. Malloc +/// arrays must be rooted when a carrier owns the family. A Longlived keys +/// array never moves or dies, so it matters only while a rooted family exposes +/// a collectible property-key leaf from its payload. Property keys are +/// strings/symbol headers and both are GC leaves; tracing through an immortal +/// key cannot discover a younger grandchild. +fn shape_keys_entry_is_minor_relevant( + table: &ShapeTable, + inner: &ShapeTableInner, + keys: u64, +) -> bool { + if keys == 0 { + return false; + } + let addr = keys as usize; + match crate::arena::classify_heap_space(addr) { + crate::arena::HeapSpace::NurseryEden + | crate::arena::HeapSpace::Survivor0 + | crate::arena::HeapSpace::Survivor1 + | crate::arena::HeapSpace::PromotedYoung => return true, + crate::arena::HeapSpace::Old => return false, + crate::arena::HeapSpace::Unknown => { + return family_has_root_carrier(table, inner, keys) + && crate::gc::young_log::addr_is_minor_collectible(addr); + } + crate::arena::HeapSpace::Longlived => {} + } + if !family_has_root_carrier(table, inner, keys) { + return false; + } + unsafe { + let Some(header) = crate::value::addr_class::try_read_tracked_gc_header(addr) else { + return false; + }; + if (*header.as_ptr()).obj_type != crate::gc::GC_TYPE_ARRAY { + return false; + } + let (slots, len) = super::keys_array_dense_slots(addr as *const ArrayHeader); + (0..len).any(|index| { + crate::gc::young_log::bits_are_minor_collectible((*slots.add(index)).to_bits()) + }) + } +} + +fn family_has_root_carrier(table: &ShapeTable, inner: &ShapeTableInner, keys: u64) -> bool { + inner.families.get(&keys).is_some_and(|ids| { + ids.as_slice().iter().any(|&id| { + table + .slab() + .get(id) + .is_some_and(|record| record.has(RECORD_FLAG_OLD_CARRIER) || record.cache_carrier()) + }) + }) +} + /// The minor-scoped walk (#9754): only the young-logged keys addresses, each /// visited exactly as the full walk visits it — the family's carrier gate, /// the record rewrite, the recycled-address retirement, the slot-index @@ -2118,9 +2211,9 @@ fn scan_shape_table_young( rewrite_phase: bool, ) { let table_len = (inner.families.len() + inner.indices.len()) as u64; - #[cfg(debug_assertions)] + #[cfg(any(debug_assertions, test))] { - let relevant = relevant_shape_keys(inner); + let relevant = relevant_shape_keys(table, inner); inner .young_keys .debug_assert_logged(SHAPE_YOUNG_LOG_NAME, &relevant); @@ -2242,10 +2335,7 @@ fn scan_shape_keys_address( inner.indices.remove(&addr); } } - ( - post, - crate::gc::young_log::addr_is_minor_relevant(post as usize), - ) + (post, shape_keys_entry_is_minor_relevant(table, inner, post)) } // #8112 sabotage switch. Suppressing the descriptor edge proves the fixture's diff --git a/crates/perry-runtime/src/object/shapes_test_support.rs b/crates/perry-runtime/src/object/shapes_test_support.rs index 6cb974c98c..21bc8f7b4a 100644 --- a/crates/perry-runtime/src/object/shapes_test_support.rs +++ b/crates/perry-runtime/src/object/shapes_test_support.rs @@ -44,6 +44,25 @@ pub(crate) struct TestRecycledKeysCheckSuppression { previous: bool, } +/// Suppress both shape-table young-log writer funnels. A test using this guard +/// must be rejected by the scanner's authoritative re-derivation. +#[cfg(test)] +pub(crate) struct TestShapeYoungLogSuppression(bool); + +#[cfg(test)] +impl TestShapeYoungLogSuppression { + pub(crate) fn new() -> Self { + Self(SHAPE_YOUNG_LOG_SUPPRESSED.with(|cell| cell.replace(true))) + } +} + +#[cfg(test)] +impl Drop for TestShapeYoungLogSuppression { + fn drop(&mut self) { + SHAPE_YOUNG_LOG_SUPPRESSED.with(|cell| cell.set(self.0)); + } +} + #[cfg(test)] impl TestRecycledKeysCheckSuppression { pub(crate) fn new() -> Self { @@ -84,6 +103,7 @@ pub(crate) fn test_clear_shape_table() { inner.by_facts.clear(); inner.families.clear(); inner.young_keys.clear(); + SHAPE_CARRIER_YOUNG_KEYS.with(|log| log.borrow_mut().clear()); // SAFETY: test-only reset with no slab reference held. unsafe { table.slab_mut().clear() }; drop(inner); diff --git a/scripts/gc_rekeyed_key_tables.json b/scripts/gc_rekeyed_key_tables.json index b31756956e..09687911b2 100644 --- a/scripts/gc_rekeyed_key_tables.json +++ b/scripts/gc_rekeyed_key_tables.json @@ -104,10 +104,10 @@ "why": "#8192: registered prune drops the whole cache entry when either weak half (prev_keys keys-array, key_ptr interned string) is dead; next_keys is a strong root and cannot be. #9754: the per-slot body shared by the full walk and the young-log walk." }, { - "site": "crates/perry-runtime/src/object/native_module/callable_exports.rs::scan_builtin_closure_metadata_roots_mut", + "site": "crates/perry-runtime/src/object/native_module/callable_exports/builtin_closure_metadata.rs::visit_owner", "table": "BUILTIN_CLOSURE_LENGTH / BUILTIN_CLOSURE_NON_CONSTRUCTABLE", "death": "dead_owner:prune_dead_builtin_closure_metadata_owners", - "why": "#8393: both tables are retained on the GC_TYPE_CLOSURE-narrowed predicate over their closure-address keys." + "why": "#8393: both tables are retained on the GC_TYPE_CLOSURE-narrowed predicate over their closure-address keys. The extracted visit_owner helper is the shared rekey path for the full and young-log walks." }, { "site": "crates/perry-runtime/src/object/shapes.rs::scan_shape_table_rekey_mut", diff --git a/scripts/thread_local_cold_allowlist.json b/scripts/thread_local_cold_allowlist.json index b27978e5db..e566b4aa29 100644 --- a/scripts/thread_local_cold_allowlist.json +++ b/scripts/thread_local_cold_allowlist.json @@ -1,6 +1,6 @@ { "_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": 405, + "_hot_declarations": 411, "files": { "crates/perry-runtime/src/agent.rs": 1, "crates/perry-runtime/src/arena/block.rs": 5, @@ -64,7 +64,8 @@ "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": 8, - "crates/perry-runtime/src/object/native_module/callable_exports.rs": 3, + "crates/perry-runtime/src/object/native_module/callable_exports.rs": 1, + "crates/perry-runtime/src/object/native_module/callable_exports/builtin_closure_metadata.rs": 2, "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": 3,