perf(runtime): gate property-tail registry probes by receiver header

changelog.d/7897-header-gated-property-tail.md

@@ -0,0 +1,57 @@
+### Performance
+
+**Property-get IC misses no longer ask the Set and Symbol registries about an
+ordinary object (#7867).** `get_field_by_name_object_tail` already classified
+Buffer and TypedArray before reading the receiver's `GcHeader`, then switched
+on that header for the rest of its exotic-receiver cases. Set and Symbol were
+the exceptions: every miss consulted their address-keyed registries first,
+including the process-global Symbol mutex and hash table.
+
+Those two exceptions need different gates. Sets are allocated with
+`GC_TYPE_SET`, so the header can rule them out before `SET_REGISTRY` is touched.
+Symbols span two storage classes: fresh `Symbol()` values are GC-backed strings,
+while `Symbol.for`, well-known, and Intl symbols are Box-leaked and have no
+`GcHeader`. Every Symbol does carry `SYMBOL_MAGIC` in its first word, so the
+existing exact-false `may_be_symbol_header` screen rules ordinary receivers out
+without excluding either storage class. In both positive cases the registry
+remains authoritative, including for stale address reuse.
+
+The ordering remains deliberate. Buffer and TypedArray are headerless and stay
+before the first `GcHeader` read. A possible headerless Symbol returns from the
+magic-gated dispatch before that read. Only then does the tail validate the
+pointer, read the header once, and use `GC_TYPE_SET` to decide whether Set
+dispatch is possible.
+
+The registered-receiver bodies are `#[cold]` and `#[inline(never)]`. This is not
+cosmetic: an initial version left them inline, grew
+`get_field_by_name_object_tail` by 40 bytes, shifted every following runtime
+function, and regressed `pipeline_big` by **+4.636%** paired geomean (30
+alternating quiet-M1-mini pairs, 95% CI +4.589% to +4.681%). Sampling showed no
+new hot leaf inside the changed tail; the regression tracked the text-layout
+shift. That version was rejected rather than hidden behind the intended probe
+saving.
+
+With the cold dispatch outlined, the same locked-host A/B measured 30
+alternating pairs at 1.693549 s base median and 1.688705 s fixed median:
+**-0.315% paired geomean**, bootstrap 95% CI -0.355% to -0.273%. Both arms
+exited zero and produced the exact `pipeline_big` oracle output
+`556260000 3 3`.
+
+The issue's earlier 1.8% `is_registered_symbol_slow` attribution is no longer
+present on current `main`: fresh debug-symbol samples contain zero such samples
+in either arm. The structural cost is still directly reproducible. The new
+test arms both registries, performs plain-object misses, and asserts the
+test-only entry counters do not move; before this change the Set counter moves
+from 801 to 803. It then sabotages the Symbol magic screen after a successful
+lookup to prove the Symbol dispatch actually ran and the Set registry remained
+untouched. A companion test exercises Set `.size`, a Box-leaked
+`Symbol.for(...).description`, and a GC-backed `Symbol(...).description`, so
+deleting a positive path or accidentally requiring every receiver to have a
+header turns the suite red.
+
+Validation includes the full serialized runtime suite (2,154 passed, 4
+ignored), its doc tests, test-registration and thread-local policy checks, the
+address-class audit, formatting, file-size, and whitespace gates. The
+address-class audit also reports the pre-existing stale `dyn_index.rs` ratchet
+entry (`baseline 2, found 1`); that adjacent cleanup is intentionally not folded
+into this PR.

crates/perry-runtime/src/object/field_get_set.rs

@@ -239,10 +239,13 @@ mod crypto_key;
 mod enumeration;
 mod field_ops;
 mod get_field_by_name;
+#[cfg(test)]
+mod get_field_by_name_probe_tests;
 mod get_field_by_name_tail;
 mod has_property;
 mod ic_miss;
 mod map_set_receiver;
+mod probe_dispatch;
 
 /// Size of the direct-mapped `(keys_ptr, key_hash, field_index)` inline
 /// cache backing `js_object_get_field_by_name`'s slow tail.

crates/perry-runtime/src/object/field_get_set/get_field_by_name_probe_tests.rs

@@ -0,0 +1,128 @@
+//! Probe-dispatch invariants for the by-name property-get slow tail (#7867).
+//!
+//! Buffer and small TypedArray allocations can be headerless, so their
+//! registry probes must stay ahead of the first `GcHeader` read. Sets cannot:
+//! every registered Set is `GC_TYPE_SET`. Symbols straddle both storage
+//! classes, but every one carries `SYMBOL_MAGIC` in its own first word. These
+//! tests pin the resulting split instead of merely checking the read result.
+
+use super::*;
+
+fn key(bytes: &[u8]) -> *const crate::StringHeader {
+    crate::string::js_string_from_bytes(bytes.as_ptr(), bytes.len() as u32)
+}
+
+fn leaked_symbol(description: &str) -> usize {
+    let description = crate::string::js_string_from_str(description);
+    let description = f64::from_bits(crate::value::js_nanbox_string(description as i64).to_bits());
+    let bits = unsafe { crate::symbol::js_symbol_for(description) }.to_bits();
+    let ptr = crate::value::js_nanbox_get_pointer(f64::from_bits(bits)) as usize;
+    assert_ne!(ptr, 0, "test premise: Symbol.for allocated a symbol");
+    assert!(
+        crate::symbol::is_registered_symbol(ptr),
+        "test premise: the leaked symbol is registered on this thread"
+    );
+    ptr
+}
+
+fn fresh_symbol(description: &str) -> usize {
+    let description = crate::string::js_string_from_str(description);
+    let description = f64::from_bits(crate::value::js_nanbox_string(description as i64).to_bits());
+    let bits = unsafe { crate::symbol::js_symbol_new(description) }.to_bits();
+    let ptr = crate::value::js_nanbox_get_pointer(f64::from_bits(bits)) as usize;
+    assert_ne!(
+        ptr, 0,
+        "test premise: Symbol(description) allocated a symbol"
+    );
+    assert!(
+        crate::symbol::is_registered_symbol(ptr),
+        "test premise: the fresh symbol is registered on this thread"
+    );
+    ptr
+}
+
+fn tail(addr: usize, property: &[u8]) -> JSValue {
+    get_field_by_name_object_tail(addr as *const ObjectHeader, key(property))
+}
+
+#[test]
+fn plain_object_miss_skips_set_and_symbol_registries() {
+    leaked_symbol("perry-7867-arm-symbol");
+    let _set = crate::set::js_set_alloc(4);
+    let object = crate::object::js_object_alloc(0, 0) as usize;
+
+    // Warm unrelated lazy state before taking the counters.
+    assert!(tail(object, b"missing").is_undefined());
+    let set_before = crate::set::test_set_registry_probe_count();
+    let symbol_before = crate::symbol::test_symbol_registry_probe_count();
+
+    assert!(tail(object, b"missing").is_undefined());
+    assert_eq!(
+        crate::set::test_set_registry_probe_count(),
+        set_before,
+        "GC_TYPE_OBJECT rules a Set out; a property miss must not probe SET_REGISTRY"
+    );
+    assert_eq!(
+        crate::symbol::test_symbol_registry_probe_count(),
+        symbol_before,
+        "a plain-object property miss must not take the process-global symbol mutex"
+    );
+
+    // Sabotage the cheap SYMBOL_MAGIC screen. The authoritative registry probe
+    // must return, while the answer and the Set counter stay unchanged.
+    let restore = crate::symbol::test_disable_symbol_magic_screen(true);
+    let set_before = crate::set::test_set_registry_probe_count();
+    let symbol_before = crate::symbol::test_symbol_registry_probe_count();
+    let answer = tail(object, b"missing");
+    let symbol_after = crate::symbol::test_symbol_registry_probe_count();
+    crate::symbol::test_disable_symbol_magic_screen(restore);
+
+    assert!(answer.is_undefined());
+    assert!(
+        symbol_after > symbol_before,
+        "with the magic screen defeated the symbol registry probe must return"
+    );
+    assert_eq!(
+        crate::set::test_set_registry_probe_count(),
+        set_before,
+        "defeating the symbol screen must not reintroduce the Set probe"
+    );
+}
+
+#[test]
+fn set_and_both_symbol_storage_classes_still_dispatch() {
+    let set = crate::set::js_set_alloc(4) as usize;
+    let set_before = crate::set::test_set_registry_probe_count();
+    let size = tail(set, b"size");
+    assert_eq!(f64::from_bits(size.bits()), 0.0);
+    assert!(
+        crate::set::test_set_registry_probe_count() > set_before,
+        "GC_TYPE_SET must still enter the authoritative Set registry"
+    );
+
+    let symbol = leaked_symbol("perry-7867-headerless-symbol");
+    assert!(
+        unsafe { crate::symbol::may_be_symbol_header(symbol as *const u8) },
+        "a Box-leaked symbol must carry SYMBOL_MAGIC without a GcHeader"
+    );
+    let symbol_before = crate::symbol::test_symbol_registry_probe_count();
+    let description = tail(symbol, b"description");
+    assert!(description.is_string());
+    assert!(
+        crate::symbol::test_symbol_registry_probe_count() > symbol_before,
+        "a possible Symbol must still enter the authoritative symbol registry"
+    );
+
+    let symbol = fresh_symbol("perry-7867-gc-symbol");
+    assert!(
+        unsafe { crate::symbol::may_be_symbol_header(symbol as *const u8) },
+        "a GC-allocated symbol must carry SYMBOL_MAGIC too"
+    );
+    let symbol_before = crate::symbol::test_symbol_registry_probe_count();
+    let description = tail(symbol, b"description");
+    assert!(description.is_string());
+    assert!(
+        crate::symbol::test_symbol_registry_probe_count() > symbol_before,
+        "a GC-allocated Symbol must still enter the authoritative symbol registry"
+    );
+}

crates/perry-runtime/src/object/field_get_set/get_field_by_name_tail.rs

@@ -451,88 +451,22 @@ pub(crate) fn get_field_by_name_object_tail(
             }
             return JSValue::undefined();
         }
