perf(runtime): an inline address window in front of the registry probes — cc --help −5.16% instructions, −6.30% cycles

changelog.d/registry-probe-address-window.md

@@ -0,0 +1,119 @@
+### Performance
+
+- **The two hottest side-table probes answer "no" from an inlined address
+  compare instead of an out-of-line registry lookup, and `is_closure_ptr` tests
+  the tag before the arena.**
+
+  `is_registered_buffer` and `lookup_typed_array_kind` are asked "is this
+  pointer special?" from ~239 and ~200 generic call sites — every property
+  get/set, every prototype walk, every element read, every `[[HasProperty]]`.
+  #9176 gave both a monotone "has anything ever been registered?" latch, which
+  makes the answer free for a program that never allocates a `Buffer` or a typed
+  array. `claude-code` allocates both, so the latch is armed and stops
+  discriminating.
+
+  How badly it stops discriminating was the finding. Measured with uprobes and
+  uretprobes on a symbolized `claude --help`, exact counts from one run — not
+  inferred from the profile:
+
+  | probe | registrations | calls | answered "yes" |
+  |---|---|---|---|
+  | `is_registered_buffer_slow` | 10 | 4,650,058 | **4** |
+  | `lookup_registered_typed_array_kind` | 42 | 3,566,956 | **0** |
+
+  1.16 million probes per "yes" for buffers, and not one "yes" in three and a
+  half million for typed arrays — every one of them going out of line to a
+  thread-local resolution, a `RefCell` borrow and a hash. The typed-array probe
+  additionally consults a direct-mapped negative cache whose cold miss *writes
+  back*, dirtying a shared cache line to record an answer nothing asked twice.
+
+  `RegistryAddrWindow` is the same monotone idea applied to the address rather
+  than to the fact of registration: a process-global `[lo, hi]` that every
+  registration widens *before* it publishes, checked inline at the call site. An
+  address outside it cannot be in any table the window covers, so rejecting is
+  sound; accepting falls through to the exact lookup that was already there. It
+  is strictly stronger than a latch — an unregistered process has the empty
+  window `[usize::MAX, 0]`, which contains nothing.
+
+  It removes 98.0% of the buffer probe's calls and 97.2% of the typed-array
+  probe's (4,650,058 → 92,965 and 3,566,956 → 100,926, measured the same way).
+
+  It is deliberately **not** a `GcHeader` tag test. A registered typed array is
+  not required to have a readable `ptr - GC_HEADER_SIZE` (see the `mprotect`ed
+  guard-page fixture in `promise::combinators`), and `native_arena`'s
+  `native_memory_copy_rejects_buffer_registry_forged_to_old_non_buffer` pins
+  that a registry entry may legitimately disagree with the header type. The
+  window never dereferences the candidate, so neither case can be misclassified.
+
+  `buffer::header` already had this filter as the thread-local
+  `BUFFER_ADDR_RANGE` — but *behind* the call, where it still paid the call, the
+  prologue, two thread-local resolutions and a tail call into `is_shared_sab`
+  for every rejection. The window is the same test hoisted in front of the call
+  and widened to cover the external and `SharedArrayBuffer` registries too, so
+  those routes keep their existing behaviour.
+
+  `is_closure_ptr` consults no registry at all; its cost was ordering. It is a
+  conjunction of a handle-band check, a heap-floor check, an alignment check,
+  arena ownership plus a GC-header read, and an exact `CLOSURE_MAGIC` tag — and
+  the tag ran last. Measured over 2,240,934 calls, the tag alone partitions them
+  231,704 / 2,009,230, which is bit for bit the partition the whole function
+  produces: in that run it decided every answer, while `classify_heap_generation`
+  and the header read ran on 100% of calls to change none of them. The tag now
+  runs first. Arena ownership still runs, below, where it does its actual job of
+  refusing a coincidental "CLOS" left in recycled arena storage — pinned by
+  `managed_error_with_closure_magic_in_padding_is_not_a_closure`, which writes
+  the magic into an `ErrorHeader`'s padding and demands `false`.
+
+  Measured on `claude-code` (`cli_2.1.112.js`, `--help`), both arms built from
+  `b3f14e9cde` in one session on the same host, `PERRY_DEBUG_SYMBOLS=1`,
+  11 interleaved reps:
+
+  | | instructions (min / median) | cycles (min / median) |
+  |---|---|---|
+  | before | 7,160,271,524 / 7,163,283,415 | 3,446,625,309 / 3,479,536,202 |
+  | after | 6,791,077,579 / 6,793,829,953 | 3,229,414,851 / 3,267,521,023 |
+  | | **−5.16% / −5.16%** | **−6.30% / −6.09%** |
+
+  Cycles fall by *more* than instructions, and IPC rises (2.077 → 2.102), so
+  none of this is work that was riding free in superscalar slack — the check
+  that a previous change in this campaign failed, having removed 24% of its
+  instructions to move cycles by +1%. The probe family's share of a symbolized
+  profile goes 6.86% → 2.65%: `is_registered_buffer_slow` 2.33% → 0.21%,
+  `lookup_registered_typed_array_kind` 1.30% → 0.11%, `is_closure_ptr`
+  1.23% → 0.42%.
+
+  Output stays byte-identical to `node cli_2.1.112.js --help` (9,175 bytes,
+  rc=0) on both arms; every number above is gated on that. Re-measuring the
+  answer distribution on the shipped binary confirms the window kept all four
+  of the run's genuine "yes" answers while removing 98.03% of the calls.
+
+  The same shape still fits four more probes that this change does not touch,
+  now the largest remaining members of the family: `is_registered_symbol_slow`
+  (0.60%), `is_registered_class_prototype_object` (0.47%), `is_registered_box_ptr`
+  (0.33%) and `is_uint8array_buffer_slow` (0.21%).
+
+### Fixed
+
+- **`RegistryAddrWindow::admit` cannot drop a registration.** It is two
+  unconditional `AcqRel` `fetch_min`/`fetch_max` calls, with no "already
+  covered?" pre-check, because two earlier drafts of that one function were
+  wrong in two different ways and a dropped registration here is a misclassified
+  pointer, not a slow path.
+
+  A `load` then `store` is a read-modify-write with a hole in it: two threads
+  registering at once both read the old bound and the *narrower* of the two
+  stores can land last, evicting the other thread's live registration from the
+  window. A `Relaxed` "skip if already covered" pre-check is the same bug moved
+  up into the memory model: a thread that skips the RMW because it *observed*
+  another thread's widening performs no acquire, so that widening never joins
+  its happens-before graph, and a reader synchronising with only this thread's
+  subsequent publish is not guaranteed to see the bound that covers the address.
+  Registration runs 52 times across a 6.9-billion-instruction `claude --help`,
+  so neither fast path bought anything measurable.
+
+  Both probes now re-derive every window rejection from the authoritative tables
+  under `debug_assertions`. The window is sound only if *every* route into the
+  guarded tables admits first; an enumeration of those routes is a snapshot a
+  later commit can invalidate in silence, so the enumeration is machine-checked
+  instead — a registration route added without `admit` panics in the first test
+  that exercises it. Compiled out entirely in release.

