From 18d5e9932bf4004bc46c01ffda1eb79e38f6bbbe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 29 Aug 2026 04:28:33 +0200 Subject: [PATCH] perf(runtime): keep the migrated key index instead of dropping it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The delete tail ends with shape_drop on the post-delete keys array, which removes exactly the index #9002's migration installs — so the migrated index was discarded immediately and the next lookup rebuilt it by re-hashing every surviving key name. (That also means #9002's own win came from pruning the STALE old entry, not from preserving a usable index; its description has been corrected.) A migrated index is shifted to match the compacted array, so it is current, not stale. Skip the drop when the migration succeeded and the rebuild really does stop happening. Unchanged when migration does not apply — a partially built index, or a delete that took a different path, still drops exactly as before. Claude-Session: https://claude.ai/code/session_01Ay8VyLkKbm8Hkc1xmvTEsP --- changelog.d/9005-keep-migrated-index.md | 20 +++++++++++++++++++ .../perry-runtime/src/object/delete_rest.rs | 10 ++++++++-- .../src/object/shapes_slot_list.rs | 13 ++++++++---- 3 files changed, 37 insertions(+), 6 deletions(-) create mode 100644 changelog.d/9005-keep-migrated-index.md diff --git a/changelog.d/9005-keep-migrated-index.md b/changelog.d/9005-keep-migrated-index.md new file mode 100644 index 0000000000..8e0da1424b --- /dev/null +++ b/changelog.d/9005-keep-migrated-index.md @@ -0,0 +1,20 @@ +`delete obj[k]` stops rebuilding the shape key index — for real this time. + +#9002 shifted the index onto the post-delete keys array, but the delete tail +ends with `shape_drop` on that same array, so the migrated index was discarded +immediately and the next lookup rebuilt it by re-hashing every surviving key +name. (#9002's own measured win came from pruning the STALE old entry, not +from preserving a usable index; its description has been corrected.) + +A migrated index is *shifted to match* the compacted array, so it is current, +not stale. Skipping the drop when the migration succeeded is what actually +stops the rebuild. Everything else is unchanged: a partially built index, or a +delete that took another path, still drops exactly as before. + +Interleaved A/B, min-of-15 at quiet load (~1.0): `bench_populated_delete` +4361 → **2781 ms, −36.2%** (mean −36.2%). Combined overwrite and +realistic-name read unchanged. + +Cumulative across #9000–#9003 and this: **5938 → 2781 ms, −53.2%** on perry's +worst object-model gap. Suite 2787 passed, no warnings; adversarial property +differential byte-identical to node. diff --git a/crates/perry-runtime/src/object/delete_rest.rs b/crates/perry-runtime/src/object/delete_rest.rs index a4889cf18d..d942ec8048 100644 --- a/crates/perry-runtime/src/object/delete_rest.rs +++ b/crates/perry-runtime/src/object/delete_rest.rs @@ -375,7 +375,7 @@ pub extern "C" fn js_object_delete_field( // property name. On a 500-key object that rebuild ran on EVERY delete. // A wrong index can only cause a miss — `shape_slot_lookup` validates // the stored key against the requested bytes before returning a slot. - super::shapes::shape_index_migrate_after_delete( + let index_migrated = super::shapes::shape_index_migrate_after_delete( keys as usize, keys_cloned as usize, i as u32, @@ -458,7 +458,13 @@ pub extern "C" fn js_object_delete_field( // not eagerly deleted because a sibling may still name one; exact // new facts are published below and weak post-trace pruning retires // dead historical descriptors. - crate::object::shapes::shape_drop(crate::object::object_keys_array(obj)); + // ...unless the migration above already shifted it to match the + // compacted array, in which case it is CURRENT, not stale, and + // dropping it would throw away the rebuild this is meant to avoid — + // the next lookup would re-hash every surviving key name. + if !index_migrated { + crate::object::shapes::shape_drop(crate::object::object_keys_array(obj)); + } 1 } } diff --git a/crates/perry-runtime/src/object/shapes_slot_list.rs b/crates/perry-runtime/src/object/shapes_slot_list.rs index 3866aae596..bc2a1ae521 100644 --- a/crates/perry-runtime/src/object/shapes_slot_list.rs +++ b/crates/perry-runtime/src/object/shapes_slot_list.rs @@ -126,23 +126,27 @@ pub(crate) fn record_shape_scan_outcome( /// index that is wrong produces a MISS and the caller's own fallback, never a /// wrong property. Only a fully-built index is carried over; a partially built /// one is dropped and rebuilt as before. +/// +/// Returns whether the index was actually carried over: the delete tail uses +/// that to skip the `shape_drop` that would otherwise discard it immediately. +#[must_use] pub(crate) fn shape_index_migrate_after_delete( old_keys_id: usize, new_keys_id: usize, removed_slot: u32, old_key_count: u32, -) { +) -> bool { if old_keys_id == 0 || new_keys_id == 0 || old_keys_id == new_keys_id { - return; + return false; } let mut inner = crate::state::state().shapes.inner.borrow_mut(); let Some(mut index) = inner.indices.remove(&old_keys_id) else { - return; + return false; }; if index.indexed_len < old_key_count { // Partially built: shifting it would leave the un-indexed tail // misaligned. Dropping it preserves the previous behaviour exactly. - return; + return false; } index.slots.retain(|_, list| { list.retain_shift(removed_slot); @@ -150,4 +154,5 @@ pub(crate) fn shape_index_migrate_after_delete( }); index.indexed_len = old_key_count - 1; inner.indices.insert(new_keys_id, index); + true }