diff --git a/changelog.d/8930-dynamic-key-scan-kill.md b/changelog.d/8930-dynamic-key-scan-kill.md new file mode 100644 index 0000000000..3becab765a --- /dev/null +++ b/changelog.d/8930-dynamic-key-scan-kill.md @@ -0,0 +1,47 @@ +Killed the per-element linear key scan behind dynamic string-keyed property +access: **−61% on baseline dynamic property throughput** (768 → 303 ms on the +`bench_dynamic_property_keys` overwrite loop; node on the same host: 12 ms). + +The `[[Set]]`/`[[Get]]` fallback walks and the `delete` path each carried their +own copy of the same loop: `for i in 0..key_count { js_array_get(keys, i) + +string compare }` — the full JS-facing array accessor (pointer cleaning, +typed-array and buffer registry probes, descriptor gates) per element, per +property operation. The pointer-keyed read plan in front never hits for a +computed key, because `o["k" + i]` allocates a fresh key string every +evaluation. Counted with a temporary in-runtime counter: **90.8 million +`js_array_get_f64` calls for 1.5 million property operations** (~60 per +access). After this change: 15.1 M, all of it in `delete`'s array compaction +rather than lookup. + +The scans now go through one shared helper (`keys_find_slot_by_bytes` / +`_by_key_ptr`): the shape hash index (`shape_slot_lookup`, content-validated) +answers in O(1) when present, with a raw dense-slot linear scan (no per-element +accessor) as the fallback and correctness backstop. + +Two hazards were found by testing and are baked into the design: + +* **Consult-only (`build=false`).** A delete drops the shape index; rebuilding + it on the next access to use it once **doubled** delete-heavy time + (1570 → 3064 ms measured) while the call counter barely moved — the time went + to rebuilds, not scans. These sites therefore only consult an index the write + path already maintains incrementally; churny receivers fall back to the raw + scan instead of thrashing rebuilds. Final: delete-heavy 1497 ms vs 1433 + baseline (within the ±10% noise band of that metric), overwrite keeps the + full win. +* **Garbage-length tolerance.** The old loops compared LENGTHS first + (`js_string_key_matches`), so a `key` pointer that is not a valid string + header was a harmless mismatch. The first helper version built a slice from + that length and panicked in an unrelated stream test with `range start index + 2613749136200 out of range`. The helper now sanity-checks + `byte_len <= capacity && < 2^28` before slicing and otherwise falls back to + the length-guarded compare. + +Also switches the shape-scanner probe memo (#8899) off std's SipHash: perf put +`RandomState::hash_one::<&(usize, bool)>` at **7.0% of total samples** on this +workload. The key folds to one word (`addr | carrier_bit`; addresses are +8-aligned so bit 0 is free) under `PtrHasher`. + +Suites: macOS **2762 passed, 0 failed** (complete); Linux x86_64 2654 passed, +0 failed with the `node_stream` error-path family excluded — that family +aborts identically on clean main (verified by stash), a pre-existing +Linux-specific failure reported separately. diff --git a/crates/perry-runtime/src/object/delete_rest.rs b/crates/perry-runtime/src/object/delete_rest.rs index 8f905d8a15..037fd19985 100644 --- a/crates/perry-runtime/src/object/delete_rest.rs +++ b/crates/perry-runtime/src/object/delete_rest.rs @@ -290,17 +290,14 @@ pub extern "C" fn js_object_delete_field( // Search through the keys array for a match let key_count = crate::array::js_array_length(keys) as usize; - let mut found_idx: Option = None; - for i in 0..key_count { - let key_val = crate::array::js_array_get(keys, i as u32); - // #1781: SSO-aware match — pre-fix `delete obj.id` on an - // object whose `id` lived as an inline SSO key reported - // success vacuously without actually deleting anything. - if crate::string::js_string_key_matches(key_val, key) { - found_idx = Some(i); - break; - } - } + // #6759: shape-index + dense-slot scan (SSO-aware via the shared + // helper, preserving #1781). The old per-element `js_array_get` walk + // made every `delete` O(keys) full-accessor calls — measured as the + // dominant residue (16.0 M of 90.8 M accessor calls) after the + // [[Set]]/[[Get]] walks were fixed. + let found_idx: Option = + crate::object::keys_find_slot_by_key_ptr(keys, key_count as u32, key) + .map(|i| i as usize); let i = match found_idx { Some(i) => i, diff --git a/crates/perry-runtime/src/object/keys_lookup.rs b/crates/perry-runtime/src/object/keys_lookup.rs new file mode 100644 index 0000000000..1be5cc08f5 --- /dev/null +++ b/crates/perry-runtime/src/object/keys_lookup.rs @@ -0,0 +1,180 @@ +//! Keys-array slot lookup helpers, in a sibling file. +//! +//! Extracted from `object/mod.rs` to keep it under the repo's 2000-line cap. +//! A child module, so these still reach the parent's private items through +//! `use super::*`. Moved verbatim apart from sharing one payload-offset helper. + +use super::*; + +/// The payload bytes of a `StringHeader`, in one place, so the key helpers +/// here do not each add a site to the payload-access ratchet. +#[inline(always)] +pub(crate) unsafe fn string_header_payload(key: *const crate::StringHeader) -> *const u8 { + (key as *const u8).add(std::mem::size_of::()) +} + +/// Raw dense-slot view of a (validated) keys array: resolve a grow-forward +/// pointer ONCE, then hand back the backing slots for direct indexing. The +/// generic `js_array_get` element getter re-runs the whole per-element +/// gauntlet — forward-resolution, lazy/Map/Set receiver probes (each a TLS + +/// registry HashMap hit), descriptor gates — on EVERY slot, which made the +/// keys_array scan loops (`own_key_present`, the sidecar/wide-index builds) +/// pay ~µs per element. Callers have already validated `keys` is a +/// `GC_TYPE_ARRAY`; keys arrays are dense (no holes), and a slot that is not +/// a string simply fails the key match. (#6748 grind) +#[inline] +pub(crate) unsafe fn keys_array_dense_slots( + keys: *const crate::array::ArrayHeader, +) -> (*const f64, usize) { + let arr = crate::array::clean_arr_ptr(keys); + if arr.is_null() { + return (std::ptr::null(), 0); + } + let len = (*arr).length.min((*arr).capacity) as usize; + ( + (arr as *const u8).add(std::mem::size_of::()) as *const f64, + len, + ) +} + +/// FNV-1a hash of the bytes behind a string header. Same hash function +/// as `key_content_hash_impl` so callers can mix paths. +#[inline(always)] +pub(crate) fn key_bytes_hash(name_ptr: *const u8, name_len: usize) -> u64 { + let mut h: u64 = 0xcbf29ce484222325; + unsafe { + for i in 0..name_len { + h ^= *name_ptr.add(i) as u64; + h = h.wrapping_mul(0x100000001b3); + } + } + h +} + +/// Find `key_bytes` among the first `key_count` keys of `keys`. +/// +/// The [[Set]]/[[Get]] fallback walks used to do this with a per-element +/// `js_array_get` + `js_string_key_matches` loop — the full JS-facing array +/// accessor (pointer cleaning, typed-array and buffer registry probes, +/// descriptor gates) per element, per property access. A computed-key site +/// allocates a fresh key string every evaluation, so the pointer-keyed read +/// plan in front of those walks never hits and every access paid the scan: +/// measured 90.8 MILLION `js_array_get_f64` calls for 1.5 M property +/// operations (~60 per access) on the dynamic-property benchmark. +/// +/// Strategy: the shared shape index (`shape_slot_lookup`, content-validated, +/// built once per shape) answers in O(1) for receivers at or above +/// `KEYS_INDEX_THRESHOLD`; below it — and as a correctness fallback if the +/// index declines — a linear scan over the DENSE raw slots +/// (`keys_array_dense_slots`, no per-element accessor) does the compare. +pub(crate) unsafe fn keys_find_slot_by_bytes( + keys: *const crate::array::ArrayHeader, + key_count: u32, + key_bytes: &[u8], +) -> Option { + if key_count >= KEYS_INDEX_THRESHOLD { + let h = key_bytes_hash(key_bytes.as_ptr(), key_bytes.len()); + // build=false — consult-only. These call sites run on delete-churn + // workloads where every delete drops the index; rebuilding it on the + // next access (500 hashes) to use it once DOUBLED delete-heavy time + // (1570 -> 3064 ms measured). Appends maintain the index incrementally + // (shape_note_append), so stable-shape workloads still hit; churny + // ones fall back to the raw dense scan below instead of thrashing. + if let Some(slot) = shapes::shape_slot_lookup(keys, key_bytes, h, key_count, false) { + return Some(slot); + } + // A miss from a fully built index is authoritative in the common case, + // but the index can decline (shrunk arrays, partial builds); the raw + // scan below is cheap enough to serve as the correctness backstop. + } + let (slots, slot_len) = keys_array_dense_slots(keys); + if slots.is_null() { + return None; + } + let n = (key_count as usize).min(slot_len); + let mut sso = [0u8; crate::value::SHORT_STRING_MAX_LEN]; + for i in 0..n { + let v = crate::JSValue::from_bits((*slots.add(i)).to_bits()); + if let Some(stored) = crate::string::js_string_key_bytes(v, &mut sso) { + if stored == key_bytes { + return Some(i as u32); + } + } + } + None +} + +/// [`keys_find_slot_by_bytes`] for a key held as a `StringHeader`. +pub(crate) unsafe fn keys_find_slot_by_key_ptr( + keys: *const crate::array::ArrayHeader, + key_count: u32, + key: *const crate::StringHeader, +) -> Option { + // Magnitude only, deliberately: the original `< 0x10000` rejected the + // handle band and nothing else, and this helper's callers tolerate a + // `key` that is not a valid header (the length guard below catches it). + if key.is_null() || !crate::value::addr_class::is_above_handle_band(key as usize) { + return None; + } + // The callers this replaced tolerated a `key` that is not actually a + // valid string header: `js_string_key_matches` compares LENGTHS first, so + // a garbage `byte_len` was just a harmless mismatch. Building a slice from + // that length instead reads it — the first version of this helper panicked + // in an unrelated stream test with `range start index 2613749136200`. + // Keep the old tolerance: a length that cannot be a real key falls back to + // the length-guarded per-candidate compare below. + let len = (*key).byte_len as usize; + if len <= (*key).capacity as usize && len < (1 << 28) { + let data = string_header_payload(key); + return keys_find_slot_by_bytes(keys, key_count, std::slice::from_raw_parts(data, len)); + } + let (slots, slot_len) = keys_array_dense_slots(keys); + if slots.is_null() { + return None; + } + let n = (key_count as usize).min(slot_len); + for i in 0..n { + let v = crate::JSValue::from_bits((*slots.add(i)).to_bits()); + if crate::string::js_string_key_matches(v, key) { + return Some(i as u32); + } + } + None +} + +/// Locate `key` in `obj`'s keys array via the shape record (#6759 C1: +/// keyed on keys_array identity — shared across same-shape objects — +/// replacing the per-object sidecar). Returns `Some(slot)` on a +/// content-validated hit, `None` on miss (caller falls through to +/// append/grow or the linear scan). +#[inline] +pub(crate) unsafe fn keys_index_lookup( + _obj: *const ObjectHeader, + keys: *const crate::array::ArrayHeader, + key_bytes: &[u8], + key_hash: u64, +) -> Option { + let key_count = crate::array::js_array_length(keys); + if key_count < KEYS_INDEX_THRESHOLD { + return None; + } + shapes::shape_slot_lookup(keys, key_bytes, key_hash, key_count, true) +} + +/// Record a new (key_hash → slot) entry on the POST-append keys array's +/// shape after a key was appended. Caller passes `crate::object::object_keys_array(obj)` +/// (the definitive post-append array — a clone or grow-realloc lands +/// under its new identity, or nowhere if no shape entry exists yet) and +/// ensures `new_count` equals the new keys_array length. +#[inline] +pub(crate) fn keys_index_insert( + keys: *const crate::array::ArrayHeader, + new_count: u32, + key_hash: u64, + slot: u32, +) { + if new_count < KEYS_INDEX_THRESHOLD { + return; + } + shapes::shape_note_append(keys, new_count, key_hash, slot); +} diff --git a/crates/perry-runtime/src/object/mod.rs b/crates/perry-runtime/src/object/mod.rs index a8c5c1f38c..c229a94b5f 100644 --- a/crates/perry-runtime/src/object/mod.rs +++ b/crates/perry-runtime/src/object/mod.rs @@ -533,80 +533,9 @@ impl ObjectHotTables { /// faster than the hash overhead (memory access, cache footprint). const KEYS_INDEX_THRESHOLD: u32 = 32; -/// Raw dense-slot view of a (validated) keys array: resolve a grow-forward -/// pointer ONCE, then hand back the backing slots for direct indexing. The -/// generic `js_array_get` element getter re-runs the whole per-element -/// gauntlet — forward-resolution, lazy/Map/Set receiver probes (each a TLS + -/// registry HashMap hit), descriptor gates — on EVERY slot, which made the -/// keys_array scan loops (`own_key_present`, the sidecar/wide-index builds) -/// pay ~µs per element. Callers have already validated `keys` is a -/// `GC_TYPE_ARRAY`; keys arrays are dense (no holes), and a slot that is not -/// a string simply fails the key match. (#6748 grind) -#[inline] -pub(crate) unsafe fn keys_array_dense_slots( - keys: *const crate::array::ArrayHeader, -) -> (*const f64, usize) { - let arr = crate::array::clean_arr_ptr(keys); - if arr.is_null() { - return (std::ptr::null(), 0); - } - let len = (*arr).length.min((*arr).capacity) as usize; - ( - (arr as *const u8).add(std::mem::size_of::()) as *const f64, - len, - ) -} - -/// FNV-1a hash of the bytes behind a string header. Same hash function -/// as `key_content_hash_impl` so callers can mix paths. -#[inline(always)] -fn key_bytes_hash(name_ptr: *const u8, name_len: usize) -> u64 { - let mut h: u64 = 0xcbf29ce484222325; - unsafe { - for i in 0..name_len { - h ^= *name_ptr.add(i) as u64; - h = h.wrapping_mul(0x100000001b3); - } - } - h -} - -/// Locate `key` in `obj`'s keys array via the shape record (#6759 C1: -/// keyed on keys_array identity — shared across same-shape objects — -/// replacing the per-object sidecar). Returns `Some(slot)` on a -/// content-validated hit, `None` on miss (caller falls through to -/// append/grow or the linear scan). -#[inline] -unsafe fn keys_index_lookup( - _obj: *const ObjectHeader, - keys: *const crate::array::ArrayHeader, - key_bytes: &[u8], - key_hash: u64, -) -> Option { - let key_count = crate::array::js_array_length(keys); - if key_count < KEYS_INDEX_THRESHOLD { - return None; - } - shapes::shape_slot_lookup(keys, key_bytes, key_hash, key_count, true) -} - -/// Record a new (key_hash → slot) entry on the POST-append keys array's -/// shape after a key was appended. Caller passes `crate::object::object_keys_array(obj)` -/// (the definitive post-append array — a clone or grow-realloc lands -/// under its new identity, or nowhere if no shape entry exists yet) and -/// ensures `new_count` equals the new keys_array length. -#[inline] -fn keys_index_insert( - keys: *const crate::array::ArrayHeader, - new_count: u32, - key_hash: u64, - slot: u32, -) { - if new_count < KEYS_INDEX_THRESHOLD { - return; - } - shapes::shape_note_append(keys, new_count, key_hash, slot); -} +#[path = "keys_lookup.rs"] +mod keys_lookup; +pub(crate) use keys_lookup::*; pub(crate) mod array_tail_transition; mod call_method_depth; @@ -834,7 +763,7 @@ pub(crate) unsafe fn interned_key_ptr(key: *const crate::StringHeader) -> usize fn key_content_hash_impl(key: *const crate::StringHeader) -> u64 { unsafe { let len = (*key).byte_len as usize; - let data = (key as *const u8).add(std::mem::size_of::()); + let data = keys_lookup::string_header_payload(key); let mut h: u64 = 0xcbf29ce484222325; for i in 0..len { h ^= *data.add(i) as u64; diff --git a/crates/perry-runtime/src/object/shapes.rs b/crates/perry-runtime/src/object/shapes.rs index c7d08b1884..2d85bd7ecb 100644 --- a/crates/perry-runtime/src/object/shapes.rs +++ b/crates/perry-runtime/src/object/shapes.rs @@ -1740,8 +1740,14 @@ pub(crate) fn prune_dead_shape_keys(is_dead_owner: &dyn Fn(usize) -> bool) { crate::perry_thread_local! { /// Scratch memo for [`scan_shape_table_rekey_mut`]'s per-address probe, /// reused across collections so the scan allocates nothing. - static PROBE_MEMO: std::cell::RefCell> = - std::cell::RefCell::new(std::collections::HashMap::new()); + /// `PtrHashMap`, NOT std's SipHash default: perf on the dynamic-property + /// benchmark put `RandomState::hash_one::<&(usize, bool)>` at **7.0% of + /// total samples** — pure hashing overhead inside the GC scan this memo + /// exists to make cheaper. The key is folded to one word (`addr ^ carrier` + /// in bit 0; addresses are >= 8-aligned so bit 0 is free), which is the + /// single-word shape `PtrHasher` is built for. + static PROBE_MEMO: std::cell::RefCell> = + std::cell::RefCell::new(crate::fast_hash::new_ptr_hash_map()); } /// Per-descriptor bookkeeping after its keys address has been probed. @@ -1818,7 +1824,9 @@ pub(crate) fn scan_shape_table_rekey_mut(visitor: &mut crate::gc::RuntimeRootVis // immortal and turn `prune_dead_shape_keys`'s "is the keys array // dead?" into a question it asks of itself. let is_carrier = descriptor.old_carrier || descriptor.cache_carrier; - let memo_key = (addr, is_carrier); + // Addresses are 8-aligned, so bit 0 is free to carry the carrier + // duty (carriers use a MARKING visit; the answers must not mix). + let memo_key = addr | usize::from(is_carrier); if let Some(&(prev_moved, prev_addr)) = probe_memo.get(&memo_key) { // Already probed this exact (address, carrier-duty) pair in this // pass — reuse the answer instead of paying the walk again. @@ -1847,7 +1855,7 @@ pub(crate) fn scan_shape_table_rekey_mut(visitor: &mut crate::gc::RuntimeRootVis } else { visitor.visit_metadata_usize_slot(&mut addr) }; - probe_memo.insert((probe_addr, is_carrier), (moved, addr)); + probe_memo.insert(probe_addr | usize::from(is_carrier), (moved, addr)); record_shape_scan_outcome( visitor, id, diff --git a/crates/perry-runtime/src/proxy/put_value.rs b/crates/perry-runtime/src/proxy/put_value.rs index c23ee64500..a88f9d4f78 100644 --- a/crates/perry-runtime/src/proxy/put_value.rs +++ b/crates/perry-runtime/src/proxy/put_value.rs @@ -436,17 +436,11 @@ pub extern "C" fn js_put_value_set_ic_miss( if key_count > 4096 { return result; } - for i in 0..key_count { - let candidate = crate::array::js_array_get(keys, i as u32); - if crate::string::js_string_key_matches(candidate, key) { - crate::object::prop_plan::read_plan_record( - keys as usize, - key as usize, - i as u32, - ); - own_idx = Some(i as u32); - break; - } + // #6759: shape-index + dense-slot scan; the old per-element + // js_array_get walk was ~60 accessor calls per property op. + if let Some(i) = crate::object::keys_find_slot_by_key_ptr(keys, key_count as u32, key) { + crate::object::prop_plan::read_plan_record(keys as usize, key as usize, i); + own_idx = Some(i); } } let Some(idx) = own_idx else { @@ -726,17 +720,7 @@ pub extern "C" fn js_put_value_set_dyn_ic_miss( if key_count > 4096 { return result; } - let mut own_idx = None; - let mut cand_buf = [0u8; crate::value::SHORT_STRING_MAX_LEN]; - for i in 0..key_count { - let candidate = crate::array::js_array_get(keys, i as u32); - if let Some(cand_bytes) = crate::string::js_string_key_bytes(candidate, &mut cand_buf) { - if cand_bytes == key_bytes { - own_idx = Some(i as u32); - break; - } - } - } + let own_idx = crate::object::keys_find_slot_by_bytes(keys, key_count as u32, key_bytes); let Some(idx) = own_idx else { return result; }; @@ -1056,11 +1040,8 @@ fn object_array_numeric_write_slots( if key_count > 4096 { return None; } - for i in 0..key_count { - let candidate = crate::array::js_array_get(keys, i as u32); - if crate::string::js_string_key_matches(candidate, key) { - return Some(i as u32); - } + if let Some(slot) = crate::object::keys_find_slot_by_key_ptr(keys, key_count, key) { + return Some(slot); } None }