crates/perry-runtime/src/buffer/header.rs

@@ -173,7 +173,7 @@ crate::perry_thread_local! {
         RefCell::new(new_ptr_hash_map());
 }
 
-use crate::registry_latch::RegistryLatch;
+use crate::registry_latch::{RegistryAddrWindow, RegistryLatch};
 
 /// Monotone "at least one `Buffer`-shaped allocation exists" latch.
 ///
@@ -191,6 +191,55 @@ use crate::registry_latch::RegistryLatch;
 /// load rather than one per registry — hence [`note_buffer_like_registered`],
 /// which `shared_sab::alloc_shared_sab` calls before publishing a backing.
 static BUFFER_LIKE_EVER_REGISTERED: RegistryLatch = RegistryLatch::new();
+
+/// Smallest and largest address ever registered as buffer-like, process-wide.
+///
+/// The latch above answers "has ANY buffer ever been registered?", which
+/// `claude-code --help` arms with one of its **10** buffer allocations and then
+/// consults 4,650,058 times — every one of them going out of line to a
+/// thread-local resolution, a `RefCell` borrow and a hash, to answer "no"
+/// 4,650,054 times out of 4,650,058 (uretprobe count, one run). This window
+/// answers the same question about the *address*, from two adjacent static
+/// loads that inline into all ~239 call sites, and removes 98.0% of those
+/// calls — 4,650,058 down to 92,965, with all four genuine "yes" answers
+/// preserved.
+///
+/// It covers every table `is_registered_buffer_slow` consults:
+///   * `BUFFER_REGISTRY` — only `register_buffer` inserts, and it admits first;
+///   * `EXTERNAL_BUFFER_REGISTRY` — both writers (`js_buffer_register_external`
+///     and `js_buffer_mark_as_crypto_key_external`) route through
+///     `register_buffer` with the same address first;
+///   * `shared_sab`'s process-global SAB registry — `alloc_shared_sab` calls
+///     [`note_buffer_like_registered`] with the backing address before it
+///     publishes.
+///
+/// Rejecting an address outside the window is therefore sound; see
+/// [`RegistryAddrWindow`] for the ordering rule that makes it so.
+static BUFFER_LIKE_ADDR_WINDOW: RegistryAddrWindow = RegistryAddrWindow::new();
+
+#[cfg(test)]
+thread_local! {
+/// Test-only count of `is_registered_buffer` calls that got past the address
+/// window and reached the registries. The window is a fast path, and a fast
+/// path nobody can prove ran is not a fast path (same contract as
+/// `typedarray::TEST_TA_REGISTRY_PROBES`, #7765).
+///
+/// Per THREAD, not per process, exactly like `TEST_TA_REGISTRY_PROBES`: the
+/// registry it guards is thread-local and `cargo test` gives each case its own
+/// thread inside one process.
+    static TEST_BUFFER_REGISTRY_PROBES: std::cell::Cell<u64> = const { std::cell::Cell::new(0) };
+}
+
+#[cfg(test)]
+pub(crate) fn test_buffer_registry_probe_count() -> u64 {
+    TEST_BUFFER_REGISTRY_PROBES.with(|c| c.get())
+}
+
+#[cfg(test)]
+pub(crate) fn test_buffer_addr_window_bounds() -> Option<(usize, usize)> {
+    BUFFER_LIKE_ADDR_WINDOW.bounds_for_tests()
+}
+
 /// Avoid a thread-local map probe in `buffer_data{,_mut}` until the first
 /// foreign-backed buffer is created. The latch is deliberately monotone;
 /// these accessors are among the hottest paths in the runtime.
