From 3b4fac9867d44245c960aaef2f1bf2eb436705ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Wed, 19 Aug 2026 19:13:56 +0200 Subject: [PATCH 1/2] perf: avoid redundant shape and array classification --- crates/perry-runtime/src/array/header.rs | 33 +++++++++ crates/perry-runtime/src/array/mod.rs | 7 +- crates/perry-runtime/src/array/push_pop.rs | 33 +++++++-- .../object/field_set_by_name/fast_paths.rs | 15 ++++- .../src/typed_feedback/guards.rs | 27 ++++---- .../perry-runtime/src/typed_feedback/tests.rs | 67 +++++++++++++++++++ 6 files changed, 155 insertions(+), 27 deletions(-) diff --git a/crates/perry-runtime/src/array/header.rs b/crates/perry-runtime/src/array/header.rs index b0b5270af3..16ca4f2c4d 100644 --- a/crates/perry-runtime/src/array/header.rs +++ b/crates/perry-runtime/src/array/header.rs @@ -68,6 +68,25 @@ pub(crate) fn array_object_flags(arr: *const ArrayHeader) -> u16 { } } +/// Read the flag word of an array that [`clean_arr_ptr`] already resolved. +/// +/// Hot operations such as `push` need the frozen, descriptor, numeric-layout, +/// and extensibility bits together. Re-entering [`array_object_flags`] for each +/// question repeats allocator ownership classification even though the first +/// clean already proved the receiver is a live, non-forwarded GC array. +/// +/// # Safety +/// +/// `arr` must be the non-null result of [`clean_arr_ptr`] or +/// [`clean_arr_ptr_mut`] with no intervening allocation or safepoint. +#[inline(always)] +pub(crate) unsafe fn array_object_flags_resolved(arr: *const ArrayHeader) -> u16 { + debug_assert!(!arr.is_null()); + let gc_header = (arr as *const u8).sub(crate::gc::GC_HEADER_SIZE) as *const crate::gc::GcHeader; + debug_assert_eq!((*gc_header).obj_type, crate::gc::GC_TYPE_ARRAY); + (*gc_header)._reserved +} + /// The `obj_type` and flag word of the `GcHeader` that precedes `arr`, read /// once, for a receiver [`clean_arr_ptr`] has already resolved. `(0, 0)` when /// `arr` is too low to carry a header — `0` is not a legal `obj_type`, so it @@ -1017,6 +1036,20 @@ pub(crate) unsafe fn canonicalize_array_numeric_store_value( f64::from_bits(canonicalize_array_numeric_store_bits(arr, value.to_bits())) } +/// Canonicalize a store using the flag word read from an already-resolved +/// array. This is the no-second-classification twin of +/// [`canonicalize_array_numeric_store_value`]. +#[inline(always)] +pub(crate) fn canonicalize_array_numeric_store_value_from_flags(flags: u16, value: f64) -> f64 { + let raw_layout = crate::gc::GC_ARRAY_RAW_F64_LAYOUT | crate::gc::GC_ARRAY_RAW_F64_HOLES; + if flags & raw_layout != 0 { + if let Some(number) = value_bits_to_number(value.to_bits()) { + return number; + } + } + value +} + #[inline] unsafe fn array_slot_bits(arr: *const ArrayHeader, index: usize) -> u64 { let slot = (arr as *const u8).add(std::mem::size_of::()) as *const u64; diff --git a/crates/perry-runtime/src/array/mod.rs b/crates/perry-runtime/src/array/mod.rs index 5a4bcc9f14..45b5af5e33 100644 --- a/crates/perry-runtime/src/array/mod.rs +++ b/crates/perry-runtime/src/array/mod.rs @@ -211,9 +211,10 @@ pub(crate) use self::header::{ array_named_property_get, array_named_property_get_by_name, array_named_property_has, array_named_property_names, array_named_property_set, array_numeric_raw_f64_get, array_numeric_raw_f64_push_inbounds, array_numeric_raw_f64_set_inbounds, array_object_flags, - array_object_flags_from_tag, array_ptr_as_proxy, array_receiver_addr, array_receiver_gc_tag, - buffer_receiver_as_uint8_typed_array, canonicalize_array_numeric_store_value, clean_arr_ptr, - clean_arr_ptr_mut, clear_array_numeric_layout, clear_array_numeric_layout_ptr, + array_object_flags_from_tag, array_object_flags_resolved, array_ptr_as_proxy, + array_receiver_addr, array_receiver_gc_tag, buffer_receiver_as_uint8_typed_array, + canonicalize_array_numeric_store_value, canonicalize_array_numeric_store_value_from_flags, + clean_arr_ptr, clean_arr_ptr_mut, clear_array_numeric_layout, clear_array_numeric_layout_ptr, gc_element_slot_range, mark_array_layout_unknown, mark_array_raw_f64_holes_fresh, normalize_array_receiver, note_array_slot, note_array_slot_layout_only, rebuild_array_layout, rebuild_array_layout_exact, refresh_array_numeric_layout, replay_array_growth_write_barriers, diff --git a/crates/perry-runtime/src/array/push_pop.rs b/crates/perry-runtime/src/array/push_pop.rs index 39aa7c0ab4..5ed024a0bd 100644 --- a/crates/perry-runtime/src/array/push_pop.rs +++ b/crates/perry-runtime/src/array/push_pop.rs @@ -22,6 +22,11 @@ fn throw_frozen_array_mutation() -> ! { #[inline] pub(crate) fn array_length_is_non_writable(arr: *const ArrayHeader) -> bool { let flags = array_object_flags(arr); + array_length_is_non_writable_with_flags(arr, flags) +} + +#[inline] +fn array_length_is_non_writable_with_flags(arr: *const ArrayHeader, flags: u16) -> bool { flags & crate::gc::OBJ_FLAG_ARRAY_DESCRIPTORS != 0 && crate::object::get_property_attrs(arr as usize, "length") .map(|a| !a.writable()) @@ -76,6 +81,13 @@ pub(crate) fn guard_writable_length(arr: *const ArrayHeader) { } } +#[inline] +fn guard_writable_length_with_flags(arr: *const ArrayHeader, flags: u16) { + if array_length_is_non_writable_with_flags(arr, flags) { + throw_non_writable_length(); + } +} + /// Guard called from the static `push_single`/`push` codegen path so that /// frozen + non-writable-`length` checks fire even for `arr.push()` with no /// arguments. ECMA-262 §23.1.3.21 always performs `Set(O,"length",…,true)`. @@ -85,10 +97,13 @@ pub extern "C" fn js_array_push_guard(arr: *mut ArrayHeader) { if arr.is_null() { return; } - if array_is_frozen(arr) { + // SAFETY: `clean_arr_ptr_mut` just proved this exact pointer is the live, + // non-forwarded array head; no allocation occurs before the flag read. + let flags = unsafe { array_object_flags_resolved(arr) }; + if flags & crate::gc::OBJ_FLAG_FROZEN != 0 { throw_frozen_array_mutation(); } - guard_writable_length(arr); + guard_writable_length_with_flags(arr, flags); } #[no_mangle] @@ -625,11 +640,17 @@ pub extern "C" fn js_array_push_f64(arr: *mut ArrayHeader, value: f64) -> *mut A return js_array_alloc(0); } let arr = cleaned; - if array_is_frozen(arr) { + // One resolved header word answers every policy/layout question below. + // Re-entering the public helpers here used to run `clean_arr_ptr` (and its + // allocator-ownership proof) once for each individual bit test. + // SAFETY: `clean_arr_ptr_mut` just returned this live head and no + // allocation or safepoint intervenes before the read. + let flags = unsafe { array_object_flags_resolved(arr) }; + if flags & crate::gc::OBJ_FLAG_FROZEN != 0 { throw_frozen_array_mutation(); } - guard_writable_length(arr); - if array_is_sealed_or_no_extend(arr) { + guard_writable_length_with_flags(arr, flags); + if flags & (crate::gc::OBJ_FLAG_SEALED | crate::gc::OBJ_FLAG_NO_EXTEND) != 0 { return arr; } unsafe { @@ -640,7 +661,7 @@ pub extern "C" fn js_array_push_f64(arr: *mut ArrayHeader, value: f64) -> *mut A return js_array_push_f64_grow(arr, length, value); } - let value = canonicalize_array_numeric_store_value(arr, value); + let value = canonicalize_array_numeric_store_value_from_flags(flags, value); let value_bits = value.to_bits(); let elements_ptr = (arr as *mut u8).add(std::mem::size_of::()) as *mut f64; // GC_STORE_AUDIT(BARRIERED): push slot is immediately recorded via note_array_slot. 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 dc5099db21..ca4b2139ee 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 @@ -38,7 +38,6 @@ pub(crate) unsafe fn try_existing_own_data_overwrite( if obj_gc.obj_type != crate::gc::GC_TYPE_OBJECT || obj_gc.gc_flags & crate::gc::GC_FLAG_FORWARDED != 0 || obj_gc._reserved & BLOCKING_FLAGS != 0 - || !crate::object::object_is_regular(obj) || (*obj).class_id == NATIVE_MODULE_CLASS_ID || crate::array::object_prototype_addr_matches(obj_addr) // URL's visible fields are live views over one backing URL. An own @@ -48,6 +47,18 @@ pub(crate) unsafe fn try_existing_own_data_overwrite( { return false; } + // The header probe above already established a live, non-forwarded + // `GC_TYPE_OBJECT`. Resolve its immutable descriptor once for both the + // ordinary-object discriminator and the live-slot bound used below. + // `object_is_regular` followed by `object_live_slot_count` repeated both + // the allocator classification and this ShapeId table lookup. + let Some(shape) = crate::object::shapes::object_shape_descriptor(obj) else { + return false; + }; + if shape.object_kind != crate::object::shapes::ShapeObjectKind::Ordinary { + return false; + } + let live_slots = shape.live_inline_slot_count; let Some(key_gc) = crate::value::addr_class::try_read_gc_header(key_addr) else { return false; @@ -99,8 +110,6 @@ pub(crate) unsafe fn try_existing_own_data_overwrite( vbits }; super::mark_object_dynamic_shape_unknown(obj); - // #8113: one bound probe, reused. It is a shape-table lookup now. - let live_slots = crate::object::object_live_slot_count(obj); let alloc_limit = std::cmp::max(live_slots, crate::object::INLINE_SLOT_FLOOR as u32) as usize; if (idx as usize) < alloc_limit { if idx >= live_slots { diff --git a/crates/perry-runtime/src/typed_feedback/guards.rs b/crates/perry-runtime/src/typed_feedback/guards.rs index 0a780bd725..3abb5c6002 100644 --- a/crates/perry-runtime/src/typed_feedback/guards.rs +++ b/crates/perry-runtime/src/typed_feedback/guards.rs @@ -1019,10 +1019,11 @@ pub unsafe extern "C" fn js_typed_feedback_method_direct_call_guard( /// /// Returns the receiver's `class_id` when every precondition the guard checks /// *other than* the class-id / shape comparison holds, and writes the -/// receiver's ShapeId through `out_shape_id`. Returns 0 — never a -/// valid user class id — when any precondition fails, and then leaves -/// output at 0 so a caller that skips the return check still cannot match a -/// real ShapeId. +/// receiver's ShapeId through `out_shape_id`. The pair is an untrusted token: +/// callers must compare it to a compiler-published `(class_id, ShapeId)` pair +/// before using it as a layout proof. Returns 0 — never a valid user class id +/// — when any precondition fails, and then leaves output at 0 so a caller that +/// skips the return check still cannot match a real ShapeId. /// /// This exists because the single-pair guard speculates the receiver's dynamic /// class is exactly the *declared* class of the expression. For a receiver @@ -1055,21 +1056,17 @@ pub unsafe extern "C" fn js_method_direct_shape_class( return 0; } let obj = object_addr as *const ObjectHeader; - // #8122: ONE shape-table probe. `object_is_regular` re-derived the GcHeader - // this function has already validated (kind + not forwarded) and probed - // for the kind; `object_shape_id` then probed again to prove the stamp - // resolves. One descriptor read answers both, and the header stamp is the - // id once it has resolved. - let Some(shape) = crate::object::shapes::object_shape_descriptor(obj) else { - return 0; - }; - if shape.object_kind != crate::object::shapes::ShapeObjectKind::Ordinary { - return 0; - } let class_id = (*obj).class_id; if class_id == 0 { return 0; } + // The emitted caller immediately compares BOTH words against one of its + // compiler-published class-shape pairs. That exact ShapeId identity is the + // descriptor proof: ids are process-unique, immutable and never reused. + // Resolving the id through the thread-local descriptor HashMap here merely + // repeated the proof before every virtual call. A class object cannot + // alias an ordinary instance's expected id: the class-kind transition + // mints its own semantic successor ShapeId. let shape_id = crate::object::shapes::object_shape_stamp(obj); if shape_id == 0 { return 0; diff --git a/crates/perry-runtime/src/typed_feedback/tests.rs b/crates/perry-runtime/src/typed_feedback/tests.rs index beac51a132..20a40837b7 100644 --- a/crates/perry-runtime/src/typed_feedback/tests.rs +++ b/crates/perry-runtime/src/typed_feedback/tests.rs @@ -1916,6 +1916,73 @@ fn typed_feedback_method_direct_guard_passes_for_exact_registered_method() { assert_eq!(site.state, "monomorphic"); } +#[test] +fn method_direct_shape_guard_requires_the_exact_compiler_pair() { + // The guard deliberately fails closed once any test in the process has + // installed a descriptor or changed a class prototype. Exercise its + // pristine fast-path state in a one-test child process instead of + // resetting those safety latches underneath unrelated tests. + const CHILD_ENV: &str = "PERRY_TEST_METHOD_DIRECT_SHAPE_PAIR_CHILD"; + if std::env::var_os(CHILD_ENV).is_none() { + let output = std::process::Command::new( + std::env::current_exe().expect("current runtime test binary"), + ) + .arg("typed_feedback::tests::method_direct_shape_guard_requires_the_exact_compiler_pair") + .arg("--exact") + .arg("--nocapture") + .env(CHILD_ENV, "1") + .output() + .expect("launch the pristine method-shape guard witness"); + assert!( + output.status.success(), + "method-shape guard witness failed:\n{}{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr), + ); + return; + } + + let _guard = typed_feedback_test_lock(); + reset_typed_feedback_for_tests(); + + let class_id = 0x7EED_1061; + let (obj, _, _, receiver) = class_instance(class_id, b"x"); + let expected_shape_id = shape_id(obj); + + assert_eq!( + unsafe { + super::guards::js_method_direct_shape_guard(receiver, class_id, expected_shape_id) + }, + 1 + ); + assert_eq!( + unsafe { + super::guards::js_method_direct_shape_guard( + receiver, + class_id.wrapping_add(1), + expected_shape_id, + ) + }, + 0 + ); + + // The classifier returns an untrusted header token; only the exact + // compiler-published pair licenses the direct call. A divergent stamp must + // miss even when it remains in the process-global ShapeId range. + unsafe { + (*obj).parent_class_id = expected_shape_id.wrapping_add(1); + } + assert_eq!( + unsafe { + super::guards::js_method_direct_shape_guard(receiver, class_id, expected_shape_id) + }, + 0 + ); + unsafe { + (*obj).parent_class_id = expected_shape_id; + } +} + #[test] fn typed_feedback_method_direct_guard_fails_for_own_method_replacement() { let _guard = typed_feedback_test_lock(); From 070b72cc95eb1a78d32b4c0bf85c75e86ee004d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Wed, 19 Aug 2026 19:15:34 +0200 Subject: [PATCH 2/2] docs: add changelog for shape classification optimization --- changelog.d/8412-shape-classification.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/8412-shape-classification.md diff --git a/changelog.d/8412-shape-classification.md b/changelog.d/8412-shape-classification.md new file mode 100644 index 0000000000..f58eed2b18 --- /dev/null +++ b/changelog.d/8412-shape-classification.md @@ -0,0 +1 @@ +Eliminate repeated GC ownership classification and ShapeId descriptor-map probes in direct method guards, array pushes, and existing-field writes. The `shapes` benchmark now beats Node in the measured sweep while retiring 5.2% fewer instructions; all 19 corpus programs remain byte-exact.