diff --git a/crates/perry-runtime/src/object/descriptor_state.rs b/crates/perry-runtime/src/object/descriptor_state.rs index 58679752ae..9b49cf8e2a 100644 --- a/crates/perry-runtime/src/object/descriptor_state.rs +++ b/crates/perry-runtime/src/object/descriptor_state.rs @@ -996,6 +996,132 @@ pub(crate) fn set_accessor_descriptor(obj: usize, key: String, acc: AccessorDesc .insert((obj, key), acc); } +/// #9103 follow-up: one-call install of a BRAND-NEW accessor property — the +/// `{ get, enumerable: true }` fast arm's tail +/// (`object_ops/define_get_accessor.rs`), which installs ~1,245 re-export +/// getters at pi startup and previously paid the full +/// `set_accessor_descriptor` + `set_property_attrs` stack twice over. +/// +/// Semantically identical to +/// `set_accessor_descriptor(obj, key.clone(), acc); +/// set_property_attrs(obj, key, attrs);` +/// with the duplicated per-call work folded to one occurrence. Each fold is +/// individually equivalence-preserving: +/// +/// * **One epoch bump.** The plan epochs are compared against snapshots for +/// equality ("changed since I cached?"); the two halves of the old sequence +/// run back-to-back on the single mutator thread with no reader in +/// between, so one bump invalidates every snapshot exactly as two did. +/// * **One `note_descriptor_target`.** Its flag writes are idempotent, and +/// its `transition_object_shape_semantics` mints a fresh semantic +/// generation whose only consumer contract is "any cached fact keyed by an +/// older ShapeId is now stale" — one fresh generation retires older ids +/// exactly as two consecutive generations did (nothing can observe the +/// intermediate id: no reader runs between the halves). +/// * **One `disable_inline_guards_for_descriptor_target`.** Both old calls +/// passed the identical `(obj, key)`; the body is idempotent (guard-slot +/// retirement plus a hash-set insert). +/// * **One meta access setting BOTH kind bits** (`accessor_key_bits` / +/// `attr_key_bits`), returning each bit's prior state. +/// * **Owner-index dedupe elided when the kind's meta bit was clear.** The +/// summary's own contract (see `note_meta_descriptor_key` / +/// `may_have_descriptor_entry`: "every insert … whose owner is +/// meta-capable sets the key's bit first, so for such owners a clear bit — +/// or a still-null meta record — proves the tables hold no entry for that +/// key") extends to the owner indexes: every `owner_index_add` site in +/// this file is preceded by the matching-kind `note_meta_descriptor_key`, +/// bits are never cleared, and index removals only shrink the index — so a +/// clear prior bit proves the index holds no entry either, and the O(N) +/// `Vec` dedupe scan (the second-largest term of the __export +/// install profile at 500 keys) can be a plain push. A set prior bit (a +/// Bloom collision, a genuine earlier entry) or a non-meta-capable owner +/// keeps the scanning `owner_index_add`. +/// +/// Callers must guarantee the property is brand new on `obj` (the fast arm +/// proves absence via `own_key_present_via_index` / +/// `obj_value_has_own_key` immediately before, with no allocation between +/// probe and install); the descriptor-table `insert`s themselves are plain +/// upserts either way, so a violated precondition degrades to the old +/// overwrite behavior, never to corruption. +pub(crate) fn install_fresh_accessor_property( + obj: usize, + key: String, + acc: AccessorDescriptor, + attrs: PropertyAttrs, +) { + super::prop_plan::prop_plan_epoch_bump(); + note_descriptor_target(obj); + let st = state(); + st.descriptors.accessors_in_use.set(true); + st.descriptors.property_attrs_in_use.set(true); + GLOBAL_DESCRIPTORS_IN_USE.store(true, Ordering::Relaxed); + disable_inline_guards_for_descriptor_target(obj, &key); + note_accessor_descriptor_key(&key); + match note_meta_descriptor_key_both(obj, &key) { + Some((accessor_bit_was_set, attr_bit_was_set)) => { + if accessor_bit_was_set { + owner_index_add(&st.descriptors.accessor_keys_by_owner, obj, &key); + } else { + owner_index_push_proven_new(&st.descriptors.accessor_keys_by_owner, obj, &key); + } + if attr_bit_was_set { + owner_index_add(&st.descriptors.attr_keys_by_owner, obj, &key); + } else { + owner_index_push_proven_new(&st.descriptors.attr_keys_by_owner, obj, &key); + } + } + // Non-meta-capable owner: no summary to consult — keep the scans. + None => { + owner_index_add(&st.descriptors.accessor_keys_by_owner, obj, &key); + owner_index_add(&st.descriptors.attr_keys_by_owner, obj, &key); + } + } + st.descriptors + .accessor_descriptors + .borrow_mut() + .insert((obj, key.clone()), acc); + st.descriptors + .property_descriptors + .borrow_mut() + .insert((obj, key), attrs); +} + +/// [`owner_index_add`] minus the dedupe scan, for a key +/// [`install_fresh_accessor_property`] has PROVEN absent via the meta +/// summary. Never call without that proof — a duplicate push would make +/// enumeration report the key twice. +fn owner_index_push_proven_new( + index: &RefCell>>, + owner: usize, + key: &str, +) { + index + .borrow_mut() + .entry(owner) + .or_default() + .push(key.to_string()); +} + +/// [`note_meta_descriptor_key`] for both kinds in ONE meta access, returning +/// each kind bit's PRIOR state `(accessor_bit_was_set, attr_bit_was_set)` — +/// `None` for a non-meta-capable owner (nothing recorded, matching the +/// single-kind form's no-op arm). +fn note_meta_descriptor_key_both(owner: usize, key: &str) -> Option<(bool, bool)> { + unsafe { + let obj = super::prototype_chain::meta_capable_object(owner)?; + // No-move window: `object_meta_ensure` allocates (see + // `note_meta_descriptor_key`). + let _no_gc = crate::gc::GcSuppressScope::new(); + let meta = super::object_meta_ensure(obj); + let bit = descriptor_key_bit(key); + let accessor_bit_was_set = (*meta).accessor_key_bits & bit != 0; + let attr_bit_was_set = (*meta).attr_key_bits & bit != 0; + (*meta).accessor_key_bits |= bit; + (*meta).attr_key_bits |= bit; + Some((accessor_bit_was_set, attr_bit_was_set)) + } +} + /// Remove an accessor descriptor for (obj, key), letting ordinary data-property /// reads and writes use the object's stored field again. pub(crate) fn clear_accessor_descriptor(obj: usize, key: &str) { diff --git a/crates/perry-runtime/src/object/mod.rs b/crates/perry-runtime/src/object/mod.rs index 44956dcd2e..772c4943c3 100644 --- a/crates/perry-runtime/src/object/mod.rs +++ b/crates/perry-runtime/src/object/mod.rs @@ -248,8 +248,9 @@ pub(crate) use descriptor_state::{ accessor_descriptor_keys_for_obj, class_field_inline_guard_enabled, class_instance_set_may_intercept, clear_accessor_descriptor, clear_property_attrs, constructor_accessor_ever_installed, descriptors_in_use, disable_class_field_inline_guard, - get_accessor_descriptor, get_property_attrs, json_object_getter_value, mark_all_keys, - object_has_descriptors, object_proto_may_intercept_key, owner_has_property_descriptors, + get_accessor_descriptor, get_property_attrs, install_fresh_accessor_property, + json_object_getter_value, mark_all_keys, object_has_descriptors, + object_proto_may_intercept_key, owner_has_property_descriptors, owner_may_have_descriptor_entries, plain_data_write_may_intercept, prune_dead_descriptor_owner_entries, reflect_getter_closure_bits, set_accessor_descriptor, set_builtin_accessor_descriptor, set_builtin_property_attrs, set_property_attrs, diff --git a/crates/perry-runtime/src/object/object_ops/define_get_accessor.rs b/crates/perry-runtime/src/object/object_ops/define_get_accessor.rs index d5e596a8f2..1ee51a821d 100644 --- a/crates/perry-runtime/src/object/object_ops/define_get_accessor.rs +++ b/crates/perry-runtime/src/object/object_ops/define_get_accessor.rs @@ -189,22 +189,21 @@ unsafe fn try_fast_install( if obj.is_null() { return Some(obj_value); } - set_accessor_descriptor( - obj as usize, - key_rust.clone(), - AccessorDescriptor { - get: get_bits, - set: 0, - }, - ); // New-property attributes for `{ get, enumerable: true }`: `enumerable` // explicit, omitted `configurable` → false, and the internal writable bit // stays `true` for a brand-new accessor (the generic arm's `has_accessor` // default) so data lookups before the accessor override don't reject a - // legitimate fallthrough write. - set_property_attrs( + // legitimate fallthrough write. The one-call installer folds the + // duplicated `set_accessor_descriptor` + `set_property_attrs` work — the + // brand-newness it requires is exactly what the presence probe above + // proved, with nothing allocating in between. + super::super::install_fresh_accessor_property( obj as usize, key_rust, + AccessorDescriptor { + get: get_bits, + set: 0, + }, PropertyAttrs::new(true, true, false), ); Some(f64::from_bits(obj_handle.get_heap_word_u64())) @@ -330,6 +329,74 @@ mod tests { } } + /// #9103 follow-up: the one-call installer's side-table state is + /// identical to the two-call `set_accessor_descriptor` + + /// `set_property_attrs` sequence it replaces — descriptor entry, attrs, + /// and the owner index (exactly one entry, so enumeration reports the + /// key once). + #[test] + fn combined_installer_matches_two_call_sequence() { + let _global = crate::gc::global_side_table_test_lock(); + let combined = js_object_alloc(0, 0); + super::super::super::install_fresh_accessor_property( + combined as usize, + "beta".to_string(), + AccessorDescriptor { get: 0, set: 0 }, + PropertyAttrs::new(true, true, false), + ); + let two_call = js_object_alloc(0, 0); + set_accessor_descriptor( + two_call as usize, + "beta".to_string(), + AccessorDescriptor { get: 0, set: 0 }, + ); + set_property_attrs( + two_call as usize, + "beta".to_string(), + PropertyAttrs::new(true, true, false), + ); + for (label, obj) in [("combined", combined), ("two_call", two_call)] { + let acc = get_accessor_descriptor(obj as usize, "beta") + .unwrap_or_else(|| panic!("{label}: accessor entry")); + assert_eq!((acc.get, acc.set), (0, 0), "{label}"); + let attrs = get_property_attrs(obj as usize, "beta") + .unwrap_or_else(|| panic!("{label}: attrs entry")); + assert!( + attrs.writable() && attrs.enumerable() && !attrs.configurable(), + "{label}" + ); + let keys = super::super::super::accessor_descriptor_keys_for_obj(obj as usize); + assert_eq!( + keys.iter().filter(|k| k.as_str() == "beta").count(), + 1, + "{label}: owner index holds the key exactly once" + ); + } + } + + /// A violated brand-newness precondition must degrade to overwrite, never + /// to a duplicated owner-index entry: the second call sees the meta bit + /// already set and takes the scanning (dedup) add. + #[test] + fn combined_installer_repeat_does_not_duplicate_owner_index() { + let _global = crate::gc::global_side_table_test_lock(); + let obj = js_object_alloc(0, 0); + for _ in 0..2 { + super::super::super::install_fresh_accessor_property( + obj as usize, + "gamma".to_string(), + AccessorDescriptor { get: 0, set: 0 }, + PropertyAttrs::new(true, true, false), + ); + } + let keys = super::super::super::accessor_descriptor_keys_for_obj(obj as usize); + assert_eq!( + keys.iter().filter(|k| k.as_str() == "gamma").count(), + 1, + "duplicate install must dedupe via the meta prior-bit path" + ); + } + /// Numeric keys are inadmissible (canonical-index semantics) and must /// flow through the materialised-descriptor generic arm — which still /// installs the accessor with the same attributes. diff --git a/crates/perry-runtime/src/object/object_ops/descriptor_helpers.rs b/crates/perry-runtime/src/object/object_ops/descriptor_helpers.rs index 65c7b8347f..fc7ecb3a71 100644 --- a/crates/perry-runtime/src/object/object_ops/descriptor_helpers.rs +++ b/crates/perry-runtime/src/object/object_ops/descriptor_helpers.rs @@ -198,11 +198,23 @@ fn desc_field_index(b: &[u8]) -> Option { /// `ToPropertyDescriptor`'s inherited-field reads, so a polluted prototype /// forces the general path. pub(super) unsafe fn object_prototype_has_desc_field() -> bool { - let op = crate::object::builtin_prototype_value("Object"); - let ptr = extract_obj_ptr(op); - if ptr.is_null() { - return false; - } + // #9103 follow-up: resolve %Object.prototype% through the memoized, + // root-scanned prototype-addr cache (one slot load + a forwarding heal) + // instead of the per-call `globalThis.Object` builtin lookup + + // `closure_get_dynamic_prop(ctor, "prototype")` walk. On the __export + // install profile that walk was the single largest per-define term + // (~2.7us of a ~6us install; the keys scan below is ~0.04us). The cache + // IS the realm intrinsic — the object `ToPropertyDescriptor` reads + // inherited fields through — so a program that rebinds + // `globalThis.Object` no longer perturbs the probe (the intrinsic + // prototype of a descriptor literal never changes), and GC moves are + // healed by `scan_prototype_addr_cache_roots_mut` exactly as they are for + // `object_prototype_addr_matches`. + let addr = crate::array::object_prototype_addr(); + if addr == 0 { + return false; // intrinsic not materialized yet — nothing own + } + let ptr = addr as *mut ObjectHeader; // NOTE: builtin init legitimately installs (non-field-named) descriptors // on Object.prototype, so the per-object flag is no signal here. Every own // install — data write, defineProperty accessor, builtin getter — mirrors