perf(runtime): concat a chain of heap strings without transient roots (iso_miss -16%)

changelog.d/7912-concat-chain-no-collect.md

@@ -0,0 +1,53 @@
+### `iso_miss` −16% — a chain of heap strings concatenates without transient roots
+
+`js_string_concat_chain` rooted every part into `RUNTIME_HANDLE_STACK` before
+allocating the result and re-read every one of them afterwards, because
+`string_storage_alloc` can collect and a copying minor would move the parts out
+from under the copy loop. Darwin has no local-exec TLS, so each `thread_local!`
+access is an `_tlv_get_addr` **call**; with the `RefCell` borrow and the `Vec`
+push that is ~10 round trips per 4-part chain. On `gc-handoff/apps/iso_miss.ts`
+— a tree-walking interpreter whose environment lookup appends
+`seen = seen + "[" + names[i] + "]"` per frame, ~9 M times — xctrace put
+`RuntimeHandleScope::root_string_ptr` at **8.48%** and
+`RuntimeHandle::get_raw_const_ptr` at **4.91%** of the whole program: more than
+the concatenation they were protecting.
+
+The roots are unnecessary whenever the allocation cannot collect, and the
+runtime can already tell. `arena_cell_alloc`'s first step is
+`try_alloc_current`, a pure bump of the block that is already open; everything
+past it (`gc_check_trigger()`, the cross-block scan, `reserve_arena_block`) is a
+collection point or can reach one. **A successful `try_alloc_current` is
+therefore a proof that nothing moved.**
+
+New `arena::arena_alloc_gc_no_collect` and `string::string_storage_alloc_no_collect`
+allocate or **refuse** — they never reach the collection point.
+`js_string_concat_chain` grows a fast arm that admits only chains whose every
+part is already a live heap string (those need no `js_jsvalue_to_string`, so
+classification allocates nothing) and allocates through it, with zero handle
+operations. On a refusal it falls through to the original rooted path: a
+refusal is not an event, nothing has collected, so the operands are still
+readable where they were. The admission scan runs before the sizing scan and
+touches nothing but the `parts` array, so a mixed chain reaches the rooted path
+having paid n register compares rather than n cold `StringHeader` loads it is
+about to discard.
+
+Retired instructions (`/usr/bin/time -l`, best-of-N, exit-checked; the dev host
+was at load 30–200, where wall clock cannot resolve this): **`iso_miss` 0.836**,
+`asyncpipe` 0.983, and the other 17 corpus programs 0.997–1.001. `interp` is
+0.9998 — the same program without the trace-string instrument, which is the
+control this change predicts.
+
+★ Two things worth carrying forward. **The whole-corpus instruction sweep caught
+a +5.5% `pipeline` regression that the targeted A/B would have shipped**: the
+first cut reached the new primitive by refactoring `arena_alloc_gc` into a
+`const MAY_COLLECT: bool` generic and routing `arena_cell_alloc`'s first
+statement through a call — two functions every allocation in the program goes
+through, both `#[inline]`, both "should" have been free. GC schedules were
+identical across the arms (`PERRY_GC_DIAG=1`: 12 copying minors / 6 steps /
+6 drains), so it was pure mutator work. Both are now byte-for-byte `main`'s and
+the no-collect entry is written out separately. **And the first version of the
+safety test could not fail**: "a small concat reached no GC trigger" is vacuous,
+because a small allocation into a block with room does not reach the trigger
+through the *collecting* allocator either — swapping the entry's body for
+`arena_alloc` left it green. The tests now fill the block until the two entries
+must diverge, and that sabotage turns two of them red.

crates/perry-runtime/src/arena/allocators.rs

@@ -47,6 +47,101 @@ pub fn arena_alloc(size: usize, align: usize) -> *mut u8 {
     }
 }
 