@@ -202,7 +251,11 @@ static FOREIGN_BACKING_EVER_REGISTERED: RegistryLatch = RegistryLatch::new();
 /// reports as buffers without them ever entering `BUFFER_REGISTRY`, so it must
 /// arm the same latch — and, per the [`crate::registry_latch`] rule, must do so
 /// *before* the backing becomes reachable.
-pub(crate) fn note_buffer_like_registered() {
+pub(crate) fn note_buffer_like_registered(addr: usize) {
+    // Widen before arming, and arm before the caller publishes: the probe
+    // checks the latch and then the window, so both must already cover this
+    // address by the time it becomes findable.
+    BUFFER_LIKE_ADDR_WINDOW.admit(addr);
     BUFFER_LIKE_EVER_REGISTERED.arm();
 }
 
@@ -304,8 +357,9 @@ pub fn register_buffer(ptr: *const BufferHeader) {
     // Arm BEFORE the insert: an arm placed afterwards leaves a window in which
     // this buffer is in the registry while `is_registered_buffer` still takes
     // the idle fast path and denies it. See `crate::registry_latch`.
-    BUFFER_LIKE_EVER_REGISTERED.arm();
     let addr = ptr as usize;
+    BUFFER_LIKE_ADDR_WINDOW.admit(addr);
+    BUFFER_LIKE_EVER_REGISTERED.arm();
     BUFFER_ADDR_RANGE.with(|r| {
         let (lo, hi) = r.get();
         r.set((lo.min(addr), hi.max(addr)));
@@ -337,6 +391,37 @@ pub fn is_registered_buffer(addr: usize) -> bool {
     if BUFFER_LIKE_EVER_REGISTERED.is_idle() {
         return false;
     }
+    // An address outside the registered window cannot be in any of the three
+    // tables the slow path consults, so reject it here — inline, without the
+    // call, the thread-local resolution, the `RefCell` borrow or the hash.
+    // Every writer widens the window before it publishes, which is what makes
+    // rejecting sound; see `BUFFER_LIKE_ADDR_WINDOW`.
+    if !BUFFER_LIKE_ADDR_WINDOW.may_contain(addr) {
+        // Machine-check the completeness of the writer set instead of trusting
+        // an enumeration of it. The window is only sound if EVERY route into
+        // the three tables below calls `admit` first; an enumeration of those
+        // routes is a snapshot that a later commit can invalidate silently, and
+        // the failure it would cause is a misclassified pointer, not a slow
+        // path. In debug builds every rejection is therefore re-derived from
+        // the authoritative tables, which turns "someone added a registration
+        // route without admitting" into a panic in the first test that
+        // exercises that route. Compiled out entirely in release.
+        #[cfg(debug_assertions)]
+        {
+            assert!(
+                !is_registered_buffer_slow(addr),
+                "BUFFER_LIKE_ADDR_WINDOW rejected {addr:#x}, but it IS a \
+                 registered buffer. Some registration route reached \
+                 BUFFER_REGISTRY, the external-buffer registry or the \
+                 shared-SAB registry without calling \
+                 `BUFFER_LIKE_ADDR_WINDOW.admit()` (via `register_buffer` or \
+                 `note_buffer_like_registered`) first."
+            );
+        }
+        return false;
+    }
+    #[cfg(test)]
+    TEST_BUFFER_REGISTRY_PROBES.with(|c| c.set(c.get().wrapping_add(1)));
     is_registered_buffer_slow(addr)
 }
 

crates/perry-runtime/src/buffer/mod.rs

@@ -63,7 +63,10 @@ pub(crate) use header::{
     finalize_collected_dead_buffer, is_foreign_backed_buffer,
 };
 #[cfg(test)]
-pub(crate) use header::{test_data_view_registry_len, test_shared_array_buffer_registry_len};
+pub(crate) use header::{
+    test_buffer_addr_window_bounds, test_buffer_registry_probe_count, test_data_view_registry_len,
+    test_shared_array_buffer_registry_len,
+};
 
 // ---- Re-exports: ArrayBuffer detach / transfer (ES2024) ----
 // `detach_array_buffer` dereferences the raw address it is given, so it stays

crates/perry-runtime/src/closure/dynamic_props.rs

@@ -505,6 +505,27 @@ pub fn is_closure_ptr(ptr: usize) -> bool {
     if !ptr.is_multiple_of(std::mem::align_of::<ClosureHeader>()) {
         return false;
     }
+    // Read the type tag BEFORE consulting the arena. The answer is the same
+    // conjunction either way — arena ownership AND the exact magic — but this
+    // is the selective, cheap term and it used to run last.
+    //
+    // Measured on `claude-code --help` (uretprobe + a tag read at the uprobe,
+    // 2,240,934 calls): the tag test partitions the calls 231,704 true /
+    // 2,009,230 false, which is bit for bit the partition the whole function
+    // produces. In that entire run it alone decided every answer, while
+    // `classify_heap_generation` and the GC-header read ran on 100% of calls to
+    // change none of them.
+    //
+    // The load is safe exactly where it is now, because the three checks above
+    // are the only guard it has ever had: the `Unknown` arm below performed
+    // this same read with nothing else in front of it, and `Unknown` means "in
+    // no arena this process knows about" — the LEAST known case, not the most.
+    // Arena ownership was never what made the load safe; it is what
+    // disambiguates a coincidental "CLOS", which is why it stays below.
+    let type_tag = unsafe { *((ptr as *const u8).add(CLOSURE_TYPE_TAG_OFFSET) as *const u32) };
+    if type_tag != CLOSURE_MAGIC {
+        return false;
+    }
     // Arena ownership gives us an authoritative discriminator. Do not let a
     // coincidental CLOSURE_MAGIC in another managed cell's payload win: in
     // particular, ErrorHeader has padding at the closure tag offset and an
@@ -523,10 +544,7 @@ pub fn is_closure_ptr(ptr: usize) -> bool {
             return false;
         }
     }
-    unsafe {
-        let type_tag = *((ptr as *const u8).add(CLOSURE_TYPE_TAG_OFFSET) as *const u32);
-        type_tag == CLOSURE_MAGIC
-    }
+    true
 }
 
 /// C-ABI predicate: returns 1 when `value_bits` (a NaN-boxed JSValue passed as