Perf #857: promote append-only string locals to a StringBuilder slot#870
Merged
Conversation
In compiled mode, a string local built by repeated `s = s + str` lowered to `String.Concat(string,string)` per iteration, copying the whole accumulator each time — O(n²). The strings benchmark was the largest remaining gap to Node (~149× warm @10k; concat-only 337×, growing unbounded vs Node's O(n) cons-strings). Promote a provably non-escaping `string` local with a string-literal initializer, used ONLY via `s = s + str`/`s += str` (statement position), `s.length`, and `s.charCodeAt(i)`, to a concrete StringBuilder slot: append → amortized-O(1) Append, length → get_Length, charCodeAt → the [int] indexer (both UTF-16 code units, identical to JS), out-of-range charCodeAt → NaN. No materialization for these uses. Turns O(n²) into O(n): bundled strings@10k 17.3ms → 0.265ms (65×), now ~2.3× Node. IL verifies. New StringAccumulatorPromotionAnalyzer enforces the conservative escape rule (any return, arg-pass, index, other method/property, comparison, reassignment, non-string append, capture, or an append used as a value disqualifies). Append must be in statement position because `s = s + E` evaluates to the new string, which a StringBuilder slot cannot produce without an O(n) ToString — as a statement the result is discarded. Candidacy is keyed PER FUNCTION SCOPE, not whole-program-per-lexeme like ArrayLocalPromotionAnalyzer: a clean `s` in one function must not be poisoned by an unrelated escaping `s` in another module (e.g. perf_hooks's `const s`) in a bundle. Cross-scope references are captures, caught by the IsVariableCaptured guard. (ArrayLocalPromotionAnalyzer has the same latent bug, dodged by uncommon names — a follow-up should port this per-scope keying there.) Phase 2 (materialize-on-escape for `return s`/pass/index via a cached companion slot) is deferred. Standalone-DLL constraint preserved (StringBuilder is pure BCL). Use-site fast paths key off the slot's CLR type, so they never misfire for a captured/object local. Plan: docs/plans/issue-857-string-accumulator-stringbuilder.md Green on dotnet test (16 new StringAccumulatorPromotionTests, both modes) except the two pre-existing stale/flaky Test262 baselines.
This was referenced Jun 21, 2026
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.
Part of #856 (perf epic). Closes the largest remaining compiled-vs-Node gap: strings.
Problem
A string local built by repeated
s = s + strlowered toString.Concat(string,string)per iteration, copying the whole accumulator each time — O(n²). Re-measured warm (content-forced), this was the dominant remaining gap to Node:Scaling (5k/10k/20k/40k = 6.5/17.8/74/351 ms) confirmed O(n²) vs Node's O(n) cons-strings — the gap grows unbounded (1340× @40k).
StringConcatOptimizeronly flattens intra-expression chains; loop accumulation was unhandled.Fix
Promote a provably non-escaping
stringlocal with a string-literal initializer, used only vias = s + str/s += str(statement position),s.length, ands.charCodeAt(i), to a concreteStringBuilderslot:Append.length→get_Length,charCodeAt(i)→ the[int]indexer (both UTF-16 code units = JS semantics), OOB →NaNO(n²) → O(n): bundled
strings.ts@10k 17.3 ms → 0.265 ms (65×), now ~2.3× Node. IL verifies.Design notes
StringAccumulatorPromotionAnalyzerenforces the conservative escape rule (any return/arg-pass/index/other-method/comparison/reassignment/non-string-append/capture, or an append used as a value, disqualifies).s = s + Eevaluates to the new string, which a StringBuilder slot can't produce without an O(n)ToString; as a statement the result is discarded.ArrayLocalPromotionAnalyzer— a cleansmust not be poisoned by an unrelated escapingsin another bundled module (e.g.perf_hooks'sconst s = findMark(...)). Cross-scope references are captures, caught byIsVariableCaptured.ArrayLocalPromotionAnalyzerhas the same latent bug (dodged by uncommon names) — a follow-up should port this per-scope keying there.return s/pass/index via a cached companion slot) is deferred. Plan:docs/plans/issue-857-string-accumulator-stringbuilder.md.Tests
StringAccumulatorPromotionTests.cs— 16 cases (both modes): positive append/length/charCodeAt, OOB→NaN, the per-scope name-collision case, and fallback-correctness for return/reassign/capture/non-string-append.dotnet test: 13967 passed, 0 real regressions. The only failures are pre-existing flaky network tests (pass in isolation) and the two documented stale Test262 baselines (drift isArray.isArray/proxy, present in interpreter mode too → unrelated to this compile-only change).SharpTS.TypeScriptConformanceis type-checker-only and unaffected by this IL-emitter change.