-        // Sets: detect via the SET_REGISTRY, which is authoritative and
-        // dereference-free. Route `.size` to `js_set_size` and synthesize
-        // method values for prototype functions such as `.has`, which Node
-        // exposes through ordinary property reads.
-        //
-        // (This used to say a `SetHeader` comes from a raw `alloc()` with no
-        // `GcHeader`, so the preceding byte could not be read. That has not
-        // been true since `js_set_alloc` moved to
-        // `arena_alloc_gc(_, _, GC_TYPE_SET)`: a registered Set IS a GC
-        // allocation and its `obj_type` classifies it. `js_array_get_f64` and
-        // `js_array_length` gate their probes on exactly that byte (#7765);
-        // this receiver is not proven to carry a header at this point, so it
-        // still asks the registry.)
-        if crate::set::is_registered_set(obj as usize) {
-            if !key.is_null() {
-                let key_ptr = (key as *const u8).add(std::mem::size_of::<crate::StringHeader>());
-                let key_len = (*key).byte_len as usize;
-                let key_bytes = std::slice::from_raw_parts(key_ptr, key_len);
-                if key_bytes == b"size" {
-                    let s = obj as *const crate::set::SetHeader;
-                    return JSValue::number(crate::set::js_set_size(s) as f64);
-                }
-                if let Some(name) = set_method_value_name(key_bytes) {
-                    // Return the SAME brand-checking thunk installed on
-                    // Set.prototype so `const m = s.forEach; m.call(badThis)`
-                    // throws a TypeError (and `m === Set.prototype.forEach`).
-                    // Falls back to the legacy instance-bound closure if the
-                    // prototype thunk isn't available.
-                    if let Ok(method_name) = std::str::from_utf8(name) {
-                        if let Some(v) =
-                            super::super::collection_proto_thunks::collection_proto_method_value(
-                                "Set",
-                                method_name,
-                            )
-                        {
-                            return JSValue::from_bits(v.to_bits());
-                        }
-                    }
-                    let this_f64 =
-                        f64::from_bits(crate::value::js_nanbox_pointer(obj as i64).to_bits());
-                    let result = js_class_method_bind(this_f64, name.as_ptr(), name.len());
-                    return JSValue::from_bits(result.to_bits());
-                }
-                // User expando keys (`s.tag = x`) live in the exotic side
-                // table (`ExoticKind::Set`); see the Map/Set arm below.
-                if let Ok(name) = std::str::from_utf8(key_bytes) {
-                    let receiver =
-                        f64::from_bits(crate::value::js_nanbox_pointer(obj as i64).to_bits());
-                    if let Some(v) = crate::object::exotic_expando::exotic_get_own_property(
-                        obj as usize,
-                        crate::object::exotic_expando::ExoticKind::Set,
-                        name,
-                        receiver,
-                    ) {
-                        return JSValue::from_bits(v.to_bits());
-                    }
-                }
-            }
-            return JSValue::undefined();
-        }
-        // Symbols: registered in SYMBOL_POINTERS by symbol.rs. Symbols
-        // allocated via Symbol.for(...) are Box-leaked (no GcHeader), so
-        // reading the byte before would be UB. Detect via the side table.
-        if crate::symbol::is_registered_symbol(obj as usize) {
-            if !key.is_null() {
-                let key_ptr = (key as *const u8).add(std::mem::size_of::<crate::StringHeader>());
-                let key_len = (*key).byte_len as usize;
-                let key_bytes = std::slice::from_raw_parts(key_ptr, key_len);
-                let sym_f64 =
-                    f64::from_bits(0x7FFD_0000_0000_0000u64 | (obj as u64 & 0x0000_FFFF_FFFF_FFFF));
-                if key_bytes == b"description" {
-                    return JSValue::from_bits(
-                        crate::symbol::js_symbol_description(sym_f64).to_bits(),
-                    );
-                }
+        // Symbols straddle two storage classes: fresh Symbol() values carry a
+        // GC_TYPE_STRING header, while Symbol.for / well-known / Intl symbols
+        // are Box-leaked and carry no GcHeader at all. Every one does carry
+        // SYMBOL_MAGIC in its own first word, so use that exact-false screen
+        // before the authoritative registry. A plain object now pays one
+        // 4-byte load instead of the process-global symbol Mutex + SipHash.
+        if crate::symbol::may_be_symbol_header(obj as *const u8) {
+            if let Some(value) = super::probe_dispatch::symbol_property_if_registered(obj, key) {
+                return value;
             }
-            return JSValue::undefined();
         }
-        // Validate this is an ObjectHeader, not some other heap type.
-        // Check GcHeader first (reliable for heap objects), then fallback to ObjectHeader.object_type
-        // for static/const objects that don't have GcHeaders.
-        // Guard: ensure we can safely read GC_HEADER_SIZE bytes before obj
+
+        // Buffer and TypedArray were the two headerless allocations that had
+        // to be classified first. A possible headerless Symbol returned above;
+        // every remaining supported receiver can now be classified once by
+        // the GcHeader the rest of this function already switches on.
         if (obj as usize) < crate::gc::GC_HEADER_SIZE + 0x1000
             || !is_valid_obj_ptr(obj as *const u8)
         {
@@ -541,8 +475,16 @@ pub(crate) fn get_field_by_name_object_tail(
         let gc_header =
             (obj as *const u8).sub(crate::gc::GC_HEADER_SIZE) as *const crate::gc::GcHeader;
         let gc_type = (*gc_header).obj_type;
-        if gc_type != crate::gc::GC_TYPE_ARRAY && !is_valid_obj_ptr(obj as *const u8) {
-            return JSValue::undefined();
+
+        // Sets are arena_alloc_gc(_, _, GC_TYPE_SET) allocations. Let the
+        // header rule every other receiver out before entering SET_REGISTRY;
+        // keep the registry authoritative for a genuine Set and for stale
+        // address reuse. Route `.size` and prototype method-value reads exactly
+        // as before.
+        if gc_type == crate::gc::GC_TYPE_SET {
+            if let Some(value) = super::probe_dispatch::set_property_if_registered(obj, key) {
+                return value;
+            }
         }
         // Issue #618: closures have their own GC type (GC_TYPE_CLOSURE=4)
         // distinct from GC_TYPE_OBJECT, but support dynamic-property storage