diff --git a/changelog.d/7912-concat-chain-no-collect.md b/changelog.d/7912-concat-chain-no-collect.md new file mode 100644 index 0000000000..868dd80bd5 --- /dev/null +++ b/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. diff --git a/crates/perry-runtime/src/arena/allocators.rs b/crates/perry-runtime/src/arena/allocators.rs index 59d62ab8c0..5276d02af8 100644 --- a/crates/perry-runtime/src/arena/allocators.rs +++ b/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 diff --git a/crates/perry-runtime/src/arena/block.rs b/crates/perry-runtime/src/arena/block.rs index 52b188a2c9..6974fc08e3 100644 --- a/crates/perry-runtime/src/arena/block.rs +++ b/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. diff --git a/crates/perry-runtime/src/arena/mod.rs b/crates/perry-runtime/src/arena/mod.rs index 4f31575bed..ca6e934510 100644 --- a/crates/perry-runtime/src/arena/mod.rs +++ b/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, }; diff --git a/crates/perry-runtime/src/arena/tests.rs b/crates/perry-runtime/src/arena/tests.rs index f94c75a18e..06baf721a7 100644 --- a/crates/perry-runtime/src/arena/tests.rs +++ b/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); + }); +} diff --git a/crates/perry-runtime/src/string/concat.rs b/crates/perry-runtime/src/string/concat.rs index b1f056c924..9f02070dc8 100644 --- a/crates/perry-runtime/src/string/concat.rs +++ b/crates/perry-runtime/src/string/concat.rs @@ -497,11 +497,125 @@ pub extern "C" fn js_string_concat_chain(parts: *const f64, n: i32) -> *mut Stri } } +/// #7912 counter: how many chains took the unrooted fast path below. A gate +/// that cannot see its subject run is not a gate — the unit tests assert this +/// moves, so a refactor that quietly stops taking the fast path is red rather +/// than "still correct, just slow again". +#[cfg(test)] +thread_local! { + pub(crate) static CONCAT_CHAIN_NO_COLLECT_HITS: std::cell::Cell = + const { std::cell::Cell::new(0) }; +} + +#[cfg(test)] +#[inline] +fn note_no_collect_hit() { + CONCAT_CHAIN_NO_COLLECT_HITS.with(|c| c.set(c.get() + 1)); +} + +#[cfg(not(test))] +#[inline(always)] +fn note_no_collect_hit() {} + +/// The unrooted arm of [`concat_chain_sized`]: every part is already a live +/// heap string, so classification touches no allocator at all, and the one +/// allocation it needs is taken through the **no-collect** entry point. +/// +/// ★ Why this is sound, stated as the invariant it rests on: +/// +/// > `string_storage_alloc_no_collect` returns `Some` only when the request +/// > was served by bumping the nursery block that was already open. That is +/// > the one path through `arena_cell_alloc` that precedes `gc_check_trigger`, +/// > so a `Some` is a proof that no collection ran and therefore that nothing +/// > moved. +/// +/// With that proof in hand the transient handle stack is not merely +/// unnecessary, it is unreachable work: the pointers read in the sizing loop +/// are still the pointers to copy from. `None` (block full, oversized result) +/// re-enters the rooted path below, which behaves exactly as it always did — +/// and re-reads its operands from `parts`, which is still valid because +/// nothing has collected *yet* either. +/// +/// The rooted path costs ~2N thread-local + `RefCell` round trips per call +/// (N `root_string_ptr` pushes in the classify loop, N +/// `get_raw_const_ptr` reads in the copy loop) plus the scope's own two. +/// On Darwin every one of those is an `_tlv_get_addr` call. On a +/// tree-walking-interpreter workload whose environment lookup appends +/// `seen = seen + "[" + names[i] + "]"` per frame, that bookkeeping measured +/// **12.6%** of total run time — more than the concatenation it was +/// protecting. +#[inline] +fn concat_chain_all_heap_strings_no_collect( + parts: *const f64, + n: usize, +) -> Option<*mut StringHeader> { + let mut piece_ptrs: [*const StringHeader; MAX_PARTS] = [std::ptr::null(); MAX_PARTS]; + let mut piece_lens: [u32; MAX_PARTS] = [0; MAX_PARTS]; + let mut piece_flags: u32 = 0; + let mut total_blen: u32 = 0; + let mut total_u16: u32 = 0; + + // Admission scan FIRST, and it touches nothing but `parts`. A chain with + // a number, an SSO value or an object in it needs `js_jsvalue_to_string`, + // which allocates, so it belongs on the rooted path — and it must reach + // that path having paid only n register compares, not n cold + // `StringHeader` loads it is about to throw away and redo. + // STRING_TAG = 0x7FFF; `is_valid_string_ptr` is a range test, no deref. + for i in 0..n { + let bits = unsafe { *parts.add(i) }.to_bits(); + if bits >> 48 != 0x7FFF { + return None; + } + let ptr = (bits & 0x0000_FFFF_FFFF_FFFF) as *const StringHeader; + if !is_valid_string_ptr(ptr) { + return None; + } + piece_ptrs[i] = ptr; + } + + for i in 0..n { + // Mirrors the rooted loop exactly, including that an EMPTY part + // contributes no flags: `piece_flags |= flags` sits inside its + // `blen > 0` guard there, and a divergence here would be a + // silent WTF-8 behaviour change rather than a slowdown. + let ptr = piece_ptrs[i]; + let blen = unsafe { (*ptr).byte_len }; + if blen > 0 { + piece_lens[i] = blen; + piece_flags |= unsafe { (*ptr).flags }; + total_blen = total_blen.saturating_add(blen); + total_u16 = total_u16.saturating_add(unsafe { (*ptr).utf16_len }); + } + } + + let (ptr, mut cursor) = string_storage_alloc_no_collect(total_blen)?; + note_no_collect_hit(); + + unsafe { + init_string_header(ptr, total_u16, total_blen, total_blen, 0, piece_flags); + for i in 0..n { + let l = piece_lens[i] as usize; + if l == 0 { + continue; + } + ptr::copy_nonoverlapping(string_data(piece_ptrs[i]), cursor, l); + cursor = cursor.add(l); + } + Some(canonicalize_surrogate_pairs(ptr)) + } +} + /// The body of [`js_string_concat_chain`], monomorphised on the scratch-array /// size. `0 < n <= MAX_PARTS` and `!parts.is_null()` are preconditions the /// dispatcher establishes. +/// +/// The `#7912` fast arm above answers first for an all-heap-string chain; +/// everything below is the original rooted path, reached when it declines. fn concat_chain_sized(parts: *const f64, n: usize) -> *mut StringHeader { debug_assert!(n > 0 && n <= MAX_PARTS); + if let Some(result) = concat_chain_all_heap_strings_no_collect::(parts, n) { + return result; + } // Per-part scratch buffer for number formatting. 32 bytes is enough // for any f64 string representation (max ~24 chars). Left UNINITIALISED: // a slot becomes readable only via `MaybeUninit::write`, on exactly the diff --git a/crates/perry-runtime/src/string/mod.rs b/crates/perry-runtime/src/string/mod.rs index 4d5656de0b..9cb50afdae 100644 --- a/crates/perry-runtime/src/string/mod.rs +++ b/crates/perry-runtime/src/string/mod.rs @@ -508,6 +508,28 @@ pub(crate) fn string_storage_alloc(capacity: u32) -> (*mut StringHeader, *mut u8 (ptr, data) } +/// [`string_storage_alloc`] with **no collection point**: `Some` means the +/// bytes came out of the nursery block that was already open, so nothing on +/// the heap moved and any raw string pointer the caller read *before* this +/// call is still valid. `None` means the caller must root its operands and +/// re-issue through [`string_storage_alloc`]. +/// +/// See `arena::arena_alloc_gc_no_collect` for why the guarantee holds. +#[inline(always)] +pub(crate) fn string_storage_alloc_no_collect( + capacity: u32, +) -> Option<(*mut StringHeader, *mut u8)> { + let payload_size = std::mem::size_of::() + capacity as usize; + let raw = crate::arena::arena_alloc_gc_no_collect(payload_size, 8, crate::gc::GC_TYPE_STRING); + if raw.is_null() { + return None; + } + let ptr = raw as *mut StringHeader; + let data = unsafe { raw.add(std::mem::size_of::()) }; + zero_alignment_padding_tail(raw, payload_size); + Some((ptr, data)) +} + #[inline] pub(crate) fn string_storage_alloc_longlived(capacity: u32) -> (*mut StringHeader, *mut u8) { let payload_size = std::mem::size_of::() + capacity as usize; diff --git a/crates/perry-runtime/src/string/tests.rs b/crates/perry-runtime/src/string/tests.rs index eb8d83f1d6..9baba08635 100644 --- a/crates/perry-runtime/src/string/tests.rs +++ b/crates/perry-runtime/src/string/tests.rs @@ -626,3 +626,215 @@ fn string_add_value_picks_the_operator_from_the_bits() { ); } } + +/// #7912: the unrooted `js_string_concat_chain` fast path. +/// +/// The change it covers replaces ~2N transient-handle round trips per chain +/// with a proof: `string_storage_alloc_no_collect` returns `Some` only when +/// the nursery block that was already open could serve the request, and that +/// is the one arena path that precedes `gc_check_trigger`. These tests hold +/// both halves — that the answer is unchanged, and that the premise the +/// answer rests on is actually true at run time. +mod concat_chain_no_collect { + use super::super::concat::CONCAT_CHAIN_NO_COLLECT_HITS; + use super::*; + + fn hits() -> u64 { + CONCAT_CHAIN_NO_COLLECT_HITS.with(|c| c.get()) + } + + fn heap(text: &str) -> f64 { + let p = js_string_from_bytes(text.as_ptr(), text.len() as u32); + f64::from_bits(crate::value::STRING_TAG | (p as u64 & 0x0000_FFFF_FFFF_FFFF)) + } + + fn chain(parts: &[f64]) -> *mut StringHeader { + js_string_concat_chain(parts.as_ptr(), parts.len() as i32) + } + + fn text(s: *mut StringHeader) -> String { + string_as_str(s).to_string() + } + + /// The exact shape a tree-walking interpreter's environment lookup emits: + /// `seen = seen + "[" + names[i] + "]"`, four heap-string parts, run in a + /// loop so the accumulator grows. + #[test] + fn four_heap_string_parts_take_the_unrooted_path_and_answer_correctly() { + let before = hits(); + let mut acc = heap(""); + let mut expected = String::new(); + for name in ["n", "fib", "go", "cat"] { + acc = { + let joined = chain(&[acc, heap("["), heap(name), heap("]")]); + expected = format!("{expected}[{name}]"); + assert_eq!(text(joined), expected); + f64::from_bits(crate::value::STRING_TAG | (joined as u64 & 0x0000_FFFF_FFFF_FFFF)) + }; + } + assert!( + hits() >= before + 4, + "every all-heap-string chain must take the unrooted path: {} -> {}", + before, + hits() + ); + } + + /// The safety premise, asserted rather than assumed: a fast-path chain + /// reaches ZERO allocation-point GC triggers. If it ever reached one, the + /// raw part pointers read in the sizing loop could have been moved out + /// from under the copy loop — which is precisely the bug the transient + /// handles used to prevent. + #[test] + fn the_unrooted_path_reaches_no_collection_point() { + // Warm the block so the very first allocation of the test is not the + // one that installs a fresh one. + let _ = chain(&[heap("warm"), heap("up")]); + crate::arena::reset_gc_trigger_arena_probe(); + let triggers_before = crate::arena::gc_trigger_arena_calls(); + let hits_before = hits(); + + let joined = chain(&[heap("a"), heap("bb"), heap("ccc")]); + assert_eq!(text(joined), "abbccc"); + + assert!( + hits() > hits_before, + "test premise: the chain must have taken the unrooted path" + ); + assert_eq!( + crate::arena::gc_trigger_arena_calls(), + triggers_before, + "the unrooted path must not reach an allocation-point collection" + ); + } + + /// A part that is not a live heap string (SSO, a number, `undefined`) + /// needs `js_jsvalue_to_string`, which allocates — so it must fall back + /// to the rooted path, and still answer correctly. + #[test] + fn non_heap_string_parts_fall_back_to_the_rooted_path() { + let sso = js_string_new_sso(b"ab".as_ptr(), 2); + for (parts, want) in [ + (vec![heap("x"), 7.0, heap("y")], "x7y"), + (vec![heap("x"), sso, heap("y")], "xaby"), + ( + vec![heap("v="), f64::from_bits(crate::value::TAG_UNDEFINED)], + "v=undefined", + ), + (vec![7.0, 8.0], "78"), + ] { + let before = hits(); + assert_eq!(text(chain(&parts)), want); + assert_eq!( + hits(), + before, + "a non-heap-string part must not take the unrooted path: {want}" + ); + } + } + + /// An EMPTY part contributes no bytes AND no flags — the rooted loop ORs + /// `piece_flags` inside its `blen > 0` guard, and a fast path that + /// diverged there would change WTF-8 behaviour silently rather than + /// visibly. + #[test] + fn empty_parts_contribute_neither_bytes_nor_flags() { + let empty = heap(""); + let joined = chain(&[empty, heap("a"), empty, heap("b"), empty]); + assert_eq!(text(joined), "ab"); + unsafe { + assert_eq!((*joined).byte_len, 2); + assert_eq!((*joined).utf16_len, 2); + assert_eq!((*joined).flags & STRING_FLAG_HAS_LONE_SURROGATES, 0); + } + } + + /// Multi-byte and surrogate handling survives the fast path: `utf16_len` + /// is summed from the parts, and an adjacent high→low pair produced by + /// the JOIN is still canonicalised to its astral form. + #[test] + fn utf16_length_and_surrogate_canonicalisation_survive_the_fast_path() { + let joined = chain(&[heap("é"), heap("漢"), heap("ab")]); + assert_eq!(text(joined), "é漢ab"); + unsafe { + assert_eq!((*joined).utf16_len, 4); + assert_eq!((*joined).byte_len, 2 + 3 + 2); + } + + let high = js_string_from_char_code(0xD83D as f64); + let low = js_string_from_char_code(0xDE00 as f64); + let hi_box = + f64::from_bits(crate::value::STRING_TAG | (high as u64 & 0x0000_FFFF_FFFF_FFFF)); + let lo_box = + f64::from_bits(crate::value::STRING_TAG | (low as u64 & 0x0000_FFFF_FFFF_FFFF)); + let merged = chain(&[hi_box, lo_box]); + assert_eq!(text(merged), "\u{1F600}"); + unsafe { + assert_eq!((*merged).utf16_len, 2); + } + } + + /// The REAL fallback: not "a part was a number", but "the open nursery + /// block could not serve the result". Driven on its own thread so filling + /// the block cannot leak into the rest of the suite. + /// + /// This is the arm that used to be reachable only in production. It has to + /// answer identically, because a refusal is not an event — nothing has + /// collected at that point, so the rooted path re-reads its operands from + /// the same `parts` array and gets the same pointers. + #[test] + fn a_full_block_falls_back_to_the_rooted_path_with_the_same_answer() { + std::thread::spawn(|| { + // Fill the open block through the same no-collect entry the concat + // uses, so the very next chain is guaranteed to be refused. + // Build the operands FIRST — `js_string_from_bytes` goes through + // the COLLECTING entry and would install a fresh block, undoing + // the fill. + let parts = [heap("a"), heap("bb"), heap("ccc"), heap("dddd")]; + + // Now fill through the no-collect entry, which by construction + // installs nothing and moves nothing, so `parts` stays valid. + // Coarse-to-fine, because refusing a 4 KB request only proves + // there is less than 4 KB left — and a 4-part chain of 10 bytes + // fits in 40. + let mut filled = false; + for chunk in [crate::gc::LARGE_OBJECT_THRESHOLD_BYTES / 4, 256, 8] { + let bound = 8 * 1024 * 1024 / chunk; + filled = false; + for _ in 0..bound { + if crate::arena::arena_alloc_gc_no_collect(chunk, 8, crate::gc::GC_TYPE_STRING) + .is_null() + { + filled = true; + break; + } + } + assert!(filled, "test premise: {chunk} B fill never refused"); + } + assert!(filled, "test premise: the block must actually be full"); + + let before = hits(); + let joined = chain(&parts); + assert_eq!(text(joined), "abbcccdddd"); + assert_eq!( + hits(), + before, + "with the block full the chain must have taken the ROOTED path" + ); + }) + .join() + .expect("full-block fallback test panicked"); + } + + /// The 4/8/32 scratch-size dispatch all route through the same fast path. + #[test] + fn every_scratch_size_class_takes_the_fast_path() { + for n in [1usize, 2, 4, 5, 8, 9, 16, 32] { + let parts: Vec = (0..n).map(|i| heap(&format!("{i}"))).collect(); + let want: String = (0..n).map(|i| i.to_string()).collect(); + let before = hits(); + assert_eq!(text(chain(&parts)), want, "n={n}"); + assert!(hits() > before, "n={n} must take the unrooted path"); + } + } +}