diff --git a/changelog.d/9016-computed-read-by-value.md b/changelog.d/9016-computed-read-by-value.md new file mode 100644 index 0000000000..dc7655b15d --- /dev/null +++ b/changelog.d/9016-computed-read-by-value.md @@ -0,0 +1,36 @@ +Computed property reads take the key by value, and the combined +overwrite+read loop drops 38 → 30 ms (node: 26); the computed-key read loop +drops 21 → **13 ms against node's 24** — nearly twice as fast as node. + +The computed-read lowering called `js_get_string_pointer_unified` before the +by-name entry, because that entry's signature wants a `*const StringHeader`. +For an SSO key that means materialising inline bytes onto the heap — an intern +hash and table probe on every read — purely to satisfy a pointer signature. +`intern_dispatch_bytes` was 5.5% of the combined loop, essentially all of it +that. + +The new by-value entry (`js_typed_feedback_object_get_field_by_value_f64`) +takes the key NaN-boxed: + +- a heap-tagged key is unmasked and passed through — no work; +- an SSO key probes the megamorphic read stub on its CONTENT bits — a hit + never builds a `StringHeader` at all; +- a stub-missing SSO key takes a READ-ONLY intern probe (an intern hit cannot + allocate or move anything) and calls through with the canonical pointer — + still no rooting; +- only a key read before its first write materialises, with the receiver + rooted across the allocation (the hazard the old codegen comment worked + around by re-deriving its handle below the unbox — that workaround is gone + because the fast path no longer allocates). + +Everything downstream is unchanged: feedback recording, exotic receivers and +prototype resolution take exactly the previous path. + +Interleaved A/B, min-of-21 at quiet load (~1.1): combined overwrite 38 → +30 ms, computed-key read 21 → 13 ms, realistic-name read and populated delete +unchanged. + +Verification: suite 2790 passed, no warnings; adversarial property +differential, computed-key differential, and a targeted stale-slot +differential (192 reads across every rotation state of a delete/re-add cycle +with a stable keys-array address) all byte-identical to node. diff --git a/changelog.d/9021-computed-read-by-value.md b/changelog.d/9021-computed-read-by-value.md new file mode 100644 index 0000000000..dc7655b15d --- /dev/null +++ b/changelog.d/9021-computed-read-by-value.md @@ -0,0 +1,36 @@ +Computed property reads take the key by value, and the combined +overwrite+read loop drops 38 → 30 ms (node: 26); the computed-key read loop +drops 21 → **13 ms against node's 24** — nearly twice as fast as node. + +The computed-read lowering called `js_get_string_pointer_unified` before the +by-name entry, because that entry's signature wants a `*const StringHeader`. +For an SSO key that means materialising inline bytes onto the heap — an intern +hash and table probe on every read — purely to satisfy a pointer signature. +`intern_dispatch_bytes` was 5.5% of the combined loop, essentially all of it +that. + +The new by-value entry (`js_typed_feedback_object_get_field_by_value_f64`) +takes the key NaN-boxed: + +- a heap-tagged key is unmasked and passed through — no work; +- an SSO key probes the megamorphic read stub on its CONTENT bits — a hit + never builds a `StringHeader` at all; +- a stub-missing SSO key takes a READ-ONLY intern probe (an intern hit cannot + allocate or move anything) and calls through with the canonical pointer — + still no rooting; +- only a key read before its first write materialises, with the receiver + rooted across the allocation (the hazard the old codegen comment worked + around by re-deriving its handle below the unbox — that workaround is gone + because the fast path no longer allocates). + +Everything downstream is unchanged: feedback recording, exotic receivers and +prototype resolution take exactly the previous path. + +Interleaved A/B, min-of-21 at quiet load (~1.1): combined overwrite 38 → +30 ms, computed-key read 21 → 13 ms, realistic-name read and populated delete +unchanged. + +Verification: suite 2790 passed, no warnings; adversarial property +differential, computed-key differential, and a targeted stale-slot +differential (192 reads across every rotation state of a delete/re-add cycle +with a stable keys-array address) all byte-identical to node. diff --git a/crates/perry-codegen/src/expr/index_get.rs b/crates/perry-codegen/src/expr/index_get.rs index a5232a03cb..1b618bdc3d 100644 --- a/crates/perry-codegen/src/expr/index_get.rs +++ b/crates/perry-codegen/src/expr/index_get.rs @@ -1911,13 +1911,17 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { // error, not a style choice: the NUMERIC sibling block below // uses that one, and a definition in THIS block does not // dominate it. - let (key_handle, str_obj_handle) = { + // The by-VALUE entry takes the key as its NaN-boxed value, so + // an SSO key is probed against the read stub on its content + // bits and a hit never materialises a `StringHeader` at all. + // That also removes the allocation the comment above works + // around on the fast path; the runtime entry roots the + // receiver across the fallback materialisation, which is where + // that hazard now lives. + let str_obj_handle = { let blk = ctx.block(); - let key_handle = unbox_str_handle(blk, &idx_box); let obj_bits = blk.bitcast_double_to_i64(&obj_box); - let str_obj_handle = - classref_preserving_handle(blk, &obj_bits, preserve_class_ref_bits); - (key_handle, str_obj_handle) + classref_preserving_handle(blk, &obj_bits, preserve_class_ref_bits) }; let site_id = emit_typed_feedback_register_site( ctx, @@ -1927,8 +1931,8 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { ); let v_str = ctx.block().call( DOUBLE, - "js_typed_feedback_object_get_field_by_name_f64", - &[(I64, &site_id), (I64, &str_obj_handle), (I64, &key_handle)], + "js_typed_feedback_object_get_field_by_value_f64", + &[(I64, &site_id), (I64, &str_obj_handle), (DOUBLE, &idx_box)], ); let str_end_lbl = ctx.block().label.clone(); ctx.block().br(&merge_lbl); diff --git a/crates/perry-codegen/src/runtime_decls/strings.rs b/crates/perry-codegen/src/runtime_decls/strings.rs index 71566af703..f62812ea52 100644 --- a/crates/perry-codegen/src/runtime_decls/strings.rs +++ b/crates/perry-codegen/src/runtime_decls/strings.rs @@ -709,6 +709,13 @@ pub fn declare_phase_b_strings(module: &mut LlModule) { // `js_string_char_at` / `js_array_get_f64` / `js_object_get_field_by_name_f64` // based on the receiver's NaN-box tag at runtime. Used by IndexGet's // fallback path when codegen can't statically prove the receiver type. + // By-value computed read: key arrives NaN-boxed so an SSO key can be + // answered from the read stub without being materialised to the heap. + module.declare_function( + "js_typed_feedback_object_get_field_by_value_f64", + DOUBLE, + &[I64, I64, DOUBLE], + ); module.declare_function("js_dyn_index_get", DOUBLE, &[DOUBLE, DOUBLE]); // #8655: guarded packed-array / dense Array-subclass read before the // fully generic dynamic dispatcher. Used by unknown-receiver loop reads. diff --git a/crates/perry-runtime/src/object/read_stub.rs b/crates/perry-runtime/src/object/read_stub.rs index 5d56c388c8..c74ad052b0 100644 --- a/crates/perry-runtime/src/object/read_stub.rs +++ b/crates/perry-runtime/src/object/read_stub.rs @@ -116,3 +116,57 @@ pub(crate) unsafe fn receiver_shape_token(obj: *const ObjectHeader) -> Option Option { + if obj.is_null() { + return None; + } + let addr = obj as usize; + let gc = crate::value::addr_class::try_read_gc_header(addr)?; + const STUB_BLOCKING: u16 = + crate::gc::OBJ_FLAG_HAS_DESCRIPTORS | crate::gc::OBJ_FLAG_TYPED_ARRAY_PROTO; + if gc.obj_type != crate::gc::GC_TYPE_OBJECT + || gc.gc_flags & crate::gc::GC_FLAG_FORWARDED != 0 + || gc._reserved & STUB_BLOCKING != 0 + { + return None; + } + let class_id = (*obj).class_id; + if class_id == 0 || class_id == crate::object::NATIVE_MODULE_CLASS_ID { + return None; + } + let token = receiver_shape_token(obj)?; + let slot = read_stub_probe(token, key_bits)?; + let limit = std::cmp::max( + crate::object::object_live_slot_count(obj), + crate::object::INLINE_SLOT_FLOOR as u32, + ); + if slot < limit { + return Some(f64::from_bits( + crate::object::js_object_get_field(obj, slot).bits(), + )); + } + crate::object::overflow_get(addr, slot as usize).map(f64::from_bits) +} diff --git a/crates/perry-runtime/src/string/intern.rs b/crates/perry-runtime/src/string/intern.rs index d9f6a0ce8b..556c526ac8 100644 --- a/crates/perry-runtime/src/string/intern.rs +++ b/crates/perry-runtime/src/string/intern.rs @@ -299,3 +299,31 @@ pub(crate) fn test_clear_intern_table_root() { }; }); } + +/// Read-only intern-table probe: the canonical pointer for `bytes`, or `None` +/// when they are not currently tabled. Never inserts, never allocates, never +/// touches GC state — safe to call with unrooted raw pointers live. +pub(crate) fn intern_lookup_bytes(bytes: &[u8]) -> Option<*const StringHeader> { + if bytes.is_empty() || bytes.len() > INTERN_MAX_BYTE_LEN as usize { + return None; + } + let mut hash = 0xcbf2_9ce4_8422_2325u64; + for &b in bytes { + hash ^= b as u64; + hash = hash.wrapping_mul(0x0100_0000_01b3); + } + let slot = (hash as usize) & INTERN_TABLE_MASK; + with_intern_table(|table| unsafe { + let entry = &(*table)[slot]; + if entry.string_ptr != 0 && entry.hash == hash { + let existing = entry.string_ptr as *const StringHeader; + if is_valid_string_ptr(existing) + && (*existing).byte_len as usize == bytes.len() + && std::slice::from_raw_parts(super::string_data(existing), bytes.len()) == bytes + { + return Some(existing); + } + } + None + }) +} diff --git a/crates/perry-runtime/src/string/mod.rs b/crates/perry-runtime/src/string/mod.rs index b5c54b0c4e..88e0318c72 100644 --- a/crates/perry-runtime/src/string/mod.rs +++ b/crates/perry-runtime/src/string/mod.rs @@ -192,6 +192,8 @@ pub use slice_ops::{ }; pub use split::{js_string_split, js_string_split_n}; +pub(crate) use intern::intern_lookup_bytes; + #[cfg(test)] pub(crate) use intern::{ test_clear_intern_table_root, test_intern_table_root, test_seed_intern_table_root, diff --git a/crates/perry-runtime/src/typed_feedback.rs b/crates/perry-runtime/src/typed_feedback.rs index 0a7c123787..748f36a1bf 100644 --- a/crates/perry-runtime/src/typed_feedback.rs +++ b/crates/perry-runtime/src/typed_feedback.rs @@ -948,6 +948,73 @@ pub extern "C" fn js_typed_feedback_observe_property_set( observe_property(site_id, TypedFeedbackSiteKind::PropertySet, obj as u64, key); } +/// By-VALUE twin of [`js_typed_feedback_object_get_field_by_name_f64`], for the +/// computed-read lowering. +/// +/// That lowering had to call `js_get_string_pointer_unified` first, because the +/// by-name entry wants a `*const StringHeader` — and for an SSO key that means +/// materialising inline bytes onto the heap (an intern hash plus table probe, +/// and an allocation on a miss) on EVERY read, purely to satisfy a pointer +/// signature. `intern_dispatch_bytes` is 5.5% of the combined overwrite loop, +/// essentially all of it that. +/// +/// An SSO key is probed against the megamorphic read stub on its CONTENT bits +/// first; a hit returns the slot without ever building a `StringHeader`. +/// Everything else falls through to exactly the previous path, so feedback +/// recording, exotic receivers and prototype resolution are unchanged. +/// +/// # Safety +/// `obj` is the raw receiver the caller already holds; the fallback below +/// materialises the key, which can allocate and therefore move `obj`, so the +/// receiver is rooted across it and re-read — the hazard the caller's own +/// lowering comment describes. +#[no_mangle] +pub extern "C" fn js_typed_feedback_object_get_field_by_value_f64( + site_id: u64, + obj: *const ObjectHeader, + key: f64, +) -> f64 { + let bits = key.to_bits(); + let top16 = bits >> 48; + // Heap string key: the pointer already exists — unmask and go. No scope, + // nothing here can allocate. + if top16 == 0x7FFF { + let key_ptr = (bits & crate::value::POINTER_MASK) as *const crate::StringHeader; + return js_typed_feedback_object_get_field_by_name_f64(site_id, obj, key_ptr); + } + if top16 == 0x7FF9 { + if let Some(v) = unsafe { crate::object::read_stub::try_read_by_content_bits(obj, bits) } { + return v; + } + // Stub miss. An intern HIT cannot allocate or move anything, so probe + // the table read-only and call through with the canonical pointer — + // still no scope. This is the steady state: the write path interns + // every key it stores, so a key being read has almost always been + // written first. The first version of this entry opened a rooted + // scope here unconditionally, and the per-read root push's write + // barrier fed the remembered set hard enough to multiply minor + // collections — cache misses rose 29x and the populated-delete + // benchmark regressed 38%. + let jsval = crate::value::JSValue::from_bits(bits); + let mut sso = [0u8; crate::value::SHORT_STRING_MAX_LEN]; + let n = jsval.short_string_to_buf(&mut sso); + if let Some(canonical) = crate::string::intern_lookup_bytes(&sso[..n]) { + return js_typed_feedback_object_get_field_by_name_f64(site_id, obj, canonical); + } + } + // Cold path only: an SSO key read before its first write, or a + // non-string key. Materialisation can allocate and move the receiver, + // so root it across the call. + let scope = crate::gc::RuntimeHandleScope::new(); + let obj_handle = scope.root_raw_const_ptr(obj); + let key_ptr = crate::value::js_get_string_pointer_unified(key) as *const crate::StringHeader; + // Scoped post-call reload (#7341): the materialisation above may have + // moved the receiver; nothing in the closure allocates. + obj_handle.with_const_ptr::(|obj| { + js_typed_feedback_object_get_field_by_name_f64(site_id, obj, key_ptr) + }) +} + #[no_mangle] pub extern "C" fn js_typed_feedback_object_get_field_by_name_f64( site_id: u64,