+/// [`arena_alloc_gc`] with its **collection point removed**: the request is
+/// served by bumping the nursery block that is already open, or the call
+/// returns null. It never runs `gc_check_trigger()`, never reserves a fresh
+/// block and never births into old-gen.
+///
+/// ★ The value here is not the handful of instructions saved on the slow
+/// branch — it is the *guarantee*. A runtime helper holding raw heap pointers
+/// it has not rooted can allocate through this and, on a non-null return,
+/// KNOW that nothing moved: the only collection point on the arena path is
+/// precisely the one this refuses to reach. That turns "root every operand
+/// into the transient handle stack, then re-read every one of them
+/// afterwards" into "read them once", for the overwhelmingly common case
+/// where a 1 MB block has room.
+///
+/// On null the caller MUST fall back: root its operands, re-issue through
+/// [`arena_alloc_gc`], and re-read the operands from their handles. Nothing
+/// has collected at that point either — a null is a refusal, not an event —
+/// so the operands are still readable where the caller last saw them.
+///
+/// Deliberately written out rather than sharing a body with `arena_alloc_gc`:
+/// that function is `#[inline(always)]` into every allocation site in the
+/// program (including user IR, through the bitcode-link path), and it is not
+/// worth risking its codegen to save twenty lines here. The two divergences
+/// are both refusals — an oversized request and a non-empty hot free list
+/// both return null instead of being served — so this can only ever hand back
+/// memory `arena_alloc_gc` would have handed back identically.
+#[inline(always)]
+pub(crate) fn arena_alloc_gc_no_collect(size: usize, align: usize, obj_type: u8) -> *mut u8 {
+    use crate::gc::{GcHeader, GC_FLAG_ARENA, GC_HEADER_SIZE};
+
+    let total = gc_padded_total_size(size, align);
+    // Old-gen birth walks page lists and can reserve — outside the contract.
+    if crate::gc::is_large_object_total_size_for_type(total, obj_type) {
+        return std::ptr::null_mut();
+    }
+    // The free-list arm of `arena_alloc_gc` cannot collect either, but nothing
+    // in the tree ever sets this latch, so serving it here would be untested
+    // code on a hot path. Refuse and let the caller take the rooted path.
+    if crate::gc::hot_arena_free_list_nonempty().get() {
+        return std::ptr::null_mut();
+    }
+
+    let raw = arena_alloc_no_collect(total, align);
+    if raw.is_null() {
+        return std::ptr::null_mut();
+    }
+
+    unsafe {
+        let header = raw as *mut GcHeader;
+        (*header).obj_type = obj_type;
+        (*header).gc_flags = GC_FLAG_ARENA | crate::gc::gc_birth_extra_flags();
+        crate::gc::gc_note_black_birth(header);
+        (*header)._reserved = 0;
+        (*header).size = total as u32;
+    }
+
+    unsafe { raw.add(GC_HEADER_SIZE) }
+}
+
+/// [`arena_alloc`] minus its collection point: serve the request from the
+/// block that is already open, or return null.
+///
+/// The inline-state sync/resync mirrors `arena_alloc`'s, so a successful
+/// allocation is indistinguishable from one taken through it. A refusal
+/// leaves every offset exactly where it was, so the caller's fallback through
+/// `arena_alloc` behaves as if this had never been called.
+#[inline(always)]
+fn arena_alloc_no_collect(size: usize, align: usize) -> *mut u8 {
+    unsafe {
+        let inline_ptr = crate::arena::hot_inline_state();
+        let arena_ptr = crate::arena::hot_arena();
+        if !(*inline_ptr).data.is_null() {
+            let offset = (*inline_ptr).offset;
+            let arena = &mut *arena_ptr;
+            let current = arena.current;
+            arena.blocks[current].offset = offset;
+        }
+        let Some(ptr) = crate::arena::arena_cell_try_alloc_current(arena_ptr, size, align) else {
+            return std::ptr::null_mut();
+        };
+        if !(*inline_ptr).data.is_null() {
+            let (data, offset, block_size) = {
+                let arena = &*arena_ptr;
+                let block = &arena.blocks[arena.current];
+                (block.data, block.offset, block.size)
+            };
+            let inline = &mut *inline_ptr;
+            inline.data = data;
+            inline.offset = offset;
+            inline.size = block_size;
+        }
+        ptr
+    }
+}
+
 /// Allocate from the longlived arena (issue #179). Unlike `arena_alloc`,
 /// this never touches the inline allocator state — the longlived arena
 /// is reserved for explicit-call allocations from cache builders

