diff --git a/changelog.d/9030-dirty-page-cache-ways.md b/changelog.d/9030-dirty-page-cache-ways.md new file mode 100644 index 0000000000..1f5341ad2f --- /dev/null +++ b/changelog.d/9030-dirty-page-cache-ways.md @@ -0,0 +1,29 @@ +The write barrier's dirty-page cache is now sixteen direct-mapped ways instead +of one entry. + +The single entry was justified by a `batch.ts` simulation whose stores arrive in +long same-page runs. The ECS component-update rows falsify that shape: each +entity's sweep stores into every component column in turn, so the store pages +alternate and one entry can never hold them. Every store then took the uncached +path — `mark_dirty_old_page_uncached` plus the `DIRTY_OLD_PAGES` thread-local +resolution measured 35–40% of both update rows' frames. + +Ways are indexed by the page number's low bits. That is safe here precisely +because page numbers are `addr >> 12` and therefore sequential, so neighbouring +columns land in distinct ways — unlike the dynprop case where low-bit folding +collapsed keys. A hit bypasses the uncached path entirely, so the thread-local +resolution and the hash insert stop executing rather than getting cheaper. + +The safety invariant is unchanged and still one-directional: a way answers +"already marked" only on an exact page match, so a stale way answers "not +marked" and the store takes the recording path. `invalidate()` clears every way +on the same removal paths as before, so the cache can still only suppress a +REPEAT recording, never a first one. + +The #8949 process-global mirror is retired rather than widened: it existed to +shave the single cell's dependent-load chain for +0.17%, and a multi-way hot-TLS +map supersedes both its mechanism and its rationale. + +`HotTls` grows by 120 bytes at `dirty_old_pages`, which sits after both offsets +codegen hardcodes (`inline_state`, `implicit_this`), so the emitted offsets are +unaffected — pinned by `hot_tls_layout_is_what_codegen_assumes`. diff --git a/crates/perry-runtime/src/gc/dirty_page_cache.rs b/crates/perry-runtime/src/gc/dirty_page_cache.rs index 1df671273b..115b61229a 100644 --- a/crates/perry-runtime/src/gc/dirty_page_cache.rs +++ b/crates/perry-runtime/src/gc/dirty_page_cache.rs @@ -85,106 +85,48 @@ use std::cell::Cell; /// `usize::MAX` would need a 76-bit address. const NO_PAGE: usize = usize::MAX; -/// The process-global mirror of the cache, tagged with the TSD base of the -/// thread that wrote it. +/// Sixteen direct-mapped ways, indexed by the page number's low bits. /// -/// The per-thread cell stays the authority, but reaching it costs the hot-TLS -/// chain — a global slot-index load, the pthread key, `mrs`, the TSD slot, -/// then the cell — four *dependent* loads on every barrier call, and the -/// profile put the barrier entry's single hottest instruction on that chain. -/// This mirror is read with one `mrs` and two loads that do not depend on -/// each other: if the owner word names the calling thread, the page word is -/// that thread's own most recent write (every path that writes or clears the -/// cell also writes here), so the compare is exactly the cell's; otherwise -/// another thread wrote last and the reader falls back to its cell. +/// The original cache was ONE entry, on the strength of a `batch.ts` +/// simulation whose store pattern was long same-page runs. The ECS +/// component-update rows falsified that shape: each entity's sweep stores +/// into EVERY component column in turn, so the store pages alternate and a +/// single entry misses almost every time — `mark_dirty_old_page_uncached` +/// plus the `DIRTY_OLD_PAGES` thread-local resolution measured 35-40% of +/// both update rows' frames. Low-bit indexing is deliberate: page numbers +/// are `addr >> 12`, so the pages of neighbouring columns land in distinct +/// ways (cf. the dynprop lesson — low-bit XOR folding collapsed keys, but +/// these are sequential page numbers, the one shape low bits are perfect +/// for). /// -/// Why a torn read across the two words is still harmless: a reader can only -/// mis-see a page another thread cached, and heaps are per thread — a slot -/// this thread stores into is never on another thread's page — so the -/// mismatch cannot answer "already dirty" for a page this thread owns. -#[cfg(all( - target_vendor = "apple", - target_arch = "aarch64", - target_pointer_width = "64" -))] -mod mirror { - use std::sync::atomic::{AtomicUsize, Ordering}; +/// The #8949 process-global mirror is retired rather than widened: it +/// existed to shave the dependent-load chain of the SINGLE cell, bought +/// +0.17% then, and a 16-way hot-TLS map supersedes both its mechanism and +/// its reason. +const WAYS: usize = 16; - static OWNER: AtomicUsize = AtomicUsize::new(0); - static PAGE: AtomicUsize = AtomicUsize::new(super::NO_PAGE); - - /// `Some(cached == page)` when the mirror is this thread's, else `None`. - #[inline(always)] - pub(super) fn probe(page: usize) -> Option { - let me = crate::tls_hot::darwin_tsd::base(); - if OWNER.load(Ordering::Relaxed) == me { - Some(PAGE.load(Ordering::Relaxed) == page) - } else { - None - } - } - - #[inline(always)] - pub(super) fn publish(page: usize) { - PAGE.store(page, Ordering::Relaxed); - OWNER.store(crate::tls_hot::darwin_tsd::base(), Ordering::Relaxed); - } - - #[inline(always)] - pub(super) fn clear() { - if OWNER.load(Ordering::Relaxed) == crate::tls_hot::darwin_tsd::base() { - PAGE.store(super::NO_PAGE, Ordering::Relaxed); - } - } -} - -#[cfg(not(all( - target_vendor = "apple", - target_arch = "aarch64", - target_pointer_width = "64" -)))] -mod mirror { - #[inline(always)] - pub(super) fn probe(_page: usize) -> Option { - None - } - #[inline(always)] - pub(super) fn publish(_page: usize) {} - #[inline(always)] - pub(super) fn clear() {} +#[inline(always)] +fn cells() -> &'static [Cell; WAYS] { + &crate::tls_hot::hot().dirty_old_pages } -/// The cache cell: an inline value in this thread's [`crate::tls_hot::HotTls`] -/// — not a `std::thread_local!` (whose `_tlv_get_addr` was ~1% of a 5k-entity -/// ECS frame by itself) and not a generic hot slot either: this is the HIT -/// path of every store the barrier consults (an old bucket taking a young -/// command each push), and the slot's extra dependent load was the measurable -/// part of what the barrier still cost after the parent/child classifications -/// were skipped on a hit. #[inline(always)] -fn cell() -> &'static Cell { - &crate::tls_hot::hot().last_dirty_old_page +fn way(page: usize) -> usize { + page & (WAYS - 1) } /// Is `page` known to be recorded already? See the module invariant. #[inline] pub(super) fn dirty_old_page_already_marked(page: usize) -> bool { debug_assert_ne!(page, NO_PAGE, "page number collides with the empty marker"); - // A stale mirror read (another thread published between the two loads) - // can only answer "not cached" for a page this thread owns — the - // conservative direction — so the cell is not re-consulted on a miss. - if let Some(hit) = mirror::probe(page) { - return hit; - } - cell().get() == page + cells()[way(page)].get() == page } /// Record that `page` is now in `DIRTY_OLD_PAGES` **and** stamped dirty in the /// arena page metadata. Callers must have established both immediately before. #[inline] pub(super) fn note_dirty_old_page_marked(page: usize) { - cell().set(page); - mirror::publish(page); + cells()[way(page)].set(page); } /// Drop the cached page. Called from every path that can remove a page from @@ -192,13 +134,40 @@ pub(super) fn note_dirty_old_page_marked(page: usize) { /// the module doc. Cheap enough (one thread-local store) that these callers do /// not check whether the page they touched is the cached one. pub(crate) fn invalidate() { - cell().set(NO_PAGE); - mirror::clear(); + for way in cells() { + way.set(NO_PAGE); + } } /// Test-only: is the cache currently empty? Lets the #7187 Phase B tests assert /// that an invalidation really happened rather than that nothing broke. #[cfg(test)] pub(super) fn is_empty_for_tests() -> bool { - cell().get() == NO_PAGE + cells().iter().all(|way| way.get() == NO_PAGE) +} + +#[cfg(test)] +mod way_tests { + use super::*; + + /// The regression the ECS update rows exposed: stores alternating between + /// two pages must BOTH stay cached. A one-entry cache thrashed here and + /// sent every store down the uncached path (thread-local resolution plus + /// a hash insert per store). + #[test] + fn alternating_pages_both_hit() { + invalidate(); + let a = 0x1000usize; + let b = 0x1001usize; + note_dirty_old_page_marked(a); + note_dirty_old_page_marked(b); + assert!( + dirty_old_page_already_marked(a), + "first page evicted by the second" + ); + assert!(dirty_old_page_already_marked(b)); + invalidate(); + assert!(!dirty_old_page_already_marked(a)); + assert!(!dirty_old_page_already_marked(b)); + } } diff --git a/crates/perry-runtime/src/gc/tests/dirty_page_cache.rs b/crates/perry-runtime/src/gc/tests/dirty_page_cache.rs index 55de1ac853..aea266e5ab 100644 --- a/crates/perry-runtime/src/gc/tests/dirty_page_cache.rs +++ b/crates/perry-runtime/src/gc/tests/dirty_page_cache.rs @@ -114,12 +114,17 @@ fn test_7187b_repeat_marks_hit_the_cache_and_a_new_page_still_gets_recorded() { assert_eq!(remembered_dirty_page_count(), 2); assert!(old_page_dirty_for(page_a) && old_page_dirty_for(page_b)); - // Returning to the first page misses (one entry), re-marks, and — this is - // the property that matters — does not lose or duplicate anything. + // Returning to the first page HITS: the direct-mapped ways hold both + // pages (adjacent page numbers land in distinct ways), which is exactly + // the alternating-store pattern the one-entry cache thrashed on. The + // completeness property is unchanged — nothing lost, nothing duplicated. let back = unsafe { barriered_young_store(parent, fields, first_index) }; assert_eq!(back, page_a); let counters = take_write_barrier_trace_counters(); - assert_eq!(counters.dirty_page_cache_hits, 0); + assert_eq!( + counters.dirty_page_cache_hits, 1, + "both alternating pages must stay cached" + ); assert_eq!(counters.new_dirty_pages, 0, "page A was already recorded"); assert_eq!(remembered_dirty_page_count(), 2); diff --git a/crates/perry-runtime/src/tls_hot.rs b/crates/perry-runtime/src/tls_hot.rs index c55ac17745..4ce2fde008 100644 --- a/crates/perry-runtime/src/tls_hot.rs +++ b/crates/perry-runtime/src/tls_hot.rs @@ -169,9 +169,12 @@ pub(crate) struct HotTls { /// aarch64 reads and writes it at the fixed byte offset /// [`HOT_TLS_IMPLICIT_THIS_OFFSET`] (see `hot_tls_layout_is_what_codegen_assumes`). pub(crate) implicit_this: Cell, - /// `gc::dirty_page_cache` — the one-entry dirty-page cache - /// (`usize::MAX` = nothing cached). - pub(crate) last_dirty_old_page: Cell, + /// `gc::dirty_page_cache` — the direct-mapped dirty-page cache, indexed + /// by the page number's low bits (`usize::MAX` = way empty). Sixteen ways + /// because real store patterns interleave a handful of pages: an ECS + /// component-update sweep alternates between each component column's + /// current page, which a single entry can never hold. + pub(crate) dirty_old_pages: [Cell; 16], /// `gc::barrier::mark_dirty_external_slot_page` — the last `(page, header)` /// pair recorded in `EXTERNAL_DIRTY_SLOT_PAGES` (`usize::MAX` = none). /// Same invariant discipline as the inline-slot cache: valid exactly while @@ -245,7 +248,7 @@ impl HotTls { learned_inline_fields: std::ptr::null_mut(), temp_roots: std::ptr::null_mut(), implicit_this: Cell::new(crate::value::TAG_UNDEFINED), - last_dirty_old_page: Cell::new(usize::MAX), + dirty_old_pages: [const { Cell::new(usize::MAX) }; 16], last_external_dirty_page: Cell::new(usize::MAX), last_external_dirty_header: Cell::new(usize::MAX), prototype_addrs: [const { Cell::new(usize::MAX) }; INLINE_PROTOTYPE_ADDR_ROWS], @@ -354,26 +357,6 @@ pub(crate) mod darwin_tsd { /// # Safety /// `slot` must be a key returned by `pthread_key_create`, so that the index /// lands inside the thread's TSD array. - /// This thread's TSD base — the per-thread constant [`get`] indexes from, - /// exposed so a hot reader can *identify* the calling thread with one - /// `mrs` and no memory access at all (the write barrier's dirty-page - /// cache mirrors its value under the writing thread's base). Same asm and - /// the same NOT-`pure` discipline as [`get`]: the value must be re-read - /// wherever execution can resume on another thread. - #[inline(always)] - pub(crate) fn base() -> usize { - let base: usize; - // SAFETY: reads a user-readable system register; no memory touched. - unsafe { - core::arch::asm!( - "mrs {b}, tpidrro_el0", - b = out(reg) base, - options(nomem, nostack, preserves_flags) - ); - } - base & !0b111 - } - #[inline(always)] pub(super) unsafe fn get(slot: usize) -> *mut u8 { let base: usize;