feat(runtime): tombstone object deletes default-on (populated delete 6.5x)

changelog.d/9029-object-tombstone-deletes.md

@@ -1,7 +1,8 @@
 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.
+2030 → **~315 ms (6.5×)**; flag-off it still gains **−11%** (the
+complete-index absence verdict below applies to ordinary deletes too), 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
@@ -33,12 +34,38 @@ 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
+Verification: suite 2807 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.
+**A complete key index proves absence.** The flag-on profile was one symbol:
+the re-add's find-before-append missed the (consult-only-stale) index and
+paid a full linear backstop scan per delete — 60.4% of the run.
+`shape_slot_lookup` now reports Found / Absent / Unindexed; when
+`indexed_len == key_count` the index covers every live slot (holes index as
+nothing, a tombstoned key's stale bucket entry fails content validation
+without disproving completeness), so the resolver returns "absent" without
+scanning. Partial and missing indexes keep the backstop.
+
+**Walker audit (default-on prerequisite), four bugs found by enumerating all
+57 files that touch keys arrays.** (1) The JSON shape-prefix template
+dereferenced a hole's bits as a StringHeader — SIGSEGV on
+`JSON.stringify` of an array of holed `class_id == 0` objects; holed shapes
+now bail to the hole-aware slow path (pinned by a unit test — the
+differentials could not reach it because their objects carry `__AnonShape`
+class ids or cache-shared keys). (2) The worker-thread serializer pairs
+keys and fields positionally, so a hole became a phantom empty-string key on
+the worker; the serializer skips the pair. (3) `diagnostics_channel`'s
+error-prop walk stringified the canonicalized hole into a phantom
+`"undefined"` prop. (4) The two lineage-carrying shape publishes hardcoded
+`hole_count: 0`, so a re-add append RESET the squeeze accounting and
+delete/re-add churn grew the keys array without bound — a memory leak
+invisible to every timing gate; they now carry `lineage.hole_count`, and a
+60-cycle churn test pins the 2×-live-size bound. Fixing it also cut flag-on
+time a further 25% (the index stays small and hot).
+
+Remaining flag-on cost is spread across the per-op fixed costs (~19% in the
+verdict lookup's TLS+hash chain); default-on is the follow-up.

changelog.d/9038-object-tombstones-default-on.md

@@ -0,0 +1,20 @@
+Tombstone object deletes (#9029) are now the DEFAULT. `delete obj[k]` on an
+owned keys array is O(1) — a hole marker plus a barriered value clear —
+with threshold compaction amortizing the debt (never more than 2x live
+size). `bench_populated_delete` drops 2030 → ~315 ms (6.5x; ~15x node on
+the same host, from ~96x before the campaign). Deletes on cache-shared
+keys arrays still clone-compact once to take ownership; every later delete
+on that object is O(1).
+
+The kill switch is `PERRY_OBJECT_TOMBSTONES=0` (also `off`/`false`),
+mirroring the moving-scavenge rollout's `PERRY_GC_MOVING_LOOP_POLLS=0`
+pattern. `=1` remains accepted and is now redundant.
+
+Shipping default-on was gated on the #9029 walker audit (all 57 files
+touching keys arrays classified; four flag-on-only bugs found and fixed,
+including a JSON-template SIGSEGV and the hole-count accounting reset that
+let delete/re-add churn dodge the squeeze bound) and on the churn-bound
+unit test that pins the 2x-live-size memory guarantee. Full suite runs
+with the default flag, so every delete-touching test now exercises the
+tombstone path; the four differentials plus the two holed-JSON-array
+repros stay byte-identical to node in BOTH flag directions.

crates/perry-runtime/src/child_process/mod.rs

@@ -11,7 +11,7 @@ pub(crate) mod ipc_transport;
 #[cfg(windows)]
 mod windows_fork;
 // #2130: V8 structured-clone codec for `serialization: 'advanced'` IPC.
-mod v8_serde;
+pub(crate) mod v8_serde;
 // #2555: sync buffered `input`, `timeout`, and `maxBuffer` execution options.
 mod sync_run;
 // #3079: setup-time command/file/args validation (`ERR_INVALID_ARG_TYPE`).

crates/perry-runtime/src/child_process/v8_serde.rs

@@ -595,6 +595,14 @@ impl Serializer {
         let mut count = 0u64;
         for f in 0..keys_len {
             let key = crate::array::js_array_get_f64(keys_arr, f);
+            // A tombstoned key slot (#9029, flag-gated deletes) reads back as
+            // undefined through `js_array_get_f64`'s hole canonicalization
+            // (#323) — and undefined is never a legal key, so this skip
+            // cannot drop a real property. Serializing the slot would emit a
+            // phantom `undefined` key node's structured clone doesn't have.
+            if key.to_bits() == crate::value::TAG_UNDEFINED {
+                continue;
+            }
             let val = if f < alloc_limit {
                 *fields_ptr.add(f as usize)
             } else {

crates/perry-runtime/src/json/mod.rs

@@ -34,7 +34,7 @@ mod stringify;
 mod stringify_api;
 mod stringify_buffer;
 mod stringify_scalars;
-mod stringify_shape_template;
+pub(crate) mod stringify_shape_template;
 mod stringify_tojson_probe;
 
 // Public FFI re-exports — preserve the `crate::json::js_json_*` path used by

crates/perry-runtime/src/json/stringify_shape_template.rs

@@ -194,6 +194,14 @@ pub(crate) unsafe fn build_shape_prefix_template(first_elem_bits: u64) -> Option
     let mut prefixes: Vec<String> = Vec::with_capacity(shape_fields as usize);
     for f in 0..shape_fields {
         let key_bits = (*keys_elements.add(f as usize)).to_bits();
+        // A tombstoned key slot (#9029, flag-gated deletes): the hole's bits
+        // are NOT a string header — the untagged-else below would deref them.
+        // Holed shapes take the generic slow path, which skips holes; the
+        // squeeze that removes them keeps the array's identity, so no stale
+        // template can outlive the holes it declined to cache.
+        if key_bits == crate::value::TAG_HOLE {
+            return None;
+        }
         let key_tag = key_bits & 0xFFFF_0000_0000_0000;
         let key_ptr = if key_tag == STRING_TAG || key_tag == POINTER_TAG {
             (key_bits & POINTER_MASK) as *const StringHeader

crates/perry-runtime/src/node_submodules/diagnostics.rs

@@ -692,6 +692,12 @@ pub fn error_user_props(error_ptr: usize) -> Vec<(String, f64)> {
         let mut out = Vec::with_capacity(len);
         for i in 0..len {
             let key_val = crate::array::js_array_get_f64(keys, i as u32);
+            // A tombstoned key slot (#9029) reads back as undefined through
+            // the hole canonicalization (#323); stringifying it would mint a
+            // phantom "undefined" prop. Undefined is never a legal key.
+            if key_val.to_bits() == crate::value::TAG_UNDEFINED {
+                continue;
+            }
             let name_ptr = crate::value::js_jsvalue_to_string(key_val);
             if name_ptr.is_null() {
                 continue;

crates/perry-runtime/src/object/delete_rest.rs

@@ -1143,15 +1143,40 @@ mod sso_tests_1781 {
 /// 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 {
+    // Test override first: the OnceLock latches at the FIRST delete anywhere
+    // in the test process, which is long before a tombstone test's own
+    // `set_var` — so tests opt in through this cell instead of the env.
+    #[cfg(test)]
+    if let Some(forced) = TOMBSTONE_TEST_OVERRIDE.with(std::cell::Cell::get) {
+        return forced;
+    }
     static ON: std::sync::OnceLock<bool> = std::sync::OnceLock::new();
     *ON.get_or_init(|| {
-        matches!(
+        // Default ON (#9029 shipped the mechanism flag-gated; the walker
+        // audit and churn-bound tests are the default-on prerequisites).
+        // `PERRY_OBJECT_TOMBSTONES=0` is the kill switch, mirroring the
+        // moving-scavenge rollout's `PERRY_GC_MOVING_LOOP_POLLS=0` pattern.
+        !matches!(
             std::env::var("PERRY_OBJECT_TOMBSTONES").as_deref(),
-            Ok("1") | Ok("on") | Ok("true")
+            Ok("0") | Ok("off") | Ok("false")
         )
     })
 }
 
+#[cfg(test)]
+thread_local! {
+    static TOMBSTONE_TEST_OVERRIDE: std::cell::Cell<Option<bool>> =
+        const { std::cell::Cell::new(None) };
+}
+
+/// Force the tombstone-delete flag for the CURRENT THREAD's asserts,
+/// bypassing the env-latched OnceLock. Pass `None` to restore env behavior;
+/// callers must do so before returning (tests share threads).
+#[cfg(test)]
+pub(crate) fn test_set_tombstone_deletes(forced: Option<bool>) {
+    TOMBSTONE_TEST_OVERRIDE.with(|cell| cell.set(forced));
+}
+
 /// 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

crates/perry-runtime/src/object/mod.rs

@@ -1868,6 +1868,8 @@ pub(super) unsafe fn mark_object_dynamic_shape_unknown(obj: *mut ObjectHeader) {
 
 #[cfg(test)]
 mod tests;
+#[cfg(test)]
+mod tombstone_tests;
 
 /// The named-property bag for a cell that has no inline slot layout of its own,
 /// creating it on first write.

crates/perry-runtime/src/object/shapes.rs

@@ -33,8 +33,10 @@ use std::cell::RefCell;
 
 #[path = "shapes_slot_list.rs"]
 mod shapes_slot_list;
+#[cfg(test)]
+pub(crate) use shapes_slot_list::shape_descriptor_keys_slot;
+pub(crate) use shapes_slot_list::shape_id_owns_keys_slot;
 pub(crate) use shapes_slot_list::{
-    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,
 };
@@ -1053,12 +1055,15 @@ pub(crate) unsafe fn stamp_object_shape(
         debug_assert_object_shape_parity(obj);
         return id;
     };
-    let id = publish_shape_result(shape_descriptor_ensure_with_generation(
+    let id = publish_shape_result(shape_descriptor_ensure_with_holes(
         keys,
         key_count,
         lineage.live_inline_slot_count,
         lineage.semantic_generation,
         lineage.object_kind,
+        // Same-array restamp: physical holes persist, so must the count
+        // (see the lineage publish below for the churn-growth rationale).
+        lineage.hole_count,
     ));
     if id != (*obj).parent_class_id {
         // Read-side lookup_ways also calls `stamp_object_shape` to populate its
@@ -1325,12 +1330,19 @@ pub(crate) unsafe fn publish_object_shape_from(
     let object_kind = lineage
         .map(|descriptor| descriptor.object_kind)
         .unwrap_or(ShapeObjectKind::Ordinary);
-    let id = publish_shape_result(shape_descriptor_ensure_with_generation(
+    // Tombstones (#9029): an append or grow-realloc keeps every hole slot
+    // physically in the array, so the successor must inherit the count — a
+    // reset would let delete/re-add churn dodge the squeeze threshold
+    // forever and grow the array unbounded. Only the squeeze itself (which
+    // physically removes the holes) publishes 0, explicitly.
+    let hole_count = lineage.map(|descriptor| descriptor.hole_count).unwrap_or(0);
+    let id = publish_shape_result(shape_descriptor_ensure_with_holes(
         keys,
         key_count,
         live_inline_slot_count,
         semantic_generation,
         object_kind,
+        hole_count,
     ));
     (*obj).parent_class_id = id;
     debug_assert_object_shape_parity_for_keys(obj, keys);
@@ -1495,56 +1507,38 @@ unsafe fn object_header_key_count(obj: *const crate::object::ObjectHeader) -> u3
     }
 }
 
-/// 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).
-///
-/// This is the seam that replaced the post-visit write-back callback. The
-/// callback existed because the header word was the strong edge and the
-/// descriptor a weak copy that had to be repaired from it, under exact-facts
-/// validation, once per traced receiver whose keys array had moved. With the
-/// descriptor holding the edge, the slot visitor writes the record directly
-/// and there is nothing left to reconcile.
-///
-/// The returned address belongs to a BOXED record, so it is stable across
-/// descriptor insertion; only `prune_dead_shape_keys` frees one, and that runs
-/// at sweep, after every enumeration of the cycle that produced it.
-#[cfg(test)]
+/// #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) fn shape_descriptor_keys_slot(shape_id: u32) -> Option<*mut u64> {
-    if !is_shape_id(shape_id) {
-        return None;
-    }
-    crate::state::state()
-        .shapes
-        .inner
-        .borrow_mut()
-        .descriptors
-        .get_mut(&shape_id)
-        .map(|record| std::ptr::addr_of_mut!(record.keys))
+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));
 }
 
-/// Is `slot` the shared `keys` word of `shape_id`'s descriptor record?
+/// Parity against an EXPLICIT keys edge.
 ///
-/// #8112: that word is a TABLE root, not a slot any receiver owns. Every
-/// sibling of the shape enumerates it, so a rewrite performed while tracing
-/// one receiver silently changes the edge of every other — including old
-/// receivers a minor never visits, for which no per-parent remembered-set page
-/// could ever be armed. The remembered-set and old→young verification paths
-/// therefore skip it and let the shape table's own root scanner cover it.
+/// `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) fn shape_id_owns_keys_slot(shape_id: u32, slot: *mut u64) -> bool {
-    if !is_shape_id(shape_id) {
-        return false;
+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"
+        );
     }
-    // Immutable borrow on purpose: this runs inside collector walks, and a
-    // `borrow_mut` here would make the predicate itself a re-entrancy hazard.
-    crate::state::state()
-        .shapes
-        .inner
-        .borrow()
-        .descriptors
-        .get(&shape_id)
-        .is_some_and(|record| std::ptr::addr_of!(record.keys) as *mut u64 == slot)
 }
 
 /// Drop the stamp iff the word currently holds one, leaving a real