Skip to content

Commit c109b08

Browse files
proggeramlugRalph Küpper
andauthored
perf(runtime): concat a chain of heap strings without transient roots (iso_miss -16%) (#7912)
* perf(runtime): concat a chain of heap strings without transient roots Claude-Session: https://claude.ai/code/session_012B8z92S82sCfqCrVqrFgS2 * perf(runtime): keep the no-collect concat off shared allocator hot paths Claude-Session: https://claude.ai/code/session_012B8z92S82sCfqCrVqrFgS2 * perf(runtime): inline(always) the no-collect allocation helpers Claude-Session: https://claude.ai/code/session_012B8z92S82sCfqCrVqrFgS2 * test(runtime): pin the no-collect contract with tests that can fail Claude-Session: https://claude.ai/code/session_012B8z92S82sCfqCrVqrFgS2 * style: cargo fmt Claude-Session: https://claude.ai/code/session_012B8z92S82sCfqCrVqrFgS2 * docs(changelog): fragment for #7912 Claude-Session: https://claude.ai/code/session_012B8z92S82sCfqCrVqrFgS2 * docs(runtime): restore the concat_chain_sized doc comment and use the PR number Claude-Session: https://claude.ai/code/session_012B8z92S82sCfqCrVqrFgS2 * style: rustfmt a use block left unformatted by #7914 Two-line reflow of the `#[cfg(test)] pub(crate) use page_meta::{..}` list. It is byte-identical to origin/main and is what `cargo fmt --all` produces — carried here only so this branch's `lint` gate can be green. Claude-Session: https://claude.ai/code/session_012B8z92S82sCfqCrVqrFgS2 --------- Co-authored-by: Ralph Küpper <ralph@skelpo.com>
1 parent 05edeac commit c109b08

8 files changed

Lines changed: 611 additions & 8 deletions

File tree

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

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

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,101 @@ pub fn arena_alloc(size: usize, align: usize) -> *mut u8 {
4747
}
4848
}
4949

