diff --git a/crates/perry-runtime/src/object/field_get_set/ic_miss.rs b/crates/perry-runtime/src/object/field_get_set/ic_miss.rs index 0ce901ebb0..f6571643a4 100644 --- a/crates/perry-runtime/src/object/field_get_set/ic_miss.rs +++ b/crates/perry-runtime/src/object/field_get_set/ic_miss.rs @@ -982,7 +982,31 @@ pub(super) fn get_field_ic_miss_impl( if diag { ic_diag_note(cache_slot, key, R::ObjectNoKeys); } - let value = js_object_get_field_by_name(obj, key); + // #10834 gated its only prime site on `miss_reason == NotOwn`, + // and this arm returns before reaching it. A receiver with no + // keys array has NO own properties at all, so "the key is not + // an own property" holds here MORE strongly than it does for + // `NotOwn` — and this is the single most common inherited-read + // shape there is: `Object.create(p)` with nothing of its own. + // + // Without this the lookup at the top of this function runs on + // every such read, always misses because nothing can ever be + // recorded, and the chain walk proceeds unchanged: measured at + // +106 instructions per read against the same binary with + // `PERRY_INHERITED_IC=0`, i.e. the cache was pure overhead for + // this shape. + if !inherited_declined { + if let Some(value) = unsafe { + crate::object::inherited_read_cache::inherited_read_cache_prime(obj, key) + } { + return f64::from_bits(value.bits()); + } + } + // Past the cache, not through it: the lookup at the top of + // this function has already asked. + let value = super::get_field_by_name::get_field_by_name_past_inherited_cache( + obj, key, + ); return f64::from_bits(value.bits()); } let key_count = shape.logical_key_count as usize; diff --git a/crates/perry-runtime/src/object/inherited_read_cache.rs b/crates/perry-runtime/src/object/inherited_read_cache.rs index 2042092f38..88bc6f1bf3 100644 --- a/crates/perry-runtime/src/object/inherited_read_cache.rs +++ b/crates/perry-runtime/src/object/inherited_read_cache.rs @@ -218,11 +218,24 @@ crate::perry_thread_local! { std::cell::UnsafeCell::new(vec![EMPTY_ENTRY; CACHE_SIZE].into_boxed_slice()); } +/// An entry is identified by (class id, ShapeId, key), so all three have to +/// reach the slot index. +/// +/// #10834 hashed only (shape, key). That is exactly wrong for the receivers +/// this cache exists to serve: `js_object_create` mints a FRESH synthetic +/// class id on every call, so N objects built by `Object.create(p)` have N +/// different class ids and ONE identical shape. Under a (shape, key) index +/// they all landed in the same direct-mapped slot and evicted one another, so +/// a site reading through eight of them primed on EVERY read and hit never: +/// measured `primes=6295655 hits=0` over ten million reads, a full chain walk +/// plus an entry write per read, +75 instructions against the same binary with +/// the cache off. #[inline(always)] -fn entry_index(shape: u32, key_ptr: usize) -> usize { +fn entry_index(class_id: u32, shape: u32, key_ptr: usize) -> usize { // Interned key pointers are 8- or 16-byte aligned, so their low bits are // zeros; fold the middle bits down before masking. - let h = ((key_ptr >> 4) as u64 ^ ((shape as u64) << 21)).wrapping_mul(0x9E37_79B9_7F4A_7C15); + let h = ((key_ptr >> 4) as u64 ^ ((shape as u64) << 21) ^ ((class_id as u64) << 43)) + .wrapping_mul(0x9E37_79B9_7F4A_7C15); (h >> 40) as usize & CACHE_MASK } @@ -395,7 +408,7 @@ pub(crate) unsafe fn inherited_read_cache_lookup( if recv_shape == 0 { return Lookup::Unknown; } - let index = entry_index(recv_shape, key as usize); + let index = entry_index(recv_class_id, recv_shape, key as usize); let entry = INHERITED_READ_CACHE.with(|cell| (*cell.get())[index]); if entry.key_ptr != key as usize || entry.recv_shape != recv_shape @@ -528,7 +541,7 @@ pub(crate) unsafe fn inherited_read_cache_prime( hop_count: note.hop_count, slot: NEGATIVE_SLOT, }; - let index = entry_index(note.recv_shape, note.key_ptr); + let index = entry_index(note.recv_class_id, note.recv_shape, note.key_ptr); INHERITED_READ_CACHE.with(|cell| { (*cell.get())[index] = entry; }); @@ -756,7 +769,7 @@ unsafe fn inherited_read_cache_walk( hop_count: hop_count as u8, slot, }; - let index = entry_index(recv_shape, key_addr); + let index = entry_index(recv_class_id, recv_shape, key_addr); INHERITED_READ_CACHE.with(|cell| { (*cell.get())[index] = entry; }); diff --git a/crates/perry-runtime/src/object/inherited_read_cache_tests.rs b/crates/perry-runtime/src/object/inherited_read_cache_tests.rs index 8e1fe8e6de..f86e11f346 100644 --- a/crates/perry-runtime/src/object/inherited_read_cache_tests.rs +++ b/crates/perry-runtime/src/object/inherited_read_cache_tests.rs @@ -264,6 +264,135 @@ fn set_prototype_of_on_an_interior_prototype_invalidates() { } } +/// Drive the read through the REAL inline-cache entry the compiled code calls, +/// not through `inherited_read_cache_prime` directly. +/// +/// That distinction is the whole point of the two tests below: both defects +/// they pin live in `get_field_ic_miss_impl`'s routing, so a test that calls +/// the cache's own functions cannot see either one. Measured against a build +/// without the fixes, these reads prime zero times (first test) or once per +/// read forever (second), and in both cases the cache is pure overhead — the +/// probe runs on every read, never serves, and the chain walk proceeds +/// unchanged. +unsafe fn read_through_the_inline_cache( + obj: *mut ObjectHeader, + k: *const crate::StringHeader, + slot: &mut crate::object::field_get_set::PicCacheSlot, + site: u64, +) -> f64 { + let bits = crate::value::js_nanbox_pointer(obj as i64).to_bits() as i64; + crate::object::field_get_set::js_object_get_field_ic(bits, k, site, slot) +} + +#[test] +fn a_receiver_with_no_own_keys_is_cached() { + let _scope = PrimeScope::new(); + unsafe { + let proto = crate::object::js_object_alloc(0, 4); + set(proto, "irc_nokeys", 11.0); + // `Object.create(p)` with nothing of its own: the receiver has no keys + // array at all, so the miss handler reports `ObjectNoKeys` rather than + // `NotOwn`. This is the single most common inherited-read shape there + // is, and the prime site was gated on `NotOwn` alone. + let created = crate::object::js_object_create(boxed(proto)); + let obj = crate::value::js_nanbox_get_pointer(created) as *mut ObjectHeader; + let k = key("irc_nokeys"); + let mut slot: crate::object::field_get_set::PicCacheSlot = std::ptr::null_mut(); + for _ in 0..4 { + let v = read_through_the_inline_cache(obj, k, &mut slot, 9001); + assert_eq!(v, 11.0, "the read must still answer correctly"); + } + assert!( + inherited_read_cache_primes() >= 1, + "a keyless receiver never reached the prime, so the cache can never \ + serve this shape and its probe is pure overhead on every read" + ); + assert!( + inherited_read_cache_hits() >= 1, + "primed but never served" + ); + } +} + +#[test] +fn several_object_create_receivers_do_not_evict_each_other() { + let _scope = PrimeScope::new(); + unsafe { + let proto = crate::object::js_object_alloc(0, 4); + set(proto, "irc_shared", 13.0); + // Eight receivers built the same way. `js_object_create` mints a FRESH + // synthetic class id per call, so these have eight DIFFERENT class ids + // and one identical shape — and the slot index hashed only + // (shape, key), so all eight landed in one direct-mapped slot. + let mut objs = Vec::new(); + for i in 0..8 { + let created = crate::object::js_object_create(boxed(proto)); + let o = crate::value::js_nanbox_get_pointer(created) as *mut ObjectHeader; + set(o, "irc_own", i as f64); + objs.push(o); + } + let k = key("irc_shared"); + let mut slot: crate::object::field_get_set::PicCacheSlot = std::ptr::null_mut(); + let rounds = 8; + for _ in 0..rounds { + for o in &objs { + let v = read_through_the_inline_cache(*o, k, &mut slot, 9002); + assert_eq!(v, 13.0, "the read must still answer correctly"); + } + } + // Account for EVERY read rather than bounding the hits, because a + // loose lower bound on hits is what an off-by-one hides in. + let primes = inherited_read_cache_primes(); + let hits = inherited_read_cache_hits(); + let declines = inherited_read_cache_declines(); + let neg = inherited_read_cache_neg_served(); + let reads = (objs.len() * rounds) as u64; + + assert_eq!( + primes, + objs.len() as u64, + "primed {primes} times for {} receivers. Exactly one prime per \ + receiver is the property: more means the entries are evicting \ + each other and every read pays a full chain walk AND an entry \ + write", + objs.len() + ); + + // At most ONE decline, and it is expected rather than tolerated. + // + // The inherited-read cache refuses to record a hop that the + // `[[Prototype]]` install funnel has not marked, and when its walk + // meets an unmarked hop it marks that hop and ABANDONS the walk + // without recording anything (`object::proto_validity`). Marking + // allocates a meta record, which can move the receiver, the hop and + // every address the walk is holding, so nothing it was holding may be + // touched afterwards — abandoning is not a shortcut, it is the only + // safe thing to do once the allocation has happened. + // + // These eight receivers share ONE prototype, so at most one read pays + // that: the first to reach an unmarked hop. Every later read finds it + // marked and primes normally. Without the marking stack in the tree + // this is 0; with it, 1. Both are correct, and the accounting below + // pins the difference to exactly that one read instead of loosening + // the hit count to absorb it. + assert!( + declines <= 1, + "{declines} declines: at most one mark-and-abandon is expected for \ + a single shared prototype" + ); + + assert_eq!( + hits + primes + declines + neg, + reads, + "every read must be exactly one of: served from an entry ({hits}), \ + the walk that recorded one ({primes}), a mark-and-abandon \ + ({declines}), or served from a negative entry ({neg}) — and they \ + sum to {}, not the {reads} reads performed", + hits + primes + declines + neg + ); + } +} + #[test] fn a_null_prototype_receiver_never_primes() { let _scope = PrimeScope::new();