diff --git a/changelog.d/8951-external-slot-cache.md b/changelog.d/8951-external-slot-cache.md new file mode 100644 index 0000000000..bed56fa295 --- /dev/null +++ b/changelog.d/8951-external-slot-cache.md @@ -0,0 +1 @@ +- **runtime/gc:** the typed-feedback array-store wrapper forwards straight to the strict store when feedback recording is off (the default); and the external-slot remembered set (`Map`/`Set` entry buffers) gets a one-entry `(page, header)` cache in `HotTls`, the twin of the inline-slot dirty-page cache, so repeated stores into the same map skip the table probe and its header-list scan. diff --git a/crates/perry-runtime/src/gc/barrier/maintenance.rs b/crates/perry-runtime/src/gc/barrier/maintenance.rs index 17b3bc5607..227fe7ca35 100644 --- a/crates/perry-runtime/src/gc/barrier/maintenance.rs +++ b/crates/perry-runtime/src/gc/barrier/maintenance.rs @@ -125,6 +125,7 @@ fn external_dirty_slot_headers_empty() -> bool { } fn clear_one_external_dirty_slot_header() -> bool { + invalidate_external_dirty_slot_cache(); EXTERNAL_DIRTY_SLOT_PAGES.with(|s| { let mut pages = s.borrow_mut(); let Some(page) = pages.keys().next().copied() else { diff --git a/crates/perry-runtime/src/gc/barrier/mod.rs b/crates/perry-runtime/src/gc/barrier/mod.rs index 3d4e716899..de97622911 100644 --- a/crates/perry-runtime/src/gc/barrier/mod.rs +++ b/crates/perry-runtime/src/gc/barrier/mod.rs @@ -1703,7 +1703,24 @@ pub(super) fn ever_dirty_old_page(page: usize) -> bool { pub(super) fn mark_dirty_external_slot_page(header_addr: usize, page: usize) -> bool { bump_write_barrier_trace_counter(BarrierTraceCounter::DirtyPageMarkAttempts); - EXTERNAL_DIRTY_SLOT_PAGES.with(|s| { + // One-entry cache over the (page → headers) table, the external-slot + // twin of `dirty_page_cache`: a `Map`'s entries buffer is an external + // slot span, so `map.set(k, v)` on one map stores into the same page + // under the same header again and again, and each store paid the + // thread-local table probe plus a linear scan of that page's header list + // (which grows with every map whose buffer shares the page). The pair is + // recorded only after the table holds it and cleared wherever the table + // drops a pair (`clear_one_external_dirty_slot_header`), so a hit means + // exactly what the probe would have found. + { + let hot = crate::tls_hot::hot(); + if hot.last_external_dirty_page.get() == page + && hot.last_external_dirty_header.get() == header_addr + { + return false; + } + } + let header_was_new = EXTERNAL_DIRTY_SLOT_PAGES.with(|s| { let mut pages = s.borrow_mut(); let page_was_new = !pages.contains_key(&page); let headers = pages.entry(page).or_insert_with(Vec::new); @@ -1717,7 +1734,19 @@ pub(super) fn mark_dirty_external_slot_page(header_addr: usize, page: usize) -> bump_write_barrier_trace_counter(BarrierTraceCounter::NewDirtyPages); } header_was_new - }) + }); + let hot = crate::tls_hot::hot(); + hot.last_external_dirty_page.set(page); + hot.last_external_dirty_header.set(header_addr); + header_was_new +} + +/// Drop the external-slot pair cache. Called from every path that removes a +/// pair from `EXTERNAL_DIRTY_SLOT_PAGES` — see `mark_dirty_external_slot_page`. +pub(super) fn invalidate_external_dirty_slot_cache() { + let hot = crate::tls_hot::hot(); + hot.last_external_dirty_page.set(usize::MAX); + hot.last_external_dirty_header.set(usize::MAX); } #[inline] diff --git a/crates/perry-runtime/src/gc/tests/barrier.rs b/crates/perry-runtime/src/gc/tests/barrier.rs index 203c53131d..7b2bfcb424 100644 --- a/crates/perry-runtime/src/gc/tests/barrier.rs +++ b/crates/perry-runtime/src/gc/tests/barrier.rs @@ -63,6 +63,53 @@ fn remembered_maintenance_entry_count() -> usize { dirty_old + external_dirty + fallback } +/// The external-slot pair cache answers a repeated `(header, page)` mark +/// without touching the table, records nothing twice, and is dropped when +/// the table drops the pair — so the next mark re-records it. +#[test] +fn external_dirty_slot_pair_cache_mirrors_the_table() { + let _guard = GcTestIsolationGuard::new(); + reset_remembered_set(); + let header = 0x7000_0000usize; + let page = 0x1234usize; + let entries = + || EXTERNAL_DIRTY_SLOT_PAGES.with(|s| s.borrow().values().map(Vec::len).sum::()); + + assert!( + super::super::barrier::mark_dirty_external_slot_page(header, page), + "first mark records the pair" + ); + assert_eq!(entries(), 1); + assert!( + !super::super::barrier::mark_dirty_external_slot_page(header, page), + "cache hit: nothing new" + ); + assert_eq!(entries(), 1, "a hit records nothing"); + // A different header on the same page is a miss that records. + assert!(super::super::barrier::mark_dirty_external_slot_page( + header + 0x100, + page + )); + assert_eq!(entries(), 2); + // Back to the first pair: the table still holds it, so not new — and the + // cache now names the second pair, so this goes through the table. + assert!(!super::super::barrier::mark_dirty_external_slot_page( + header, page + )); + assert_eq!(entries(), 2); + + // Clearing the remembered set drops the pairs and the cache with them: + // the same mark records again. + reset_remembered_set(); + assert_eq!(entries(), 0); + assert!( + super::super::barrier::mark_dirty_external_slot_page(header + 0x100, page), + "re-recorded after the clear" + ); + assert_eq!(entries(), 1); + reset_remembered_set(); +} + #[test] fn incremental_mark_barrier_active_count_tracks_thread_activation() { let _guard = GcTestIsolationGuard::new(); diff --git a/crates/perry-runtime/src/tls_hot.rs b/crates/perry-runtime/src/tls_hot.rs index 0e42ffd6bb..e2977e0bc4 100644 --- a/crates/perry-runtime/src/tls_hot.rs +++ b/crates/perry-runtime/src/tls_hot.rs @@ -166,6 +166,12 @@ pub(crate) struct HotTls { /// `gc::dirty_page_cache` — the one-entry dirty-page cache /// (`usize::MAX` = nothing cached). pub(crate) last_dirty_old_page: Cell, + /// `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 + /// the pair is still recorded, cleared wherever a pair is removed. + pub(crate) last_external_dirty_page: Cell, + pub(crate) last_external_dirty_header: Cell, /// `array::prototype_addr` — this thread's memoized intrinsic prototype /// addresses, `usize::MAX` = not yet computed. Rewritten by the /// collector's root scan like the slot it replaced. @@ -223,6 +229,8 @@ impl HotTls { learned_inline_fields: std::ptr::null_mut(), temp_roots: std::ptr::null_mut(), last_dirty_old_page: Cell::new(usize::MAX), + 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], box_ptr_cache: [const { Cell::new(0) }; INLINE_BOX_PTR_CACHE_SLOTS], i32_box_ptr_cache: [const { Cell::new(0) }; INLINE_BOX_PTR_CACHE_SLOTS], diff --git a/crates/perry-runtime/src/typed_feedback.rs b/crates/perry-runtime/src/typed_feedback.rs index 1a8d9762ba..735cd3a33b 100644 --- a/crates/perry-runtime/src/typed_feedback.rs +++ b/crates/perry-runtime/src/typed_feedback.rs @@ -2554,6 +2554,13 @@ pub extern "C" fn js_typed_feedback_array_set_index_or_string( idx: f64, value: f64, ) -> *mut ArrayHeader { + // #5094 for the assignment site: with recording off (the default) every + // helper below early-returns, but the index conversion and two + // out-of-line calls to reach those returns were 1.5% of an ECS frame on + // `column[index] = record`. One flag test, then the store. + if !typed_feedback_enabled() { + return crate::array::js_array_set_index_or_string_strict(arr, idx, value); + } let index = finite_nonnegative_u32_index(idx).unwrap_or(u32::MAX); observe_array(site_id, arr, index); if index == u32::MAX {