50+
/// [`arena_alloc_gc`] with its **collection point removed**: the request is
51+
/// served by bumping the nursery block that is already open, or the call
52+
/// returns null. It never runs `gc_check_trigger()`, never reserves a fresh
53+
/// block and never births into old-gen.
54+
///
55+
/// ★ The value here is not the handful of instructions saved on the slow
56+
/// branch — it is the *guarantee*. A runtime helper holding raw heap pointers
57+
/// it has not rooted can allocate through this and, on a non-null return,
58+
/// KNOW that nothing moved: the only collection point on the arena path is
59+
/// precisely the one this refuses to reach. That turns "root every operand
60+
/// into the transient handle stack, then re-read every one of them
61+
/// afterwards" into "read them once", for the overwhelmingly common case
62+
/// where a 1 MB block has room.
63+
///
64+
/// On null the caller MUST fall back: root its operands, re-issue through
65+
/// [`arena_alloc_gc`], and re-read the operands from their handles. Nothing
66+
/// has collected at that point either — a null is a refusal, not an event —
67+
/// so the operands are still readable where the caller last saw them.
68+
///
69+
/// Deliberately written out rather than sharing a body with `arena_alloc_gc`:
70+
/// that function is `#[inline(always)]` into every allocation site in the
71+
/// program (including user IR, through the bitcode-link path), and it is not
72+
/// worth risking its codegen to save twenty lines here. The two divergences
73+
/// are both refusals — an oversized request and a non-empty hot free list
74+
/// both return null instead of being served — so this can only ever hand back
75+
/// memory `arena_alloc_gc` would have handed back identically.
76+
#[inline(always)]
77+
pub(crate) fn arena_alloc_gc_no_collect(size: usize, align: usize, obj_type: u8) -> *mut u8 {
78+
use crate::gc::{GcHeader, GC_FLAG_ARENA, GC_HEADER_SIZE};
79+
80+
let total = gc_padded_total_size(size, align);
81+
// Old-gen birth walks page lists and can reserve — outside the contract.
82+
if crate::gc::is_large_object_total_size_for_type(total, obj_type) {
83+
return std::ptr::null_mut();
84+
}
85+
// The free-list arm of `arena_alloc_gc` cannot collect either, but nothing
86+
// in the tree ever sets this latch, so serving it here would be untested
87+
// code on a hot path. Refuse and let the caller take the rooted path.
88+
if crate::gc::hot_arena_free_list_nonempty().get() {
89+
return std::ptr::null_mut();
90+
}
91+
92+
let raw = arena_alloc_no_collect(total, align);
93+
if raw.is_null() {
94+
return std::ptr::null_mut();
95+
}
96+
97+
unsafe {
98+
let header = raw as *mut GcHeader;
99+
(*header).obj_type = obj_type;
100+
(*header).gc_flags = GC_FLAG_ARENA | crate::gc::gc_birth_extra_flags();
101+
crate::gc::gc_note_black_birth(header);
102+
(*header)._reserved = 0;
103+
(*header).size = total as u32;
104+
}
105+
106+
unsafe { raw.add(GC_HEADER_SIZE) }
107+
}
108+
109+
/// [`arena_alloc`] minus its collection point: serve the request from the
110+
/// block that is already open, or return null.
111+
///
112+
/// The inline-state sync/resync mirrors `arena_alloc`'s, so a successful
113+
/// allocation is indistinguishable from one taken through it. A refusal
114+
/// leaves every offset exactly where it was, so the caller's fallback through
115+
/// `arena_alloc` behaves as if this had never been called.
116+
#[inline(always)]
117+
fn arena_alloc_no_collect(size: usize, align: usize) -> *mut u8 {
118+
unsafe {
119+
let inline_ptr = crate::arena::hot_inline_state();
120+
let arena_ptr = crate::arena::hot_arena();
121+
if !(*inline_ptr).data.is_null() {
122+
let offset = (*inline_ptr).offset;
123+
let arena = &mut *arena_ptr;
124+
let current = arena.current;
125+
arena.blocks[current].offset = offset;
126+
}
127+
let Some(ptr) = crate::arena::arena_cell_try_alloc_current(arena_ptr, size, align) else {
128+
return std::ptr::null_mut();
129+
};
130+
if !(*inline_ptr).data.is_null() {
131+
let (data, offset, block_size) = {
132+
let arena = &*arena_ptr;
133+
let block = &arena.blocks[arena.current];
134+
(block.data, block.offset, block.size)
135+
};
136+
let inline = &mut *inline_ptr;
137+
inline.data = data;
138+
inline.offset = offset;
139+
inline.size = block_size;
140+
}
141+
ptr
142+
}
143+
}
144+
50145
/// Allocate from the longlived arena (issue #179). Unlike `arena_alloc`,
51146
/// this never touches the inline allocator state — the longlived arena
52147
/// is reserved for explicit-call allocations from cache builders

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

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -774,6 +774,36 @@ impl Arena {
774774
/// # Safety
775775
/// `arena` must be the `UnsafeCell` payload of a live thread-local `Arena` for
776776
/// the current thread.
777+
/// [`arena_cell_alloc`]'s FIRST step, and only that step: serve the request
778+
/// from the block that is already open, or report that it cannot.
779+
///
780+
/// Everything past that step in `arena_cell_alloc` is either the
781+
/// allocation-point collection (`gc_check_trigger`) or a block reservation
782+
/// that can reach one, so a `Some` from here is the runtime's proof that
783+
/// **no collection ran and therefore nothing moved**. That proof is what
784+
/// [`super::arena_alloc_gc_no_collect`] sells to helpers holding raw heap
785+
/// pointers they have not rooted.
786+
///
787+
/// Deliberately a copy of the two lines rather than a refactor of
788+
/// `arena_cell_alloc` to call it: that function is `#[inline]`d into every
789+
/// arena allocation in the program, and interposing a call there moved
790+
/// `pipeline` by +5.5% retired instructions on a measured A/B while the
791+
/// concatenation change it was supposed to be serving moved nothing there.
792+
/// A shared allocation path is not the place to find out whether the
793+
/// inliner agrees with you.
794+
///
795+
/// # Safety
796+
/// Same as [`arena_cell_alloc`].
797+
#[inline(always)]
798+
pub(crate) unsafe fn arena_cell_try_alloc_current(
799+
arena: *mut Arena,
800+
size: usize,
801+
align: usize,
802+
) -> Option<*mut u8> {
803+
let _borrow = ArenaBorrowGuard::new();
804+
(*arena).try_alloc_current(size, align)
805+
}
806+
777807
#[inline]
778808
pub(crate) unsafe fn arena_cell_alloc(arena: *mut Arena, size: usize, align: usize) -> *mut u8 {
779809
// Try current block first, under a borrow that ends with this statement.

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

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -34,11 +34,11 @@ pub(crate) use allocators::{
3434
inactive_survivor_index, with_survivor_arena, with_survivor_arena_mut,
3535
};
3636
pub(crate) use block::{
37-
arena_cell_alloc, drain_block_pool_if_requested, old_gen_in_use_bytes_sub, release_arena_block,
38-
request_block_pool_drain, Arena, ArenaBlock, ArenaBlockRelease, BlockPoolDrainStats,
39-
ACTIVE_SURVIVOR, ARENA, ARENA_TOTAL_BYTES, BLOCK_SIZE, FRESH_GENERAL_BLOCK_MIN_USED_BYTES,
40-
INLINE_STATE, LONGLIVED_ARENA, OLD_ARENA, OLD_GEN_IN_USE_BYTES, SURVIVOR_ARENA_0,
41-
SURVIVOR_ARENA_1,
37+
arena_cell_alloc, arena_cell_try_alloc_current, drain_block_pool_if_requested,
38+
old_gen_in_use_bytes_sub, release_arena_block, request_block_pool_drain, Arena, ArenaBlock,
39+
ArenaBlockRelease, BlockPoolDrainStats, ACTIVE_SURVIVOR, ARENA, ARENA_TOTAL_BYTES, BLOCK_SIZE,
40+
FRESH_GENERAL_BLOCK_MIN_USED_BYTES, INLINE_STATE, LONGLIVED_ARENA, OLD_ARENA,
41+
OLD_GEN_IN_USE_BYTES, SURVIVOR_ARENA_0, SURVIVOR_ARENA_1,
4242
};
4343
/// #7469 hot-TLS plumbing — see `crate::tls_hot`. The `*_hot_addr` half is
4444
/// consumed by `tls_hot::fill`; the `hot_*` half is the cached accessor the
@@ -71,7 +71,8 @@ pub use allocators::{
7171
arena_alloc_longlived, arena_alloc_old, js_arena_alloc,
7272
};
7373
pub(crate) use allocators::{
74-
arena_alloc_gc_old_born_tenured, arena_alloc_gc_old_excluding_pages, arena_alloc_gc_survivor,
74+
arena_alloc_gc_no_collect, arena_alloc_gc_old_born_tenured, arena_alloc_gc_old_excluding_pages,
75+
arena_alloc_gc_survivor,
7576
};
7677

7778
// walk.rs
@@ -144,6 +145,6 @@ pub(crate) use page_meta::{
144145
deferred_old_page_registrations_len, generation_page_base,
145146
old_arena_page_index_clear_for_tests, old_page_meta_for_tests,
146147
old_page_meta_snapshot_calls_for_tests, pending_promoted_page_runs,
147-
reset_old_page_meta_snapshot_calls_for_tests,
148-
DEFERRED_OLD_PAGE_REGISTRATION_CAP, GENERATION_CLASS_SHIFT, GENERATION_PAGE_SIZE,
148+
reset_old_page_meta_snapshot_calls_for_tests, DEFERRED_OLD_PAGE_REGISTRATION_CAP,
149+
GENERATION_CLASS_SHIFT, GENERATION_PAGE_SIZE,
149150
};

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

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1829,3 +1829,79 @@ fn batched_flush_matches_eager_registration() {
18291829
);
18301830
});
18311831
}
1832+
1833+
// ---------------------------------------------------------------------------
1834+
// #7912: `arena_alloc_gc_no_collect` — the "allocate without a collection
1835+
// point" entry point.
1836+
//
1837+
// Its whole value is a guarantee, not a speed: a caller holding raw heap
1838+
// pointers it has not rooted may allocate through it and, on a non-null
1839+
// return, KNOW nothing moved. That is only true if it REFUSES rather than
1840+
// reaching `gc_check_trigger()` when the open block cannot serve the request,
1841+
// so that is what these tests pin.
1842+
//
1843+
// ★ An earlier cut of this coverage asserted only "a small concat reached no
1844+
// trigger", which is vacuous: a small allocation into a block with room does
1845+
// not reach the trigger through `arena_alloc` either. Replacing the entry's
1846+
// body with the COLLECTING `arena_alloc` left that test green. These two
1847+
// drive the block to the point where the two entries must diverge.
1848+
// ---------------------------------------------------------------------------
1849+
1850+
#[test]
1851+
fn no_collect_alloc_refuses_a_full_block_instead_of_collecting() {
1852+
run_with_fresh_arenas(|| {
1853+
reset_gc_trigger_arena_probe();
1854+
// Comfortably under LARGE_OBJECT_THRESHOLD_BYTES, so every request
1855+
// takes the nursery bump path rather than old-gen birth.
1856+
let chunk = LARGE_OBJECT_THRESHOLD_BYTES / 4;
1857+
let bound = 8 * BLOCK_SIZE / chunk;
1858+
let mut served = 0usize;
1859+
let mut refused = false;
1860+
for _ in 0..bound {
1861+
if arena_alloc_gc_no_collect(chunk, 8, GC_TYPE_STRING).is_null() {
1862+
refused = true;
1863+
break;
1864+
}
1865+
served += 1;
1866+
}
1867+
assert!(
1868+
refused,
1869+
"the no-collect entry must REFUSE once the open block is full — it \
1870+
served {served} chunks of {chunk} B without ever declining, which \
1871+
means it reached the block-reservation/collection path it exists \
1872+
to avoid"
1873+
);
1874+
assert!(
1875+
served > 0,
1876+
"test premise: the entry must serve from an open block at all"
1877+
);
1878+
assert_eq!(
1879+
gc_trigger_arena_calls(),
1880+
0,
1881+
"the no-collect entry reached the allocation-point GC trigger; \
1882+
every raw pointer a caller read before it is now potentially \
1883+
from-space"
1884+
);
1885+
// A refusal is a refusal, not damage: the same request through the
1886+
// collecting entry still works, which is the caller's fallback.
1887+
assert!(
1888+
!arena_alloc_gc(chunk, 8, GC_TYPE_STRING).is_null(),
1889+
"the collecting fallback must still serve after a refusal"
1890+
);
1891+
});
1892+
}
1893+
1894+
#[test]
1895+
fn no_collect_alloc_refuses_an_oversized_request() {
1896+
run_with_fresh_arenas(|| {
1897+
reset_gc_trigger_arena_probe();
1898+
// Old-gen birth walks page lists and can reserve, so it is outside the
1899+
// contract even though it is not itself `gc_check_trigger`.
1900+
assert!(
1901+
arena_alloc_gc_no_collect(LARGE_OBJECT_THRESHOLD_BYTES * 2, 8, GC_TYPE_STRING)
1902+
.is_null(),
1903+
"a large-object request must be refused, not born tenured"
1904+
);
1905+
assert_eq!(gc_trigger_arena_calls(), 0);
1906+
});
1907+
}

0 commit comments

Comments
 (0)