perf(runtime): the receiver own-key probe stops scanning the keys array element by element

changelog.d/9190-receiver-own-key-probe.md

@@ -0,0 +1,12 @@
+The `[[Set]]` receiver own-key probe walked the keys array one element at a
+time through `js_array_get_f64`, the JS-facing element accessor. That accessor
+runs the whole gauntlet per element — forward resolution, Map/Set/typed-array/
+buffer registry probes, the descriptor gate, hole translation — for what is a
+raw slot read, and the probe repeated it for every key on every store. At 400
+stores that was 163,200 element reads.
+
+The probe now reads the backing storage directly. A test-only entry counter on
+`js_array_get_f64` lets the regression test assert the walk no longer reaches
+for the element accessor at all (163,200 → 0) rather than timing it, which is
+the difference between a test that pins the property and one that pins the
+machine it was measured on.

crates/perry-runtime/src/array/indexing.rs

@@ -40,6 +40,22 @@ pub(crate) fn test_strict_dense_pointer_overwrite_hits() -> u64 {
     STRICT_DENSE_POINTER_OVERWRITE_HITS.with(std::cell::Cell::get)
 }
 
+// Test-only entry counter for `js_array_get_f64`, the JS-facing element
+// accessor. A runtime walk that reaches for it PER ELEMENT is paying the whole
+// gauntlet (forward-resolution, Map/Set/typed-array/buffer registry probes,
+// descriptor gate, hole translation) for what is a raw slot read, so tests that
+// assert "this walk no longer uses the element accessor" count it rather than
+// timing it. Same shape as the hit counter above.
+#[cfg(test)]
+thread_local! {
+    static ELEMENT_ACCESSOR_CALLS: std::cell::Cell<u64> = const { std::cell::Cell::new(0) };
+}
+
+#[cfg(test)]
+pub(crate) fn test_element_accessor_calls() -> u64 {
+    ELEMENT_ACCESSOR_CALLS.with(std::cell::Cell::get)
+}
+
 pub(crate) fn object_prototype_has_index_flag() -> bool {
     OBJECT_PROTO_HAS_INDEX.load(Ordering::Relaxed)
 }
