perf(gc): direct-mapped dirty-page cache — sixteen ways instead of one

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`.

crates/perry-runtime/src/gc/dirty_page_cache.rs

@@ -85,120 +85,89 @@ 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<bool> {
-        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<bool> {
-        None
-    }
-    #[inline(always)]
-    pub(super) fn publish(_page: usize) {}
-    #[inline(always)]
-    pub(super) fn clear() {}
+#[inline(always)]
+fn cells() -> &'static [Cell<usize>; 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<usize> {
-    &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
 /// `DIRTY_OLD_PAGES` or un-stamp / discard its arena metadata — see rule 2 in
 /// 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));
+    }
 }

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);
 

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<u64>,
-    /// `gc::dirty_page_cache` — the one-entry dirty-page cache
-    /// (`usize::MAX` = nothing cached).
-    pub(crate) last_dirty_old_page: Cell<usize>,
+    /// `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<usize>; 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;