From 824c7465e44aa9ea980687c0ea5957a025628891 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Fri, 28 Aug 2026 18:23:52 +0200 Subject: [PATCH 1/3] perf(runtime): read fast lane resolves keys via the shape index, not a scan MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The read-plan cache's MISS path in js_object_get_field_by_name's fast lane was an open-coded keys_array_slot + js_string_key_matches walk — up to key_count string compares, run in full every time the epoch-guarded plan was flushed (on each GC, and on descriptor / prototype / delete mutations). On a 500-key receiver that put js_string_key_matches at 9.6% self time in a computed-key read loop, second only to the entry itself. Route it through the same keys_find_slot_by_key_ptr helper that #8936 and #8950 put on the write, delete and [[Get]]-fallback paths: shape hash index first, raw dense-slot scan as its own fallback and correctness backstop. Claude-Session: https://claude.ai/code/session_01Ay8VyLkKbm8Hkc1xmvTEsP --- .../object/field_get_set/get_field_by_name.rs | 50 ++++++++++++------- 1 file changed, 33 insertions(+), 17 deletions(-) diff --git a/crates/perry-runtime/src/object/field_get_set/get_field_by_name.rs b/crates/perry-runtime/src/object/field_get_set/get_field_by_name.rs index bde9b72270..d9fdf0e043 100644 --- a/crates/perry-runtime/src/object/field_get_set/get_field_by_name.rs +++ b/crates/perry-runtime/src/object/field_get_set/get_field_by_name.rs @@ -189,23 +189,39 @@ pub extern "C" fn js_object_get_field_by_name( let key_count = crate::array::keys_array_len_capped_to_capacity(keys); if key_count <= 4096 { - for i in 0..key_count { - let kv = crate::array::keys_array_slot(keys, i as u32); - if crate::string::js_string_key_matches(kv, key) { - super::super::prop_plan::read_plan_record( - keys as usize, - key as usize, - i as u32, - ); - return if i < alloc_limit { - super::accessors::js_object_get_field(o, i as u32) - } else { - match super::super::overflow_get(raw, i) { - Some(b) => JSValue::from_bits(b), - None => JSValue::undefined(), - } - }; - } + // #8936/#8950's shared resolver: the shape's + // hash index answers in O(1), with the raw + // dense-slot scan as its own fallback. The + // open-coded `keys_array_slot` + + // `js_string_key_matches` walk this replaces + // was the read-plan cache's MISS path, so it + // ran in full — up to `key_count` string + // compares — every time the epoch-guarded + // plan was flushed (each GC, and every + // descriptor / prototype / delete mutation). + // On a 500-key receiver that put + // `js_string_key_matches` at 9.6% self time + // in a computed-key read loop, second only to + // this function itself. + if let Some(i) = crate::object::keys_find_slot_by_key_ptr( + keys, + key_count as u32, + key, + ) { + let i = i as usize; + super::super::prop_plan::read_plan_record( + keys as usize, + key as usize, + i as u32, + ); + return if i < alloc_limit { + super::accessors::js_object_get_field(o, i as u32) + } else { + match super::super::overflow_get(raw, i) { + Some(b) => JSValue::from_bits(b), + None => JSValue::undefined(), + } + }; } } } From 2a258af8e977213181148c4b79e66ee06b5fc020 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Fri, 28 Aug 2026 18:33:37 +0200 Subject: [PATCH 2/3] perf(runtime): the three remaining per-element key scans use the shape index Same transformation as the read lane in the parent commit, applied to the write fast path's read-plan miss fallback, the write tail, and the read tail. Two of them ran js_array_get per key, which additionally probes for a per-index accessor. Both tail sites keep their original js_string_key_matches test as the gate, so the resolver can only narrow the candidate slot, never widen what is accepted. Claude-Session: https://claude.ai/code/session_01Ay8VyLkKbm8Hkc1xmvTEsP --- changelog.d/8971-key-scan-kills-read-lane.md | 36 +++++++++++++++++++ .../field_get_set/get_field_by_name_tail.rs | 7 +++- .../object/field_set_by_name/fast_paths.rs | 16 +++++---- .../src/object/field_set_by_name/tail.rs | 8 ++++- 4 files changed, 58 insertions(+), 9 deletions(-) create mode 100644 changelog.d/8971-key-scan-kills-read-lane.md diff --git a/changelog.d/8971-key-scan-kills-read-lane.md b/changelog.d/8971-key-scan-kills-read-lane.md new file mode 100644 index 0000000000..537af0c66b --- /dev/null +++ b/changelog.d/8971-key-scan-kills-read-lane.md @@ -0,0 +1,36 @@ +The last four per-element key scans on the property paths now resolve through +the shape hash index. + +#8936 and #8950 replaced the `js_array_get` + `js_string_key_matches` walks on +the `[[Set]]`, `delete` and `[[Get]]`-fallback paths, but four copies survived +in the plan-miss and cache-miss fallbacks: + +- `js_object_get_field_by_name`'s FAST LANE — the read-plan cache's miss path, + a full walk of up to 4096 keys. The plan is epoch-guarded and flushed on + every GC and on descriptor / prototype / delete mutations, so this ran in + full each time it was flushed. It put `js_string_key_matches` at **9.6% self + time** in a computed-key read loop, second only to the entry itself. +- the write fast path's read-plan miss fallback, and +- the write tail and the read tail, whose loops walked every key (two of them + via `js_array_get`, which additionally probes each index for a per-index + accessor). + +All four go through `keys_find_slot_by_key_ptr`: shape hash index first, raw +dense-slot scan as its own fallback and correctness backstop. The two tail +sites keep their original `js_string_key_matches` test as the gate, so the +resolver can only narrow the candidate slot, never widen what is accepted. + +Interleaved A/B, min-of-15 at quiet load (node on the same host in brackets): + +| loop | main | this PR | | +|---|---|---|---| +| read only | 38 ms (25) | **34 ms** | −10.5% min, −18.6% mean | +| combined overwrite | 79 ms (29) | **77 ms** | −2.5% min, −13.2% mean | +| write only | 44 ms (23) | 43 ms | −2.3% | + +The read-only loop is where this lands: in the combined loop the write primes +the read plan, so the read never reaches the miss path at all. + +Computed-key differential output (delete/re-add ordering, `Map`/`Set` keys, +the SSO boundary, non-ASCII, floats, negatives, 1e21) is byte-identical to +node. Suite: 2779 passed, 0 failed. diff --git a/crates/perry-runtime/src/object/field_get_set/get_field_by_name_tail.rs b/crates/perry-runtime/src/object/field_get_set/get_field_by_name_tail.rs index 6e90a4a9ab..104af0ee78 100644 --- a/crates/perry-runtime/src/object/field_get_set/get_field_by_name_tail.rs +++ b/crates/perry-runtime/src/object/field_get_set/get_field_by_name_tail.rs @@ -1670,7 +1670,12 @@ pub(crate) fn get_field_by_name_object_tail( return JSValue::undefined(); } - for i in 0..key_count { + // #8936/#8950's resolver narrows the candidate to one slot via the + // shape hash index instead of walking every key; the original match + // below still gates the hit, so this cannot widen what is accepted. + for i in crate::object::keys_find_slot_by_key_ptr(keys, key_count as u32, key) + .map(|v| v as usize) + { let key_val = crate::array::keys_array_slot(keys, i as u32); // #1781: accept inline SSO short keys here too — the // slow-path lookup is what backs `obj[k]` for ≤5-byte 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 c9001ca497..0f8c379d89 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 @@ -111,13 +111,15 @@ pub(crate) unsafe fn try_existing_own_data_overwrite( if key_count > 4096 { return false; } - for i in 0..key_count { - let kv = crate::array::js_array_get(keys, i as u32); - if crate::string::js_string_key_matches(kv, key) { - super::prop_plan::read_plan_record(keys_addr, key_addr, i as u32); - own_idx = Some(i as u32); - break; - } + // The write twin of the read lane's resolver: shape hash index first, + // raw dense-slot scan as its own fallback. The open-coded walk this + // replaces ran `js_array_get` (which additionally probes for per-index + // accessors) plus a string compare per key, in full, every time the + // epoch-guarded read plan was flushed — the same miss-path cost #8936 + // and #8950 removed from their sides of the property paths. + own_idx = crate::object::keys_find_slot_by_key_ptr(keys, key_count as u32, key); + if let Some(i) = own_idx { + super::prop_plan::read_plan_record(keys_addr, key_addr, i); } } let Some(idx) = own_idx else { diff --git a/crates/perry-runtime/src/object/field_set_by_name/tail.rs b/crates/perry-runtime/src/object/field_set_by_name/tail.rs index 7c5b1fee44..7cefee1409 100644 --- a/crates/perry-runtime/src/object/field_set_by_name/tail.rs +++ b/crates/perry-runtime/src/object/field_set_by_name/tail.rs @@ -856,7 +856,13 @@ pub(crate) fn set_field_by_name_object_tail( return; } - for i in 0..key_count { + // #8936/#8950's resolver narrows the candidate to one slot via the + // shape hash index instead of walking every key (and probing each for + // per-index accessors via `js_array_get`); the original match below + // still gates the hit, so this cannot widen what is accepted. + for i in crate::object::keys_find_slot_by_key_ptr(keys, key_count as u32, key) + .map(|v| v as usize) + { let key_val = crate::array::js_array_get(keys, i as u32); // #1781: SSO-aware match — keys are stored as either a // STRING_TAG pointer OR a SHORT_STRING_TAG inline value for From 26abfa34496bdcdcfea5eeafb6975ee4e5246023 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Fri, 28 Aug 2026 18:59:26 +0200 Subject: [PATCH 3/3] fix(object): if let, not a for loop over an Option `for_loops_over_fallibles` is a `-D warnings` error, so the `warnings` job was red. Neither body uses `continue`/`break`, so this is a pure substitution. --- .../src/object/field_get_set/get_field_by_name_tail.rs | 2 +- crates/perry-runtime/src/object/field_set_by_name/tail.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/perry-runtime/src/object/field_get_set/get_field_by_name_tail.rs b/crates/perry-runtime/src/object/field_get_set/get_field_by_name_tail.rs index 104af0ee78..f56ff584e1 100644 --- a/crates/perry-runtime/src/object/field_get_set/get_field_by_name_tail.rs +++ b/crates/perry-runtime/src/object/field_get_set/get_field_by_name_tail.rs @@ -1673,7 +1673,7 @@ pub(crate) fn get_field_by_name_object_tail( // #8936/#8950's resolver narrows the candidate to one slot via the // shape hash index instead of walking every key; the original match // below still gates the hit, so this cannot widen what is accepted. - for i in crate::object::keys_find_slot_by_key_ptr(keys, key_count as u32, key) + if let Some(i) = crate::object::keys_find_slot_by_key_ptr(keys, key_count as u32, key) .map(|v| v as usize) { let key_val = crate::array::keys_array_slot(keys, i as u32); diff --git a/crates/perry-runtime/src/object/field_set_by_name/tail.rs b/crates/perry-runtime/src/object/field_set_by_name/tail.rs index 7cefee1409..5d5d80ae42 100644 --- a/crates/perry-runtime/src/object/field_set_by_name/tail.rs +++ b/crates/perry-runtime/src/object/field_set_by_name/tail.rs @@ -860,7 +860,7 @@ pub(crate) fn set_field_by_name_object_tail( // shape hash index instead of walking every key (and probing each for // per-index accessors via `js_array_get`); the original match below // still gates the hit, so this cannot widen what is accepted. - for i in crate::object::keys_find_slot_by_key_ptr(keys, key_count as u32, key) + if let Some(i) = crate::object::keys_find_slot_by_key_ptr(keys, key_count as u32, key) .map(|v| v as usize) { let key_val = crate::array::js_array_get(keys, i as u32);