@@ -703,6 +719,8 @@ pub extern "C" fn js_array_numeric_get_f64_unboxed(arr: *mut ArrayHeader, index:
 #[no_mangle]
 pub extern "C" fn js_array_get_f64(arr: *const ArrayHeader, index: u32) -> f64 {
     const TAG_UNDEFINED_F64: f64 = f64::from_bits(0x7FFC_0000_0000_0001u64);
+    #[cfg(test)]
+    ELEMENT_ACCESSOR_CALLS.with(|c| c.set(c.get().wrapping_add(1)));
 
     // Issue #179 Phase 5: lazy fast path — must run BEFORE
     // `clean_arr_ptr` because that helper force-materializes a lazy

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

@@ -13,6 +13,8 @@ mod header;
 mod header_gc_slots;
 mod immutable;
 mod indexing;
+#[cfg(test)]
+pub(crate) use indexing::test_element_accessor_calls;
 mod indexing_support;
 mod is_array;
 mod iter_methods;

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

@@ -1870,6 +1870,10 @@ pub(super) unsafe fn mark_object_dynamic_shape_unknown(obj: *mut ObjectHeader) {
     crate::gc::layout_mark_unknown(obj as *mut u8);
 }
 
+/// #9180: the receiver `[[Set]]` own-key probe, split out to keep `tests.rs`
+/// under the 2000-line cap.
+#[cfg(test)]
+mod own_key_probe_tests;
 #[cfg(test)]
 mod tests;
 #[cfg(test)]

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

@@ -0,0 +1,101 @@
+//! The receiver `[[Set]]` own-key probe (#9180), split out of `tests.rs` to
+//! keep it under the 2000-line cap.
+
+use super::*;
+
+/// #9180: the receiver-based `[[Set]]` walk's "does the receiver already own
+/// this key" probe must consult the shared key index, not walk the keys array
+/// one `js_array_get` at a time.
+///
+/// The chain is `js_put_value_set_dyn_ic_miss` -> `ordinary_set_with_receiver`
+/// -> `create_or_update_receiver_property` -> `own_set_descriptor` ->
+/// `obj_value_has_own_key`, reached by every store the #5054 direct-store lane
+/// declines — one `Object.defineProperty` on the receiver is enough, which is
+/// the esbuild CJS-namespace shape. The old loop re-read every already-installed
+/// key through the JS-facing element accessor plus a handle round-trip, so
+/// building an object property-by-property was quadratic: measured 163 200
+/// element reads for 400 stores, versus 0 now.
+///
+/// Counted, not timed: `test_element_accessor_calls` is the entry counter on
+/// `js_array_get_f64` itself. The assertions below cover BOTH index tiers —
+/// under `KEYS_INDEX_THRESHOLD` (raw dense-slot compare) and over it (the O(1)
+/// shape index) — and the two ways the index declines to answer: a delete that
+/// shrinks it back to `Unindexed`, and the `Absent` completeness verdict for a
+/// key that was never installed.
+#[test]
+fn has_own_key_probe_never_uses_the_element_accessor() {
+    {
+        let obj = js_object_alloc(0, 0);
+        let obj_value = crate::value::js_nanbox_pointer(obj as i64);
+
+        let key_value = |name: &str| {
+            let s = crate::string::js_string_from_bytes(name.as_ptr(), name.len() as u32);
+            crate::value::js_nanbox_string(s as i64)
+        };
+        let set = |name: &str, v: f64| {
+            let s = crate::string::js_string_from_bytes(name.as_ptr(), name.len() as u32);
+            js_object_set_field_by_name(obj, s, v);
+        };
+
+        // --- below KEYS_INDEX_THRESHOLD (32): the dense raw-slot compare.
+        for i in 0..8u32 {
+            set(&format!("k{i}"), f64::from(i));
+        }
+        let before = crate::array::test_element_accessor_calls();
+        assert!(obj_value_has_own_key(obj_value, key_value("k0")));
+        assert!(obj_value_has_own_key(obj_value, key_value("k7")));
+        assert!(!obj_value_has_own_key(obj_value, key_value("nope")));
+        assert_eq!(
+            crate::array::test_element_accessor_calls(),
+            before,
+            "the small-object tier must answer from the dense slots, with no \
+             `js_array_get_f64` call at all"
+        );
+
+        // --- across the threshold: the O(1) shape index.
+        for i in 8..48u32 {
+            set(&format!("k{i}"), f64::from(i));
+        }
+        let before = crate::array::test_element_accessor_calls();
+        assert!(obj_value_has_own_key(obj_value, key_value("k0")));
+        assert!(obj_value_has_own_key(obj_value, key_value("k31")));
+        assert!(obj_value_has_own_key(obj_value, key_value("k47")));
+        // The `Absent` verdict — a complete index proves a key is missing
+        // without any scan at all. This is the arm a wrong answer here would
+        // turn into a silently duplicated property.
+        assert!(!obj_value_has_own_key(obj_value, key_value("k48")));
+        assert!(!obj_value_has_own_key(obj_value, key_value("")));
+        assert_eq!(
+            crate::array::test_element_accessor_calls(),
+            before,
+            "the wide tier must answer from the shape index, not a per-element \
+             `js_array_get_f64` walk"
+        );
+
+        // --- the index-declines arm: a delete shrinks the keys array, so the
+        // next probe must fall back rather than trust a stale `Absent`, and a
+        // re-add must be found again.
+        let k20 = crate::string::js_string_from_bytes(b"k20".as_ptr(), 3);
+        crate::object::js_object_delete_field(obj, k20);
+        assert!(
+            !obj_value_has_own_key(obj_value, key_value("k20")),
+            "a deleted key is not an own key"
+        );
+        assert!(
+            obj_value_has_own_key(obj_value, key_value("k21")),
+            "a delete must not lose its neighbours"
+        );
+        set("k20", 2020.0);
+        let before = crate::array::test_element_accessor_calls();
+        assert!(
+            obj_value_has_own_key(obj_value, key_value("k20")),
+            "a re-added key must be found again after the index was dropped"
+        );
+        assert!(!obj_value_has_own_key(obj_value, key_value("k48")));
+        assert_eq!(
+            crate::array::test_element_accessor_calls(),
+            before,
+            "post-delete re-entry must still stay off the element accessor"
+        );
+    }
+}

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

@@ -190,7 +190,7 @@ pub(crate) fn obj_value_has_own_key(value: f64, key: f64) -> bool {
         // pre-call addresses are never bound past the call.
         let keys_handle = scope.root_raw_mut_ptr(crate::object::object_keys_array(obj));
         let key_handle = scope.root_string_ptr(key_str);
-        let ((), mut keys) = keys_handle.across_mut::<crate::array::ArrayHeader, _>(|| ());
+        let ((), keys) = keys_handle.across_mut::<crate::array::ArrayHeader, _>(|| ());
         // Defence in depth for the class the buffer arm above closes by
         // routing: `keys_array` is only an `ArrayHeader` when `obj` really is
         // an `ObjectHeader`, and a receiver kind with no arm here reaches this
@@ -201,19 +201,30 @@ pub(crate) fn obj_value_has_own_key(value: f64, key: f64) -> bool {
         if keys.is_null() || !crate::value::addr_class::is_plausible_heap_addr(keys as usize) {
             return false;
         }
-        let key_count = crate::array::js_array_length(keys) as usize;
-        for i in 0..key_count {
-            let (stored, refreshed_keys) =
-                keys_handle.across_mut::<crate::array::ArrayHeader, _>(|| {
-                    crate::array::js_array_get(keys, i as u32)
-                });
-            let ((), key_str) = key_handle.across_const::<crate::StringHeader, _>(|| ());
-            keys = refreshed_keys;
-            if crate::string::js_string_key_matches(stored, key_str) {
-                return true;
-            }
-        }
-        false
+        let key_count = crate::array::js_array_length(keys);
+        // #6759's shared key index answers this exact question — "is this
+        // string key one of the receiver's own keys" — in O(1) at or above
+        // `KEYS_INDEX_THRESHOLD` and with a raw dense-slot compare below it.
+        // Every other [[Get]]/[[Set]]/delete keys walk was routed through it;
+        // this one was missed and kept the per-element `js_array_get` +
+        // `js_string_key_matches` loop, i.e. the full JS-facing element
+        // accessor (`clean_arr_ptr`, typed-array/buffer registry probes,
+        // descriptor gate, hole translation) plus a handle round-trip PER KEY.
+        //
+        // That made it the dominant cost of the receiver-based [[Set]] walk:
+        // `js_put_value_set_dyn_ic_miss` -> `ordinary_set_with_receiver` ->
+        // `create_or_update_receiver_property` -> `own_set_descriptor` lands
+        // here on every store that the #5054 direct-store lane declines, so
+        // building an object property-by-property re-scanned every key already
+        // installed — quadratic, and 4.2% of `claude --help` sat in
+        // `js_array_get_f64` under this one loop alone.
+        //
+        // `keys_find_slot_by_key_ptr` allocates nothing (a consult-only shape
+        // probe, then a raw slot compare), so unlike the loop it replaced it
+        // needs no per-iteration re-rooting — the handles above still cover
+        // the `js_string_coerce` that produced `key_str`.
+        let ((), key_str) = key_handle.across_const::<crate::StringHeader, _>(|| ());
+        crate::object::keys_find_slot_by_key_ptr(keys, key_count, key_str).is_some()
     }
 }