perf(runtime): concat a chain of heap strings without transient roots (iso_miss -16%) - #7912
Merged
Merged
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughThe runtime adds current-nursery-block allocation helpers that refuse requests requiring collection. Heap-string concatenation uses these helpers through an unrooted fast path and falls back to the existing rooted path when operands or allocation are unsupported. ChangesNo-collection concatenation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant concat_chain_sized
participant concat_chain_all_heap_strings_no_collect
participant string_storage_alloc_no_collect
participant arena_alloc_gc_no_collect
concat_chain_sized->>concat_chain_all_heap_strings_no_collect: Try heap-string fast path
concat_chain_all_heap_strings_no_collect->>string_storage_alloc_no_collect: Request result storage
string_storage_alloc_no_collect->>arena_alloc_gc_no_collect: Request current-block allocation
arena_alloc_gc_no_collect-->>string_storage_alloc_no_collect: Return storage or refusal
string_storage_alloc_no_collect-->>concat_chain_all_heap_strings_no_collect: Return allocation result
concat_chain_all_heap_strings_no_collect-->>concat_chain_sized: Return string or use rooted fallback
Possibly related PRs
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
proggeramlug
pushed a commit
that referenced
this pull request
Aug 12, 2026
added 8 commits
August 12, 2026 08:10
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
proggeramlug
force-pushed
the
perf/isomiss-concat-no-collect
branch
from
August 12, 2026 06:26
a3075ed to
925f3b5
Compare
proggeramlug
marked this pull request as ready for review
August 12, 2026 06:28
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
iso_missis the worst ratio in the 19-program corpus (2.89× node). A quarterof it was transient-root bookkeeping in
js_string_concat_chain, and none ofthat bookkeeping was doing anything.
The mechanism
gc-handoff/apps/iso_miss.tsis a tree-walking interpreter whose environmentlookup appends a trace string per frame:
That is one
js_string_concat_chain(parts, 4)per frame walked, ~9 M times.concat_chain_sizedroots every part intoRUNTIME_HANDLE_STACKbefore theresult allocation and re-reads every one of them after it, because
string_storage_alloccan collect and a copying minor would move the partsout from under the copy loop.
Darwin has no local-exec TLS, so each
thread_local!access is an_tlv_get_addrcall; with theRefCellborrow and theVecpush that is~10 round trips per 4-part chain. xctrace on
6d9f12e60:iso_missRuntimeHandleScope::root_string_ptrRuntimeHandle::get_raw_const_ptrjs_string_concat_chainitself13.4% of the program was root bookkeeping — more than the concatenation it
was protecting.
The fix: buy the guarantee instead of paying for the roots
arena_cell_alloc's FIRST step istry_alloc_current, a pure bump of theblock that is already open. Everything after it —
gc_check_trigger(), thecross-block scan,
reserve_arena_block— is a collection point or can reachone. So:
With that proof the transient roots are not merely unnecessary, they are
unreachable work.
arena::arena_alloc_gc_no_collect/string::string_storage_alloc_no_collect— allocate, or refuse; never reach the collection point.
js_string_concat_chaingrows a fast arm that admits only chains whose everypart is already a live heap string (those need no
js_jsvalue_to_string, soclassification allocates nothing), sizes them, and allocates through the
no-collect entry. Zero handle operations.
refusal is not an event — nothing has collected — so the operands are still
readable where they were, and the rooted path re-roots and re-reads them
exactly as it always did.
The admission scan runs before the sizing scan and touches nothing but the
partsarray, so a chain with a number in it reaches the rooted path havingpaid n register compares rather than n cold
StringHeaderloads it is about tothrow away.
Results — retired instructions, dev box, best-of-N, exit-checked
The dev host ran at load 30–200 all session (five concurrent agents), so wall
clock cannot resolve this.
/usr/bin/time -lreports retired instructions onApple Silicon; those are load-independent and reproduce to ~0.05% here. Full
19-program corpus, both arms, byte-exact output and exit 0 in every cell.
Both arms built from scratch in their own
CARGO_TARGET_DIRwith an identical-pset, offc4b2c1c8e. ★ Re-measure your own baseline: #7906 landedmid-round and inlines the
RuntimeHandleaccessors — it movediso_miss'sbase 18.456 → 18.325 Gi and took that much off this change's headroom
(−16.4% → −15.8%). Measured, not assumed. The final binaries were rebuilt from
branch HEAD and reproduce 15.423 Gi (0.03% from the measured arm).
interpis 0.9998 —interp.tsis the same program without the trace-stringinstrument, which is exactly the control this change predicts.
★ The corpus sweep caught a +5.5% regression the targeted A/B would have shipped
The first cut measured
iso_miss−16.7% andinterp0.0%, and regressedpipelineby +5.53% — a program whose profile does not containjs_string_concat_chainat all. It was not the concatenation change. To reachthe no-collect primitive, that cut had refactored two functions every
allocation in the program goes through:
arena_alloc_gcinto aconst MAY_COLLECT: boolgeneric, andarena_cell_alloc's first statementinto a call. Both
#[inline]/#[inline(always)], both "should" have beenfree.
PERRY_GC_DIAG=1showed identical GC schedules across the arms(12 copying minors / 6 steps / 6 drains), so it was pure mutator work.
The fix was to stop touching them.
arena_alloc_gcandarena_cell_allocarenow byte-for-byte
main's, and the no-collect entry is written outseparately — its only divergences are two refusals (oversized request; the hot
free-list latch set, which nothing in the tree ever sets), so it can only ever
hand back memory
arena_alloc_gcwould have handed back identically.Coverage, and the first version of it that could not fail
Nine tests: seven in
string::tests::concat_chain_no_collect, two inarena::tests.★ The first cut of the safety test asserted only "a small concat reached no
GC trigger". That is vacuous — a small allocation into a block with room does
not reach the trigger through the collecting
arena_alloceither. Replacingthe no-collect entry's body with
arena_allocleft it green. The tests nowdrive the block to the point where the two entries must diverge:
no_collect_alloc_refuses_a_full_block_instead_of_collecting— fills theopen block through the entry, asserts it eventually refuses, that
gc_trigger_arena_calls()stayed at 0 across the whole fill, and that thecollecting fallback still serves afterwards.
a_full_block_falls_back_to_the_rooted_path_with_the_same_answer— buildsthe operands, fills the block (through the no-collect entry, so nothing
moves), then asserts the chain took the rooted path and produced the same
bytes. This is the arm that used to be reachable only in production.
non-heap-string part falls back; an empty part contributes neither bytes nor
flags (the rooted loop ORs
piece_flagsinside itsblen > 0guard, and adivergence there would be a silent WTF-8 change);
utf16_lensumming andhigh→low surrogate canonicalisation survive the fast path.
Sabotage, run, RED:
arena_alloccanonicalize_surrogate_pairsValidation
cargo test -p perry-runtime --lib— 2169 passed, 0 failedPERRY_GC_PROTECT_FROMSPACE=1 …_DEPTH=200 PERRY_GC_VERIFY_EVACUATION=1,with the instrument shown live (
retired_set=#49, 940 MB protected oniso_miss) rather than assumedchecksum 437840 misses 0test_gap_*suite vsnode 26.5.1— 516 pass / 17 fail /13 node-skip, and every one of the 17 is IDENTICAL on the baseline
(re-run arm-by-arm, not assumed)
cargo fmt --all -- --check,scripts/check_file_size.shRebased onto
05edeac94(after #7907 / #7913 / #7914) and re-verified there:cargo test -p perry-runtime --lib2178 passed, 0 failed. #7914 touchesthe promoted-page index and this change touches the allocation entry point —
unrelated mechanisms, and the rebase was a clean textual merge in
arena/mod.rs(twouselists) with both intact.One unrelated line in the diff:
arena/mod.rs's#[cfg(test)] pub(crate) use page_meta::{..}block is reflowed. That block is byte-identical tomainandis what
cargo fmt --allproduces —mainis currently fmt-dirty there from#7914, and this branch carries the two-line fix only so its own
lintgate canbe green.
Summary by CodeRabbit
Performance
Bug Fixes
Tests