From a1b201ba029d5c24a3d5e93cd7daa26fa06ee991 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 30 Aug 2026 19:27:02 +0200 Subject: [PATCH 1/2] perf(runtime): the [[Set]] own-key probe stops walking the keys array one element accessor at a time (163 200 -> 0 element reads at 400 stores) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `obj_value_has_own_key`'s ordinary-object arm answered "does this receiver already own this key" with a per-element `js_array_get` + `js_string_key_matches` loop, each iteration wrapped in a `RuntimeHandle::across_mut` round-trip. That is the full JS-facing element accessor — forward resolution, Map/Set/typed-array /buffer registry probes, the descriptor gate, hole translation — for what is a raw slot compare, paid once per already-installed key, per store. It is on the receiver-based `[[Set]]` walk: js_put_value_set_dyn_ic_miss -> proxy::ordinary_set_with_receiver -> proxy::create_or_update_receiver_property -> proxy::own_set_descriptor -> object::obj_value_has_own_key reached by every store the #5054 direct-store lane declines. One `Object.defineProperty` on the receiver is enough to decline it — which is the esbuild CJS-namespace shape, and `claude-code`'s bundle carries 1 526 of them — so building a module namespace property-by-property re-scanned every key already installed. Quadratic, and on a symbolized profile of `claude --help` `js_array_get_f64` (4.20%) plus `across_mut::` (2.44%) plus `obj_value_has_own_key` itself (1.02%) were all this one loop. #6759's shared key index already answers exactly this question — O(1) at or above `KEYS_INDEX_THRESHOLD`, a raw dense-slot compare below it — and its own doc comment records replacing these walks ("measured 90.8 MILLION `js_array_get_f64` calls for 1.5 M property operations"). Thirteen other `[[Get]]`/`[[Set]]`/delete sites route through it; this one was missed. Route it through `keys_find_slot_by_key_ptr`, which allocates nothing, so the per-iteration re-rooting the old loop needed goes with it. Counted, not timed. Element reads through `js_array_get_f64` for the esbuild-namespace shape (one `defineProperty`, then N stores): N before after 25 825 0 50 2 900 0 100 10 800 0 200 41 600 0 400 163 200 0 and process-total `js_array_get_f64` calls at N=400 fall 165 135 -> 1 935. Symbol-level proof: the `across_mut::` monomorphization — the exact profile frame — is gone from the runtime archive (1 -> 0), as is its `js_array_get_f64` relocation (1 -> 0); `keys_find_slot_by_key_ptr` appears in its place, and the only surviving array call in the function is one `js_array_length` for the key count. `.text` of a compiled program: 11 044 244 -> 11 043 924 (-320 B). `test_gap_9180_receiver_set_own_key_scan.ts` is byte-identical to node and covers the correctness surface the walk owns: both index tiers (8 keys and 40, crossing the 32-key threshold), the two ways the index declines to answer — a delete that shrinks it back to `Unindexed`, and the `Absent` completeness verdict for a key never installed — plus `Reflect.set` with receiver !== target, a proxy receiver, an own accessor on the receiver, a non-writable own data property, `Object.defineProperty` interaction, prototype shadowing, a non-extensible receiver, and index-vs-name keys. `has_own_key_probe_never_uses_the_element_accessor` pins it as an executable fact against the new test-only `js_array_get_f64` entry counter; it fails on the parent commit (17 accessor calls for three probes of an 8-key object) and passes here at 0. Validation: `cargo test -p perry-runtime --lib -- --test-threads=1` 2848 passed / 0 failed; `cargo test -p perry-codegen --lib` 1357 passed / 0 failed; `--test native_proof_regressions` 285 passed / 0 failed. A 40-fixture node-differential sweep over the object/proxy/descriptor/prototype test-files is 38/38 identical (one pre-existing unsettled-await mismatch in `test_gap_2159`, reproduced unchanged on the parent commit). Claude-Session: https://claude.ai/code/session_01Ay8VyLkKbm8Hkc1xmvTEsP --- crates/perry-runtime/src/array/indexing.rs | 18 ++ crates/perry-runtime/src/array/mod.rs | 2 + .../src/object/reflect_support.rs | 39 ++-- crates/perry-runtime/src/object/tests.rs | 97 ++++++++++ ...test_gap_9180_receiver_set_own_key_scan.ts | 183 ++++++++++++++++++ 5 files changed, 325 insertions(+), 14 deletions(-) create mode 100644 test-files/test_gap_9180_receiver_set_own_key_scan.ts diff --git a/crates/perry-runtime/src/array/indexing.rs b/crates/perry-runtime/src/array/indexing.rs index 81f3679a9d..72fce1cc91 100644 --- a/crates/perry-runtime/src/array/indexing.rs +++ b/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 = 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 diff --git a/crates/perry-runtime/src/array/mod.rs b/crates/perry-runtime/src/array/mod.rs index 5032a8313d..31a15ec68e 100644 --- a/crates/perry-runtime/src/array/mod.rs +++ b/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; diff --git a/crates/perry-runtime/src/object/reflect_support.rs b/crates/perry-runtime/src/object/reflect_support.rs index 6963e1f73f..f3ab3c8e84 100644 --- a/crates/perry-runtime/src/object/reflect_support.rs +++ b/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::(|| ()); + let ((), keys) = keys_handle.across_mut::(|| ()); // 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::js_array_get(keys, i as u32) - }); - let ((), key_str) = key_handle.across_const::(|| ()); - 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::object::keys_find_slot_by_key_ptr(keys, key_count, key_str).is_some() } } diff --git a/crates/perry-runtime/src/object/tests.rs b/crates/perry-runtime/src/object/tests.rs index 5dcfb7c0c9..b2be10196b 100644 --- a/crates/perry-runtime/src/object/tests.rs +++ b/crates/perry-runtime/src/object/tests.rs @@ -1989,3 +1989,100 @@ fn object_is_regular_excludes_a_heap_class_object() { ); } } + +/// #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" + ); + } +} diff --git a/test-files/test_gap_9180_receiver_set_own_key_scan.ts b/test-files/test_gap_9180_receiver_set_own_key_scan.ts new file mode 100644 index 0000000000..bef8a38f74 --- /dev/null +++ b/test-files/test_gap_9180_receiver_set_own_key_scan.ts @@ -0,0 +1,183 @@ +// #9180 — the receiver-based [[Set]] walk's "does the receiver already own +// this key" probe (`obj_value_has_own_key`) used a per-element `js_array_get` +// scan of the receiver's keys array. Every store that the direct-store lane +// declines re-scanned every key already installed, so building an object +// property-by-property was quadratic; it is now the shared #6759 key index +// (O(1) at/above 32 keys, raw dense-slot compare below it). +// +// This exercises BOTH tiers and, critically, the paths where the index +// declines or reports absence: below-threshold objects, an object grown +// across the 32-key threshold, a delete that shrinks the index (the +// `Unindexed` re-entry), and a re-add after the delete (the `Absent` +// completeness verdict). Plus the correctness surface of the walk itself: +// receiver !== target, proxies, accessors, non-writable data, defineProperty, +// prototype shadowing, non-extensible receivers, and index-vs-name keys. + +function out(label: string, v: any): void { + console.log(label + "=" + JSON.stringify(v)); +} + +// ---------------------------------------------------------------- 1. below +// the index threshold: dense raw-slot scan. Receiver has a non-default +// prototype, so the direct-store lane declines and the spec walk runs. +const base1: any = { inherited: 1 }; +const small: any = Object.create(base1); +for (let i = 0; i < 8; i++) small["k" + i] = i; +small.k3 = 99; +out("small.keys", Object.keys(small)); +out("small.k3", small.k3); +out("small.inherited", small.inherited); +out("small.hasOwn.inherited", Object.prototype.hasOwnProperty.call(small, "inherited")); + +// -------------------------------------------------- 2. across the threshold +// 40 keys crosses KEYS_INDEX_THRESHOLD (32): the first 31 stores take the +// dense scan, the rest consult the shape index. Overwrites after the crossing +// must still find the ALREADY-PRESENT key (an `Absent` verdict here would +// silently create a duplicate or drop the update). +const base2: any = { tail: "proto" }; +const wide: any = Object.create(base2); +for (let i = 0; i < 40; i++) wide["p" + i] = i; +for (let i = 0; i < 40; i += 7) wide["p" + i] = -i; +out("wide.keyCount", Object.keys(wide).length); +out("wide.p0", wide.p0); +out("wide.p7", wide.p7); +out("wide.p35", wide.p35); +out("wide.p36", wide.p36); +out("wide.tail", wide.tail); + +// ------------------------------------------------- 3. delete then re-add +// A delete shrinks the keys array, which invalidates the shape index. The +// next probe must NOT trust a stale `Absent`: the re-added key has to land as +// one property, not two, and the overwrite after it has to find it. +delete wide.p10; +delete wide.p11; +out("wide.afterDelete.count", Object.keys(wide).length); +out("wide.afterDelete.p10", wide.p10); +wide.p10 = 1000; +wide.p10 = 1001; +out("wide.readd.count", Object.keys(wide).length); +out("wide.readd.p10", wide.p10); +out("wide.readd.hasOwn", Object.prototype.hasOwnProperty.call(wide, "p10")); + +// ------------------------------------- 4. receiver !== target (Reflect.set) +// This is what `ordinary_set_with_receiver` exists for: the property is +// created on the RECEIVER, never on the target. +const target4: any = Object.create({ z: 0 }); +const receiver4: any = Object.create({ z: 0 }); +for (let i = 0; i < 34; i++) receiver4["r" + i] = i; +out("reflect.set", Reflect.set(target4, "r5", 555, receiver4)); +out("reflect.receiver.r5", receiver4.r5); +out("reflect.target.r5", target4.r5); +out("reflect.target.hasOwn", Object.prototype.hasOwnProperty.call(target4, "r5")); +out("reflect.set.fresh", Reflect.set(target4, "brandNew", 7, receiver4)); +out("reflect.receiver.brandNew", receiver4.brandNew); +out("reflect.target.brandNew", target4.brandNew); + +// --------------------------------------------- 5. non-writable own data +// An existing non-writable own property on the receiver rejects the store. +const nw: any = Object.create({ q: 0 }); +for (let i = 0; i < 33; i++) nw["n" + i] = i; +Object.defineProperty(nw, "locked", { value: 1, writable: false, enumerable: true, configurable: true }); +out("nonwritable.reflect", Reflect.set(nw, "locked", 2)); +out("nonwritable.value", nw.locked); +out("nonwritable.stillFindsOthers", Reflect.set(nw, "n7", 77)); +out("nonwritable.n7", nw.n7); + +// --------------------------------------------------- 6. accessor on the +// prototype: the setter fires and no own property is created. +let sawSetter: any = null; +const accProto: any = {}; +Object.defineProperty(accProto, "acc", { + get() { return sawSetter; }, + set(v: any) { sawSetter = v; }, + configurable: true, +}); +const accObj: any = Object.create(accProto); +for (let i = 0; i < 33; i++) accObj["a" + i] = i; +accObj.acc = "viaSetter"; +out("accessor.value", accObj.acc); +out("accessor.hasOwn", Object.prototype.hasOwnProperty.call(accObj, "acc")); +out("accessor.sawSetter", sawSetter); + +// ------------------------------------------- 7. own accessor on receiver +// An own accessor on the RECEIVER makes the CreateDataProperty tail return +// false without invoking the setter (OrdinarySetWithOwnDescriptor 2.d.i). +let receiverSetterCalls = 0; +const t7: any = Object.create({ w: 0 }); +const r7: any = Object.create({ w: 0 }); +for (let i = 0; i < 33; i++) r7["s" + i] = i; +Object.defineProperty(r7, "own", { + get() { return "g"; }, + set(_v: any) { receiverSetterCalls++; }, + configurable: true, +}); +out("receiverAccessor.reflect", Reflect.set(t7, "own", 5, r7)); +out("receiverAccessor.calls", receiverSetterCalls); +out("receiverAccessor.value", r7.own); + +// ----------------------------------------------------- 8. non-extensible +const sealedObj: any = Object.create({ v: 0 }); +for (let i = 0; i < 33; i++) sealedObj["e" + i] = i; +Object.preventExtensions(sealedObj); +out("nonextensible.existing", Reflect.set(sealedObj, "e4", 44)); +out("nonextensible.e4", sealedObj.e4); +out("nonextensible.new", Reflect.set(sealedObj, "brandNew", 1)); +out("nonextensible.brandNew", sealedObj.brandNew); + +// ------------------------------------------ 9. index-like vs name keys +// Canonical integer-index STRING keys and their numeric twins are the same +// property; a leading-zero form is a distinct ordinary name. +const idx: any = Object.create({ y: 0 }); +for (let i = 0; i < 33; i++) idx["i" + i] = i; +idx[2] = "two"; +idx["2"] = "TWO"; +idx["02"] = "ohtwo"; +out("index.2", idx[2]); +out("index.str2", idx["2"]); +out("index.02", idx["02"]); +out("index.count", Object.keys(idx).length); + +// ------------------------------------------------------------ 10. proxy +// A proxy RECEIVER routes the tail through [[DefineOwnProperty]]. +const proxyTarget: any = Object.create({ pp: 0 }); +for (let i = 0; i < 33; i++) proxyTarget["x" + i] = i; +const trapLog: string[] = []; +const proxied: any = new Proxy(proxyTarget, { + defineProperty(t: any, k: any, d: any) { + trapLog.push("define:" + String(k)); + return Reflect.defineProperty(t, k, d); + }, + set(t: any, k: any, v: any, r: any) { + trapLog.push("set:" + String(k)); + return Reflect.set(t, k, v, r); + }, +}); +proxied.x4 = 444; +proxied.newOne = 1; +out("proxy.x4", proxyTarget.x4); +out("proxy.newOne", proxyTarget.newOne); +out("proxy.trapLog", trapLog); + +// ----------------------------------- 11. prototype-chain shadowing order +const shadowProto: any = { shared: "proto" }; +const shadowObj: any = Object.create(shadowProto); +for (let i = 0; i < 33; i++) shadowObj["h" + i] = i; +out("shadow.before", shadowObj.shared); +shadowObj.shared = "own"; +out("shadow.after", shadowObj.shared); +out("shadow.proto", shadowProto.shared); +out("shadow.hasOwn", Object.prototype.hasOwnProperty.call(shadowObj, "shared")); + +// ------------------------------- 12. quadratic shape: many distinct keys +// The scan this replaces was O(own-key-count) per store. Answer must be +// exact, not merely fast. +const big: any = Object.create({ tailKey: "t" }); +for (let i = 0; i < 300; i++) big["b" + i] = i; +let sum = 0; +for (let i = 0; i < 300; i++) sum += big["b" + i]; +out("big.count", Object.keys(big).length); +out("big.sum", sum); +out("big.first", big.b0); +out("big.last", big.b299); +out("big.absent", big.b300); +out("big.tailKey", big.tailKey); From e720f6909ab9461f5f0bfe107aa0e510a87c3997 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 30 Aug 2026 22:12:45 +0200 Subject: [PATCH 2/2] test: split the own-key probe test into its own module for the 2000-line cap object/tests.rs reached 2088 lines, over check_file_size.sh's cap. Extracts the new #9180 test into object/own_key_probe_tests.rs. Adds the changelog fragment. --- changelog.d/9190-receiver-own-key-probe.md | 12 +++ crates/perry-runtime/src/object/mod.rs | 4 + .../src/object/own_key_probe_tests.rs | 101 ++++++++++++++++++ crates/perry-runtime/src/object/tests.rs | 97 ----------------- 4 files changed, 117 insertions(+), 97 deletions(-) create mode 100644 changelog.d/9190-receiver-own-key-probe.md create mode 100644 crates/perry-runtime/src/object/own_key_probe_tests.rs diff --git a/changelog.d/9190-receiver-own-key-probe.md b/changelog.d/9190-receiver-own-key-probe.md new file mode 100644 index 0000000000..b9d541ba1a --- /dev/null +++ b/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. diff --git a/crates/perry-runtime/src/object/mod.rs b/crates/perry-runtime/src/object/mod.rs index 772c4943c3..05f2d13b96 100644 --- a/crates/perry-runtime/src/object/mod.rs +++ b/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)] diff --git a/crates/perry-runtime/src/object/own_key_probe_tests.rs b/crates/perry-runtime/src/object/own_key_probe_tests.rs new file mode 100644 index 0000000000..4c8314f9d1 --- /dev/null +++ b/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" + ); + } +} diff --git a/crates/perry-runtime/src/object/tests.rs b/crates/perry-runtime/src/object/tests.rs index b2be10196b..5dcfb7c0c9 100644 --- a/crates/perry-runtime/src/object/tests.rs +++ b/crates/perry-runtime/src/object/tests.rs @@ -1989,100 +1989,3 @@ fn object_is_regular_excludes_a_heap_class_object() { ); } } - -/// #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" - ); - } -}