diff --git a/changelog.d/9029-object-tombstone-deletes.md b/changelog.d/9029-object-tombstone-deletes.md new file mode 100644 index 0000000000..d48d0429c5 --- /dev/null +++ b/changelog.d/9029-object-tombstone-deletes.md @@ -0,0 +1,44 @@ +O(1) object deletes via tombstones, flag-gated (`PERRY_OBJECT_TOMBSTONES=1`, +default OFF). With the flag on, `bench_populated_delete` drops +2089 → **1050 ms (−50%)**, with the combined overwrite and realistic-name-read +loops unchanged. + +`delete obj[k]` on an OWNED keys array (no `GC_FLAG_SHAPE_SHARED` — the same +authority two existing call sites already trust for the clone-or-mutate +decision) writes a hole marker over the key slot and clears the value through +the barriered stores, exactly #9020's Map-tombstone idiom. Survivors keep +their slots: no value shift, no layout rebuild, no index shift, no live-bound +update. Tombstones are squeezed out when they reach half the slots, one +overlap-safe pass mirroring `compact_map_entries`. + +**Shape identity per delete is forced, and its lifecycle is the hard part.** +The per-site dyn-IC ways live in generated-code globals the runtime cannot +reach, so retiring a deleted key's cached `(token, key) → slot` entries +requires changing the token itself: every hole-delete publishes a successor id +(fresh semantic generation + `hole_count`, now part of `ShapeFacts` identity +and covered by the facts-exhaustiveness test). The first version left every +predecessor id alive against ONE stable array address — the reverse-index list +grew per delete and every publish walked it, measuring **26× slower** than the +compacting delete. Retiring just the direct predecessor halved that; +delete-then-re-add cycles also mint an id on the APPEND side, so the publish +now sweeps EVERY stale id for the owned address (each is unreachable the +moment the header word is restamped; single owner by the same flag the gate +trusts). With the sweep the flag-on path is 2× faster than flag-off. + +Walkers: `js_object_keys`' raw-push fast path, `getOwnPropertyNames`' walk, +and `JSON.stringify`'s field walk skip the marker explicitly. Note +`js_array_get` translates `TAG_HOLE` to `undefined` per OrdinaryGet (#323), so +key walks reading through it must skip BOTH forms — comparing `TAG_HOLE` alone +was dead code and let holes reach output as JSON `null`; `undefined` is never +a legal key, so the two-form skip is safe. Every other enumeration path +resolves keys through `js_string_key_bytes`, which rejects the marker. + +Verification: suite 2799 passed, all 60 lint gates pass. Four differentials +byte-identical to node in BOTH flag states: the enumeration suite +(keys/values/entries/for-in/stringify/spread/rest across interleaved deletes, +re-adds, threshold crossings, and overflow-slot objects — this suite caught +the getOwnPropertyNames and hole-canonicalization bugs), the adversarial +property suite, the computed-key suite, and the stale-slot suite. + +Remaining flag-on cost is the per-delete publish/sweep machinery (~5 µs/op vs +node's 0.1); reducing that is the follow-up before default-on. diff --git a/crates/perry-runtime/src/json/stringify.rs b/crates/perry-runtime/src/json/stringify.rs index 5979836d49..dccb80d460 100644 --- a/crates/perry-runtime/src/json/stringify.rs +++ b/crates/perry-runtime/src/json/stringify.rs @@ -1244,6 +1244,10 @@ pub(crate) unsafe fn stringify_object_inner(ptr: *const u8, buf: &mut String, de }; for j in 0..actual_fields { let f = pos(j); + // Tombstoned slot from an O(1) delete: not a key, not serialized. + if key_at(f).to_bits() == crate::value::TAG_HOLE { + continue; + } // Private elements (`#x`) live in a class instance's keys_array but are // not serializable own properties. (`has_prototype_chain` == class_id != 0.) if has_prototype_chain diff --git a/crates/perry-runtime/src/object/delete_rest.rs b/crates/perry-runtime/src/object/delete_rest.rs index 38faaf6301..ac5044ab87 100644 --- a/crates/perry-runtime/src/object/delete_rest.rs +++ b/crates/perry-runtime/src/object/delete_rest.rs @@ -348,6 +348,60 @@ pub extern "C" fn js_object_delete_field( let keys_gc_header = (keys as *const u8).sub(crate::gc::GC_HEADER_SIZE) as *const crate::gc::GcHeader; let keys_owned = (*keys_gc_header).gc_flags & crate::gc::GC_FLAG_SHAPE_SHARED == 0; + // O(1) tombstone delete (flag-gated, #9020's Map pattern applied to + // objects). An OWNED keys array can take a hole marker in place of + // the deleted key: survivors keep their slots, so nothing shifts, no + // layout rebuilds, and the live inline-slot bound is untouched. The + // shape publish mints a fresh semantic generation, which is what + // retires every cached (token, key) pair for this receiver — a + // deleted key must stop hitting even though the array address and + // every surviving slot are byte-identical, or a stale IC hit would + // return the cleared slot instead of walking the prototype chain. + // The read-plan epoch was already bumped at this function's entry. + // + // Tombstones are squeezed out when they reach half the slots (the + // Map threshold), which amortizes compaction to O(1) per delete and + // bounds the array at 2x its live size. + if keys_owned && object_tombstone_deletes_enabled() { + let holes = super::shapes::object_shape_hole_count(obj); + let threshold_hit = key_count >= 16 && (holes + 1) * 2 > key_count as u32; + if !threshold_hit { + let successor = super::shapes::publish_object_shape_holes(obj, holes + 1); + if successor != 0 { + let elements = (keys as *mut u8).add(std::mem::size_of::()) + as *mut f64; + // Barriered stores, exactly the Map delete's idiom: the + // hole overwrites a key POINTER and the clear overwrites + // the value, so SATB marking must shade both children. + crate::gc::runtime_store_external_jsvalue_slot( + keys as usize, + elements.add(i) as usize, + crate::value::TAG_HOLE, + ); + if i < alloc_limit { + let fields_ptr = + (obj as *mut u8).add(std::mem::size_of::()) as *mut u64; + crate::gc::runtime_store_jsvalue_slot( + obj as usize, + fields_ptr.add(i) as usize, + i, + crate::value::TAG_UNDEFINED, + ); + } else { + overflow_set(obj as usize, i, crate::value::TAG_UNDEFINED); + } + return 1; + } + // Unstamped/unshaped receiver: fall through to the + // compacting delete below, which needs no shape stamp. + } else { + // Threshold: squeeze every hole plus this key in one pass, + // then continue through the ordinary compaction bookkeeping + // is unnecessary — the squeeze does its own. + squeeze_holes_and_delete(obj, keys, i, key_count, alloc_limit, field_count); + return 1; + } + } let index_migrated = if keys_owned { let elements = (keys as *mut u8).add(std::mem::size_of::()) as *mut f64; @@ -1084,3 +1138,82 @@ mod sso_tests_1781 { } } } + +/// Gate for O(1) tombstone deletes (`PERRY_OBJECT_TOMBSTONES=1`). Default OFF +/// while the walker audit and differentials bake; the sibling Map tombstones +/// (#9020) shipped default-on after the same sequence. +fn object_tombstone_deletes_enabled() -> bool { + static ON: std::sync::OnceLock = std::sync::OnceLock::new(); + *ON.get_or_init(|| { + matches!( + std::env::var("PERRY_OBJECT_TOMBSTONES").as_deref(), + Ok("1") | Ok("on") | Ok("true") + ) + }) +} + +/// Threshold compaction for a tombstoned keys array: squeeze every hole AND +/// the key at `delete_slot` out in one overlap-safe pass, values moved to +/// match, then republish layout, live bound and shape. The cost equals what +/// ONE pre-tombstone delete paid, amortized over the deletes that created +/// the holes — `compact_map_entries`' argument, applied to objects. +/// +/// # Safety +/// `obj` live and owned `keys` as its current keys array; `delete_slot < +/// key_count`; caller already bumped the read-plan epoch. +unsafe fn squeeze_holes_and_delete( + obj: *mut ObjectHeader, + keys: *const crate::ArrayHeader, + delete_slot: usize, + key_count: usize, + alloc_limit: usize, + field_count: u32, +) { + let keys = keys as *mut crate::ArrayHeader; + let elements = (keys as *mut u8).add(std::mem::size_of::()) as *mut f64; + let fields_ptr = (obj as *mut u8).add(std::mem::size_of::()) as *mut u64; + let mut out = 0usize; + for s in 0..key_count { + let kv = std::ptr::read(elements.add(s)); + if s == delete_slot || kv.to_bits() == crate::value::TAG_HOLE { + continue; + } + if out != s { + // Keys move DOWN within one buffer (out < s always) — same + // overlap argument as `compact_map_entries`. + // GC_STORE_AUDIT(EXTERNAL_BARRIERED): the dirty-span barrier after + // this loop covers every surviving key slot written here, exactly + // as compact_map_entries' squeeze is audited. + std::ptr::write(elements.add(out), kv); + // Value follows its key. Read through the index path against the + // PRE-squeeze bound (the same boundary it was written under), + // then store through the barriered inline/overflow split. + let v = + crate::object::field_get_set::object_field_at_with_live(obj, s as u32, field_count); + if out < alloc_limit { + crate::gc::runtime_store_jsvalue_slot( + obj as usize, + fields_ptr.add(out) as usize, + out, + v.bits(), + ); + } else { + overflow_set(obj as usize, out, v.bits()); + } + } + out += 1; + } + (*keys).length = out as u32; + if out > 0 { + // GC_STORE_AUDIT(EXTERNAL_BARRIERED): dirty-span barrier over the + // compacted key slots, mirroring compact_map_entries. + crate::gc::runtime_write_barrier_external_slot_span(keys as usize, elements as usize, out); + } + super::rebuild_array_layout_from_slots(keys); + set_object_live_slot_count(obj, std::cmp::min(out, alloc_limit) as u32); + // Slots moved: the per-array key index and any stale descriptors for the + // pre-squeeze states are wrong now. Drop the index (rebuilt on demand) + // and publish the squeezed shape at hole_count = 0. + crate::object::shapes::shape_drop(keys); + super::shapes::publish_object_shape_holes(obj, 0); +} diff --git a/crates/perry-runtime/src/object/descriptors.rs b/crates/perry-runtime/src/object/descriptors.rs index 5c65969c00..91863b5d42 100644 --- a/crates/perry-runtime/src/object/descriptors.rs +++ b/crates/perry-runtime/src/object/descriptors.rs @@ -1625,6 +1625,19 @@ fn js_object_get_own_property_names_shape(obj_value: f64) -> f64 { let mut sso_buf = [0u8; crate::value::SHORT_STRING_MAX_LEN]; for i in 0..len { let key_val = crate::array::js_array_get(keys, pos(i)); + // Tombstoned slot from an O(1) delete: not a key. Same raw-push + // hole hazard as `js_object_keys`' fast path — this loop emitted + // the marker itself (visible as `null` in getOwnPropertyNames). + if key_val.bits() == crate::value::TAG_HOLE + || key_val.bits() == crate::value::TAG_UNDEFINED + { + // Tombstoned slot from an O(1) delete. `js_array_get` translates + // TAG_HOLE to `undefined` per OrdinaryGet (#323), so the marker + // arrives here in EITHER form — and `undefined` is never a legal + // key, so both are skips. Comparing TAG_HOLE alone was dead code + // and let the hole reach the output as JSON `null`. + continue; + } if hide_private || hide_wasi_state { if let Some(b) = crate::string::js_string_key_bytes(key_val, &mut sso_buf) { if super::field_get_set::is_internal_runtime_key_bytes(b) diff --git a/crates/perry-runtime/src/object/field_get_set/enumeration.rs b/crates/perry-runtime/src/object/field_get_set/enumeration.rs index 001b9ddd0a..d5a60122ab 100644 --- a/crates/perry-runtime/src/object/field_get_set/enumeration.rs +++ b/crates/perry-runtime/src/object/field_get_set/enumeration.rs @@ -1302,6 +1302,20 @@ fn js_object_keys_shape(obj: *const ObjectHeader) -> *mut ArrayHeader { let out = crate::array::js_array_alloc(len as u32); for j in 0..len { let key_val = crate::array::js_array_get(keys, pos(j)); + // Tombstoned slot from an O(1) delete: not a key. The slow + // path below skips holes for free (`js_string_key_bytes` + // rejects them); this raw-push path must skip explicitly or + // `Object.keys` would emit the hole marker itself. + if key_val.bits() == crate::value::TAG_HOLE + || key_val.bits() == crate::value::TAG_UNDEFINED + { + // Tombstoned slot from an O(1) delete. `js_array_get` translates + // TAG_HOLE to `undefined` per OrdinaryGet (#323), so the marker + // arrives here in EITHER form — and `undefined` is never a legal + // key, so both are skips. Comparing TAG_HOLE alone was dead code + // and let the hole reach the output as JSON `null`. + continue; + } crate::array::js_array_push_f64(out, f64::from_bits(key_val.bits())); } return out; diff --git a/crates/perry-runtime/src/object/keys_lookup.rs b/crates/perry-runtime/src/object/keys_lookup.rs index 1be5cc08f5..6c3c266e70 100644 --- a/crates/perry-runtime/src/object/keys_lookup.rs +++ b/crates/perry-runtime/src/object/keys_lookup.rs @@ -80,12 +80,19 @@ pub(crate) unsafe fn keys_find_slot_by_bytes( // (1570 -> 3064 ms measured). Appends maintain the index incrementally // (shape_note_append), so stable-shape workloads still hit; churny // ones fall back to the raw dense scan below instead of thrashing. - if let Some(slot) = shapes::shape_slot_lookup(keys, key_bytes, h, key_count, false) { - return Some(slot); + match shapes::shape_slot_lookup_verdict(keys, key_bytes, h, key_count, false) { + shapes::KeysIndexVerdict::Found(slot) => return Some(slot), + // A COMPLETE index (indexed_len == key_count) proves absence: + // every present key is indexed, holes index as nothing, and a + // stale bucket entry for a tombstoned key fails its content + // validation without disproving completeness. Skipping the + // backstop here is what makes tombstone-delete churn cheap — the + // re-add's find-before-append otherwise linear-scanned up to 2x + // the live keys per delete (60.4% of the flag-on + // bench_populated_delete profile in one symbol). + shapes::KeysIndexVerdict::Absent => return None, + shapes::KeysIndexVerdict::Unindexed => {} } - // A miss from a fully built index is authoritative in the common case, - // but the index can decline (shrunk arrays, partial builds); the raw - // scan below is cheap enough to serve as the correctness backstop. } let (slots, slot_len) = keys_array_dense_slots(keys); if slots.is_null() { diff --git a/crates/perry-runtime/src/object/shapes.rs b/crates/perry-runtime/src/object/shapes.rs index 50c88cb9c9..15846ea28f 100644 --- a/crates/perry-runtime/src/object/shapes.rs +++ b/crates/perry-runtime/src/object/shapes.rs @@ -34,8 +34,9 @@ use std::cell::RefCell; #[path = "shapes_slot_list.rs"] mod shapes_slot_list; pub(crate) use shapes_slot_list::{ - record_shape_scan_outcome, shape_index_migrate_after_delete, shape_index_shift_in_place, - SlotList, + debug_assert_object_shape_parity, debug_assert_object_shape_parity_for_keys, + object_shape_hole_count, publish_object_shape_holes, record_shape_scan_outcome, + shape_index_migrate_after_delete, shape_index_shift_in_place, SlotList, }; pub(crate) struct ShapeIndex { @@ -115,6 +116,10 @@ pub(crate) struct ShapeDescriptor { /// the authoritative descriptor rather than `GcHeader::_reserved`, whose /// bits belong to the GC layout/age protocol and object feature flags. pub(crate) object_kind: ShapeObjectKind, + /// Tombstoned key slots (`TAG_HOLE`) left by O(1) deletes; the live key + /// count is `logical_key_count - hole_count`. Immutable per id like every + /// other identity fact — a hole-delete publishes a successor id. + pub(crate) hole_count: u32, } /// Shape identity is the FACTS, never the storage address. A descriptor value @@ -207,6 +212,11 @@ struct ShapeFacts { live_inline_slot_count: u32, semantic_generation: u64, object_kind: ShapeObjectKind, + /// Tombstoned key slots in the keys array (`TAG_HOLE` markers left by + /// O(1) deletes). Part of identity: two shapes over the same array with + /// different hole sets must be distinct ids, or a stale IC entry for a + /// deleted key would keep hitting. + hole_count: u32, } struct ShapeTableInner { @@ -319,6 +329,7 @@ fn descriptor_facts(descriptor: ShapeDescriptor) -> ShapeFacts { live_inline_slot_count: descriptor.live_inline_slot_count, semantic_generation: descriptor.semantic_generation, object_kind: descriptor.object_kind, + hole_count: descriptor.hole_count, } } @@ -329,6 +340,7 @@ fn descriptor_facts_with_keys(descriptor: ShapeDescriptor, keys: u64) -> ShapeFa live_inline_slot_count: descriptor.live_inline_slot_count, semantic_generation: descriptor.semantic_generation, object_kind: descriptor.object_kind, + hole_count: descriptor.hole_count, } } @@ -490,6 +502,27 @@ fn shape_descriptor_ensure_with_generation( live_inline_slot_count: u32, semantic_generation: u64, object_kind: ShapeObjectKind, +) -> Result { + shape_descriptor_ensure_with_holes( + keys, + logical_key_count, + live_inline_slot_count, + semantic_generation, + object_kind, + 0, + ) +} + +/// [`shape_descriptor_ensure_with_generation`] with an explicit tombstone +/// count — the publish half of an O(1) hole-delete, which must mint a shape +/// identity distinct from every hole state of the same array. +fn shape_descriptor_ensure_with_holes( + keys: *const ArrayHeader, + logical_key_count: u32, + live_inline_slot_count: u32, + semantic_generation: u64, + object_kind: ShapeObjectKind, + hole_count: u32, ) -> Result { let keys_id = keys as usize; if keys_id == 0 && logical_key_count != 0 { @@ -501,6 +534,7 @@ fn shape_descriptor_ensure_with_generation( live_inline_slot_count, semantic_generation, object_kind, + hole_count, }; let mut inner = crate::state::state().shapes.inner.borrow_mut(); if let Some(id) = inner @@ -522,6 +556,7 @@ fn shape_descriptor_ensure_with_generation( live_inline_slot_count, semantic_generation, object_kind, + hole_count, }; // Publish by-id first, then the reverse accelerator. An ObjectHeader is // stamped only after this function returns, so a visible id always has a @@ -789,7 +824,7 @@ pub extern "C" fn js_object_shape_id_for_keys(keys: u64, key_count: u32) -> u32 /// slot representations must never share a pre-baked GC descriptor. pub(crate) fn mint_registered_typed_shape_id(keys: *const ArrayHeader, key_count: u32) -> u32 { let id = alloc_shape_id().unwrap_or_else(|_| shape_id_exhausted_abort()); - if !install_external_shape_id(id, keys, key_count, key_count) { + if !shapes_slot_list::install_external_shape_id(id, keys, key_count, key_count) { invalid_shape_facts_abort(); } id @@ -802,56 +837,7 @@ pub(crate) fn install_registered_typed_shape_id( keys: *const ArrayHeader, key_count: u32, ) -> bool { - install_external_shape_id(id, keys, key_count, key_count) -} - -/// Install a process-global id into this agent's local descriptor table. -/// Module globals are initialized once per process, while workers own distinct -/// runtime state and moving keys pointers. Global id uniqueness makes a local -/// first installation unambiguous; an existing different descriptor fails -/// closed and the caller mints a fresh local id instead. -fn install_external_shape_id( - id: u32, - keys: *const ArrayHeader, - logical_key_count: u32, - live_inline_slot_count: u32, -) -> bool { - if !is_shape_id(id) || (keys.is_null() && logical_key_count != 0) { - return false; - } - let descriptor = ShapeDescriptor { - keys: keys as usize as u64, - indexed_keys: keys as usize as u64, - record: 0, - old_carrier: false, - old_carrier_seen: false, - cache_carrier: false, - logical_key_count, - live_inline_slot_count, - semantic_generation: 0, - object_kind: ShapeObjectKind::Ordinary, - }; - let facts = descriptor_facts(descriptor); - let mut inner = crate::state::state().shapes.inner.borrow_mut(); - if let Some(existing) = inner.descriptors.get(&id) { - return **existing == descriptor; - } - // A worker can have minted an equivalent local descriptor before module - // 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_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)); - // An equivalent local descriptor can predate module initialization. Keep - // both reverse-index entries and prefer the external id for subsequent - // births in this agent; already-published local ids remain resolvable. - inner.ids_by_facts.entry(facts).or_default().insert(0, id); - insert_descriptor_id_sorted(inner.ids_by_keys.entry(descriptor.keys).or_default(), id); - true + shapes_slot_list::install_external_shape_id(id, keys, key_count, key_count) } // --------------------------------------------------------------------------- @@ -1121,7 +1107,12 @@ pub(crate) unsafe fn birth_stamp_object_shape( let key_count = current.logical_key_count; let supplied_id_is_local = descriptor_matches_object(runtime_shape_id, obj, live_inline_slot_count) - || install_external_shape_id(runtime_shape_id, keys, key_count, live_inline_slot_count); + || shapes_slot_list::install_external_shape_id( + runtime_shape_id, + keys, + key_count, + live_inline_slot_count, + ); if supplied_id_is_local { (*obj).parent_class_id = runtime_shape_id; debug_assert_object_shape_parity(obj); @@ -1379,6 +1370,19 @@ pub(crate) unsafe fn transition_object_shape_semantics( id } +/// Publish the successor shape for an O(1) hole-delete on `obj`'s CURRENT +/// keys array: same address, same surviving slots, one more tombstone. +/// +/// Modeled on [`transition_object_shape_semantics`]: the structural facts are +/// unchanged except `hole_count`, and the fresh process-unique generation is +/// what retires every cached `(token, key)` pair for this receiver — a +/// deleted key must stop hitting even though the array address and every +/// surviving slot are byte-identical, or a stale IC hit would return the +/// cleared slot instead of walking the prototype chain. +/// +/// Returns the successor id, or 0 when the object is not stamped/shaped — +/// the caller falls back to the compacting delete. + /// Turn a class-expression object into a class receiver. The kind is part of /// the exact immutable descriptor, so it cannot alias GC layout bits and every /// pre-mark ShapeId guard permanently misses afterward. @@ -1491,40 +1495,6 @@ unsafe fn object_header_key_count(obj: *const crate::object::ObjectHeader) -> u3 } } -/// #8113: the live-slot bound is no longer independently observable, so parity -/// is now exactly "the stamp resolves, and its structural keys facts match the -/// keys edge the receiver is about to carry". The bound cannot disagree with -/// itself. -#[inline] -pub(crate) unsafe fn debug_assert_object_shape_parity(obj: *const crate::object::ObjectHeader) { - debug_assert_object_shape_parity_for_keys(obj, crate::object::object_keys_array(obj)); -} - -/// Parity against an EXPLICIT keys edge. -/// -/// `publish_object_shape_from` stamps the successor before the header store -/// (that is what makes the keys mutation mint-then-stamp), so for that one -/// window the authoritative edge is the caller's argument, not the header word. -#[inline] -pub(crate) unsafe fn debug_assert_object_shape_parity_for_keys( - obj: *const crate::object::ObjectHeader, - keys: *mut ArrayHeader, -) { - let id = object_shape_stamp(obj); - if id != 0 { - let key_count = if keys.is_null() { - 0 - } else { - crate::array::keys_array_len_capped_to_capacity(keys) as u32 - }; - debug_assert!( - shape_descriptor_by_id(id) - .is_some_and(|d| { d.keys == keys as u64 && d.logical_key_count == key_count }), - "published ShapeId disagrees with authoritative ObjectHeader facts" - ); - } -} - /// The address of the ONE `keys` word the collector rewrites for `shape_id`, /// or `None` when the id names no descriptor in this agent (#8112). /// @@ -1627,6 +1597,21 @@ unsafe fn index_range(shape: &mut ShapeIndex, keys: *const ArrayHeader, key_coun /// historical thresholds: write path ≥ `KEYS_INDEX_THRESHOLD`, read path /// ≥ `WIDE_KEY_INDEX_MIN_KEYS`) — but an entry that already exists is /// consulted regardless, so a read may reuse the index a write built. +/// A key-index consultation's answer, distinguishing "this COMPLETE index +/// proves the key absent" from "the index cannot answer". +pub(crate) enum KeysIndexVerdict { + Found(u32), + /// The index covers every slot of the array (`indexed_len == key_count`) + /// and holds no entry for this key: the key is not present, and the + /// caller may skip its linear backstop scan. Trusting absence is what + /// makes tombstone-delete churn O(1) — the re-add's find-before-append + /// otherwise pays a full scan per delete, measured at 60.4% of the + /// flag-on `bench_populated_delete` profile. + Absent, + /// No index, a partial build, or a declined consult — scan. + Unindexed, +} + pub(crate) unsafe fn shape_slot_lookup( keys: *const ArrayHeader, key_bytes: &[u8], @@ -1634,6 +1619,19 @@ pub(crate) unsafe fn shape_slot_lookup( key_count: u32, build: bool, ) -> Option { + match shape_slot_lookup_verdict(keys, key_bytes, key_hash, key_count, build) { + KeysIndexVerdict::Found(slot) => Some(slot), + _ => None, + } +} + +pub(crate) unsafe fn shape_slot_lookup_verdict( + keys: *const ArrayHeader, + key_bytes: &[u8], + key_hash: u64, + key_count: u32, + build: bool, +) -> KeysIndexVerdict { let keys_id = keys as usize; let mut inner = crate::state::state().shapes.inner.borrow_mut(); let shape = match inner.indices.get_mut(&keys_id) { @@ -1641,13 +1639,13 @@ pub(crate) unsafe fn shape_slot_lookup( if s.indexed_len > key_count { // Shrink (delete/compaction): slots are untrustworthy. inner.indices.remove(&keys_id); - return None; + return KeysIndexVerdict::Unindexed; } s } None => { if !build { - return None; + return KeysIndexVerdict::Unindexed; } inner.indices.entry(keys_id).or_insert(ShapeIndex { indexed_len: 0, @@ -1658,7 +1656,15 @@ pub(crate) unsafe fn shape_slot_lookup( if shape.indexed_len < key_count { index_range(shape, keys, key_count); } - let candidates = shape.slots.get(&key_hash)?; + let complete = shape.indexed_len == key_count; + let absent = if complete { + KeysIndexVerdict::Absent + } else { + KeysIndexVerdict::Unindexed + }; + let Some(candidates) = shape.slots.get(&key_hash) else { + return absent; + }; let mut sso = [0u8; crate::value::SHORT_STRING_MAX_LEN]; let (slots, slot_len) = super::keys_array_dense_slots(keys); for &i in candidates.iter() { @@ -1668,11 +1674,13 @@ pub(crate) unsafe fn shape_slot_lookup( let v = crate::JSValue::from_bits((*slots.add(i as usize)).to_bits()); if let Some(stored) = crate::string::js_string_key_bytes(v, &mut sso) { if stored == key_bytes { - return Some(i); + return KeysIndexVerdict::Found(i); } } } - None + // Hash-bucket candidates existed but none matched: with a complete index + // that still proves absence (the bucket held colliding OTHER keys). + absent } /// Record a freshly appended key: `keys` (the POST-append array — a clone diff --git a/crates/perry-runtime/src/object/shapes_slot_list.rs b/crates/perry-runtime/src/object/shapes_slot_list.rs index eefdba9125..da6136b700 100644 --- a/crates/perry-runtime/src/object/shapes_slot_list.rs +++ b/crates/perry-runtime/src/object/shapes_slot_list.rs @@ -191,3 +191,152 @@ pub(crate) fn shape_index_migrate_after_delete( inner.indices.insert(new_keys_id, index); true } + +/// The receiver's current tombstone count, 0 when unshaped. +pub(crate) unsafe fn object_shape_hole_count(obj: *const crate::object::ObjectHeader) -> u32 { + super::object_shape_descriptor(obj) + .map(|d| d.hole_count) + .unwrap_or(0) +} + +pub(crate) unsafe fn publish_object_shape_holes( + obj: *mut crate::object::ObjectHeader, + hole_count: u32, +) -> u32 { + if obj.is_null() || !super::shape_word_is_writable(obj) { + return 0; + } + let Some(current) = super::object_shape_descriptor(obj) else { + return 0; + }; + let generation = super::SHAPE_SEMANTIC_NEXT.fetch_add(1, std::sync::atomic::Ordering::Relaxed); + if generation == 0 { + super::shape_id_exhausted_abort(); + } + let id = super::publish_shape_result(super::shape_descriptor_ensure_with_holes( + current.keys as usize as *mut super::ArrayHeader, + current.logical_key_count, + current.live_inline_slot_count, + generation, + current.object_kind, + hole_count, + )); + (*obj).parent_class_id = id; + // Retire the predecessor. Its keys array is OWNED (the tombstone path is + // gated on that), so this object is the only carrier of the old stamp and + // the id becomes unreachable the moment the header word above is written: + // stale IC tokens already miss on the stamp compare, and + // `shape_descriptor_by_id` of a removed id is `None`. Without this, a + // delete-churn loop minted one descriptor per delete against ONE stable + // address forever — the reverse-index Vec under that address grew by one + // per delete and every later publish walked it, which measured as a 26x + // slowdown (2.06 s → 53.6 s) on `bench_populated_delete` before this + // line existed. + { + let mut inner = crate::state::state().shapes.inner.borrow_mut(); + // Sweep EVERY other id for this keys address, not just the direct + // predecessor: the delete-then-re-add cycle publishes an id on the + // APPEND side too, and nothing else retires those — the post-trace + // dead-key pruning only fires when the keys ARRAY dies, and this + // array lives at a stable address for the object's whole life. + // Retiring only the predecessor halved the descriptor pile-up + // (53.6 s → 25.1 s on the churn benchmark) but ids still accumulated + // one per iteration from the append publish. + let stale: Vec = inner + .ids_by_keys + .get(&(current.keys)) + .map(|ids| ids.iter().copied().filter(|&other| other != id).collect()) + .unwrap_or_default(); + for other in stale { + super::remove_descriptor_and_reverse_indices(&mut inner, other); + } + } + super::debug_assert_object_shape_parity(obj); + id +} + +/// Install a process-global id into this agent's local descriptor table. +/// Module globals are initialized once per process, while workers own distinct +/// runtime state and moving keys pointers. Global id uniqueness makes a local +/// first installation unambiguous; an existing different descriptor fails +/// closed and the caller mints a fresh local id instead. +pub(super) fn install_external_shape_id( + id: u32, + keys: *const super::ArrayHeader, + logical_key_count: u32, + live_inline_slot_count: u32, +) -> bool { + if !super::is_shape_id(id) || (keys.is_null() && logical_key_count != 0) { + return false; + } + let descriptor = super::ShapeDescriptor { + keys: keys as usize as u64, + indexed_keys: keys as usize as u64, + record: 0, + old_carrier: false, + old_carrier_seen: false, + cache_carrier: false, + logical_key_count, + live_inline_slot_count, + semantic_generation: 0, + object_kind: super::ShapeObjectKind::Ordinary, + hole_count: 0, + }; + let facts = super::descriptor_facts(descriptor); + let mut inner = crate::state::state().shapes.inner.borrow_mut(); + if let Some(existing) = inner.descriptors.get(&id) { + return **existing == descriptor; + } + // A worker can have minted an equivalent local descriptor before module + // 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_ways cache has to be invalidated here (the fresh-id insert in + // `intern_shape_descriptor` cannot, and deliberately does not). + super::invalidate_shape_lookup_cache(); + inner + .descriptors + .insert(id, super::box_descriptor(descriptor)); + // An equivalent local descriptor can predate module initialization. Keep + // both reverse-index entries and prefer the external id for subsequent + // births in this agent; already-published local ids remain resolvable. + inner.ids_by_facts.entry(facts).or_default().insert(0, id); + super::insert_descriptor_id_sorted(inner.ids_by_keys.entry(descriptor.keys).or_default(), id); + true +} + +/// #8113: the live-slot bound is no longer independently observable, so parity +/// is now exactly "the stamp resolves, and its structural keys facts match the +/// keys edge the receiver is about to carry". The bound cannot disagree with +/// itself. +#[inline] +pub(crate) unsafe fn debug_assert_object_shape_parity(obj: *const crate::object::ObjectHeader) { + debug_assert_object_shape_parity_for_keys(obj, crate::object::object_keys_array(obj)); +} + +/// Parity against an EXPLICIT keys edge. +/// +/// `publish_object_shape_from` stamps the successor before the header store +/// (that is what makes the keys mutation mint-then-stamp), so for that one +/// window the authoritative edge is the caller's argument, not the header word. +#[inline] +pub(crate) unsafe fn debug_assert_object_shape_parity_for_keys( + obj: *const crate::object::ObjectHeader, + keys: *mut crate::array::ArrayHeader, +) { + let id = super::object_shape_stamp(obj); + if id != 0 { + let key_count = if keys.is_null() { + 0 + } else { + crate::array::keys_array_len_capped_to_capacity(keys) as u32 + }; + debug_assert!( + super::shape_descriptor_by_id(id) + .is_some_and(|d| { d.keys == keys as u64 && d.logical_key_count == key_count }), + "published ShapeId disagrees with authoritative ObjectHeader facts" + ); + } +} diff --git a/crates/perry-runtime/src/object/shapes_tests.rs b/crates/perry-runtime/src/object/shapes_tests.rs index aad1961dc9..2195801861 100644 --- a/crates/perry-runtime/src/object/shapes_tests.rs +++ b/crates/perry-runtime/src/object/shapes_tests.rs @@ -490,7 +490,7 @@ mod descriptor_tests_8067 { let local = shape_descriptor_ensure(keys as *const ArrayHeader, 1, 1) .expect("shape range unexpectedly exhausted"); let external = alloc_shape_id().expect("shape range unexpectedly exhausted"); - assert!(install_external_shape_id( + assert!(shapes_slot_list::install_external_shape_id( external, keys as *const ArrayHeader, 1, @@ -563,7 +563,7 @@ mod descriptor_tests_8067 { .expect("shape range unexpectedly exhausted"); let worker_keys = 0x8067_0000_0000_1900usize; std::thread::spawn(move || { - assert!(install_external_shape_id( + assert!(shapes_slot_list::install_external_shape_id( module_id, worker_keys as *const ArrayHeader, 2, @@ -827,6 +827,7 @@ fn shape_facts_hash_folds_every_field() { live_inline_slot_count: 3, semantic_generation: 9, object_kind: ShapeObjectKind::Ordinary, + hole_count: 0, }; let variants = [ @@ -858,6 +859,13 @@ fn shape_facts_hash_folds_every_field() { ..base }, ), + ( + "hole_count", + ShapeFacts { + hole_count: 1, + ..base + }, + ), ( "object_kind", ShapeFacts { diff --git a/scripts/shape_descriptor_census.py b/scripts/shape_descriptor_census.py index 9c946127c1..120768081b 100644 --- a/scripts/shape_descriptor_census.py +++ b/scripts/shape_descriptor_census.py @@ -222,6 +222,7 @@ def assert_before(body: str, first: str, second: str, label: str) -> None: def assert_authority_surfaces(sources: dict[str, str]) -> None: authority_paths = ( "crates/perry-runtime/src/object/shapes.rs", + "crates/perry-runtime/src/object/shapes_slot_list.rs", "crates/perry-runtime/src/object/mod.rs", "crates/perry-runtime/src/object/live_slots.rs", "crates/perry-codegen/src/lower_call/new_alloc.rs", @@ -246,7 +247,17 @@ def assert_authority_surfaces(sources: dict[str, str]) -> None: "shape descriptor authority source missing: " + ", ".join(missing) ) clean = stripped_sources({path: sources[path] for path in authority_paths}) - shapes = clean["crates/perry-runtime/src/object/shapes.rs"] + # `shapes.rs` sits against the repo's 2000-line cap, so helpers keep being + # split into the `shapes_slot_list.rs` sibling as it grows. Read the two as + # ONE logical unit: every `function_body(shapes, ...)` below then finds its + # target wherever it currently lives, instead of silently matching nothing + # the next time a pinned function crosses the split — #8918's exact failure + # mode, where a census inspecting an empty body reports success. + shapes = ( + clean["crates/perry-runtime/src/object/shapes.rs"] + + "\n" + + clean["crates/perry-runtime/src/object/shapes_slot_list.rs"] + ) object_mod = clean["crates/perry-runtime/src/object/mod.rs"] live_slots = clean["crates/perry-runtime/src/object/live_slots.rs"] codegen_alloc = clean["crates/perry-codegen/src/lower_call/new_alloc.rs"] @@ -376,7 +387,10 @@ def assert_authority_surfaces(sources: dict[str, str]) -> None: "descriptor is the authoritative edge since #8112" ) - ensure = function_body(shapes, "shape_descriptor_ensure_with_generation") + # The insert/reverse-index body lives in the `_with_holes` variant since + # the tombstone-delete work; `_with_generation` is a thin forwarding + # wrapper. The authority ordering is checked where the writes are. + ensure = function_body(shapes, "shape_descriptor_ensure_with_holes") assert_before( ensure, "inner.descriptors.insert",