perf(runtime): stop hoisting cold-path thread-locals into o[k]'s fast lane (−5.8%)

changelog.d/10651-dynamic-key-cold-tls.md

@@ -0,0 +1,41 @@
+**perf(runtime): stop hoisting cold-path thread-locals into `o[k]`'s fast lane.**
+`js_object_get_field_by_name` resolved two thread-locals unconditionally in its
+prologue — the `PROXIES` registry and `RUNTIME_HANDLE_STACK`'s fallback — on a
+dynamic-key read loop that never touches a Proxy and never uses a `.size` key.
+Both belong to arms that are guarded at the Rust level and never taken.
+
+A `thread_local!` address resolution is `readnone` to LLVM, so once a guarded
+cold arm is inlined the optimizer may hoist *just the address computation* above
+its guard: the gate survives, the TLS call escapes it. Marking both arms
+`#[cold] #[inline(never)]` keeps them opaque to the inliner. The
+`runtime_handles` half is the general fix — `RuntimeHandleScope::new()` is called
+from dozens of small guarded arms across the runtime, and splitting its fallback
+lets the published fast arm stay `#[inline(always)]` without dragging a raw TLS
+call to every call site.
+
+Also: `try_data_get_bytes` called `is_plausible_heap_addr` explicitly and again
+inside `try_read_gc_header`, which LLVM could not CSE across
+`classify_heap_generation`'s intervening cache write; that one site now uses
+`try_read_gc_header_known_plausible`.
+
+**544.7 → 512.9 instructions per `o[k]` access (−5.8%)**, differenced within each
+binary against a bare-loop control reading 0.00 / −0.10 so layout and fixed
+per-process cost cancel. Found by disassembly, not by reading: the profile
+charged both `_tlv_get_addr` calls to `js_object_get_field_by_name` itself rather
+than to a callee, and `otool -tV` plus `nm` named which two thread-locals they
+were. Fixing the Proxy block alone left the other in place by a different route.
+
+Negative results worth not re-running: `try_data_get_bytes`'s `from_utf8` and
+Bloom-hash preamble is spec-required work; `is_anon_shape_class_id`'s remaining
+11.2% is the per-image `current()` lookup that #10570 already reduced to a hash
+plus an 8-slot probe, and a process-global mirror would be unsound across images;
+`keys_find_slot_by_bytes`'s `memcmp` is genuine key-byte comparison.
+
+Validation: new `test_gap_dynamic_key_proxy_receiver.ts` covers trapped,
+pass-through and nested Proxies plus interleaved plain/Proxy receivers — the arm
+made cold must still be correct when taken — byte-identical to node 26.5.1;
+#10570's read-paths test unchanged; four GC-stress runs (seeds 1 and 42, from-space
+protection, evacuation verification, scan-abort) all exit 0 with `dangling=0`,
+`missing_rewrites=0` and non-zero copying minors (32/29/241/247); gap suite 831/838
+with all 6 failures pre-existing; `perry-runtime --lib` 4,016 passed with the 2
+failures reproduced on pristine `origin/main`.

crates/perry-runtime/src/gc/roots/runtime_handles.rs

@@ -57,6 +57,34 @@ fn runtime_handle_stack() -> StackRef {
             return unsafe { &*(stack as *const RuntimeHandleStack) };
         }
     }
