diff --git a/changelog.d/7863-barrier-file-split.md b/changelog.d/7863-barrier-file-split.md new file mode 100644 index 0000000000..aef5a4975b --- /dev/null +++ b/changelog.d/7863-barrier-file-split.md @@ -0,0 +1,7 @@ +**`gc/barrier.rs` split at the 2000-line cap.** The evening merges pushed it to +2,157 lines, going red on `main`'s file-size gate. The remembered-set +inspection/drain/clear group moves to `gc/barrier/maintenance.rs` — the #7830 +recipe: a cohesive function group, explicit re-export, no logic change. The +moved items' `pub(super)` visibilities widen one level to `pub(in crate::gc)` +(a re-export cannot widen), and the thread-local cold-inventory entries re-key +to the new path. diff --git a/crates/perry-runtime/src/gc/barrier/maintenance.rs b/crates/perry-runtime/src/gc/barrier/maintenance.rs new file mode 100644 index 0000000000..17b3bc5607 --- /dev/null +++ b/crates/perry-runtime/src/gc/barrier/maintenance.rs @@ -0,0 +1,235 @@ +//! Remembered-set inspection, drain and clear helpers — split from +//! `barrier/mod.rs` for the 2000-line file-size gate (the #7830 recipe: +//! extract a cohesive function group into a sibling file, re-export +//! explicitly). No logic change. + +use super::*; + +pub fn remembered_set_size() -> usize { + remembered_dirty_page_count() + REMEMBERED_SET.with(|s| s.borrow().len()) +} + +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub(in crate::gc) struct MaintenanceClearStep { + pub(in crate::gc) done: bool, + pub(in crate::gc) work_units: usize, +} + +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +enum RememberedSetClearSubphase { + DirtyOldPages, + ExternalDirtySlots, + FallbackHeaders, + Done, +} + +pub(in crate::gc) struct RememberedSetClearState { + subphase: RememberedSetClearSubphase, +} + +impl RememberedSetClearState { + pub(in crate::gc) fn new() -> Self { + Self { + subphase: RememberedSetClearSubphase::DirtyOldPages, + } + } + + pub(in crate::gc) fn step(&mut self, budget: usize) -> bool { + self.step_counted(budget).done + } + + pub(in crate::gc) fn step_counted(&mut self, budget: usize) -> MaintenanceClearStep { + let mut work_units = 0usize; + loop { + match self.subphase { + RememberedSetClearSubphase::DirtyOldPages => { + if dirty_old_pages_empty() { + self.subphase = RememberedSetClearSubphase::ExternalDirtySlots; + continue; + } + if work_units == budget { + break; + } + if clear_one_dirty_old_page() { + work_units = work_units.saturating_add(1); + } + } + RememberedSetClearSubphase::ExternalDirtySlots => { + if external_dirty_slot_headers_empty() { + self.subphase = RememberedSetClearSubphase::FallbackHeaders; + continue; + } + if work_units == budget { + break; + } + if clear_one_external_dirty_slot_header() { + work_units = work_units.saturating_add(1); + } + } + RememberedSetClearSubphase::FallbackHeaders => { + if fallback_remembered_set_empty() { + self.subphase = RememberedSetClearSubphase::Done; + continue; + } + if work_units == budget { + break; + } + if clear_one_fallback_remembered_header() { + work_units = work_units.saturating_add(1); + } + } + RememberedSetClearSubphase::Done => { + return MaintenanceClearStep { + done: true, + work_units, + }; + } + } + } + MaintenanceClearStep { + done: self.subphase == RememberedSetClearSubphase::Done, + work_units, + } + } +} + +fn dirty_old_pages_empty() -> bool { + DIRTY_OLD_PAGES.with(|s| s.borrow().is_empty()) +} + +/// The **sole** path that removes a page from `DIRTY_OLD_PAGES`. Every other +/// touch of that set is an insert, a read, or the snapshot — which is why +/// #7187 Phase B's cache needs exactly one invalidation point on this side. +fn clear_one_dirty_old_page() -> bool { + DIRTY_OLD_PAGES.with(|s| { + let mut pages = s.borrow_mut(); + let Some(page) = pages.iter().next().copied() else { + return false; + }; + crate::arena::old_page_clear_dirty(page); + pages.remove(&page); + // DELIBERATELY redundant with `old_page_clear_dirty`, which invalidates + // too (#7187 Phase B rule 2). The cache's invariant has two halves and + // this line owns the modbuf one: an edit that stops the arena side from + // invalidating — or a page whose metadata entry no longer exists, so + // `old_page_clear_dirty` finds nothing to clear — must not silently + // leave the cache asserting a page this function just removed. The cost + // is one thread-local store on the cold clear path. + super::dirty_page_cache::invalidate(); + true + }) +} + +fn external_dirty_slot_headers_empty() -> bool { + EXTERNAL_DIRTY_SLOT_PAGES.with(|s| s.borrow().is_empty()) +} + +fn clear_one_external_dirty_slot_header() -> bool { + EXTERNAL_DIRTY_SLOT_PAGES.with(|s| { + let mut pages = s.borrow_mut(); + let Some(page) = pages.keys().next().copied() else { + return false; + }; + let remove_page = match pages.get_mut(&page) { + Some(headers) => { + headers.pop(); + headers.is_empty() + } + None => false, + }; + if remove_page { + pages.remove(&page); + } + true + }) +} + +fn fallback_remembered_set_empty() -> bool { + REMEMBERED_SET.with(|s| s.borrow().is_empty()) +} + +fn clear_one_fallback_remembered_header() -> bool { + REMEMBERED_SET.with(|s| { + let mut headers = s.borrow_mut(); + let Some(header) = headers.iter().next().copied() else { + return false; + }; + headers.remove(&header); + true + }) +} + +pub(in crate::gc) struct ConservativePinClearState { + done: bool, +} + +impl ConservativePinClearState { + pub(in crate::gc) fn new() -> Self { + Self { done: false } + } + + pub(in crate::gc) fn step_counted(&mut self, budget: usize) -> MaintenanceClearStep { + if self.done { + return MaintenanceClearStep { + done: true, + work_units: 0, + }; + } + + let mut work_units = 0usize; + while work_units < budget { + if clear_one_conservative_pin() { + work_units = work_units.saturating_add(1); + } else { + self.done = true; + break; + } + } + + if !self.done && conservative_pins_empty() { + self.done = true; + } + + MaintenanceClearStep { + done: self.done, + work_units, + } + } +} + +fn conservative_pins_empty() -> bool { + CONS_PINNED.with(|s| s.borrow().is_empty()) +} + +fn clear_one_conservative_pin() -> bool { + CONS_PINNED.with(|s| { + let mut pinned = s.borrow_mut(); + let Some(header) = pinned.iter().next().copied() else { + return false; + }; + pinned.remove(&header); + true + }) +} + +/// Gen-GC Phase C: clear the remembered set. Will be called by +/// minor GC after the rs-scan completes (Phase C3). Test-only +/// for now to enable test isolation. +pub fn remembered_set_clear() { + let mut state = RememberedSetClearState::new(); + while !state.step(usize::MAX) {} +} + +/// #7035: is `addr`'s old page currently in the remembered set? +pub(in crate::gc) fn dirty_now_for_addr(addr: usize) -> bool { + DIRTY_OLD_PAGES.with(|s| { + s.borrow() + .contains(&crate::arena::generation_page_for_addr(addr)) + }) +} + +/// #7035: was `addr`'s old page EVER dirtied? Distinguishes "barrier never ran" +/// from "edge was recorded then lost". +pub(in crate::gc) fn ever_dirty_for_addr(addr: usize) -> bool { + ever_dirty_old_page(crate::arena::generation_page_for_addr(addr)) +} diff --git a/crates/perry-runtime/src/gc/barrier.rs b/crates/perry-runtime/src/gc/barrier/mod.rs similarity index 91% rename from crates/perry-runtime/src/gc/barrier.rs rename to crates/perry-runtime/src/gc/barrier/mod.rs index f877418800..bb924cc262 100644 --- a/crates/perry-runtime/src/gc/barrier.rs +++ b/crates/perry-runtime/src/gc/barrier/mod.rs @@ -1669,7 +1669,7 @@ thread_local! { /// point (edge recorded-then-LOST by a clear/restore gap) or never recorded /// at all (a store path that skips the barrier) — the decisive split for the /// missing old→young edge bug. Empty/unused unless the verifier env is set. - static EVER_DIRTY_OLD_PAGES: std::cell::RefCell> = + pub(super) static EVER_DIRTY_OLD_PAGES: std::cell::RefCell> = std::cell::RefCell::new(crate::fast_hash::new_ptr_hash_set()); } @@ -1927,231 +1927,8 @@ pub(super) fn remembered_dirty_page_count() -> usize { /// by tests and `PERRY_GC_DIAG=1` output to confirm barrier /// activity. Returns 0 in Phase C1 since no codegen-emitted /// barrier has fired yet. -pub fn remembered_set_size() -> usize { - remembered_dirty_page_count() + REMEMBERED_SET.with(|s| s.borrow().len()) -} - -#[derive(Clone, Copy, Debug, Eq, PartialEq)] -pub(super) struct MaintenanceClearStep { - pub(super) done: bool, - pub(super) work_units: usize, -} - -#[derive(Clone, Copy, Debug, Eq, PartialEq)] -enum RememberedSetClearSubphase { - DirtyOldPages, - ExternalDirtySlots, - FallbackHeaders, - Done, -} - -pub(super) struct RememberedSetClearState { - subphase: RememberedSetClearSubphase, -} - -impl RememberedSetClearState { - pub(super) fn new() -> Self { - Self { - subphase: RememberedSetClearSubphase::DirtyOldPages, - } - } - - pub(super) fn step(&mut self, budget: usize) -> bool { - self.step_counted(budget).done - } - - pub(super) fn step_counted(&mut self, budget: usize) -> MaintenanceClearStep { - let mut work_units = 0usize; - loop { - match self.subphase { - RememberedSetClearSubphase::DirtyOldPages => { - if dirty_old_pages_empty() { - self.subphase = RememberedSetClearSubphase::ExternalDirtySlots; - continue; - } - if work_units == budget { - break; - } - if clear_one_dirty_old_page() { - work_units = work_units.saturating_add(1); - } - } - RememberedSetClearSubphase::ExternalDirtySlots => { - if external_dirty_slot_headers_empty() { - self.subphase = RememberedSetClearSubphase::FallbackHeaders; - continue; - } - if work_units == budget { - break; - } - if clear_one_external_dirty_slot_header() { - work_units = work_units.saturating_add(1); - } - } - RememberedSetClearSubphase::FallbackHeaders => { - if fallback_remembered_set_empty() { - self.subphase = RememberedSetClearSubphase::Done; - continue; - } - if work_units == budget { - break; - } - if clear_one_fallback_remembered_header() { - work_units = work_units.saturating_add(1); - } - } - RememberedSetClearSubphase::Done => { - return MaintenanceClearStep { - done: true, - work_units, - }; - } - } - } - MaintenanceClearStep { - done: self.subphase == RememberedSetClearSubphase::Done, - work_units, - } - } -} - -fn dirty_old_pages_empty() -> bool { - DIRTY_OLD_PAGES.with(|s| s.borrow().is_empty()) -} - -/// The **sole** path that removes a page from `DIRTY_OLD_PAGES`. Every other -/// touch of that set is an insert, a read, or the snapshot — which is why -/// #7187 Phase B's cache needs exactly one invalidation point on this side. -fn clear_one_dirty_old_page() -> bool { - DIRTY_OLD_PAGES.with(|s| { - let mut pages = s.borrow_mut(); - let Some(page) = pages.iter().next().copied() else { - return false; - }; - crate::arena::old_page_clear_dirty(page); - pages.remove(&page); - // DELIBERATELY redundant with `old_page_clear_dirty`, which invalidates - // too (#7187 Phase B rule 2). The cache's invariant has two halves and - // this line owns the modbuf one: an edit that stops the arena side from - // invalidating — or a page whose metadata entry no longer exists, so - // `old_page_clear_dirty` finds nothing to clear — must not silently - // leave the cache asserting a page this function just removed. The cost - // is one thread-local store on the cold clear path. - super::dirty_page_cache::invalidate(); - true - }) -} - -fn external_dirty_slot_headers_empty() -> bool { - EXTERNAL_DIRTY_SLOT_PAGES.with(|s| s.borrow().is_empty()) -} - -fn clear_one_external_dirty_slot_header() -> bool { - EXTERNAL_DIRTY_SLOT_PAGES.with(|s| { - let mut pages = s.borrow_mut(); - let Some(page) = pages.keys().next().copied() else { - return false; - }; - let remove_page = match pages.get_mut(&page) { - Some(headers) => { - headers.pop(); - headers.is_empty() - } - None => false, - }; - if remove_page { - pages.remove(&page); - } - true - }) -} - -fn fallback_remembered_set_empty() -> bool { - REMEMBERED_SET.with(|s| s.borrow().is_empty()) -} - -fn clear_one_fallback_remembered_header() -> bool { - REMEMBERED_SET.with(|s| { - let mut headers = s.borrow_mut(); - let Some(header) = headers.iter().next().copied() else { - return false; - }; - headers.remove(&header); - true - }) -} - -pub(super) struct ConservativePinClearState { - done: bool, -} - -impl ConservativePinClearState { - pub(super) fn new() -> Self { - Self { done: false } - } - - pub(super) fn step_counted(&mut self, budget: usize) -> MaintenanceClearStep { - if self.done { - return MaintenanceClearStep { - done: true, - work_units: 0, - }; - } - - let mut work_units = 0usize; - while work_units < budget { - if clear_one_conservative_pin() { - work_units = work_units.saturating_add(1); - } else { - self.done = true; - break; - } - } - - if !self.done && conservative_pins_empty() { - self.done = true; - } - - MaintenanceClearStep { - done: self.done, - work_units, - } - } -} - -fn conservative_pins_empty() -> bool { - CONS_PINNED.with(|s| s.borrow().is_empty()) -} - -fn clear_one_conservative_pin() -> bool { - CONS_PINNED.with(|s| { - let mut pinned = s.borrow_mut(); - let Some(header) = pinned.iter().next().copied() else { - return false; - }; - pinned.remove(&header); - true - }) -} - -/// Gen-GC Phase C: clear the remembered set. Will be called by -/// minor GC after the rs-scan completes (Phase C3). Test-only -/// for now to enable test isolation. -pub fn remembered_set_clear() { - let mut state = RememberedSetClearState::new(); - while !state.step(usize::MAX) {} -} - -/// #7035: is `addr`'s old page currently in the remembered set? -pub(super) fn dirty_now_for_addr(addr: usize) -> bool { - DIRTY_OLD_PAGES.with(|s| { - s.borrow() - .contains(&crate::arena::generation_page_for_addr(addr)) - }) -} - -/// #7035: was `addr`'s old page EVER dirtied? Distinguishes "barrier never ran" -/// from "edge was recorded then lost". -pub(super) fn ever_dirty_for_addr(addr: usize) -> bool { - ever_dirty_old_page(crate::arena::generation_page_for_addr(addr)) -} +// Remembered-set inspection and drain/maintenance helpers live in a sibling +// module purely for the 2000-line file-size gate; same module tree, same +// visibility semantics (the statics they read are pub(super)/pub(crate)). +mod maintenance; +pub use maintenance::*; diff --git a/scripts/thread_local_cold_allowlist.json b/scripts/thread_local_cold_allowlist.json index de275ddcdd..d2a86daa2f 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!`. Every entry is a declaration that pays `_tlv_get_addr` on Darwin; the count is a ratchet, so adding one to an already-listed file fails too. New code should use `crate::perry_thread_local!` \u2014 see crates/perry-runtime/src/tls_hot.rs. Regenerate with scripts/check_thread_locals.py --update.", - "_hot_declarations": 160, + "_hot_declarations": 163, "files": { "crates/perry-runtime/src/agent.rs": 1, "crates/perry-runtime/src/arena/block.rs": 2, @@ -30,7 +30,7 @@ "crates/perry-runtime/src/fs/filehandle.rs": 1, "crates/perry-runtime/src/fs/mod.rs": 1, "crates/perry-runtime/src/fs/stream.rs": 1, - "crates/perry-runtime/src/gc/barrier.rs": 2, + "crates/perry-runtime/src/gc/barrier/mod.rs": 2, "crates/perry-runtime/src/gc/barrier_arming.rs": 1, "crates/perry-runtime/src/gc/cycle.rs": 1, "crates/perry-runtime/src/gc/dirty_page_cache.rs": 1,