crates/perry-runtime/src/arena/block.rs

@@ -774,6 +774,36 @@ impl Arena {
 /// # Safety
 /// `arena` must be the `UnsafeCell` payload of a live thread-local `Arena` for
 /// the current thread.
+/// [`arena_cell_alloc`]'s FIRST step, and only that step: serve the request
+/// from the block that is already open, or report that it cannot.
+///
+/// Everything past that step in `arena_cell_alloc` is either the
+/// allocation-point collection (`gc_check_trigger`) or a block reservation
+/// that can reach one, so a `Some` from here is the runtime's proof that
+/// **no collection ran and therefore nothing moved**. That proof is what
+/// [`super::arena_alloc_gc_no_collect`] sells to helpers holding raw heap
+/// pointers they have not rooted.
+///
+/// Deliberately a copy of the two lines rather than a refactor of
+/// `arena_cell_alloc` to call it: that function is `#[inline]`d into every
+/// arena allocation in the program, and interposing a call there moved
+/// `pipeline` by +5.5% retired instructions on a measured A/B while the
+/// concatenation change it was supposed to be serving moved nothing there.
+/// A shared allocation path is not the place to find out whether the
+/// inliner agrees with you.
+///
+/// # Safety
+/// Same as [`arena_cell_alloc`].
+#[inline(always)]
+pub(crate) unsafe fn arena_cell_try_alloc_current(
+    arena: *mut Arena,
+    size: usize,
+    align: usize,
+) -> Option<*mut u8> {
+    let _borrow = ArenaBorrowGuard::new();
+    (*arena).try_alloc_current(size, align)
+}
+
 #[inline]
 pub(crate) unsafe fn arena_cell_alloc(arena: *mut Arena, size: usize, align: usize) -> *mut u8 {
     // Try current block first, under a borrow that ends with this statement.

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

@@ -34,11 +34,11 @@ pub(crate) use allocators::{
     inactive_survivor_index, with_survivor_arena, with_survivor_arena_mut,
 };
 pub(crate) use block::{
-    arena_cell_alloc, drain_block_pool_if_requested, old_gen_in_use_bytes_sub, release_arena_block,
-    request_block_pool_drain, Arena, ArenaBlock, ArenaBlockRelease, BlockPoolDrainStats,
-    ACTIVE_SURVIVOR, ARENA, ARENA_TOTAL_BYTES, BLOCK_SIZE, FRESH_GENERAL_BLOCK_MIN_USED_BYTES,
-    INLINE_STATE, LONGLIVED_ARENA, OLD_ARENA, OLD_GEN_IN_USE_BYTES, SURVIVOR_ARENA_0,
-    SURVIVOR_ARENA_1,
+    arena_cell_alloc, arena_cell_try_alloc_current, drain_block_pool_if_requested,
+    old_gen_in_use_bytes_sub, release_arena_block, request_block_pool_drain, Arena, ArenaBlock,
+    ArenaBlockRelease, BlockPoolDrainStats, ACTIVE_SURVIVOR, ARENA, ARENA_TOTAL_BYTES, BLOCK_SIZE,
+    FRESH_GENERAL_BLOCK_MIN_USED_BYTES, INLINE_STATE, LONGLIVED_ARENA, OLD_ARENA,
+    OLD_GEN_IN_USE_BYTES, SURVIVOR_ARENA_0, SURVIVOR_ARENA_1,
 };
 /// #7469 hot-TLS plumbing — see `crate::tls_hot`. The `*_hot_addr` half is
 /// consumed by `tls_hot::fill`; the `hot_*` half is the cached accessor the
@@ -71,7 +71,8 @@ pub use allocators::{
     arena_alloc_longlived, arena_alloc_old, js_arena_alloc,
 };
 pub(crate) use allocators::{
-    arena_alloc_gc_old_born_tenured, arena_alloc_gc_old_excluding_pages, arena_alloc_gc_survivor,
+    arena_alloc_gc_no_collect, arena_alloc_gc_old_born_tenured, arena_alloc_gc_old_excluding_pages,
+    arena_alloc_gc_survivor,
 };
 
 // walk.rs
@@ -144,6 +145,6 @@ pub(crate) use page_meta::{
     deferred_old_page_registrations_len, generation_page_base,
     old_arena_page_index_clear_for_tests, old_page_meta_for_tests,
     old_page_meta_snapshot_calls_for_tests, pending_promoted_page_runs,
-    reset_old_page_meta_snapshot_calls_for_tests,
-    DEFERRED_OLD_PAGE_REGISTRATION_CAP, GENERATION_CLASS_SHIFT, GENERATION_PAGE_SIZE,
+    reset_old_page_meta_snapshot_calls_for_tests, DEFERRED_OLD_PAGE_REGISTRATION_CAP,
+    GENERATION_CLASS_SHIFT, GENERATION_PAGE_SIZE,
 };

crates/perry-runtime/src/arena/tests.rs

@@ -1829,3 +1829,79 @@ fn batched_flush_matches_eager_registration() {
         );
     });
 }
