diff --git a/changelog.d/keystroke-property-key-decode.md b/changelog.d/keystroke-property-key-decode.md new file mode 100644 index 0000000000..50d22562ce --- /dev/null +++ b/changelog.d/keystroke-property-key-decode.md @@ -0,0 +1,28 @@ +### Performance + +- **Property reads no longer UTF-8-validate ASCII keys, and no longer copy + the key or take the async-resource registry lock for ordinary receivers.** + The generic read ladder decodes the key `StringHeader` at several layers + per miss (`js_object_get_field_ic_miss`, closure expando lookup, accessor + and reflection probes, the typed-feedback class-field guards, async- + resource dispatch); `core::str::from_utf8` on those decodes was 2 % of the + claude-code keystroke profile, the guard's `String` copy was a `malloc` + per guarded class-field access, and `async_resource_property` copied the + key and locked the registry before asking whether any AsyncResource + handle existed at all. + + - `crates/perry-runtime/src/string/mod.rs` — `header_str_checked`: a + header whose `utf16_len == byte_len` is pure ASCII, so it is borrowed + unchecked; anything else takes the `from_utf8` scan it always took + (WTF-8 payloads still answer `None`). Used by `has_own_helpers`, + `closure_dynamic_prop_by_key`, the accessor probes, + `typedarray_props::string_header_str` and the typed-feedback guards + (which now borrow instead of allocating a `String`; every consumer is a + Rust-side table read, so the payload cannot move while borrowed). + - `crates/perry-runtime/src/async_hooks.rs` — `is_async_resource_handle` + answers from the atomic handle count before touching the mutex, and the + IC-miss handler / `async_resource_property` ask it before decoding or + copying the key. + + Test: `header_str_checked_matches_from_utf8_on_every_payload_class` + (ASCII, non-ASCII scalar, lone surrogate, empty). diff --git a/crates/perry-runtime/src/async_hooks.rs b/crates/perry-runtime/src/async_hooks.rs index 16a41515dd..e05f322da0 100644 --- a/crates/perry-runtime/src/async_hooks.rs +++ b/crates/perry-runtime/src/async_hooks.rs @@ -207,8 +207,15 @@ pub struct AsyncResourceHandle { event_emitter: i64, } +/// Is `handle` a live `AsyncResource` backing? One relaxed load answers "no" +/// while none was ever created; only then the registry lock. The generic +/// property-read ladder asks this BEFORE decoding or copying the key, so an +/// ordinary receiver — the overwhelming case — pays neither. +#[inline] pub(crate) fn is_async_resource_handle(handle: i64) -> bool { - handle != 0 && ASYNC_RESOURCE_HANDLES.lock().unwrap().contains(&handle) + ASYNC_RESOURCE_HANDLE_COUNT.load(Ordering::Relaxed) != 0 + && handle != 0 + && ASYNC_RESOURCE_HANDLES.lock().unwrap().contains(&handle) } /// Resolve either a native `AsyncResource` handle or the ordinary object used @@ -1371,9 +1378,7 @@ fn async_resource_bind_method_value(handle: i64) -> f64 { } pub fn try_async_resource_property_dispatch(handle: i64, property: &str) -> Option { - if ASYNC_RESOURCE_HANDLE_COUNT.load(Ordering::Relaxed) == 0 - || !ASYNC_RESOURCE_HANDLES.lock().unwrap().contains(&handle) - { + if !is_async_resource_handle(handle) { return None; } // User-defined own properties shadow AsyncResource.prototype just as they diff --git a/crates/perry-runtime/src/object/field_get_set/accessors.rs b/crates/perry-runtime/src/object/field_get_set/accessors.rs index 6e8d91ed66..8c477cdb50 100644 --- a/crates/perry-runtime/src/object/field_get_set/accessors.rs +++ b/crates/perry-runtime/src/object/field_get_set/accessors.rs @@ -623,9 +623,7 @@ pub(crate) unsafe fn primitive_builtin_prototype_property( // inside `invoke_accessor_getter` — not the prototype object the accessor // happens to live on (which a plain field read below would hand it). if crate::state::state().descriptors.accessors_in_use.get() { - let key_ptr = (key as *const u8).add(std::mem::size_of::()); - let key_len = (*key).byte_len as usize; - if let Ok(name) = std::str::from_utf8(std::slice::from_raw_parts(key_ptr, key_len)) { + if let Some(name) = crate::string::header_str_checked(key) { if let Some(acc) = get_accessor_descriptor(proto_ptr as usize, name) { if acc.get == 0 { return Some(JSValue::undefined()); @@ -678,9 +676,7 @@ pub(crate) unsafe fn array_subclass_prototype_field( { return None; } - let key_ptr = crate::object::string_header_payload(key); - let key_len = (*key).byte_len as usize; - let name = std::str::from_utf8(std::slice::from_raw_parts(key_ptr, key_len)).ok()?; + let name = crate::string::header_str_checked(key)?; // `array_prototype_property_value` copies `name` before its first // allocation and roots the receiver across the prototype lookup. array_prototype_property_value(name, obj as usize) diff --git a/crates/perry-runtime/src/object/field_get_set/get_field_by_name_async.rs b/crates/perry-runtime/src/object/field_get_set/get_field_by_name_async.rs index 427f7c4832..0120b49a3c 100644 --- a/crates/perry-runtime/src/object/field_get_set/get_field_by_name_async.rs +++ b/crates/perry-runtime/src/object/field_get_set/get_field_by_name_async.rs @@ -7,7 +7,7 @@ pub(crate) fn async_resource_property( obj: *const ObjectHeader, key: *const crate::StringHeader, ) -> Option { - if key.is_null() { + if key.is_null() || !crate::async_hooks::is_async_resource_handle(obj as i64) { return None; } let key = unsafe { crate::string::OwnedStringBytes::copy_from_header(key) }; diff --git a/crates/perry-runtime/src/object/field_get_set/has_property.rs b/crates/perry-runtime/src/object/field_get_set/has_property.rs index 783b27021d..a81839c513 100644 --- a/crates/perry-runtime/src/object/field_get_set/has_property.rs +++ b/crates/perry-runtime/src/object/field_get_set/has_property.rs @@ -1387,9 +1387,7 @@ pub(crate) unsafe fn closure_dynamic_prop_by_key( if key.is_null() { return None; } - let key_ptr = (key as *const u8).add(std::mem::size_of::()); - let key_len = (*key).byte_len as usize; - let name = std::str::from_utf8(std::slice::from_raw_parts(key_ptr, key_len)).ok()?; + let name = crate::string::header_str_checked(key)?; let val = crate::closure::closure_get_dynamic_prop(obj, name); if val.to_bits() != crate::value::TAG_UNDEFINED { return Some(val); diff --git a/crates/perry-runtime/src/object/field_get_set/ic_miss.rs b/crates/perry-runtime/src/object/field_get_set/ic_miss.rs index be3027adb6..9e1660cbf5 100644 --- a/crates/perry-runtime/src/object/field_get_set/ic_miss.rs +++ b/crates/perry-runtime/src/object/field_get_set/ic_miss.rs @@ -526,11 +526,9 @@ pub extern "C" fn js_object_get_field_ic_miss( // `< 0x100000` proxy / HANDLE_PROPERTY_DISPATCH routing below — matching // the ordering in `js_object_get_field_by_name`. The macOS heap floor // (0x200_0000_0000 in is_valid_obj_ptr) masked this; Linux's is 0x1000. - if !key.is_null() { + if !key.is_null() && crate::async_hooks::is_async_resource_handle(obj as i64) { unsafe { - let key_ptr = crate::string::string_data(key); - let key_len = (*key).byte_len as usize; - if let Ok(name) = std::str::from_utf8(std::slice::from_raw_parts(key_ptr, key_len)) { + if let Some(name) = crate::string::header_str_checked(key) { if let Some(value) = crate::async_hooks::try_async_resource_property_dispatch(obj as i64, name) { diff --git a/crates/perry-runtime/src/object/has_own_helpers.rs b/crates/perry-runtime/src/object/has_own_helpers.rs index bd6b1d185c..83de47f064 100644 --- a/crates/perry-runtime/src/object/has_own_helpers.rs +++ b/crates/perry-runtime/src/object/has_own_helpers.rs @@ -72,10 +72,7 @@ unsafe fn string_header_as_str<'a>(key: *const crate::StringHeader) -> Option<&' if key.is_null() { return None; } - let len = (*key).byte_len as usize; - let data = (key as *const u8).add(std::mem::size_of::()); - let bytes = std::slice::from_raw_parts(data, len); - std::str::from_utf8(bytes).ok() + crate::string::header_str_checked(key) } pub(super) unsafe fn string_primitive_own_key_present( diff --git a/crates/perry-runtime/src/string/mod.rs b/crates/perry-runtime/src/string/mod.rs index 77c33aa738..f673d45d22 100644 --- a/crates/perry-runtime/src/string/mod.rs +++ b/crates/perry-runtime/src/string/mod.rs @@ -1027,6 +1027,32 @@ pub(crate) fn is_ascii_string(s: *const StringHeader) -> bool { unsafe { (*s).utf16_len == (*s).byte_len } } +/// Borrow a header's payload as `&str`, answering `None` for a WTF-8 payload +/// (lone surrogates), like `std::str::from_utf8(..).ok()` — but without the +/// scan when the header already proves the answer: `utf16_len == byte_len` +/// holds iff every byte is a one-byte code unit, i.e. pure ASCII, which is +/// what nearly every property key is. The generic property-read ladder +/// decodes the key at several layers per read (`ic_miss`, closure expandos, +/// accessor and reflection probes, async-resource dispatch), and +/// `core::str::from_utf8` was 2 % of the claude-code keystroke profile on +/// those decodes alone. +/// +/// Same borrow rule as [`string_as_str`]: the slice must not outlive any +/// call that can move the payload. +/// +/// # Safety +/// `s` must point at a live `StringHeader`. +#[inline] +pub(crate) unsafe fn header_str_checked<'a>(s: *const StringHeader) -> Option<&'a str> { + let len = (*s).byte_len as usize; + let bytes = slice::from_raw_parts(string_data(s), len); + if (*s).utf16_len as usize == len { + Some(str::from_utf8_unchecked(bytes)) + } else { + str::from_utf8(bytes).ok() + } +} + /// `PERRY_GC_CENSUS`: the fixed-size intern table (slots, bytes). Entries /// point into the GC heap; only the table itself is counted. pub(crate) fn intern_table_census() -> (usize, usize) { diff --git a/crates/perry-runtime/src/string/tests.rs b/crates/perry-runtime/src/string/tests.rs index 5968961bce..c001ba15d9 100644 --- a/crates/perry-runtime/src/string/tests.rs +++ b/crates/perry-runtime/src/string/tests.rs @@ -1296,3 +1296,36 @@ mod split_empty_delimiter_code_units { assert_eq!(crate::array::js_array_length(arr), 3); } } + +/// `header_str_checked` answers exactly like `from_utf8(..).ok()` — a pure +/// ASCII key without the scan, a non-ASCII scalar key by validation, and a +/// WTF-8 payload (lone surrogate) as `None`. +#[test] +fn header_str_checked_matches_from_utf8_on_every_payload_class() { + let scope = crate::gc::RuntimeHandleScope::new(); + let ascii = scope.root_string_ptr(js_string_from_bytes(b"userName".as_ptr(), 8)); + let cjk = "名前"; + let scalar = scope.root_string_ptr(js_string_from_bytes(cjk.as_ptr(), cjk.len() as u32)); + let lone = [0xEDu8, 0xA0, 0x80, b'x']; + let wtf8 = scope.root_string_ptr(js_string_from_wtf8_bytes(lone.as_ptr(), lone.len() as u32)); + let empty = scope.root_string_ptr(js_string_from_bytes(b"".as_ptr(), 0)); + for (root, expect) in [ + (&ascii, Some("userName")), + (&scalar, Some(cjk)), + (&wtf8, None), + (&empty, Some("")), + ] { + let got = root.with_const_ptr::(|s| unsafe { header_str_checked(s) }); + assert_eq!(got, expect); + let via_std = root.with_const_ptr::(|s| { + std::str::from_utf8(string_as_bytes_for_test(s)) + .ok() + .map(|s| s.to_string()) + }); + assert_eq!(got.map(|s| s.to_string()), via_std); + } +} + +fn string_as_bytes_for_test<'a>(s: *const StringHeader) -> &'a [u8] { + unsafe { slice::from_raw_parts(string_data(s), (*s).byte_len as usize) } +} diff --git a/crates/perry-runtime/src/typed_feedback/guards.rs b/crates/perry-runtime/src/typed_feedback/guards.rs index 339bc3ccff..b165f6659a 100644 --- a/crates/perry-runtime/src/typed_feedback/guards.rs +++ b/crates/perry-runtime/src/typed_feedback/guards.rs @@ -147,17 +147,17 @@ fn method_direct_call_contract( (shape_addr, class_id, gc_type, name_hash, valid) } -fn key_as_str(key: *const crate::StringHeader) -> Option { +/// Borrow the key text for the guard's side-table lookups. Every consumer +/// (`class_getter_in_chain`, `descriptor_blocks_class_field_*`, +/// `get_accessor_descriptor`, `get_property_attrs`) reads Rust-side tables +/// and allocates nothing on the GC heap, so the payload cannot move while the +/// borrow is live; the `String` this used to return was one `malloc` + UTF-8 +/// scan per guarded class-field access. +fn key_as_str<'a>(key: *const crate::StringHeader) -> Option<&'a str> { if !valid_string_key(key) { return None; } - unsafe { - let len = (*key).byte_len as usize; - let data = (key as *const u8).add(std::mem::size_of::()); - std::str::from_utf8(std::slice::from_raw_parts(data, len)) - .ok() - .map(|s| s.to_string()) - } + unsafe { crate::string::header_str_checked(key) } } fn class_setter_in_chain(class_id: u32, key_name: &str) -> bool { @@ -314,8 +314,8 @@ fn class_field_get_contract( expected_field_index, require_raw_f64, ) - && !class_getter_in_chain(class_id, &key_name) - && !descriptor_blocks_class_field_get(object_addr, class_id, &key_name); + && !class_getter_in_chain(class_id, key_name) + && !descriptor_blocks_class_field_get(object_addr, class_id, key_name); (shape_addr, class_id, gc_type, valid) } } @@ -596,8 +596,8 @@ fn class_field_set_contract( expected_field_index, true, ))) - && !class_setter_in_chain(class_id, &key_name) - && !descriptor_blocks_class_field_set(object_addr, class_id, &key_name); + && !class_setter_in_chain(class_id, key_name) + && !descriptor_blocks_class_field_set(object_addr, class_id, key_name); (shape_addr, class_id, gc_type, valid) } } diff --git a/crates/perry-runtime/src/typedarray_props.rs b/crates/perry-runtime/src/typedarray_props.rs index b32a57b78c..c56fff0d30 100644 --- a/crates/perry-runtime/src/typedarray_props.rs +++ b/crates/perry-runtime/src/typedarray_props.rs @@ -172,9 +172,7 @@ unsafe fn string_header_str<'a>(key: *const crate::string::StringHeader) -> Opti if key.is_null() || (key as usize) < 0x10000 { return None; } - let len = (*key).byte_len as usize; - let data = (key as *const u8).add(std::mem::size_of::()); - std::str::from_utf8(std::slice::from_raw_parts(data, len)).ok() + crate::string::header_str_checked(key) } fn unsigned_canonical_index(name: &str) -> Option {