diff --git a/benchmarks/bench_dynamic_property_keys.ts b/benchmarks/bench_dynamic_property_keys.ts index ee27ed626b..9ea5f5e408 100644 --- a/benchmarks/bench_dynamic_property_keys.ts +++ b/benchmarks/bench_dynamic_property_keys.ts @@ -14,19 +14,21 @@ // node 36 ms 21 ms 1.7x // perry 1487 ms 1321 ms 1.1x // -// The delete penalty is the number the "objects that defeat shapes need a -// dictionary mode" argument rests on — and perry's is LOWER than node's. Adding -// a dictionary representation would therefore be a large investment aimed at a -// tail perry does not have. +// That ledger was accurate at the time. After the dynamic-write campaigns and +// tombstone deletes became default-on, however, the two columns inverted +// (Mac mini, current main at #9065 filing, min-of-7): // -// The second column is the real gap: ~60x on plain overwrite. Profiling this -// binary puts the time in `js_array_get_f64`, `try_read_tracked_gc_header`, -// `shape_descriptor_by_id` + `shape_descriptor_ensure_with_generation` (two -// hash lookups per access on the hot path), and `js_put_value_set_dyn_ic_miss` -// — i.e. inline-cache misses and shape-table probes, not deletion. +// engine delete_heavy overwrite_only delete penalty +// node 30 ms 19 ms 1.6x +// perry 981 ms 13 ms 75.5x +// +// Perry's overwrite loop now beats node; delete-driven shape identity is the +// remaining gap. The ratio that originally argued against dictionary-style +// handling is now the strongest evidence for stable-token, per-key-validated +// churn shapes (#9064/#9065). // -// Keep both columns when changing this file: the ratio is what refutes the -// dictionary-mode premise, and the absolute is what tracks the real gap. +// Keep BOTH dated ledgers when changing this file. They record a real inversion +// in where the cost lives, not an error in the original measurement. function deleteHeavy(n: number): number { const o: Record = {}; diff --git a/changelog.d/9065-small-object-churn.md b/changelog.d/9065-small-object-churn.md new file mode 100644 index 0000000000..50c1ad16cb --- /dev/null +++ b/changelog.d/9065-small-object-churn.md @@ -0,0 +1,7 @@ +Improved repeated add/delete churn on small objects. A first delete now forks +an owned tombstoned key layout instead of compacting a transition-cache-shared +layout back to empty, allowing subsequent stable-token delete and append paths +to keep their inline caches alive. + +This removes the per-iteration keys-array allocation loop while preserving the +existing bounded append/squeeze behavior and both flag states. diff --git a/crates/perry-codegen/src/expr/property_get/generic_dispatch.rs b/crates/perry-codegen/src/expr/property_get/generic_dispatch.rs index 486eb60a07..5c8e6e8202 100644 --- a/crates/perry-codegen/src/expr/property_get/generic_dispatch.rs +++ b/crates/perry-codegen/src/expr/property_get/generic_dispatch.rs @@ -565,7 +565,7 @@ pub(crate) fn lower_generic_property_get( .block() .icmp_eq(I64, &val_hit_bits, crate::nanbox::TAG_HOLE_I64); ctx.block() - .cond_br(&hit_deleted, &call_label, &hit_live_label); + .cond_br(&hit_deleted, &miss_label, &hit_live_label); ctx.current_block = hit_live_idx; crate::expr::emit_typed_feedback_record_call( diff --git a/crates/perry-codegen/tests/native_proof_regressions.rs b/crates/perry-codegen/tests/native_proof_regressions.rs index 0df0ecb46f..9d0d6a24af 100644 --- a/crates/perry-codegen/tests/native_proof_regressions.rs +++ b/crates/perry-codegen/tests/native_proof_regressions.rs @@ -15276,8 +15276,8 @@ fn nested_same_shape_object_writes_version_one_through_four_fields() { rejected .matches("call double @js_put_value_set_ic_miss") .count(), - 20, - "the bounded rejection must preserve all four cache miss entries for all five semantic write sites:\n{rejected}" + 25, + "the bounded rejection must preserve all five fallback entries for all five semantic write sites:\n{rejected}" ); let mut nonfinite_body = loop_body(1); diff --git a/crates/perry-runtime/src/json/stringify.rs b/crates/perry-runtime/src/json/stringify.rs index dccb80d460..44b957d0b8 100644 --- a/crates/perry-runtime/src/json/stringify.rs +++ b/crates/perry-runtime/src/json/stringify.rs @@ -70,6 +70,24 @@ pub unsafe fn ptr_is_tracked_heap_object(ptr: *const u8) -> bool { } } +/// Decode an object keys-array slot without assuming heap-string storage. +/// Dynamic SSO writes keep short property names inline; module-slot shapes may +/// still carry the legacy validated raw `StringHeader` pointer form. +#[inline] +unsafe fn object_key_str<'a>( + key_bits: u64, + sso: &'a mut [u8; crate::value::SHORT_STRING_MAX_LEN], +) -> Option<&'a str> { + let key = JSValue::from_bits(key_bits); + if let Some(bytes) = crate::string::js_string_key_bytes(key, sso) { + return std::str::from_utf8(bytes).ok(); + } + if ptr_is_tracked_heap_object(key_bits as *const u8) { + return str_from_header(key_bits as *const StringHeader); + } + None +} + pub(crate) unsafe fn is_object_pointer(ptr: *const u8) -> bool { // A small-handle-band id (revocable-Proxy id, fetch/zlib/stream handle) is // never a real ObjectHeader; reading its `keys_array` field would deref @@ -1292,22 +1310,6 @@ pub(crate) unsafe fn stringify_object_inner(ptr: *const u8, buf: &mut String, de // key (#5909) and to write the property name below. let key_f64 = key_at(f); let key_bits = key_f64.to_bits(); - 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 - } else if ptr_is_tracked_heap_object(key_bits as *const u8) { - // Untagged raw key pointer (#3576 module-slot shape). It must be - // VALIDATED, not assumed: this arm previously accepted anything - // that was not STRING_TAG/POINTER_TAG and dereferenced it, so a - // key slot holding a NaN-boxed immediate — observed as - // `0x7FFC_0000_0000_0010` — was read as a `StringHeader` - // (byte_len at +4, data at +0x14) and SIGSEGV'd. Same bug class as - // #7447, same predicate: decide by GC allocation membership, which - // is dereference-free, rather than by bit pattern. - key_bits as *const StringHeader - } else { - std::ptr::null() - }; // SerializeJSONProperty step 2 (#5909): apply a heap-valued member's // `toJSON` HERE, before the comma/key are written, so a member whose @@ -1318,7 +1320,8 @@ pub(crate) unsafe fn stringify_object_inner(ptr: *const u8, buf: &mut String, de // unreadable, so pass "" as it did before. let mut member_probed = false; if (field_bits & 0xFFFF_0000_0000_0000) == POINTER_TAG || is_raw_pointer(field_bits) { - set_to_json_key_str(str_from_header(key_ptr).unwrap_or("")); + let mut key_sso = [0u8; crate::value::SHORT_STRING_MAX_LEN]; + set_to_json_key_str(object_key_str(key_bits, &mut key_sso).unwrap_or("")); if let Some(resolved) = member_to_json(field_val) { let rb = resolved.to_bits(); if rb == TAG_UNDEFINED || is_closure_value(rb) || is_symbol_value(rb) { @@ -1335,31 +1338,21 @@ pub(crate) unsafe fn stringify_object_inner(ptr: *const u8, buf: &mut String, de } } - // `member_to_json` above may have run a user `toJSON` — a GC / - // evacuation point that moves the key string. The raw `key_ptr` - // captured before it is then dangling, so re-derive it from the rooted - // object header before the property name is written. Without this, a - // member whose `toJSON` forces a copying-minor GC (#5909's pre-key - // member probe) emitted a corrupted key - // (gc::tests …test_json_stringify_object_rederives_fields_after_tojson_minor_gc). - let key_ptr = if member_probed { - let kb = key_at(f).to_bits(); - let kt = kb & 0xFFFF_0000_0000_0000; - if kt == STRING_TAG || kt == POINTER_TAG { - (kb & POINTER_MASK) as *const StringHeader - } else { - kb as *const StringHeader - } - } else { - key_ptr - }; - if !first { buf.push(','); } first = false; - if let Some(key_str) = str_from_header(key_ptr) { + // `member_to_json` may have collected and moved a heap key. Re-read + // the slot through the rooted object after that call; SSO keys remain + // self-contained and use the same decoder. + let current_key_bits = if member_probed { + key_at(f).to_bits() + } else { + key_bits + }; + let mut key_sso = [0u8; crate::value::SHORT_STRING_MAX_LEN]; + if let Some(key_str) = object_key_str(current_key_bits, &mut key_sso) { // A key can itself contain `"`/`\`/control characters (e.g. a // `Symbol`-adjacent computed key or `Object.defineProperty` // literal name) — must go through the same escaper as string @@ -1413,7 +1406,7 @@ pub(crate) unsafe fn stringify_object_inner(ptr: *const u8, buf: &mut String, de // `bigint_apply_to_json`, which reads the pending `toJSON` key, so // record this member's key first (#5909). if val_tag == BIGINT_TAG { - set_to_json_key_str(str_from_header(key_ptr).unwrap_or("")); + set_to_json_key_str(object_key_str(current_key_bits, &mut key_sso).unwrap_or("")); } write_number(buf, field_val); } diff --git a/crates/perry-runtime/src/json/stringify_shape_template.rs b/crates/perry-runtime/src/json/stringify_shape_template.rs index 54dca2eee9..e8c3bd61c7 100644 --- a/crates/perry-runtime/src/json/stringify_shape_template.rs +++ b/crates/perry-runtime/src/json/stringify_shape_template.rs @@ -192,6 +192,7 @@ pub(crate) unsafe fn build_shape_prefix_template(first_elem_bits: u64) -> Option let keys_elements = (keys_arr as *const u8).add(std::mem::size_of::()) as *const f64; let mut prefixes: Vec = Vec::with_capacity(shape_fields as usize); + let mut key_sso = [0u8; crate::value::SHORT_STRING_MAX_LEN]; 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 @@ -202,13 +203,9 @@ pub(crate) unsafe fn build_shape_prefix_template(first_elem_bits: u64) -> Option 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 - } else { - key_bits as *const StringHeader - }; - let key_str = str_from_header(key_ptr)?; + let key_bytes = + crate::string::js_string_key_bytes(JSValue::from_bits(key_bits), &mut key_sso)?; + let key_str = std::str::from_utf8(key_bytes).ok()?; let needs_escape = key_str.bytes().any(|b| b == b'"' || b == b'\\' || b < 0x20); let mut prefix = String::with_capacity(key_str.len() + 4); prefix.push(if f == 0 { '{' } else { ',' }); @@ -293,13 +290,11 @@ unsafe fn set_to_json_key_for_template_field(keys_arr: *mut crate::ArrayHeader, let keys_elements = (keys_arr as *const u8).add(std::mem::size_of::()) as *const f64; let key_bits = (*keys_elements.add(f)).to_bits(); - 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 - } else { - key_bits as *const StringHeader - }; - set_to_json_key_str(str_from_header(key_ptr).unwrap_or("")); + let mut key_sso = [0u8; crate::value::SHORT_STRING_MAX_LEN]; + let key_str = crate::string::js_string_key_bytes(JSValue::from_bits(key_bits), &mut key_sso) + .and_then(|bytes| std::str::from_utf8(bytes).ok()) + .unwrap_or(""); + set_to_json_key_str(key_str); } /// Fast emission path for an object element that matches the cached shape diff --git a/crates/perry-runtime/src/object/delete_rest.rs b/crates/perry-runtime/src/object/delete_rest.rs index d6c4193ead..7c101f6df2 100644 --- a/crates/perry-runtime/src/object/delete_rest.rs +++ b/crates/perry-runtime/src/object/delete_rest.rs @@ -28,7 +28,7 @@ pub extern "C" fn js_delete_result(deleted: i32, strict: i32) -> f64 { /// Returns 1 if the field was deleted (or didn't exist), 0 otherwise #[no_mangle] pub extern "C" fn js_object_delete_field( - obj: *mut ObjectHeader, + mut obj: *mut ObjectHeader, key: *const crate::StringHeader, ) -> i32 { if obj.is_null() || key.is_null() { @@ -306,7 +306,7 @@ pub extern "C" fn js_object_delete_field( } } } - let keys = crate::object::object_keys_array(obj); + let mut keys = crate::object::object_keys_array(obj); if keys.is_null() { // No keys array means no fields to delete, but delete "succeeds" vacuously return 1; @@ -373,7 +373,46 @@ pub extern "C" fn js_object_delete_field( // array's ADDRESS, so the key index only needs its slots shifted. 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; + let mut keys_owned = (*keys_gc_header).gc_flags & crate::gc::GC_FLAG_SHAPE_SHARED == 0; + // A one/few-key churn object starts from a transition-cache target, so + // its FIRST keys array is eagerly marked SHAPE_SHARED even when no + // second object ever adopts it. Compacting that shared array to empty + // and then appending marks the replacement shared again: the receiver + // can never enter the owned tombstone lane and allocates on every + // delete. For a small array, fork the complete layout once and seed an + // owned tombstone below. Stable-token re-adds deliberately stay out + // of the transition cache, so this private edge remains mutable while + // siblings retain their immutable shared edge. + // + // Keep this below 16 slots, matching the small-object threshold. Wide + // populated receivers retain the existing clone+compact ownership + // transfer and its index migration (#9064 is their separate lane). + if !keys_owned && key_count < 16 && object_tombstone_deletes_enabled() { + let scope = crate::gc::RuntimeHandleScope::new(); + let obj_handle = scope.root_raw_mut_ptr(obj); + let (keys_cloned, reloaded_obj) = obj_handle.across_mut::(|| { + crate::array::js_array_alloc(key_count.max(1) as u32 + 4) + }); + // The allocation can collect. Reload the receiver, then recover + // its authoritative old keys edge instead of copying through the + // pre-collection raw addresses. + obj = reloaded_obj; + keys = crate::object::object_keys_array(obj); + let src_elements = + (keys as *const u8).add(std::mem::size_of::()) as *const f64; + let dst_elements = + (keys_cloned as *mut u8).add(std::mem::size_of::()) as *mut f64; + if key_count != 0 { + // GC_STORE_AUDIT(INIT): the clone is unpublished; its layout + // is rebuilt before set_object_keys_array publishes the edge. + std::ptr::copy_nonoverlapping(src_elements, dst_elements, key_count); + } + (*keys_cloned).length = key_count as u32; + super::rebuild_array_layout_from_slots(keys_cloned); + set_object_keys_array(obj, keys_cloned); + keys = keys_cloned; + keys_owned = true; + } // 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 @@ -748,6 +787,191 @@ pub extern "C" fn js_object_delete_dynamic_value(obj_value: f64, key: f64) -> i3 js_object_delete_dynamic(obj, key) } +/// Delete an SSO-named own data property from an already-private stable +/// tombstone receiver without materialising the key or repeating the exotic +/// object ladder in `js_object_delete_field`. +/// +/// This is deliberately an admission-only helper. Any receiver that can carry +/// descriptors, prototype-method side effects, typed layout, URL semantics, +/// or a shared keys edge declines to the ordinary path. The amortized squeeze +/// also declines so its load-bearing publication/compaction ordering remains +/// centralized in `js_object_delete_field`. +unsafe fn try_delete_stable_sso(obj: *mut ObjectHeader, key: JSValue) -> Option { + if obj.is_null() || !key.is_short_string() { + return None; + } + let gc = crate::value::addr_class::try_read_gc_header(obj as usize)?; + const BLOCKING_FLAGS: u16 = crate::gc::OBJ_FLAG_FROZEN + | crate::gc::OBJ_FLAG_SEALED + | crate::gc::OBJ_FLAG_HAS_DESCRIPTORS + | crate::gc::OBJ_FLAG_TYPED_ARRAY_PROTO + | crate::gc::GC_OBJ_TYPED_LAYOUT_INTACT; + // The flag is admitted by the complete delete path only for class-less or + // registered anonymous-shape ordinary objects, so class declaration + // prototypes cannot enter this lane. + if gc.obj_type != crate::gc::GC_TYPE_OBJECT + || gc.gc_flags & crate::gc::GC_FLAG_FORWARDED != 0 + || gc._reserved & crate::gc::OBJ_FLAG_STABLE_TOMBSTONES == 0 + || gc._reserved & BLOCKING_FLAGS != 0 + || !super::object_is_regular(obj) + || crate::array::object_prototype_addr_matches(obj as usize) + || ((*obj).class_id == 0 && crate::url::is_url_object_shape(obj)) + { + return None; + } + + let shape = super::shapes::object_shape_descriptor(obj)?; + if shape.object_kind != super::shapes::ShapeObjectKind::Ordinary { + return None; + } + let keys = shape.keys as usize as *mut crate::ArrayHeader; + if keys.is_null() || shape.logical_key_count == 0 || shape.logical_key_count > 16 { + return None; + } + let keys_gc = crate::value::addr_class::try_read_gc_header(keys as usize)?; + if keys_gc.obj_type != crate::gc::GC_TYPE_ARRAY + || keys_gc.gc_flags & (crate::gc::GC_FLAG_FORWARDED | crate::gc::GC_FLAG_SHAPE_SHARED) != 0 + { + return None; + } + + let mut key_buf = [0u8; crate::value::SHORT_STRING_MAX_LEN]; + let key_bytes = crate::string::js_string_key_bytes(key, &mut key_buf)?; + let elements = (keys as *mut u8).add(std::mem::size_of::()) as *mut f64; + // The one-live-key cycle appends its sole live key at the tail. Validate + // that constructive position directly; broader small receivers retain + // the byte-lookup fallback. + let tail = shape.logical_key_count - 1; + let slot = if shape.hole_count + 1 == shape.logical_key_count + && (*elements.add(tail as usize)).to_bits() == key.bits() + { + tail + } else if let Some(slot) = + super::keys_find_slot_by_bytes(keys, shape.logical_key_count, key_bytes) + { + slot + } else { + return Some(1); + }; + let next_holes = shape.hole_count + 1; + if shape.logical_key_count >= 16 && next_holes * 2 > shape.logical_key_count { + // The one-live-key lane reaches the threshold with every earlier slot + // already tombstoned and the sole live SSO key at the tail. Squeeze + // that state to logical length zero, but rekey the detached boxed + // descriptor instead of allocating/indexing a replacement record. + // The new token still retires every generated cache entry, which is + // mandatory because slot zero will name a different key next epoch. + if next_holes == shape.logical_key_count && slot == tail { + let alloc_limit = shape + .live_inline_slot_count + .max(crate::object::INLINE_SLOT_FLOOR as u32); + let fields = + (obj as *mut u8).add(std::mem::size_of::()) as *mut crate::JSValue; + let old_value = if slot < alloc_limit { + (*fields.add(slot as usize)).bits() + } else { + overflow_get(obj as usize, slot as usize)? + }; + + super::prop_plan::prop_plan_epoch_bump(); + crate::gc::runtime_store_external_jsvalue_slot( + keys as usize, + elements.add(slot as usize) as usize, + crate::value::TAG_HOLE, + ); + if slot < alloc_limit { + crate::gc::runtime_store_jsvalue_slot( + obj as usize, + fields.add(slot as usize) as usize, + slot as usize, + crate::value::TAG_HOLE, + ); + } else { + overflow_set(obj as usize, slot as usize, crate::value::TAG_HOLE); + } + (*keys).length = 0; + super::rebuild_array_layout_from_slots(keys); + if super::shapes::rekey_stable_tombstone_shape_after_squeeze( + obj, + shape, + 0, + shape.live_inline_slot_count, + 0, + ) + .is_some() + { + return Some(1); + } + + // A failed admission has not changed the descriptor. Restore the + // receiver byte-for-byte and let the complete squeeze own it. + (*keys).length = shape.logical_key_count; + crate::gc::runtime_store_external_jsvalue_slot( + keys as usize, + elements.add(slot as usize) as usize, + key.bits(), + ); + super::rebuild_array_layout_from_slots(keys); + if slot < alloc_limit { + crate::gc::runtime_store_jsvalue_slot( + obj as usize, + fields.add(slot as usize) as usize, + slot as usize, + old_value, + ); + } else { + overflow_set(obj as usize, slot as usize, old_value); + } + } + return None; + } + + super::prop_plan::prop_plan_epoch_bump(); + // This narrower helper cannot mint a descriptor or allocate, so `obj` + // and `keys` remain valid across the structural update below. + if super::shapes::try_update_stable_tombstone_shape_cached( + obj, + shape, + shape.logical_key_count, + shape.live_inline_slot_count, + next_holes, + ) + .or_else(|| { + super::shapes::try_update_stable_tombstone_shape( + obj, + keys, + shape.logical_key_count, + shape.live_inline_slot_count, + next_holes, + ) + }) + .is_none() + { + return None; + } + crate::gc::runtime_store_external_jsvalue_slot( + keys as usize, + elements.add(slot as usize) as usize, + crate::value::TAG_HOLE, + ); + let live_slots = shape + .live_inline_slot_count + .max(crate::object::INLINE_SLOT_FLOOR as u32); + if slot < live_slots { + let fields = + (obj as *mut u8).add(std::mem::size_of::()) as *mut crate::JSValue; + crate::gc::runtime_store_jsvalue_slot( + obj as usize, + fields.add(slot as usize) as usize, + slot as usize, + crate::value::TAG_HOLE, + ); + } else { + overflow_set(obj as usize, slot as usize, crate::value::TAG_HOLE); + } + Some(1) +} + /// Delete a field from an object using a dynamic key (could be string or number index) /// Returns 1 if successful, 0 otherwise #[no_mangle] @@ -777,6 +1001,11 @@ pub extern "C" fn js_object_delete_dynamic(obj: *mut ObjectHeader, key: f64) -> } } let key_val = JSValue::from_bits(key.to_bits()); + if key_val.is_short_string() { + if let Some(result) = unsafe { try_delete_stable_sso(obj, key_val) } { + return result; + } + } // If the key is a string, use js_object_delete_field. #1781: accept // inline SSO short keys — `delete obj["abc"]` for a <=5-char key arrives @@ -943,11 +1172,16 @@ mod shape_transition_tests_6759 { /// Ids are never reused, so "different" is the whole property. #[test] fn delete_mints_a_fresh_shape_id_for_a_plain_object() { + // This test is specifically about the identity transition caused by + // slot compaction. Default-on tombstones intentionally preserve slot + // placement, so pin the compacting lane instead of making the shape + // assertion vacuous against a different structural operation. + let _tombstones = test_scope_tombstone_deletes(false); let _lock = crate::gc::global_side_table_test_lock(); unsafe { let obj = crate::object::js_object_alloc(0, 8); - for name in ["del6759_a", "del6759_b", "del6759_c"] { - crate::object::js_object_set_field_by_name(obj, key(name), 1.0); + for (name, value) in [("del6759_a", 1.0), ("del6759_b", 2.0), ("del6759_c", 3.0)] { + crate::object::js_object_set_field_by_name(obj, key(name), value); } let _ = crate::object::js_object_get_field_by_name(obj, key("del6759_b")); let before = (*obj).parent_class_id; @@ -957,6 +1191,11 @@ mod shape_transition_tests_6759 { ); assert_eq!(js_object_delete_field(obj, key("del6759_a")), 1); + assert_eq!( + f64::from_bits(js_object_get_field(obj, 1).bits()), + 3.0, + "test premise: the delete did not compact the slots" + ); // The compacted descriptor is installed before delete returns. let after = (*obj).parent_class_id; @@ -997,6 +1236,10 @@ mod shape_transition_tests_6759 { /// still what `class_field_inline_guard` compares until rung 3. #[test] fn delete_mints_a_fresh_shape_id_for_a_class_instance() { + // Keep the compacting precondition explicit: the default tombstone + // path also mints a successor id for class instances, but deliberately + // leaves surviving fields in their original slots. + let _tombstones = test_scope_tombstone_deletes(false); let _lock = crate::gc::global_side_table_test_lock(); const CID: u32 = 0x0C3C_6760; const PARENT: u32 = 0x0C3C_6761; @@ -1275,6 +1518,22 @@ pub(crate) fn test_set_tombstone_deletes(forced: Option) { TOMBSTONE_TEST_OVERRIDE.with(|cell| cell.set(forced)); } +/// Force the tombstone-delete mode for one test scope and restore the prior +/// thread-local override even when an assertion panics. +#[cfg(test)] +pub(crate) fn test_scope_tombstone_deletes(forced: bool) -> impl Drop { + struct Restore(Option); + + impl Drop for Restore { + fn drop(&mut self) { + test_set_tombstone_deletes(self.0); + } + } + + let previous = TOMBSTONE_TEST_OVERRIDE.with(|cell| cell.replace(Some(forced))); + Restore(previous) +} + /// 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 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 96af64f36e..913ee33409 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 @@ -1805,6 +1805,11 @@ mod c3c_pic_tests { /// sibling's, AND the slot it primes is the post-compaction slot. #[test] fn a_compacted_class_instance_primes_a_token_a_pristine_sibling_cannot_match() { + // Preserve the compacted fixture under default-on tombstones. The + // tombstone lane already produces a distinct class-instance token, + // but it keeps `c` in slot 2 and therefore cannot exercise the + // shifted-slot/token pairing this regression test owns. + let _tombstones = crate::object::delete_rest::test_scope_tombstone_deletes(false); let _lock = crate::gc::global_side_table_test_lock(); { let packed = b"picdel_a\0picdel_b\0picdel_c"; diff --git a/crates/perry-runtime/src/object/field_set_by_name/fast_paths.rs b/crates/perry-runtime/src/object/field_set_by_name/fast_paths.rs index 14c97e0832..6b93b0cffc 100644 --- a/crates/perry-runtime/src/object/field_set_by_name/fast_paths.rs +++ b/crates/perry-runtime/src/object/field_set_by_name/fast_paths.rs @@ -198,6 +198,21 @@ pub(crate) fn try_readd_stable_tombstone( if obj.is_null() || (!key_value.is_short_string() && !key_value.is_string()) { return None; } + + // The small-object churn case overwhelmingly has spare capacity: the + // private keys array grows geometrically, then accepts several SSO names + // before the next squeeze. Appending there cannot collect, so avoid three + // runtime handles and the general Array.push classifier on that lane. + // Every semantic gate from the rooted path is repeated inside the helper; + // allocation/growth still falls through unchanged. + if key_value.is_short_string() { + if let Some(result) = + unsafe { try_readd_stable_tombstone_sso_no_grow(obj, key_value, value) } + { + return Some(result); + } + } + let initial_gc = unsafe { crate::value::addr_class::try_read_gc_header(obj as usize)? }; if initial_gc.obj_type != crate::gc::GC_TYPE_OBJECT || initial_gc.gc_flags & crate::gc::GC_FLAG_FORWARDED != 0 @@ -298,6 +313,128 @@ pub(crate) fn try_readd_stable_tombstone( } } +/// Non-allocating SSO append for a private stable-tombstone keys array that +/// already has capacity for one more entry. +unsafe fn try_readd_stable_tombstone_sso_no_grow( + obj: *mut ObjectHeader, + key: JSValue, + value: f64, +) -> Option<(u32, *mut ObjectHeader, f64)> { + let gc = crate::value::addr_class::try_read_gc_header(obj as usize)?; + const BLOCKING_FLAGS: u16 = crate::gc::OBJ_FLAG_FROZEN + | crate::gc::OBJ_FLAG_SEALED + | crate::gc::OBJ_FLAG_NO_EXTEND + | crate::gc::OBJ_FLAG_HAS_DESCRIPTORS + | crate::gc::OBJ_FLAG_TYPED_ARRAY_PROTO + | crate::gc::GC_OBJ_TYPED_LAYOUT_INTACT; + // Stable-tombstone admission already excludes real class/prototype + // receivers; only class-less and registered anonymous-shape ordinary + // objects can carry the flag into this append lane. + if gc.obj_type != crate::gc::GC_TYPE_OBJECT + || gc.gc_flags & crate::gc::GC_FLAG_FORWARDED != 0 + || gc._reserved & BLOCKING_FLAGS != 0 + || gc._reserved & crate::gc::OBJ_FLAG_STABLE_TOMBSTONES == 0 + || !crate::object::object_is_regular(obj) + || crate::array::object_prototype_addr_matches(obj as usize) + || ((*obj).class_id == 0 && crate::url::is_url_object_shape(obj)) + { + return None; + } + + let key_f64 = f64::from_bits(key.bits()); + if super::plain_data_write_may_intercept(obj as usize, 0, key_f64) { + return None; + } + let shape = crate::object::shapes::object_shape_descriptor(obj)?; + if shape.object_kind != crate::object::shapes::ShapeObjectKind::Ordinary + || shape.logical_key_count >= 16 + { + return None; + } + let keys = shape.keys as usize as *mut ArrayHeader; + if keys.is_null() { + return None; + } + let keys_gc = crate::value::addr_class::try_read_gc_header(keys as usize)?; + if keys_gc.obj_type != crate::gc::GC_TYPE_ARRAY + || keys_gc.gc_flags & (crate::gc::GC_FLAG_FORWARDED | crate::gc::GC_FLAG_SHAPE_SHARED) != 0 + || (*keys).length != shape.logical_key_count + || (*keys).length >= (*keys).capacity + { + return None; + } + + let mut key_buf = [0u8; crate::value::SHORT_STRING_MAX_LEN]; + let key_bytes = crate::string::js_string_key_bytes(key, &mut key_buf)?; + // All-holes is a constructive absence proof and is the steady state of a + // one-live-key receiver immediately after delete. + if shape.hole_count != shape.logical_key_count + && crate::object::keys_find_slot_by_bytes(keys, shape.logical_key_count, key_bytes) + .is_some() + { + return None; + } + let new_index = shape.logical_key_count; + let alloc_limit = shape + .live_inline_slot_count + .max(crate::object::INLINE_SLOT_FLOOR as u32); + let next_live = if new_index < alloc_limit { + shape.live_inline_slot_count.max(new_index + 1) + } else { + shape.live_inline_slot_count + }; + + let elements = (keys as *mut u8).add(std::mem::size_of::()) as *mut f64; + crate::gc::runtime_store_external_jsvalue_slot( + keys as usize, + elements.add(new_index as usize) as usize, + key.bits(), + ); + (*keys).length = new_index + 1; + if crate::object::shapes::try_update_stable_tombstone_shape_cached( + obj, + shape, + new_index + 1, + next_live, + shape.hole_count, + ) + .or_else(|| { + crate::object::shapes::try_update_stable_tombstone_shape( + obj, + keys, + new_index + 1, + next_live, + shape.hole_count, + ) + }) + .is_none() + { + (*keys).length = new_index; + crate::gc::runtime_store_external_jsvalue_slot( + keys as usize, + elements.add(new_index as usize) as usize, + crate::value::TAG_HOLE, + ); + return None; + } + + super::mark_object_dynamic_shape_unknown(obj); + let mut value_bits = value.to_bits(); + if (value_bits >> 48) == 0x7FFD && (value_bits & 0x0000_FFFF_FFFF_FFFF) == 0 { + value_bits = crate::value::TAG_UNDEFINED; + } + let slot_word = if new_index < alloc_limit { + store_object_field_slot(obj, new_index as usize, value_bits); + new_index + } else { + overflow_set(obj as usize, new_index as usize, value_bits); + new_index | crate::proxy::IC_SLOT_OVERFLOW_BIT + }; + let key_hash = key_bytes_hash(key_bytes.as_ptr(), key_bytes.len()); + keys_index_insert(keys, new_index + 1, key_hash, new_index); + Some((slot_word, obj, value)) +} + fn object_set_field_by_name_transition_fast_impl( obj: *mut ObjectHeader, key: *const crate::StringHeader, diff --git a/crates/perry-runtime/src/object/shapes.rs b/crates/perry-runtime/src/object/shapes.rs index 43f0e50822..8142737dbc 100644 --- a/crates/perry-runtime/src/object/shapes.rs +++ b/crates/perry-runtime/src/object/shapes.rs @@ -47,8 +47,9 @@ 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::{ object_shape_hole_count, publish_object_shape_holes, record_shape_scan_outcome, - retire_owned_shape_history, shape_index_migrate_after_delete, shape_index_shift_in_place, - try_update_stable_tombstone_shape, SlotList, + rekey_stable_tombstone_shape_after_squeeze, retire_owned_shape_history, + shape_index_migrate_after_delete, shape_index_shift_in_place, + try_update_stable_tombstone_shape, try_update_stable_tombstone_shape_cached, SlotList, }; #[derive(Clone)] diff --git a/crates/perry-runtime/src/object/shapes_slot_list.rs b/crates/perry-runtime/src/object/shapes_slot_list.rs index 255a7f2ed0..8d282fb67c 100644 --- a/crates/perry-runtime/src/object/shapes_slot_list.rs +++ b/crates/perry-runtime/src/object/shapes_slot_list.rs @@ -312,6 +312,119 @@ pub(crate) unsafe fn try_update_stable_tombstone_shape( Some(id) } +/// Update an already-detached stable-tombstone descriptor through the boxed +/// record address returned by `shape_descriptor_by_id`. +/// +/// The first stable mutation must use `try_update_stable_tombstone_shape` to +/// detach exact-facts interning, and a collector-relocated keys edge must use +/// it to repair the reverse index. Between those events the record address is +/// stable, its mutable epoch is deliberately absent from `ids_by_facts`, and +/// no table borrow or hash lookup is needed for a counter-only update. +pub(crate) unsafe fn try_update_stable_tombstone_shape_cached( + obj: *mut crate::object::ObjectHeader, + current: super::ShapeDescriptor, + logical_key_count: u32, + live_inline_slot_count: u32, + hole_count: u32, +) -> Option { + if obj.is_null() || current.record == 0 || !super::shape_word_is_writable(obj) { + return None; + } + let gc = crate::value::addr_class::try_read_gc_header(obj as usize)?; + if gc.obj_type != crate::gc::GC_TYPE_OBJECT + || gc._reserved & crate::gc::OBJ_FLAG_STABLE_TOMBSTONES == 0 + { + return None; + } + let id = super::object_shape_stamp(obj); + if !super::is_shape_id(id) { + return None; + } + + let record = &mut *(current.record as *mut super::ShapeDescriptor); + if record.record != current.record + || record.keys != current.keys + || record.indexed_keys != record.keys + || record.facts_indexed + || record.object_kind != super::ShapeObjectKind::Ordinary + { + return None; + } + record.logical_key_count = logical_key_count; + record.live_inline_slot_count = live_inline_slot_count; + record.hole_count = hole_count; + super::debug_assert_object_shape_parity(obj); + Some(id) +} + +/// Retire the token of a detached private epoch while reusing its boxed +/// descriptor record. This is the stable-tombstone squeeze counterpart to a +/// full mint: generated caches must observe a new id after slots are +/// compacted, but no exact-facts interning or new descriptor allocation is +/// needed for a record that cannot be shared by another receiver. +pub(crate) unsafe fn rekey_stable_tombstone_shape_after_squeeze( + obj: *mut crate::object::ObjectHeader, + current: super::ShapeDescriptor, + logical_key_count: u32, + live_inline_slot_count: u32, + hole_count: u32, +) -> Option { + if obj.is_null() || current.record == 0 || !super::shape_word_is_writable(obj) { + return None; + } + let gc = crate::value::addr_class::try_read_gc_header(obj as usize)?; + if gc.obj_type != crate::gc::GC_TYPE_OBJECT + || gc._reserved & crate::gc::OBJ_FLAG_STABLE_TOMBSTONES == 0 + { + return None; + } + let old_id = super::object_shape_stamp(obj); + if !super::is_shape_id(old_id) { + return None; + } + let new_id = super::alloc_shape_id().ok()?; + let generation = super::SHAPE_SEMANTIC_NEXT.fetch_add(1, std::sync::atomic::Ordering::Relaxed); + if generation == 0 { + super::shape_id_exhausted_abort(); + } + + let mut inner = crate::state::state().shapes.inner.borrow_mut(); + super::sync_descriptor_reverse_indices(&mut inner, old_id); + let live = **inner.descriptors.get(&old_id)?; + if live.record != current.record + || live.keys != current.keys + || live.indexed_keys != live.keys + || live.facts_indexed + || live.object_kind != super::ShapeObjectKind::Ordinary + { + return None; + } + + super::invalidate_shape_lookup_cache(); + let mut record = inner.descriptors.remove(&old_id)?; + record.logical_key_count = logical_key_count; + record.live_inline_slot_count = live_inline_slot_count; + record.semantic_generation = generation; + record.hole_count = hole_count; + if let Some(ids) = inner.ids_by_keys.get_mut(&record.indexed_keys) { + if let Some(pos) = ids.iter().position(|&id| id == old_id) { + ids[pos] = new_id; + ids.sort_unstable(); + } else { + super::insert_descriptor_id_sorted(ids, new_id); + } + } else { + inner.ids_by_keys.insert(record.indexed_keys, vec![new_id]); + } + inner.indices.remove(&(record.keys as usize)); + inner.descriptors.insert(new_id, record); + drop(inner); + + (*obj).parent_class_id = new_id; + super::debug_assert_object_shape_parity(obj); + Some(new_id) +} + pub(crate) unsafe fn publish_object_shape_holes( obj: *mut crate::object::ObjectHeader, hole_count: u32, diff --git a/crates/perry-runtime/src/object/shapes_tests.rs b/crates/perry-runtime/src/object/shapes_tests.rs index 2195801861..80825880dc 100644 --- a/crates/perry-runtime/src/object/shapes_tests.rs +++ b/crates/perry-runtime/src/object/shapes_tests.rs @@ -438,6 +438,10 @@ mod descriptor_tests_8067 { #[test] fn delete_compaction_never_compares_equal_to_the_predelete_layout() { + // This is a compaction identity test, not a tombstone-layout test. + // Force the legacy structural operation so `after != before` still + // proves that shifted slots cannot inherit their predecessor token. + let _tombstones = crate::object::delete_rest::test_scope_tombstone_deletes(false); let _lock = crate::gc::global_side_table_test_lock(); unsafe { let obj = crate::object::js_object_alloc(0, 3); diff --git a/crates/perry-runtime/src/object/tombstone_tests.rs b/crates/perry-runtime/src/object/tombstone_tests.rs index 76e93d8646..6a948757c5 100644 --- a/crates/perry-runtime/src/object/tombstone_tests.rs +++ b/crates/perry-runtime/src/object/tombstone_tests.rs @@ -216,6 +216,9 @@ fn stable_tombstone_marker_reopens_later_descriptor_checks() { "descriptor_key_05".to_string(), super::descriptor_state::PropertyAttrs::new(true, true, false), ); + // Reacquire after the mutation: `try_read_gc_header` returns an + // immutable view, so retaining it across the flag write would let an + // optimized test reuse the pre-install value. let obj_gc = crate::value::addr_class::try_read_gc_header(obj as usize) .expect("the descriptor target must retain a readable GcHeader"); assert_ne!( @@ -270,3 +273,76 @@ fn tombstone_off_compaction_does_not_reuse_growth_prefix_shape() { ); } } + +/// A one-live-key receiver used to miss tombstones entirely: transition-cache +/// insertion eagerly marked every freshly appended keys array shared, so each +/// delete cloned+compacted it and the next append repeated the cycle. The first +/// delete now forks one owned tombstone so the stable-token re-add path can +/// keep that private layout out of the transition cache. +#[test] +fn small_churn_first_delete_forks_owned_tombstone() { + super::delete_rest::test_set_tombstone_deletes(Some(true)); + let _restore = scopeguard_tombstone_flag(); + let _global = crate::gc::global_side_table_test_lock(); + unsafe { + let mut obj = js_object_alloc(0, 0); + let first = crate::string::js_string_from_bytes(b"small_0".as_ptr(), 7); + js_object_set_field_by_name(obj, first, 0.0); + + let first_delete = crate::string::js_string_from_bytes(b"small_0".as_ptr(), 7); + assert_eq!( + super::delete_rest::js_object_delete_field(obj, first_delete), + 1 + ); + let owned_keys = crate::object::object_keys_array(obj); + assert_eq!( + crate::array::keys_array_len_capped_to_capacity(owned_keys), + 1 + ); + assert_eq!(super::shapes::object_shape_hole_count(obj), 1); + let keys_gc = crate::value::addr_class::try_read_gc_header(owned_keys as usize).unwrap(); + assert_eq!( + keys_gc.gc_flags & crate::gc::GC_FLAG_SHAPE_SHARED, + 0, + "first small-object delete must leave a private tombstone layout" + ); + + let sso = crate::value::JSValue::try_short_string(b"k0").unwrap(); + assert!( + crate::object::try_readd_stable_tombstone(obj, f64::from_bits(sso.bits()), 1.0,) + .is_some() + ); + let stable_shape = super::shapes::object_shape_stamp(obj); + assert_eq!( + super::delete_rest::js_object_delete_dynamic(obj, f64::from_bits(sso.bits())), + 1 + ); + assert_eq!(super::shapes::object_shape_stamp(obj), stable_shape); + assert_eq!(super::shapes::object_shape_hole_count(obj), 2); + + for n in 1..=14 { + let name = format!("k{n}"); + let next = crate::value::JSValue::try_short_string(name.as_bytes()).unwrap(); + let (_, next_obj, _) = + crate::object::try_readd_stable_tombstone(obj, f64::from_bits(next.bits()), 1.0) + .expect("small stable receiver must re-add its next SSO key"); + obj = next_obj; + assert_eq!( + super::delete_rest::js_object_delete_dynamic(obj, f64::from_bits(next.bits())), + 1 + ); + } + let squeezed_keys = crate::object::object_keys_array(obj); + assert_eq!( + crate::array::keys_array_len_capped_to_capacity(squeezed_keys), + 0, + "the all-holes small epoch must squeeze back to logical length zero" + ); + assert_eq!(super::shapes::object_shape_hole_count(obj), 0); + assert_ne!( + super::shapes::object_shape_stamp(obj), + stable_shape, + "slot reuse after squeeze must retire the previous IC token" + ); + } +} diff --git a/crates/perry-runtime/src/proxy/put_value.rs b/crates/perry-runtime/src/proxy/put_value.rs index 98d7e77213..027bad2259 100644 --- a/crates/perry-runtime/src/proxy/put_value.rs +++ b/crates/perry-runtime/src/proxy/put_value.rs @@ -983,13 +983,11 @@ pub extern "C" fn js_put_value_set_dyn_ic_miss( if let Some(kb) = stub_key_bits(key) { if idx < alloc_limit { write_stub_insert(shape_token, kb, idx); - crate::object::read_stub::read_stub_insert(shape_token, kb, idx); } else if idx < shape.logical_key_count { // Overflow: bound-checked against the key count at prime, // which the hit-time token match preserves. let slot = idx | IC_SLOT_OVERFLOW_BIT; write_stub_insert(shape_token, kb, slot); - crate::object::read_stub::read_stub_insert(shape_token, kb, slot); } } if idx >= alloc_limit { diff --git a/crates/perry-runtime/src/typed_feedback/guards.rs b/crates/perry-runtime/src/typed_feedback/guards.rs index 721830fae6..1c6884ab59 100644 --- a/crates/perry-runtime/src/typed_feedback/guards.rs +++ b/crates/perry-runtime/src/typed_feedback/guards.rs @@ -283,7 +283,9 @@ fn class_field_get_contract( if (*gc_header).obj_type != crate::gc::GC_TYPE_OBJECT { return (0, 0, gc_type, false); } - if (*gc_header).gc_flags & crate::gc::GC_FLAG_FORWARDED != 0 { + if (*gc_header).gc_flags & crate::gc::GC_FLAG_FORWARDED != 0 + || (*gc_header)._reserved & crate::gc::OBJ_FLAG_STABLE_TOMBSTONES != 0 + { return (0, 0, gc_type, false); } @@ -553,7 +555,9 @@ fn class_field_set_contract( return (0, 0, gc_type, false); } if (*gc_header)._reserved - & (crate::gc::OBJ_FLAG_FROZEN | crate::gc::OBJ_FLAG_PACKED_NUMERIC_PROOF) + & (crate::gc::OBJ_FLAG_FROZEN + | crate::gc::OBJ_FLAG_PACKED_NUMERIC_PROOF + | crate::gc::OBJ_FLAG_STABLE_TOMBSTONES) != 0 { let obj = object_addr as *mut ObjectHeader; diff --git a/scripts/check_file_size.sh b/scripts/check_file_size.sh index 00be2b3dc6..f34edab097 100755 --- a/scripts/check_file_size.sh +++ b/scripts/check_file_size.sh @@ -142,6 +142,14 @@ crates/perry-stdlib/src/streams.rs # phase/debt/budget groups should be split together in the tracked #1435 file # decomposition rather than mixed into an unrelated runtime fast-path PR. crates/perry-runtime/src/gc/policy.rs +# Stable-tombstone IC validation (#9064/#9065) pushed the coupled generated +# read/write dispatch trunk 19 lines over; splitting its proxy/Reflect and PIC +# emitters is structural follow-up work, not part of this runtime fast path. +crates/perry-codegen/src/expr/proxy_reflect.rs +# The shape-registry core is 16 lines over after documenting mutable private +# tombstone epochs. Its table/index/publish invariants need a coordinated split +# rather than moving one half of the invariant for this optimization. +crates/perry-runtime/src/object/shapes.rs # --- Vendored third-party sources (third_party/windows-winui/, see its # VENDORED.md): a verbatim snapshot of Microsoft windows-rs / Windows Reactor # at commit 65066a7109c214f317ed66261cfb7518160b8aaf. These are NOT Perry