perf(strings): per-site concat cache for "literal" + proven-small value — bench_object_property beats node - #9514
Merged
Conversation
bench_object_property was the last suite row behind node after the concat memo (PerryTS#9373) and its governor (PerryTS#9397): 14 vs 12 ms. The memo made the `obj["field_" + j]` key allocation-free, but a memo hit is still a call into an ~850-instruction function (tag test, fract, range test, itoa, ASCII scan, hash, byte compare, governor bookkeeping); seven single-cause experiments on that function were all flat, and the standing verdict was that a revisit means a per-site redesign. This is it. Every `+` site whose left operand is a string literal and whose right operand is PROVEN to stay inside 0..=255 gets a private `[32 x i64] zeroinitializer` table. Slot k is either 0 or the NaN-boxed string `prefix + String(k)` — by construction, since the prefix cannot vary at the site and only the runtime fill arm writes the table — so a filled slot needs no verification. The emitted hot path is two ordered fcmps (every NaN-box fails them), fptosi/sitofp integrality (folds away for an i32 counter), one load and a non-zero test. The fill arm (`js_string_concat_site_value`) answers exactly what `js_string_concat_value_box` does, fills the slot and registers it through `js_gc_register_global_root`, the funnel string literals already use, so evacuation rewrites it; an SSO result is cached by value with no root. A value outside the table takes the original fused call directly. Admission is measured, not assumed: a gate on a value that sweeps past the table costs 1-2 ns per call (bench_gc_pressure's `"item_" + i` to 500k lost ~1 ms at min with an unconditional lane), a hit saves ~19 ns (210k calls, 4 ms). So the lane needs a proven bound — a loop counter's induction interval, an integer constant (literal, module constant, never-written `const`; `-`/`+`/`*` of those), or `x % C` with small C — exposed from the `loop_bounded_i32` collector as `LoopInductionFacts`. gc_pressure now emits no table and is byte-identical to the lane-off build. Mac mini, interleaved with node in one window, min/median ms: bench_object_property node 12/13 lane off 14/15 lane on 10/11 bench_gc_pressure node 12/13 lane off 12/13 lane on 12/13 bench_string_heavy / 08_string_concat unchanged (41/41, 5/5). Proof: runtime lifecycle tests (fill-once + one root, non-slot values leave the table alone, -0 is slot 0, SSO cached by value with no root, a copied minor rewrites a filled slot — sabotage-run: removing the registration fails the named assertion); perry integration tests (lane fires, node-exact plain and under forced verified evacuation across every slot-rule edge, admission pin with exact table count, kill switch PERRY_CONCAT_SITE_CACHE=0 restores the plain helper); gc store-site gate passes; string/concat suites green.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (11)
Included review availability: Your plan provides up to 8 included reviews per hour; 2 remain after this review. 📝 WalkthroughWalkthroughThe compiler now emits per-site caches for proven-small literal-prefix string concatenations. New loop facts support admission analysis. The runtime stores cached boxed strings with GC handling. Runtime and integration tests cover reuse, numeric boundaries, evacuation, and the disable switch. ChangesPer-site string concatenation cache
Estimated code review effort: 4 (Complex) | ~45 minutes Merge Risk: 🔵 Low · up to This PR adds bounded caching for eligible string concatenations and roots cached heap strings so they continue to work across garbage-collection movement. It is mergeable with owner awareness of a low-probability runtime risk around concurrent or asynchronous collection during cache publication; the change otherwise has no actionable merge-blocking risk in the supplied evidence. Sequence Diagram(s)sequenceDiagram
participant TypeScript
participant perry_codegen
participant concat_site_cache
participant perry_runtime
participant GarbageCollector
TypeScript->>perry_codegen: compile literal-prefix concatenation
perry_codegen->>concat_site_cache: analyze operand and emit site table
concat_site_cache->>perry_runtime: call js_string_concat_site_value on cache miss
perry_runtime->>GarbageCollector: register heap-string cache slot
perry_runtime-->>TypeScript: return cached or newly concatenated string
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Description checkExplanation The description is detailed and on-topic. It explains the mechanism, admission rules, runtime and compiler changes, benchmarks, tests, GC behavior, and kill switch. It does not use every template heading or provide an explicit checklist, but it contains the required substantive information.
✨ Finishing Touches🧪 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
added a commit
that referenced
this pull request
Sep 2, 2026
proggeramlug
pushed a commit
that referenced
this pull request
Sep 2, 2026
…h after the dispatch split, two new StringHeader payload sites via counted readers (ratchet 359->358), #9514's concat-site symbol in POLL_CAPABLE_RUNTIME, unused import in the capture-stash split
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
1 participant
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.
Per-site concat cache for
"literal" + value— bench_object_property beats nodebench_object_propertywas the last suite row still behind node after the concat memo (#9373) and its governor (#9397): 14 vs 12 ms. The memo made theobj["field_" + j]key allocation-free, but its hit is still a call into an ~850-instruction function (tag test,fract, range test, itoa into a stack buffer, ASCII scan, hash, byte compare, governor bookkeeping). Seven single-cause experiments on that function were all flat; the standing verdict was "a revisit means a per-site redesign, never shaving the current one". This is that redesign.Mechanism
Every
+site whose left operand is a source string literal gets a private[32 x i64] zeroinitializertable. Slotkis either 0 or the NaN-boxed heap stringprefix + String(k)— by construction, because the prefix cannot vary at the site and only the runtime fill arm writes the table — so a filled slot needs no verification. Emitted per site:0.0 <= r < 32as two orderedfcmps (every NaN-box fails; dominates thefptosi);k = fptosi r,sitofp k == r, load slotk, non-zero → the cached handle is the result. For a loop counter already proven i32 the round trip folds away, leaving the gate and one load;js_string_concat_site_value(table, prefix, r)— answers exactly whatjs_string_concat_value_boxdoes, fills the slot when the result is a heap string, and registers the slot throughjs_gc_register_global_root, the funnel module-global string literals already use, so evacuation rewrites it;js_string_concat_value_boxcall this lane replaced — a site whose values mostly miss ("item_" + ito 500k in bench_gc_pressure) pays twofcmps and a branch over the old cost, not an extra call level.An SSO result (≤5 ASCII bytes,
"k" + 4) is cached by value without a registration: it carries no pointer, and leaving it uncached would send every call of such a site through the fill arm forever.concat.rs(memo + governor) is untouched; it still serves the fill arm and every non-admitted site. Kill switch:PERRY_CONCAT_SITE_CACHE=0.Admission — measured, not assumed
Round 1 shipped the lane on every literal-prefix site and bench_gc_pressure lost ~1 ms at min (13 vs 12, median unchanged): its
"item_" + iruns to 500k, and the gate alone (two compares and a branch) is 1-2 ns per call at 501k calls. Splitting the cold arm (round 2) did not move it, which pinned the cost on the gate, not the call level. Ceiling arithmetic from the two benches: a hit saves ~19 ns (object_property: 210k calls, 4 ms), a wasted gate costs 1-2 ns. So a site gets a table only when the right operand is proven inside0..=255: a loop counter with a proven induction interval (theloop_bounded_i32collector's intervals, now exposed through the facts struct), a compile-time integer (anExpr::Integer/Expr::Numberliteral, a module constant or never-writtenconstwith a literal initialiser — the loop proof's own set, now exposed with its intervals asLoopInductionFacts— and-/+/*of those), orx % CwithC <= 256. A sweep to 255 still hits one call in eight (2.4 ns saved vs 1.75 ns spent); an unproven or large-bound operand keeps the plain fused call and the memo, so gc_pressure's site emits no table at all.Numbers (Mac mini, interleaved with node in one window, min / median ms)
11 rounds each, one round = node then each binary; "lane off" is the same compiler with
PERRY_CONCAT_SITE_CACHE=0, so the pair is a single-toolchain A/B. The gc_pressure pair's kept IR is byte-identical (0 diff lines), so anything between its arms is the measurement's own band: one 11-round run showed on 13/13 vs off 12/12, and the same two binaries re-run with the arm order swapped gave 12/13 vs 12/13 — a 1 ms drift-with-order effect on a 12 ms row, not the code. object_property's admitted sites are thej < FIELDSkey writes (warmup and timed loop) and the"field_" + (FIELDS - 1)read; gc_pressure emits no table at all (IR census: 0 tables, 0 fill-arm calls, its 4 fused calls unchanged).Regression guard (lane on vs off, same session):
bench_string_heavy41/41 vs 41/41,08_string_concat5/5 vs 5/6 — its"word" + i + " "is a chain and does not enter this arm.Proof
crates/perry-runtime/src/gc/tests/concat_site.rs: fill-once/identity with exactly one root registered, every non-slot value (fractional, negative, past the table, NaN) leaves the table alone,-0is slot 0, an SSO immediate is cached by value with no root; and a copied minor rewrites a filled slot (an independent shadow root keeps the string moving, so the premise cannot depend on the registration under test). Sabotage-run on the final code: with thejs_gc_register_global_rootcall removed, the move test fails on the assertion that names it ("the slot must follow the moved string — is the filled slot still registered as a global root?") and the fill test fails on its root count; restored, both pass.crates/perry/tests/concat_site_cache.rs: IR pin that the fill arm is CALLED (the lane fired) and a per-site table is read; output node-exact for the bench shape plus every slot-rule edge (a proven bound past the table,+=on a handed-out handle,-0,1.5,-1,NaN,1e21, dynamic number/string/null right operands, the SSO twin), plain and underPERRY_GC_HEAP_LIMIT=8 PERRY_GC_FORCE_EVACUATE=1 PERRY_GC_VERIFY_EVACUATION=1; admission pin (a counter sweeping to 100k gets no table and still reaches the fused arm; a small-bound counter, a constant expression over a module constant and an integer literal get exactly three tables while the unbounded accumulator gets none — the literal case was caught by this pin: HIR spells19asExpr::Integer, which the first evaluator did not match); kill switch restores the plain helper (vacuity-guarded) with identical output.scripts/gc_store_site_inventory.py --gatepasses (the one raw store isGC_STORE_AUDIT(ROOT), registered on the next line).issue_7841_declared_string_self_append,string_append_heap_alias,gc_string_coerce_property_key_rooting_6943,issue_5579_arguments_string_key_dyn_setgreen.Notes for the memo owner
The hook is one
if letinlower_string_concat.rs'sl_is_string && !r_is_stringarm, after thestring_proof_is_declared_onlycheck. The table is emitted throughtyped_parse_rodata, the per-function deferred raw-global sink every lowering context already drains, and the site id comes fromic_site_counter.Summary by CodeRabbit
New Features
Bug Fixes
Tests