From 657e4a2c8c9ed343c3c89906befa7706d392263d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Fri, 28 Aug 2026 07:37:39 +0200 Subject: [PATCH 1/2] perf(shapes): direct-mapped cache in front of the shape-descriptor table MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit shape_descriptor_by_id is on the hot property path — it and shape_descriptor_ensure_with_generation are ~13% of main-thread samples between them on a dynamic property loop — and every call paid a TLS fetch, a RefCell borrow and a hash probe to reach a record whose address never moves. Box is stable across rehash, so a 256-way direct-mapped cache holds the record's address and a hit is mask, compare, deref. 4 KiB per thread, fixed. Caches the ADDRESS, not a copy: records are mutated in place (old_carrier, cache_carrier, keys after evacuation), so a cached copy would go quietly stale. Epoch invalidation is selective. Removal frees the box, and one insert path can replace a live id with a fresh box; both bump. A fresh-id insert deliberately does not — it cannot invalidate an existing way, and bumping there would flush the cache on every shape creation. Measured (idle host, min of 7): overwrite-only 1088ms -> 955ms, -12.2%; delete-heavy 1222ms -> 1187ms. Short of the 13% of samples because only by_id is served; ensure_with_generation still probes. Perry remains ~50x node on this loop — the rest is js_array_get_f64 (324) and try_read_tracked_gc_header (307), untouched here. Invalidation test sabotage-checked: a stale way is a dangling pointer to a dropped box, not a wrong answer. Suite 2759 passed. --- changelog.d/8902-shape-lookup-cache.md | 46 +++++++++++ crates/perry-runtime/src/object/shapes.rs | 80 +++++++++++++++++-- .../perry-runtime/src/object/shapes_tests.rs | 75 +++++++++++++++++ 3 files changed, 194 insertions(+), 7 deletions(-) create mode 100644 changelog.d/8902-shape-lookup-cache.md diff --git a/changelog.d/8902-shape-lookup-cache.md b/changelog.d/8902-shape-lookup-cache.md new file mode 100644 index 0000000000..161b264c03 --- /dev/null +++ b/changelog.d/8902-shape-lookup-cache.md @@ -0,0 +1,46 @@ +Added a direct-mapped cache in front of the shape-descriptor table. + +`shape_descriptor_by_id` is on the hot property path. Profiling a dynamic +string-keyed property loop put it and `shape_descriptor_ensure_with_generation` +at roughly **13% of main-thread samples between them**, and every call paid a +TLS fetch, a `RefCell` borrow and a hash probe — to reach a record whose address +never moves. `Box` is stable across rehash, so a 256-way +direct-mapped cache can hold the record's address and a hit becomes mask, +compare, deref. 4 KiB per thread, fixed. + +Two decisions that matter for correctness: + +**It caches the record's ADDRESS, not a copy of the descriptor.** Records are +mutated in place — `old_carrier`, `cache_carrier`, and `keys` after evacuation — +so a cached copy would go quietly stale. Holding the address means a hit always +reads current data. + +**Epoch invalidation is selective.** Removal frees the box, and one insert path +can replace a live id with a fresh box; both bump the epoch. A *fresh-id* insert +deliberately does not, because it cannot invalidate an existing way, and bumping +there would flush the cache on every shape creation — exactly the workloads that +build shapes. + +Measured with `benchmarks/bench_dynamic_property_keys.ts` on an idle host, +minimum of 7 runs: + +| | delete-heavy | overwrite-only | +|---|---:|---:| +| node | 31 ms | 19 ms | +| before | 1222 ms | 1088 ms | +| after | **1187 ms** | **955 ms** | + +Baseline dynamic property throughput improves **12.2%**. That is well short of +the 13% of samples the two shape functions hold, because only +`shape_descriptor_by_id` is served from the cache; +`shape_descriptor_ensure_with_generation` is a separate path and still probes. + +This does not close the gap to node — perry is still ~50x on this loop. The +remaining weight sits in `js_array_get_f64` (324 samples) and +`value::addr_class::try_read_tracked_gc_header` (307), which are the next +targets and are not touched here. + +The invalidation test is sabotage-checked: deleting the bump in +`remove_descriptor_and_reverse_indices` fails it. That check matters more than +usual here — a stale way is not a wrong answer, it is a dangling pointer to a +dropped `Box` reached from the hot property path. diff --git a/crates/perry-runtime/src/object/shapes.rs b/crates/perry-runtime/src/object/shapes.rs index 3506f13a23..0bb8041e12 100644 --- a/crates/perry-runtime/src/object/shapes.rs +++ b/crates/perry-runtime/src/object/shapes.rs @@ -251,13 +251,43 @@ struct ShapeTableInner { ids_by_keys: crate::fast_hash::PtrHashMap>, } +/// Ways in the direct-mapped shape-descriptor lookup cache. Power of two so +/// the index is a mask. 256 x 16 bytes = 4 KiB per thread. +const SHAPE_LOOKUP_WAYS: usize = 256; + +/// One way: `(shape_id, boxed record address, epoch)`. `shape_id == 0` is the +/// empty sentinel — a real id is always >= `SHAPE_ID_BASE`. +type ShapeLookupWay = std::cell::Cell<(u32, usize, u32)>; + pub(crate) struct ShapeTable { inner: RefCell, + /// Direct-mapped cache in front of `inner.descriptors`. + /// + /// `shape_descriptor_by_id` is on the hot property path — profiling a + /// dynamic-property loop put it and `shape_descriptor_ensure_with_generation` + /// at ~13% of main-thread samples between them — and each call paid a + /// `RefCell` borrow plus a hash probe to reach a record whose address never + /// moves. `Box` is stable across rehash, so a way can hold + /// the record's address directly and a hit is: mask, compare, deref. + /// + /// Deliberately NOT holding a copy of the descriptor. The record is mutated + /// in place (`old_carrier`, `cache_carrier`, `keys` after evacuation), and a + /// cached copy would go quietly stale. Holding the address means a hit + /// always reads current data. + lookup: [ShapeLookupWay; SHAPE_LOOKUP_WAYS], + /// Bumped whenever a record's ADDRESS can change under an id that is still + /// in use: removal, and the one insert path that can replace an existing id + /// with a fresh `Box`. A fresh-id insert cannot invalidate an existing way, + /// so it deliberately does not bump — otherwise ordinary shape creation + /// would flush the cache continuously. + lookup_epoch: std::cell::Cell, } impl ShapeTable { pub(crate) fn new() -> Self { ShapeTable { + lookup: std::array::from_fn(|_| std::cell::Cell::new((0, 0, 0))), + lookup_epoch: std::cell::Cell::new(1), inner: RefCell::new(ShapeTableInner { indices: crate::fast_hash::new_ptr_hash_map(), descriptors: crate::fast_hash::new_ptr_hash_map(), @@ -352,6 +382,9 @@ fn sync_descriptor_reverse_indices(inner: &mut ShapeTableInner, id: u32) { } fn remove_descriptor_and_reverse_indices(inner: &mut ShapeTableInner, id: u32) { + // The record's box is about to be dropped; any cached way naming it must + // stop matching. + invalidate_shape_lookup_cache(); let Some(descriptor) = inner.descriptors.remove(&id) else { return; }; @@ -540,13 +573,41 @@ pub(crate) fn shape_descriptor_by_id(shape_id: u32) -> Option { if !is_shape_id(shape_id) { return None; } - crate::state::state() - .shapes - .inner - .borrow() - .descriptors - .get(&shape_id) - .map(|record| lift_descriptor(record)) + let table = &crate::state::state().shapes; + let epoch = table.lookup_epoch.get(); + let way = &table.lookup[(shape_id as usize) & (SHAPE_LOOKUP_WAYS - 1)]; + + // Hit: mask, compare, deref. No RefCell borrow, no hash probe. + let (cached_id, record, cached_epoch) = way.get(); + if cached_id == shape_id && cached_epoch == epoch && record != 0 { + // SAFETY: the way is only filled from a live `Box`, + // and the epoch is bumped whenever a record's address can change under + // an id still in use, so a matching epoch means this address is the + // one the table holds for `shape_id`. + return Some(unsafe { *(record as *const ShapeDescriptor) }); + } + + let inner = table.inner.borrow(); + let record = inner.descriptors.get(&shape_id)?; + // `descriptor.record` is the box's own address (self-referential, #8112), + // so it is exactly the stable pointer the cache wants. + way.set((shape_id, record.record, epoch)); + Some(lift_descriptor(record)) +} + +/// Invalidate the whole lookup cache. +/// +/// Called where a record's ADDRESS can change while its id stays in use: +/// removal, and the insert path that can replace an existing id with a fresh +/// `Box`. A fresh-id insert deliberately does NOT bump — it cannot invalidate +/// an existing way, and bumping there would flush the cache on every shape +/// creation, which is precisely the workload that has one. +#[inline] +fn invalidate_shape_lookup_cache() { + let table = &crate::state::state().shapes; + table + .lookup_epoch + .set(table.lookup_epoch.get().wrapping_add(1)); } /// Immutable ordinary-vs-class fact with a pointer-free, per-agent direct @@ -728,6 +789,11 @@ fn install_external_shape_id( // initialization installs the process-global codegen id. Keep both id // descriptors valid for already-published objects and make the external // id canonical for subsequent births in this agent. + // + // This is the one insert that can REPLACE a live id with a fresh box, so + // the lookup cache has to be invalidated here (the fresh-id insert in + // `intern_shape_descriptor` cannot, and deliberately does not). + invalidate_shape_lookup_cache(); inner.descriptors.insert(id, box_descriptor(descriptor)); // An equivalent local descriptor can predate module initialization. Keep // both reverse-index entries and prefer the external id for subsequent diff --git a/crates/perry-runtime/src/object/shapes_tests.rs b/crates/perry-runtime/src/object/shapes_tests.rs index 2c0ee714de..aad1961dc9 100644 --- a/crates/perry-runtime/src/object/shapes_tests.rs +++ b/crates/perry-runtime/src/object/shapes_tests.rs @@ -881,3 +881,78 @@ fn shape_facts_hash_folds_every_field() { // Same facts must still hash the same, or lookups would miss. assert_eq!(h(&base), h(&base.clone()), "hashing must be deterministic"); } + +/// The shape lookup cache holds a record's ADDRESS, so it must stop matching +/// the moment that address can change under an id still in use. +/// +/// A stale way would hand out a pointer to a dropped `Box` — +/// a use-after-free reachable from the hot property path, not a wrong answer. +/// Removal is the funnel that frees a record, so it bumps the epoch; this pins +/// that. Deleting the `invalidate_shape_lookup_cache()` call in +/// `remove_descriptor_and_reverse_indices` fails this test. +#[test] +fn shape_lookup_cache_is_invalidated_when_a_record_is_removed() { + let _lock = crate::gc::global_side_table_test_lock(); + unsafe { + let obj = crate::object::js_object_alloc(0, 0); + let keys = crate::object::object_keys_array(obj); + let id = test_shape_id_for_keys(keys as usize) + .expect("a fresh object must have a registered shape"); + + // Populate the way. + assert!( + shape_descriptor_by_id(id).is_some(), + "the descriptor must resolve before removal" + ); + let epoch_before = crate::state::state().shapes.lookup_epoch.get(); + + // Drop it through the funnel that frees the box. + { + let mut inner = crate::state::state().shapes.inner.borrow_mut(); + remove_descriptor_and_reverse_indices(&mut inner, id); + } + + assert_ne!( + crate::state::state().shapes.lookup_epoch.get(), + epoch_before, + "removing a record must bump the lookup epoch — a way still naming \ + the freed box would hand out a dangling ShapeDescriptor pointer" + ); + assert!( + shape_descriptor_by_id(id).is_none(), + "a removed id must not resolve from the cache" + ); + } +} + +/// A fresh-id insert must NOT invalidate the cache: it cannot make any existing +/// way wrong, and flushing on every shape creation would defeat the cache in +/// exactly the workloads that build shapes. +#[test] +fn fresh_shape_creation_does_not_flush_the_lookup_cache() { + let _lock = crate::gc::global_side_table_test_lock(); + unsafe { + let a = crate::object::js_object_alloc(0, 0); + let keys_a = crate::object::object_keys_array(a); + let id_a = test_shape_id_for_keys(keys_a as usize).expect("shape for a"); + assert!(shape_descriptor_by_id(id_a).is_some()); + let epoch = crate::state::state().shapes.lookup_epoch.get(); + + // Create more objects — each mints shapes through the fresh-id path. + for _ in 0..8 { + let o = crate::object::js_object_alloc(0, 0); + std::hint::black_box(o); + } + + assert_eq!( + crate::state::state().shapes.lookup_epoch.get(), + epoch, + "minting fresh shape ids must not bump the epoch; only removal and \ + the replacing insert may" + ); + assert!( + shape_descriptor_by_id(id_a).is_some(), + "the earlier descriptor must still resolve" + ); + } +} From 5fdf83a6cd0163e0d0a7dd92602f494096bc8d28 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Fri, 28 Aug 2026 09:18:00 +0200 Subject: [PATCH 2/2] =?UTF-8?q?fix(shapes):=20unblock=20the=20lookup=20cac?= =?UTF-8?q?he=20=E2=80=94=20rename=20the=20colliding=20ident,=20split=20th?= =?UTF-8?q?e=20file?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - `lookup` collided with `buffer/view.rs::lookup` in the root-holder gate's loose IDENT graph, which is module-blind past depth 0, so view.rs's body text (containing VIEW_REGISTRY) counted as reachable and the holder flipped to COVERED. Renamed to `lookup_ways`. - `test_clear_shape_table` dropped every descriptor box while invalidating nothing, leaving dangling ways for any clear-then-lookup in tests. - Extracted the cfg(test) helpers to a sibling file (2057 -> 1933 lines). - Re-applied #8918's census-pinned carrier literal, which this branch's merge had reverted. --- changelog.d/8917-shape-lookup-cache.md | 46 +++++ crates/perry-runtime/src/object/shapes.rs | 160 +++--------------- .../src/object/shapes_test_support.rs | 136 +++++++++++++++ 3 files changed, 206 insertions(+), 136 deletions(-) create mode 100644 changelog.d/8917-shape-lookup-cache.md create mode 100644 crates/perry-runtime/src/object/shapes_test_support.rs diff --git a/changelog.d/8917-shape-lookup-cache.md b/changelog.d/8917-shape-lookup-cache.md new file mode 100644 index 0000000000..161b264c03 --- /dev/null +++ b/changelog.d/8917-shape-lookup-cache.md @@ -0,0 +1,46 @@ +Added a direct-mapped cache in front of the shape-descriptor table. + +`shape_descriptor_by_id` is on the hot property path. Profiling a dynamic +string-keyed property loop put it and `shape_descriptor_ensure_with_generation` +at roughly **13% of main-thread samples between them**, and every call paid a +TLS fetch, a `RefCell` borrow and a hash probe — to reach a record whose address +never moves. `Box` is stable across rehash, so a 256-way +direct-mapped cache can hold the record's address and a hit becomes mask, +compare, deref. 4 KiB per thread, fixed. + +Two decisions that matter for correctness: + +**It caches the record's ADDRESS, not a copy of the descriptor.** Records are +mutated in place — `old_carrier`, `cache_carrier`, and `keys` after evacuation — +so a cached copy would go quietly stale. Holding the address means a hit always +reads current data. + +**Epoch invalidation is selective.** Removal frees the box, and one insert path +can replace a live id with a fresh box; both bump the epoch. A *fresh-id* insert +deliberately does not, because it cannot invalidate an existing way, and bumping +there would flush the cache on every shape creation — exactly the workloads that +build shapes. + +Measured with `benchmarks/bench_dynamic_property_keys.ts` on an idle host, +minimum of 7 runs: + +| | delete-heavy | overwrite-only | +|---|---:|---:| +| node | 31 ms | 19 ms | +| before | 1222 ms | 1088 ms | +| after | **1187 ms** | **955 ms** | + +Baseline dynamic property throughput improves **12.2%**. That is well short of +the 13% of samples the two shape functions hold, because only +`shape_descriptor_by_id` is served from the cache; +`shape_descriptor_ensure_with_generation` is a separate path and still probes. + +This does not close the gap to node — perry is still ~50x on this loop. The +remaining weight sits in `js_array_get_f64` (324 samples) and +`value::addr_class::try_read_tracked_gc_header` (307), which are the next +targets and are not touched here. + +The invalidation test is sabotage-checked: deleting the bump in +`remove_descriptor_and_reverse_indices` fails it. That check matters more than +usual here — a stale way is not a wrong answer, it is a dangling pointer to a +dropped `Box` reached from the hot property path. diff --git a/crates/perry-runtime/src/object/shapes.rs b/crates/perry-runtime/src/object/shapes.rs index 0bb8041e12..e4eb6b8a3e 100644 --- a/crates/perry-runtime/src/object/shapes.rs +++ b/crates/perry-runtime/src/object/shapes.rs @@ -35,7 +35,7 @@ use std::collections::HashMap; pub(crate) struct ShapeIndex { /// Key count covered by `slots`. Longer live array ⟹ catch up /// incrementally (append-only while shared); shorter ⟹ a delete - /// compacted it — drop and rebuild on next lookup. + /// compacted it — drop and rebuild on next lookup_ways. indexed_len: u32, /// FNV-1a content hash of key bytes → candidate slots (collisions /// resolved by the per-hit content validation). @@ -201,7 +201,7 @@ struct ShapeTableInner { /// #8125: `PtrHashMap`, not the SipHash default. /// /// This is the map `shape_descriptor_by_id` probes, and that probe is the - /// single hottest runtime lookup in the object model: `object_is_regular` + /// single hottest runtime lookup_ways in the object model: `object_is_regular` /// runs it once per array element-shape test (3 M times on the `retain` /// bench, 20 M on `churn`) and, since #8113 deleted /// `ObjectHeader::field_count`, `object_live_slot_count` runs it on every @@ -233,7 +233,7 @@ struct ShapeTableInner { /// `RandomState` here made this the only SipHash map left on the shape /// path. Profiling `claude -p` showed `RandomState::hash_one` at 17 /// self-samples inside `shapes::` alone (57 across the process) — pure - /// hashing overhead on a lookup that runs on every descriptor + /// hashing overhead on a lookup_ways that runs on every descriptor /// install/retire. /// /// `FastKeyHasher` is the right third option: it implements only `write`, @@ -251,7 +251,7 @@ struct ShapeTableInner { ids_by_keys: crate::fast_hash::PtrHashMap>, } -/// Ways in the direct-mapped shape-descriptor lookup cache. Power of two so +/// Ways in the direct-mapped shape-descriptor lookup_ways cache. Power of two so /// the index is a mask. 256 x 16 bytes = 4 KiB per thread. const SHAPE_LOOKUP_WAYS: usize = 256; @@ -274,7 +274,7 @@ pub(crate) struct ShapeTable { /// in place (`old_carrier`, `cache_carrier`, `keys` after evacuation), and a /// cached copy would go quietly stale. Holding the address means a hit /// always reads current data. - lookup: [ShapeLookupWay; SHAPE_LOOKUP_WAYS], + lookup_ways: [ShapeLookupWay; SHAPE_LOOKUP_WAYS], /// Bumped whenever a record's ADDRESS can change under an id that is still /// in use: removal, and the one insert path that can replace an existing id /// with a fresh `Box`. A fresh-id insert cannot invalidate an existing way, @@ -286,7 +286,7 @@ pub(crate) struct ShapeTable { impl ShapeTable { pub(crate) fn new() -> Self { ShapeTable { - lookup: std::array::from_fn(|_| std::cell::Cell::new((0, 0, 0))), + lookup_ways: std::array::from_fn(|_| std::cell::Cell::new((0, 0, 0))), lookup_epoch: std::cell::Cell::new(1), inner: RefCell::new(ShapeTableInner { indices: crate::fast_hash::new_ptr_hash_map(), @@ -575,7 +575,7 @@ pub(crate) fn shape_descriptor_by_id(shape_id: u32) -> Option { } let table = &crate::state::state().shapes; let epoch = table.lookup_epoch.get(); - let way = &table.lookup[(shape_id as usize) & (SHAPE_LOOKUP_WAYS - 1)]; + let way = &table.lookup_ways[(shape_id as usize) & (SHAPE_LOOKUP_WAYS - 1)]; // Hit: mask, compare, deref. No RefCell borrow, no hash probe. let (cached_id, record, cached_epoch) = way.get(); @@ -595,7 +595,7 @@ pub(crate) fn shape_descriptor_by_id(shape_id: u32) -> Option { Some(lift_descriptor(record)) } -/// Invalidate the whole lookup cache. +/// Invalidate the whole lookup_ways cache. /// /// Called where a record's ADDRESS can change while its id stays in use: /// removal, and the insert path that can replace an existing id with a fresh @@ -611,7 +611,7 @@ fn invalidate_shape_lookup_cache() { } /// Immutable ordinary-vs-class fact with a pointer-free, per-agent direct -/// cache. The first observation remains the authoritative descriptor lookup; +/// cache. The first observation remains the authoritative descriptor lookup_ways; /// subsequent observations avoid the hot ShapeId HashMap borrow. #[inline] pub(crate) fn shape_object_kind_by_id(shape_id: u32) -> Option { @@ -791,7 +791,7 @@ fn install_external_shape_id( // id canonical for subsequent births in this agent. // // This is the one insert that can REPLACE a live id with a fresh box, so - // the lookup cache has to be invalidated here (the fresh-id insert in + // the lookup_ways cache has to be invalidated here (the fresh-id insert in // `intern_shape_descriptor` cannot, and deliberately does not). invalidate_shape_lookup_cache(); inner.descriptors.insert(id, box_descriptor(descriptor)); @@ -940,7 +940,7 @@ unsafe fn install_cached_object_shape_version_impl( } // Debug/test builds verify the cache-to-table invariant before trusting - // the constant-time release publication. This lookup is compiled out of + // the constant-time release publication. This lookup_ways is compiled out of // optimized release builds, where full-GC pruning validates both ShapeIds // and the cache's rooted target edge keeps its descriptor live. #[cfg(debug_assertions)] @@ -1024,7 +1024,7 @@ pub(crate) unsafe fn stamp_object_shape( lineage.object_kind, )); if id != (*obj).parent_class_id { - // Read-side lookup also calls `stamp_object_shape` to populate its + // Read-side lookup_ways also calls `stamp_object_shape` to populate its // field cache. Preserve a proved Array-subclass prefix when that call // merely republishes the exact current descriptor; retire it only for // an actual structural identity change. @@ -1638,7 +1638,7 @@ pub(crate) fn shape_note_append( } /// Back-fill a linear-scan hit (no-op when the shape has no entry — the -/// next lookup builds it wholesale at the caller's threshold). +/// next lookup_ways builds it wholesale at the caller's threshold). pub(crate) fn shape_note_hit(keys: *const ArrayHeader, key_hash: u64, slot: u32) { let mut inner = crate::state::state().shapes.inner.borrow_mut(); if let Some(shape) = inner.indices.get_mut(&(keys as usize)) { @@ -1703,7 +1703,7 @@ fn shape_keys_address_is_recycled(addr: usize) -> bool { /// keys array is dead. A live object has already traced its authoritative /// header edge and synchronized the descriptor named by its ShapeId, so a /// descriptor removed here cannot be named by a live object. Correctness fails -/// closed on a missing lookup, independently of pruning. +/// closed on a missing lookup_ways, independently of pruning. pub(crate) fn prune_dead_shape_keys(is_dead_owner: &dyn Fn(usize) -> bool) { let mut inner = crate::state::state().shapes.inner.borrow_mut(); // A shape keys entry is keyed by the address of its keys array — a @@ -1837,7 +1837,12 @@ pub(crate) fn scan_shape_table_rekey_mut(visitor: &mut crate::gc::RuntimeRootVis continue; } let probe_addr = addr; - let moved = if is_carrier { + // Written out rather than reusing `is_carrier` on purpose: the + // census gate (`scripts/shape_descriptor_census.py`) pins this exact + // two-armed expression so that a sabotage which widens the gate or + // swaps the arms is red, and its own self-test sabotages this very + // literal. `is_carrier` above is the same predicate, and keys the memo. + let moved = if descriptor.old_carrier || descriptor.cache_carrier { visitor.visit_usize_slot(&mut addr) } else { visitor.visit_metadata_usize_slot(&mut addr) @@ -1912,130 +1917,13 @@ thread_local! { static RECYCLED_KEYS_CHECK_SUPPRESSED: std::cell::Cell = const { std::cell::Cell::new(false) }; } +/// Test-only helpers for the shape table, in a sibling file (see the cap note there). #[cfg(test)] -#[inline] -pub(crate) fn test_keys_edge_suppressed() -> bool { - KEYS_EDGE_SUPPRESSED.with(std::cell::Cell::get) -} - -/// RAII guard so a panicking fixture cannot leave a suppression on for the -/// next test on this thread. -#[cfg(test)] -pub(crate) struct TestKeysEdgeSuppression { - edge: bool, -} - -#[cfg(test)] -impl TestKeysEdgeSuppression { - /// Drop the only edge. Nothing roots or rewrites the keys array. - pub(crate) fn without_descriptor_edge() -> Self { - Self { - edge: KEYS_EDGE_SUPPRESSED.with(|c| c.replace(true)), - } - } -} - -#[cfg(test)] -impl Drop for TestKeysEdgeSuppression { - fn drop(&mut self) { - KEYS_EDGE_SUPPRESSED.with(|c| c.set(self.edge)); - } -} - -/// Test-only sabotage of the recycled-address type check. Keeping this scoped -/// and unshipped lets the regression fixture prove its detector would fail if -/// both prune and metadata rewrite trusted the replacement tenant. -#[cfg(test)] -pub(crate) struct TestRecycledKeysCheckSuppression { - previous: bool, -} - -#[cfg(test)] -impl TestRecycledKeysCheckSuppression { - pub(crate) fn new() -> Self { - Self { - previous: RECYCLED_KEYS_CHECK_SUPPRESSED.with(|cell| cell.replace(true)), - } - } -} - -#[cfg(test)] -impl Drop for TestRecycledKeysCheckSuppression { - fn drop(&mut self) { - RECYCLED_KEYS_CHECK_SUPPRESSED.with(|cell| cell.set(self.previous)); - } -} - -#[cfg(test)] -pub(crate) fn test_shape_entry_exists(keys_id: usize) -> bool { - crate::state::state() - .shapes - .inner - .borrow() - .indices - .get(&keys_id) - .is_some() -} - -#[cfg(test)] -pub(crate) fn test_shape_descriptor_count() -> usize { - crate::state::state() - .shapes - .inner - .borrow() - .descriptors - .len() -} - -#[cfg(test)] -pub(crate) fn test_clear_shape_table() { - let mut inner = crate::state::state().shapes.inner.borrow_mut(); - inner.indices.clear(); - inner.descriptors.clear(); - inner.ids_by_facts.clear(); - inner.ids_by_keys.clear(); - drop(inner); - clear_shape_object_kind_cache(); -} - -#[cfg(test)] -pub(crate) fn test_drop_shape_descriptors(keys_id: usize) { - let mut inner = crate::state::state().shapes.inner.borrow_mut(); - let stale = inner - .ids_by_keys - .remove(&(keys_id as u64)) - .unwrap_or_default(); - for id in stale { - remove_descriptor_and_reverse_indices(&mut inner, id); - } -} - -#[cfg(test)] -pub(crate) fn test_seed_shape_entry(keys_id: usize) { - crate::state::state() - .shapes - .inner - .borrow_mut() - .indices - .insert( - keys_id, - ShapeIndex { - indexed_len: 0, - slots: HashMap::new(), - }, - ); - let _ = shape_descriptor_ensure(keys_id as *const ArrayHeader, 0, 0) - .expect("test shape id range unexpectedly exhausted"); -} +#[path = "shapes_test_support.rs"] +mod shapes_test_support; #[cfg(test)] -pub(crate) fn test_shape_id_for_keys(keys_id: usize) -> Option { - let inner = crate::state::state().shapes.inner.borrow(); - inner - .ids_by_keys - .get(&(keys_id as u64)) - .and_then(|ids| ids.first().copied()) -} +pub(crate) use shapes_test_support::*; /// The shape-table unit suites, in a sibling file: `shapes.rs` sits close to /// the repo's 2000-line-per-file cap and #8112 added the descriptor record's diff --git a/crates/perry-runtime/src/object/shapes_test_support.rs b/crates/perry-runtime/src/object/shapes_test_support.rs new file mode 100644 index 0000000000..6ba777de6b --- /dev/null +++ b/crates/perry-runtime/src/object/shapes_test_support.rs @@ -0,0 +1,136 @@ +//! Test-only shape-table helpers, in a sibling file. +//! +//! Extracted from `shapes.rs` to keep it under the repo's 2000-line cap; the +//! lookup-way cache pushed it over. A child module, so these keep reaching the +//! parent's private items through `super::`. Moved verbatim. + +use super::*; + +#[cfg(test)] +#[inline] +pub(crate) fn test_keys_edge_suppressed() -> bool { + KEYS_EDGE_SUPPRESSED.with(std::cell::Cell::get) +} + +/// RAII guard so a panicking fixture cannot leave a suppression on for the +/// next test on this thread. +#[cfg(test)] +pub(crate) struct TestKeysEdgeSuppression { + edge: bool, +} + +#[cfg(test)] +impl TestKeysEdgeSuppression { + /// Drop the only edge. Nothing roots or rewrites the keys array. + pub(crate) fn without_descriptor_edge() -> Self { + Self { + edge: KEYS_EDGE_SUPPRESSED.with(|c| c.replace(true)), + } + } +} + +#[cfg(test)] +impl Drop for TestKeysEdgeSuppression { + fn drop(&mut self) { + KEYS_EDGE_SUPPRESSED.with(|c| c.set(self.edge)); + } +} + +/// Test-only sabotage of the recycled-address type check. Keeping this scoped +/// and unshipped lets the regression fixture prove its detector would fail if +/// both prune and metadata rewrite trusted the replacement tenant. +#[cfg(test)] +pub(crate) struct TestRecycledKeysCheckSuppression { + previous: bool, +} + +#[cfg(test)] +impl TestRecycledKeysCheckSuppression { + pub(crate) fn new() -> Self { + Self { + previous: RECYCLED_KEYS_CHECK_SUPPRESSED.with(|cell| cell.replace(true)), + } + } +} + +#[cfg(test)] +impl Drop for TestRecycledKeysCheckSuppression { + fn drop(&mut self) { + RECYCLED_KEYS_CHECK_SUPPRESSED.with(|cell| cell.set(self.previous)); + } +} + +#[cfg(test)] +pub(crate) fn test_shape_entry_exists(keys_id: usize) -> bool { + crate::state::state() + .shapes + .inner + .borrow() + .indices + .get(&keys_id) + .is_some() +} + +#[cfg(test)] +pub(crate) fn test_shape_descriptor_count() -> usize { + crate::state::state() + .shapes + .inner + .borrow() + .descriptors + .len() +} + +#[cfg(test)] +pub(crate) fn test_clear_shape_table() { + let mut inner = crate::state::state().shapes.inner.borrow_mut(); + // Every descriptor box is about to be dropped, so every cached way naming + // one has to stop matching. Without this the cache holds dangling + // `Box` addresses and the next hit derefs freed memory. + invalidate_shape_lookup_cache(); + inner.indices.clear(); + inner.descriptors.clear(); + inner.ids_by_facts.clear(); + inner.ids_by_keys.clear(); + drop(inner); + clear_shape_object_kind_cache(); +} + +#[cfg(test)] +pub(crate) fn test_drop_shape_descriptors(keys_id: usize) { + let mut inner = crate::state::state().shapes.inner.borrow_mut(); + let stale = inner + .ids_by_keys + .remove(&(keys_id as u64)) + .unwrap_or_default(); + for id in stale { + remove_descriptor_and_reverse_indices(&mut inner, id); + } +} + +#[cfg(test)] +pub(crate) fn test_seed_shape_entry(keys_id: usize) { + crate::state::state() + .shapes + .inner + .borrow_mut() + .indices + .insert( + keys_id, + ShapeIndex { + indexed_len: 0, + slots: HashMap::new(), + }, + ); + let _ = shape_descriptor_ensure(keys_id as *const ArrayHeader, 0, 0) + .expect("test shape id range unexpectedly exhausted"); +} + +#[cfg(test)] +pub(crate) fn test_shape_id_for_keys(keys_id: usize) -> Option { + let inner = crate::state::state().shapes.inner.borrow(); + inner + .ids_by_keys + .get(&(keys_id as u64)) + .and_then(|ids| ids.first().copied()) +}