+
+// ---------------------------------------------------------------------------
+// #7912: `arena_alloc_gc_no_collect` — the "allocate without a collection
+// point" entry point.
+//
+// Its whole value is a guarantee, not a speed: a caller holding raw heap
+// pointers it has not rooted may allocate through it and, on a non-null
+// return, KNOW nothing moved. That is only true if it REFUSES rather than
+// reaching `gc_check_trigger()` when the open block cannot serve the request,
+// so that is what these tests pin.
+//
+// ★ An earlier cut of this coverage asserted only "a small concat reached no
+// trigger", which is vacuous: a small allocation into a block with room does
+// not reach the trigger through `arena_alloc` either. Replacing the entry's
+// body with the COLLECTING `arena_alloc` left that test green. These two
+// drive the block to the point where the two entries must diverge.
+// ---------------------------------------------------------------------------
+
+#[test]
+fn no_collect_alloc_refuses_a_full_block_instead_of_collecting() {
+    run_with_fresh_arenas(|| {
+        reset_gc_trigger_arena_probe();
+        // Comfortably under LARGE_OBJECT_THRESHOLD_BYTES, so every request
+        // takes the nursery bump path rather than old-gen birth.
+        let chunk = LARGE_OBJECT_THRESHOLD_BYTES / 4;
+        let bound = 8 * BLOCK_SIZE / chunk;
+        let mut served = 0usize;
+        let mut refused = false;
+        for _ in 0..bound {
+            if arena_alloc_gc_no_collect(chunk, 8, GC_TYPE_STRING).is_null() {
+                refused = true;
+                break;
+            }
+            served += 1;
+        }
+        assert!(
+            refused,
+            "the no-collect entry must REFUSE once the open block is full — it \
+             served {served} chunks of {chunk} B without ever declining, which \
+             means it reached the block-reservation/collection path it exists \
+             to avoid"
+        );
+        assert!(
+            served > 0,
+            "test premise: the entry must serve from an open block at all"
+        );
+        assert_eq!(
+            gc_trigger_arena_calls(),
+            0,
+            "the no-collect entry reached the allocation-point GC trigger; \
+             every raw pointer a caller read before it is now potentially \
+             from-space"
+        );
+        // A refusal is a refusal, not damage: the same request through the
+        // collecting entry still works, which is the caller's fallback.
+        assert!(
+            !arena_alloc_gc(chunk, 8, GC_TYPE_STRING).is_null(),
+            "the collecting fallback must still serve after a refusal"
+        );
+    });
+}
+
+#[test]
+fn no_collect_alloc_refuses_an_oversized_request() {
+    run_with_fresh_arenas(|| {
+        reset_gc_trigger_arena_probe();
+        // Old-gen birth walks page lists and can reserve, so it is outside the
+        // contract even though it is not itself `gc_check_trigger`.
+        assert!(
+            arena_alloc_gc_no_collect(LARGE_OBJECT_THRESHOLD_BYTES * 2, 8, GC_TYPE_STRING)
+                .is_null(),
+            "a large-object request must be refused, not born tenured"
+        );
+        assert_eq!(gc_trigger_arena_calls(), 0);
+    });
+}