+    runtime_handle_stack_cold()
+}
+
+/// Fallback arm of [`runtime_handle_stack`]: the raw `thread_local!` lookup,
+/// reached only before this thread's `HotTls` is published (or from inside
+/// `HotTls::fill` itself). `#[inline(never)]` on purpose, not just `#[cold]`.
+///
+/// `RuntimeHandleScope::new()` is called from dozens of arms throughout the
+/// runtime, many of them small and gated behind a cheap guard deep inside an
+/// otherwise hot function (`js_object_get_field_by_name`'s `.size`-key arm is
+/// one: see its own `RuntimeHandleScope::new()` call site, guarded on the key
+/// bytes equalling `"size"`, with a comment already defending against making
+/// the SCOPE unconditional). `crate::tls_hot`'s #7469 note explains why that
+/// defense is not enough on its own: a `thread_local!` address resolution is
+/// `readnone` from the optimizer's point of view — it has no observable side
+/// effect — so once the fallback arm above is visible to the inliner at such a
+/// call site, LLVM can (and does) hoist JUST that address computation out of
+/// every surrounding guard and run it unconditionally, regardless of how
+/// deeply the Rust-level scope construction is gated. Measured: on an `o[k]`
+/// loop over a two-property plain object (never touching a `.size` key or a
+/// Proxy), this fallback's `_tlv_get_addr` call sat directly in
+/// `js_object_get_field_by_name`'s prologue. Keeping this arm opaque to the
+/// inliner is what lets the FAST (published) arm above stay `#[inline(always)]`
+/// without dragging the raw TLS call along with it at every call site.
+#[inline(never)]
+#[cold]
+#[cfg(not(any(target_os = "android", target_env = "ohos")))]
+fn runtime_handle_stack_cold() -> StackRef {
     RUNTIME_HANDLE_STACK.with(|stack| {
         // SAFETY: the metadata is const-initialized and has no Drop. Its
         // cells remain valid throughout thread teardown. Cell is !Sync, so

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

@@ -22,6 +22,33 @@ fn handle_proto_inherited_field(
     }
 }
 
+/// #2846 Proxy-receiver forwarding for a generic property read. Split out and
+/// `#[inline(never)]` on purpose: `js_proxy_is_proxy` (via `lookup`) and
+/// `js_proxy_get` (via `RuntimeHandleScope::new`) each resolve their own
+/// `thread_local!` (the proxy registry, the transient-handle root stack).
+/// Both accessors are pure address computations from LLVM's point of view —
+/// `readnone`, no observable side effect — so once this code was inlined into
+/// `js_object_get_field_by_name` the optimizer hoisted BOTH out of the
+/// `is_proxy_id_band` guard above them and ran them unconditionally on every
+/// call, proxy receiver or not. Measured on an `o[k]` loop over a two-property
+/// plain object (never a Proxy): two `_tlv_get_addr` calls sitting directly in
+/// `js_object_get_field_by_name`'s prologue, 9.2% of the whole access.
+/// `#[inline(never)]` keeps the optimizer from seeing inside this function at
+/// the call site, so it cannot hoist anything out of it; `is_proxy_id_band`
+/// itself stays inline in the caller since it touches no thread-local.
+#[cold]
+#[inline(never)]
+fn proxy_receiver_get(raw_addr: u64, key: *const crate::StringHeader) -> Option<JSValue> {
+    const POINTER_TAG: u64 = 0x7FFD_0000_0000_0000;
+    let boxed = f64::from_bits(POINTER_TAG | (raw_addr & 0x0000_FFFF_FFFF_FFFF));
+    if crate::proxy::js_proxy_is_proxy(boxed) == 0 {
+        return None;
+    }
+    let key_f64 = f64::from_bits(crate::value::js_nanbox_string(key as i64).to_bits());
+    let v = crate::proxy::js_proxy_get(boxed, key_f64);
+    Some(JSValue::from_bits(v.to_bits()))
+}
+
 #[no_mangle]
 pub extern "C" fn js_object_get_field_by_name(
     obj: *const ObjectHeader,
@@ -105,12 +132,8 @@ pub extern "C" fn js_object_get_field_by_name(
             addr
         };
         if crate::value::addr_class::is_proxy_id_band(raw_addr as usize) && !key.is_null() {
-            const POINTER_TAG: u64 = 0x7FFD_0000_0000_0000;
-            let boxed = f64::from_bits(POINTER_TAG | (raw_addr & 0x0000_FFFF_FFFF_FFFF));
-            if crate::proxy::js_proxy_is_proxy(boxed) != 0 {
-                let key_f64 = f64::from_bits(crate::value::js_nanbox_string(key as i64).to_bits());
-                let v = crate::proxy::js_proxy_get(boxed, key_f64);
-                return JSValue::from_bits(v.to_bits());
+            if let Some(value) = proxy_receiver_get(raw_addr, key) {
+                return value;
             }
         }
     }

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

@@ -70,7 +70,10 @@ pub(crate) unsafe fn try_data_get_bytes(receiver: JSValue, key: &[u8]) -> Option
         {
             return None;
         }
-        let header = crate::value::addr_class::try_read_gc_header(addr)?;
+        // `is_plausible_heap_addr(addr)` was just proven true above; skip
+        // `try_read_gc_header`'s own re-derivation of it (see
+        // `try_read_gc_header_known_plausible`'s doc comment).
+        let header = crate::value::addr_class::try_read_gc_header_known_plausible(addr)?;
         if header.obj_type != crate::gc::GC_TYPE_OBJECT
             || header.gc_flags & crate::gc::GC_FLAG_FORWARDED != 0
             || header._reserved & crate::gc::OBJ_FLAG_TYPED_ARRAY_PROTO != 0

crates/perry-runtime/src/value/addr_class.rs

@@ -238,6 +238,29 @@ pub(crate) unsafe fn try_read_gc_header(addr: usize) -> Option<&'static GcHeader
     if !is_plausible_heap_addr(addr) {
         return None;
     }
+    try_read_gc_header_known_plausible(addr)
+}
+
+/// [`try_read_gc_header`] for a caller that already ran
+/// [`is_plausible_heap_addr`] on this exact `addr` earlier in the same
+/// straight-line scope, with no intervening collection or reassignment of
+/// `addr`. Skips re-deriving that magnitude check.
+///
+/// `native_get::try_data_get_bytes`'s prototype-chain loop already branches
+/// on `is_plausible_heap_addr(addr)` (paired with the arena-generation
+/// classification) one statement above every call site this exists for, so
+/// the plain [`try_read_gc_header`] was re-running the same handle-band /
+/// heap-range compare a second time per step for free. `classify_heap_generation`
+/// runs in between and writes its own cache, which is enough to stop LLVM's
+/// CSE from eliding the duplicate call on its own (its side effect isn't
+/// provably unrelated to `is_plausible_heap_addr`'s inputs from the
+/// optimizer's point of view), so the redundancy was real, not just apparent.
+///
+/// # Safety
+/// As [`try_read_gc_header`], plus: `is_plausible_heap_addr(addr)` must be
+/// `true` for this `addr` already (unchecked here).
+#[inline(always)]
+pub(crate) unsafe fn try_read_gc_header_known_plausible(addr: usize) -> Option<&'static GcHeader> {
     // Small-buffer slab allocations are heap-plausible but carry NO GcHeader —
     // `addr - GC_HEADER_SIZE` is the previous slab entry's data bytes, so a
     // brand probe (Temporal/Date/Map/Set `obj_type` check) would read a

test-files/test_gap_dynamic_key_proxy_receiver.ts

@@ -0,0 +1,60 @@
+// Dynamic-key reads, `o[k]`, through a Proxy receiver.
+//
+// `js_object_get_field_by_name`'s Proxy-forwarding block never satisfies the
+// ordinary-object fast data-get lane above it (a Proxy's boxed encoding is a
+// small registry id, not a real heap pointer), so it was pulled out into its
+// own `#[inline(never)]` helper (`proxy_receiver_get`): inlined in place, the
+// compiler proved the proxy registry's and the transient-handle root stack's
+// `thread_local!` address resolutions were side-effect-free and hoisted BOTH
+// out of the `is_proxy_id_band` guard, so an `o[k]` loop over a plain
+// two-property object — never touching a Proxy — paid two `_tlv_get_addr`
+// calls on every access. This exercises that the extraction is behavior
+// preserving: trapped reads, pass-through reads (no `get` trap), a
+// forward-to-target hop through a nested Proxy, and a Proxy loop running
+// right alongside an ordinary-object loop on the same key.
+const target: any = { a: 1, b: 2 };
+
+const trapped = new Proxy(target, {
+  get(t, prop, receiver) {
+    if (prop === "special") return "trapped!";
+    return Reflect.get(t, prop, receiver);
+  },
+});
+for (const k of ["a", "b", "special", "missing"]) {
+  console.log("trapped", k, String(trapped[k]));
+}
+
+// No `get` trap: falls through to the target's own [[Get]].
+const passthrough = new Proxy(target, {});
+for (const k of ["a", "b", "missing"]) {
+  console.log("passthrough", k, String(passthrough[k]));
+}
+
+// Nested Proxy: a dynamic-key read that recurses through the
+// forward-to-target hop inside the outlined helper.
+const inner = new Proxy(target, {
+  get(t, p) {
+    return (t as any)[p];
+  },
+});
+const outer = new Proxy(inner, {});
+console.log("nested", outer["a"]);
+
+// An ordinary-object loop and a Proxy loop on the same key, back to back —
+// the ordinary loop must not pay for the Proxy path, and the Proxy loop must
+// still resolve correctly through it.
+const plain: any = { k: 1.5, other: 2 };
+let plainTotal = 0;
+for (let i = 0; i < 20; i++) plainTotal += plain["k"];
+console.log("plain total", plainTotal);
+
+let proxyTotal = 0;
+for (let i = 0; i < 20; i++) proxyTotal += Number(trapped["a"]);
+console.log("proxy total", proxyTotal);
+
+// A Proxy over an array, read by dynamic numeric-string key.
+const arrTarget = [10, 20, 30];
+const arrProxy = new Proxy(arrTarget, {});
+for (const k of ["0", "1", "2", "length"]) {
+  console.log("arr proxy", k, String((arrProxy as any)[k]));
+}