From ac2eaf7168920b88453bc4c333c8c8884767f835 Mon Sep 17 00:00:00 2001 From: Nick Nassiri Date: Sat, 20 Jun 2026 18:42:29 -0700 Subject: [PATCH] Perf #857: promote append-only string locals to a StringBuilder slot MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- Compilation/CompilationContext.cs | 13 ++ Compilation/ILCompiler.cs | 2 + Compilation/ILEmitter.Calls.MethodDispatch.cs | 10 + Compilation/ILEmitter.Expressions.cs | 17 ++ Compilation/ILEmitter.Operators.cs | 14 ++ Compilation/ILEmitter.Properties.cs | 53 +++++ Compilation/ILEmitter.Statements.cs | 17 ++ .../StringAccumulatorPromotionAnalyzer.cs | 182 ++++++++++++++++++ .../StringAccumulatorPromotionTests.cs | 176 +++++++++++++++++ TypeSystem/TypeMap.cs | 19 ++ ...ue-857-string-accumulator-stringbuilder.md | 122 ++++++++++++ 11 files changed, 625 insertions(+) create mode 100644 Compilation/StringAccumulatorPromotionAnalyzer.cs create mode 100644 SharpTS.Tests/SharedTests/StringAccumulatorPromotionTests.cs create mode 100644 docs/plans/issue-857-string-accumulator-stringbuilder.md diff --git a/Compilation/CompilationContext.cs b/Compilation/CompilationContext.cs index 4e2ce55a..f5e1db70 100644 --- a/Compilation/CompilationContext.cs +++ b/Compilation/CompilationContext.cs @@ -270,6 +270,19 @@ public IReadOnlyList TakePendingLoopLabels() return null; } + /// + /// If currently binds to a promoted string-accumulator local + /// (a slot whose CLR type is StringBuilder, declared by the #857 promotion path), returns its + /// ; otherwise null. The slot's CLR type is the single source of truth, so + /// this is automatically scope-correct under shadowing and never misfires for a captured/object local + /// (no other code path declares a user local with a StringBuilder slot). + /// + public LocalBuilder? TryGetPromotedStringAccumulator(string variableName) + { + if (!Locals.TryGetLocal(variableName, out var local)) return null; + return Locals.GetLocalType(variableName) == Types.StringBuilder ? local : null; + } + /// /// Resolves the generated shape struct for a promoted object-literal local by its canonical shape /// key (#862), or null if shapes are not threaded into this context / the key is unknown. Used at the diff --git a/Compilation/ILCompiler.cs b/Compilation/ILCompiler.cs index 2d4bf9f0..f9970430 100644 --- a/Compilation/ILCompiler.cs +++ b/Compilation/ILCompiler.cs @@ -425,6 +425,7 @@ public void Compile(List statements, TypeMap typeMap, DeadCodeInfo? deadCo Phase1_EmitRuntimeTypes(); Phase2_AnalyzeClosures(statements); ArrayLocalPromotionAnalyzer.Analyze(statements, _typeMap, _closures.Analyzer); + StringAccumulatorPromotionAnalyzer.Analyze(statements, _typeMap, _closures.Analyzer); NonEscapingArrowLocalAnalyzer.Analyze(statements, _closures.DirectCallArrowBindings, _closures.Analyzer); ObjectLocalPromotionAnalyzer.Analyze(statements, _typeMap, _closures.Analyzer); Phase3_CreateProgramType(); @@ -955,6 +956,7 @@ public void CompileModules(List modules, ModuleResolver resolver, Phase1_EmitRuntimeTypes(); Phase2_AnalyzeClosures(allStatements); ArrayLocalPromotionAnalyzer.Analyze(allStatements, _typeMap, _closures.Analyzer); + StringAccumulatorPromotionAnalyzer.Analyze(allStatements, _typeMap, _closures.Analyzer); NonEscapingArrowLocalAnalyzer.Analyze(allStatements, _closures.DirectCallArrowBindings, _closures.Analyzer); ObjectLocalPromotionAnalyzer.Analyze(allStatements, _typeMap, _closures.Analyzer); Phase3_CreateProgramType(); diff --git a/Compilation/ILEmitter.Calls.MethodDispatch.cs b/Compilation/ILEmitter.Calls.MethodDispatch.cs index 1000479b..fef9ae5b 100644 --- a/Compilation/ILEmitter.Calls.MethodDispatch.cs +++ b/Compilation/ILEmitter.Calls.MethodDispatch.cs @@ -25,6 +25,16 @@ private void EmitMethodCall(Expr.Get methodGet, List arguments) return; } + // Promoted string-accumulator charCodeAt (#857): read the StringBuilder's UTF-16 code unit + // directly via its [int] indexer (== JS charCodeAt); out-of-range yields NaN (JS semantics). + // Must intercept before the string-method path, which would emit the slot as a string receiver. + if (methodName == "charCodeAt" && !methodGet.Optional && methodGet.Object is Expr.Variable ccVar + && _ctx.TryGetPromotedStringAccumulator(ccVar.Name.Lexeme) is { } ccSb) + { + EmitPromotedStringCharCodeAt(ccSb, arguments); + return; + } + // Try direct dispatch for known class instance methods TypeSystem.TypeInfo? objType = _ctx.TypeMap?.Get(methodGet.Object); if (TryEmitDirectMethodCall(methodGet.Object, objType, methodName, arguments)) diff --git a/Compilation/ILEmitter.Expressions.cs b/Compilation/ILEmitter.Expressions.cs index 6012e8bd..f1f34ff4 100644 --- a/Compilation/ILEmitter.Expressions.cs +++ b/Compilation/ILEmitter.Expressions.cs @@ -318,6 +318,23 @@ protected override void EmitAssign(Expr.Assign a) // rest of the module body and ultimately trips PathStackDepth at the final ret. if (TryEmitCjsAssign(a)) return; + // Promoted string-accumulator append (#857): `s = s + E` where `s` is a StringBuilder slot. + // Emit `sb.Append(E)` instead of evaluating `s + E` (String.Concat) and storing — turning the + // O(n²) accumulation into O(n). The analyzer promotes `s` only when every such append is in + // statement position, so the Append-returned builder left on the stack is the single value + // `Stmt.Expression` pops (the discarded assignment-expression result). + if (a.Value is Expr.Binary { Operator.Type: TokenType.PLUS, Left: Expr.Variable accLeft } accBin + && accLeft.Name.Lexeme == a.Name.Lexeme + && _ctx.TryGetPromotedStringAccumulator(a.Name.Lexeme) is { } accSb) + { + IL.Emit(OpCodes.Ldloc, accSb); + EmitExpression(accBin.Right); + EnsureString(); + IL.Emit(OpCodes.Callvirt, _ctx.Types.StringBuilderAppendString); + SetStackUnknown(); + return; + } + EmitExpression(a.Value); // 0. Per-iteration loop-binding cell (#650): write through the StrongBox so the diff --git a/Compilation/ILEmitter.Operators.cs b/Compilation/ILEmitter.Operators.cs index eb621d10..66f235f4 100644 --- a/Compilation/ILEmitter.Operators.cs +++ b/Compilation/ILEmitter.Operators.cs @@ -439,6 +439,20 @@ protected override void EmitUnary(Expr.Unary u) protected override void EmitCompoundAssign(Expr.CompoundAssign ca) { + // Promoted string-accumulator append (#857): `s += E` where `s` is a StringBuilder slot → + // `sb.Append(E)`. Promotion guarantees statement position, so the Append-returned builder on the + // stack is the value Stmt.Expression pops. See EmitAssign and StringAccumulatorPromotionAnalyzer. + if (ca.Operator.Type == TokenType.PLUS_EQUAL + && _ctx.TryGetPromotedStringAccumulator(ca.Name.Lexeme) is { } accSb) + { + IL.Emit(OpCodes.Ldloc, accSb); + EmitExpression(ca.Value); + EnsureString(); + IL.Emit(OpCodes.Callvirt, _ctx.Types.StringBuilderAppendString); + SetStackUnknown(); + return; + } + var local = _ctx.Locals.GetLocal(ca.Name.Lexeme); FieldBuilder? topLevelField = null; _ctx.TopLevelStaticVars?.TryGetValue(ca.Name.Lexeme, out topLevelField); diff --git a/Compilation/ILEmitter.Properties.cs b/Compilation/ILEmitter.Properties.cs index 632336b4..4db7a8e7 100644 --- a/Compilation/ILEmitter.Properties.cs +++ b/Compilation/ILEmitter.Properties.cs @@ -277,6 +277,18 @@ protected override void EmitGet(Expr.Get g) return; } + // Promoted string-accumulator `.length` (#857): direct StringBuilder.Length. .NET StringBuilder + // .Length is UTF-16 code units, identical to JS string .length — no materialization. + if (!g.Optional && g.Name.Lexeme == "length" && g.Object is Expr.Variable accLenVar + && _ctx.TryGetPromotedStringAccumulator(accLenVar.Name.Lexeme) is { } accLenSb) + { + IL.Emit(OpCodes.Ldloc, accLenSb); + IL.Emit(OpCodes.Callvirt, _ctx.Types.GetProperty(_ctx.Types.StringBuilder, "Length").GetGetMethod()!); + IL.Emit(OpCodes.Conv_R8); + SetStackType(StackType.Double); + return; + } + // Try direct getter dispatch for known class instance types TypeInfo? objType = _ctx.TypeMap?.Get(g.Object); if (TryEmitDirectGetterCall(g.Object, objType, g.Name.Lexeme)) @@ -1234,6 +1246,47 @@ private void EmitPromotedArrayPush(LocalBuilder list, ArrayElementsDescriptor de SetStackType(StackType.Double); } + /// + /// Emits s.charCodeAt(i) for a promoted string-accumulator (StringBuilder slot): reads the + /// UTF-16 code unit directly via the this[int] indexer (identical to JS charCodeAt), with an + /// out-of-range (incl. negative, via unsigned compare) result of NaN. Leaves a boxed double, matching + /// the string-method call convention. See EmitMethodCall and StringAccumulatorPromotionAnalyzer. + /// + private void EmitPromotedStringCharCodeAt(LocalBuilder sb, List arguments) + { + var getLength = _ctx.Types.GetProperty(_ctx.Types.StringBuilder, "Length").GetGetMethod()!; + var getChars = _ctx.Types.GetMethod(_ctx.Types.StringBuilder, "get_Chars", _ctx.Types.Int32); + + var idxLocal = IL.DeclareLocal(_ctx.Types.Int32); + if (arguments.Count > 0) EmitExpressionAsDouble(arguments[0]); + else IL.Emit(OpCodes.Ldc_R8, 0.0); + IL.Emit(OpCodes.Conv_I4); + IL.Emit(OpCodes.Stloc, idxLocal); + + var oob = IL.DefineLabel(); + var end = IL.DefineLabel(); + + // if ((uint)idx >= (uint)sb.Length) -> NaN (unsigned fold catches negative indices too) + IL.Emit(OpCodes.Ldloc, idxLocal); + IL.Emit(OpCodes.Ldloc, sb); + IL.Emit(OpCodes.Callvirt, getLength); + IL.Emit(OpCodes.Bge_Un, oob); + + IL.Emit(OpCodes.Ldloc, sb); + IL.Emit(OpCodes.Ldloc, idxLocal); + IL.Emit(OpCodes.Callvirt, getChars); + IL.Emit(OpCodes.Conv_R8); + IL.Emit(OpCodes.Box, _ctx.Types.Double); + IL.Emit(OpCodes.Br, end); + + IL.MarkLabel(oob); + IL.Emit(OpCodes.Ldc_R8, double.NaN); + IL.Emit(OpCodes.Box, _ctx.Types.Double); + + IL.MarkLabel(end); + SetStackUnknown(); + } + /// /// Emits the common List<object?> / $Array set path with frozen checks and fallback. /// Shared by all descriptor-driven SetIndex paths (typed miss fallthrough and object direct). diff --git a/Compilation/ILEmitter.Statements.cs b/Compilation/ILEmitter.Statements.cs index a43a0849..1c413b76 100644 --- a/Compilation/ILEmitter.Statements.cs +++ b/Compilation/ILEmitter.Statements.cs @@ -249,6 +249,23 @@ protected override void EmitVarDeclaration(Stmt.Var v) return; } + // Append-only string-accumulator promotion (#857): 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)`, is backed by a StringBuilder slot — turning O(n²) repeated + // String.Concat (each copies the whole accumulator) into amortized-O(1) Append. StringBuilder + // .Length and the [i] indexer are UTF-16 code units, identical to JS .length/charCodeAt, so those + // reads need no materialization. Reached only AFTER the capture branches above (the analyzer + // excludes captured names); the append/length/charCodeAt fast paths key off the slot's CLR type. + if (_ctx.TypeMap != null && _ctx.TypeMap.IsPromotableStringAccumulator(v.Name) + && v.Initializer is Expr.Literal { Value: string seedStr }) + { + var sbLocal = _ctx.Locals.DeclareLocal(v.Name.Lexeme, _ctx.Types.StringBuilder); + IL.Emit(OpCodes.Ldstr, seedStr); + IL.Emit(OpCodes.Newobj, _ctx.Types.StringBuilderStringCtor); + IL.Emit(OpCodes.Stloc, sbLocal); + return; + } + // Determine if this local can use unboxed double type Type localType = CanUseUnboxedLocal(v) ? _ctx.Types.Double : _ctx.Types.Object; var local = _ctx.Locals.DeclareLocal(v.Name.Lexeme, localType); diff --git a/Compilation/StringAccumulatorPromotionAnalyzer.cs b/Compilation/StringAccumulatorPromotionAnalyzer.cs new file mode 100644 index 00000000..958d6521 --- /dev/null +++ b/Compilation/StringAccumulatorPromotionAnalyzer.cs @@ -0,0 +1,182 @@ +using SharpTS.Parsing; +using SharpTS.Parsing.Visitors; +using SharpTS.TypeSystem; + +namespace SharpTS.Compilation; + +/// +/// Whole-program analysis that flags string local declarations which can be promoted from the +/// default object slot to a concrete StringBuilder slot (#857). Repeated +/// s = s + str / s += str on an object/string slot lowers to +/// String.Concat, which copies the whole accumulator every iteration — O(n²). A StringBuilder +/// slot turns that into amortized-O(1) Append (O(n) total). StringBuilder.Length and the +/// this[int] indexer are UTF-16 code units, identical to JS .length and +/// charCodeAt(i), so those reads need no materialization. +/// +/// Deliberately-conservative first cut (mirrors ): a local +/// is promoted only when provably non-escaping AND every use is one of a tiny permitted set, so the bare +/// StringBuilder (which is NOT a string) is never observed anywhere that expects a string. +/// A candidate s qualifies iff ALL hold: +/// +/// declared const/let with a string-literal initializer ("" or any "…"); +/// the name is declared exactly once in the whole program (conservative shadowing guard); +/// the name is not captured by any closure (a captured local is routed to an object +/// display-class field, never a typed slot); +/// every use is one of: an append in statement position (s = s + E or s += E +/// as an expression statement, where E is statically string and does not reference +/// s), s.length, or s.charCodeAt(i). Any other appearance — return, argument +/// pass, s[i], other method/property, comparison, template literal, reassignment to a +/// non-append value, a non-string append, or an append used as a value — disqualifies it. +/// +/// +/// Append must be in statement position because s = s + E evaluates to the new string; with a +/// StringBuilder slot that result cannot be produced without an O(n) ToString(). As an expression +/// statement the result is discarded (Stmt.Expression pops it), so the emitter leaves the +/// Append-returned builder on the stack as the one popped value. Materialize-on-escape +/// (return s, pass, index, other methods) is a deliberate Phase-2 follow-up. +/// +public static class StringAccumulatorPromotionAnalyzer +{ + public static void Analyze(List program, TypeMap? typeMap, ClosureAnalyzer? closures) + { + if (typeMap == null) return; + + var visitor = new Visitor(typeMap); + foreach (var stmt in program) + visitor.Visit(stmt); + + foreach (var (key, nameToken) in visitor.Candidates) + { + if (visitor.Disqualified.Contains(key)) continue; + if (visitor.DeclCount.GetValueOrDefault(key) != 1) continue; + // IsVariableCaptured is lexeme-global (conservative): a captured local is routed to an + // object display-class field, never a StringBuilder slot, so capture must disqualify. + if (closures?.IsVariableCaptured(key.Name) == true) continue; + typeMap.MarkPromotableStringAccumulator(nameToken); + } + } + + private sealed class Visitor(TypeMap typeMap) : AstVisitorBase + { + private readonly TypeMap _typeMap = typeMap; + + // Candidacy/disqualification is keyed by (function scope, lexeme), NOT whole-program lexeme: + // a common accumulator name like `s` in one function must not be poisoned by an unrelated, + // escaping `s` in another (e.g. perf_hooks's `const s = findMark(...)` in a bundle). Each + // function/arrow body is its own scope; cross-scope references are captures, handled by the + // IsVariableCaptured guard. The module top level is scope 0. + private int _scope; + private int _nextScope; + + /// (scope, name) → candidate declaration's name token (string-literal-initialized local). + public Dictionary<(int Scope, string Name), Token> Candidates { get; } = new(); + + /// How many times each (scope, name) is declared. + public Dictionary<(int Scope, string Name), int> DeclCount { get; } = new(); + + /// (scope, name) pairs with at least one disqualifying occurrence. + public HashSet<(int Scope, string Name)> Disqualified { get; } = new(); + + protected override void VisitFunction(Stmt.Function stmt) => InScope(() => base.VisitFunction(stmt)); + protected override void VisitArrowFunction(Expr.ArrowFunction expr) => InScope(() => base.VisitArrowFunction(expr)); + + private void InScope(Action body) + { + var saved = _scope; + _scope = ++_nextScope; + body(); + _scope = saved; + } + + protected override void VisitVar(Stmt.Var stmt) => + HandleDeclaration(stmt.Name, stmt.Initializer); + + protected override void VisitConst(Stmt.Const stmt) => + HandleDeclaration(stmt.Name, stmt.Initializer); + + private void HandleDeclaration(Token name, Expr? initializer) + { + var key = (_scope, name.Lexeme); + DeclCount[key] = DeclCount.GetValueOrDefault(key) + 1; + + if (initializer is Expr.Literal { Value: string } && !Candidates.ContainsKey(key)) + Candidates[key] = name; + + // A string-literal initializer has no sub-uses, but a non-candidate initializer may + // reference other accumulators. + if (initializer is not Expr.Literal { Value: string } && initializer != null) + Visit(initializer); + } + + protected override void VisitExpressionStmt(Stmt.Expression stmt) + { + // Permitted append in statement position (result discarded): `s = s + E` / `s += E` + // with E statically string. Consume by visiting ONLY E — not the target, not the inner + // `s` read. Visiting E still disqualifies if E references s (the VisitVariable catch-all) + // or escapes any other accumulator. + switch (stmt.Expr) + { + case Expr.Assign { Value: Expr.Binary { Operator.Type: TokenType.PLUS, Left: Expr.Variable lv } bin } asg + when lv.Name.Lexeme == asg.Name.Lexeme && IsStaticString(bin.Right): + Visit(bin.Right); + return; + case Expr.CompoundAssign { Operator.Type: TokenType.PLUS_EQUAL } ca when IsStaticString(ca.Value): + Visit(ca.Value); + return; + } + base.VisitExpressionStmt(stmt); + } + + protected override void VisitGet(Expr.Get expr) + { + // `s.length` — permitted; skip the receiver variable. + if (expr.Name.Lexeme == "length" && expr.Object is Expr.Variable && !expr.Optional) + return; + Visit(expr.Object); + } + + protected override void VisitCall(Expr.Call expr) + { + // `s.charCodeAt(i)` — permitted; visit the index args but skip the receiver variable. + if (expr.Callee is Expr.Get { Object: Expr.Variable, Optional: false } get + && get.Name.Lexeme == "charCodeAt") + { + foreach (var arg in expr.Arguments) + Visit(arg); + return; + } + base.VisitCall(expr); + } + + protected override void VisitAssign(Expr.Assign expr) + { + // Reached only when NOT consumed as a statement-position append above — i.e. a reassign, + // a non-string/prepend append, or an append used as a value. Disqualify. + Disqualified.Add((_scope, expr.Name.Lexeme)); + base.VisitAssign(expr); + } + + protected override void VisitCompoundAssign(Expr.CompoundAssign expr) + { + Disqualified.Add((_scope, expr.Name.Lexeme)); + base.VisitCompoundAssign(expr); + } + + protected override void VisitVariable(Expr.Variable expr) + { + // Catch-all: any bare variable occurrence not consumed by a permitted-use override above + // is an escape (returned, passed, indexed, compared, concatenated as a value, etc.). + Disqualified.Add((_scope, expr.Name.Lexeme)); + } + + private bool IsStaticString(Expr e) => IsStringTypeInfo(_typeMap.Get(e)); + + private static bool IsStringTypeInfo(TypeInfo? type) => type switch + { + TypeInfo.String => true, + TypeInfo.StringLiteral => true, + TypeInfo.Union u => u.FlattenedTypes.All(IsStringTypeInfo), + _ => false + }; + } +} diff --git a/SharpTS.Tests/SharedTests/StringAccumulatorPromotionTests.cs b/SharpTS.Tests/SharedTests/StringAccumulatorPromotionTests.cs new file mode 100644 index 00000000..7b2f8f75 --- /dev/null +++ b/SharpTS.Tests/SharedTests/StringAccumulatorPromotionTests.cs @@ -0,0 +1,176 @@ +using SharpTS.Tests.Infrastructure; +using Xunit; + +namespace SharpTS.Tests.SharedTests; + +/// +/// Tests for the append-only string-accumulator promotion optimization (#857): 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), is compiled to a StringBuilder slot (O(n²) String.Concat → O(n) Append). +/// +/// These run against BOTH the interpreter and the compiler. The positive cases exercise the +/// promoted fast paths (append / length / charCodeAt); the non-promotable cases must fall back to +/// the general string path and still produce correct results — interpreter/compiled parity must +/// hold whether or not promotion fires. A wrong escape rule or a wrong fast-path lowering would +/// surface here as a compiled-mode mismatch. +/// +public class StringAccumulatorPromotionTests +{ + // ── Positive cases: promotable shapes ────────────────────────────────── + + [Theory] + [MemberData(nameof(ExecutionModes.All), MemberType = typeof(ExecutionModes))] + public void Promoted_BuildThenScan_CharCodeSum(ExecutionMode mode) + { + // The stringWork shape: append in a loop, then sweep length + charCodeAt. + var source = """ + function stringWork(n: number): number { + let s: string = ""; + for (let i: number = 0; i < n; i++) { s = s + "ab"; } + let sum: number = 0; + for (let i: number = 0; i < s.length; i++) { sum = sum + s.charCodeAt(i); } + return sum; + } + console.log(stringWork(3)); + """; + + // "ababab": 3×('a'97 + 'b'98) = 3×195 = 585 + Assert.Equal("585\n", TestHarness.Run(source, mode)); + } + + [Theory] + [MemberData(nameof(ExecutionModes.All), MemberType = typeof(ExecutionModes))] + public void Promoted_CompoundAppend_Length(ExecutionMode mode) + { + var source = """ + function f(): number { + let s: string = "x"; + for (let i: number = 0; i < 3; i++) { s += "y"; } + return s.length; + } + console.log(f()); + """; + + // "xyyy" → length 4 + Assert.Equal("4\n", TestHarness.Run(source, mode)); + } + + [Theory] + [MemberData(nameof(ExecutionModes.All), MemberType = typeof(ExecutionModes))] + public void Promoted_CharCodeAt_OutOfRange_IsNaN(ExecutionMode mode) + { + var source = """ + function f(): number { + let s: string = ""; + s = s + "AB"; + return s.charCodeAt(5); + } + console.log(f()); + """; + + // charCodeAt past the end → NaN (JS semantics) + Assert.Equal("NaN\n", TestHarness.Run(source, mode)); + } + + [Theory] + [MemberData(nameof(ExecutionModes.All), MemberType = typeof(ExecutionModes))] + public void Promoted_PerScope_NameCollisionDoesNotPoison(ExecutionMode mode) + { + // `s` in build() is a clean append-only accumulator and must promote even though an + // unrelated, escaping `s` exists in other() — the per-function-scope candidacy guards + // against the whole-program per-lexeme collision (e.g. perf_hooks's `const s` in a bundle). + var source = """ + function makeName(): string { return "hi"; } + function build(): number { + let s: string = ""; + for (let i: number = 0; i < 4; i++) { s = s + "ab"; } + return s.length; + } + function other(): string { + let s: string = makeName(); + return s.toUpperCase(); + } + console.log(build()); + console.log(other()); + """; + + // "abababab" → 8 ; "hi".toUpperCase() → "HI" + Assert.Equal("8\nHI\n", TestHarness.Run(source, mode)); + } + + // ── Non-promotable cases: must fall back and stay correct ─────────────── + + [Theory] + [MemberData(nameof(ExecutionModes.All), MemberType = typeof(ExecutionModes))] + public void NotPromoted_Returned_StillCorrect(ExecutionMode mode) + { + // `return s` escapes the accumulator (a StringBuilder slot can't be returned as a string + // without materialization) — must fall back to the general path, still correct. + var source = """ + function f(): string { + let s: string = ""; + for (let i: number = 0; i < 3; i++) { s = s + "z"; } + return s; + } + console.log(f()); + """; + + Assert.Equal("zzz\n", TestHarness.Run(source, mode)); + } + + [Theory] + [MemberData(nameof(ExecutionModes.All), MemberType = typeof(ExecutionModes))] + public void NotPromoted_Reassigned_StillCorrect(ExecutionMode mode) + { + // `s = "b"` is a non-append reassignment → disqualified. + var source = """ + function f(): string { + let s: string = "a"; + s = "b"; + s = s + "c"; + return s; + } + console.log(f()); + """; + + Assert.Equal("bc\n", TestHarness.Run(source, mode)); + } + + [Theory] + [MemberData(nameof(ExecutionModes.All), MemberType = typeof(ExecutionModes))] + public void NotPromoted_Captured_StillCorrect(ExecutionMode mode) + { + // `s` is captured by a closure → routed to an object display-class field, never a + // StringBuilder slot. The closure must observe the live value after the append. + var source = """ + function f(): number { + let s: string = ""; + const get = (): number => s.length; + s = s + "abc"; + return get(); + } + console.log(f()); + """; + + Assert.Equal("3\n", TestHarness.Run(source, mode)); + } + + [Theory] + [MemberData(nameof(ExecutionModes.All), MemberType = typeof(ExecutionModes))] + public void NotPromoted_NonStringAppend_StillCorrect(ExecutionMode mode) + { + // `s = s + i` appends a number — not statically string, so not the promotable shape; + // must fall back to the general coercing concat path (JS ToString on the number). + var source = """ + function f(): string { + let s: string = ""; + for (let i: number = 0; i < 3; i++) { s = s + i; } + return s; + } + console.log(f()); + """; + + Assert.Equal("012\n", TestHarness.Run(source, mode)); + } +} diff --git a/TypeSystem/TypeMap.cs b/TypeSystem/TypeMap.cs index 13a4b8b8..79d834a7 100644 --- a/TypeSystem/TypeMap.cs +++ b/TypeSystem/TypeMap.cs @@ -21,6 +21,7 @@ public class TypeMap private readonly HashSet _undefinedReachableNumericLocals = new(ReferenceEqualityComparer.Instance); private readonly HashSet _undefinedReachableNumericParams = new(ReferenceEqualityComparer.Instance); private readonly Dictionary _promotableArrayLocals = new(ReferenceEqualityComparer.Instance); + private readonly HashSet _promotableStringAccumulators = new(ReferenceEqualityComparer.Instance); private readonly Dictionary _promotableObjectLocals = new(ReferenceEqualityComparer.Instance); /// @@ -145,6 +146,24 @@ public void MarkPromotableArrayLocal(Token nameToken, TokenType elementToken) => public bool IsPromotableArrayLocal(Token nameToken, out TokenType elementToken) => _promotableArrayLocals.TryGetValue(nameToken, out elementToken); + /// + /// Flags a const/let string local with a string-literal initializer that is provably + /// non-escaping and used only via append (s = s + str/s += str in statement position), + /// s.length, and s.charCodeAt(i). The IL compiler promotes such a local to a concrete + /// StringBuilder slot (#857), turning O(n²) repeated String.Concat into O(n) Append. + /// Keyed by reference on the declaration's name token (stable across Stmt.Var/ + /// Stmt.Const). Purely a compiler hint — set by the IL compiler's promotion analyzer. + /// + public void MarkPromotableStringAccumulator(Token nameToken) => + _promotableStringAccumulators.Add(nameToken); + + /// + /// True if the declaration with name token was flagged by + /// . + /// + public bool IsPromotableStringAccumulator(Token nameToken) => + _promotableStringAccumulators.Contains(nameToken); + /// /// Flags a const/let object-literal local declaration whose literal has a fixed, /// statically-known primitive shape and which is provably non-escaping (only used via constant-key diff --git a/docs/plans/issue-857-string-accumulator-stringbuilder.md b/docs/plans/issue-857-string-accumulator-stringbuilder.md new file mode 100644 index 00000000..7a95665e --- /dev/null +++ b/docs/plans/issue-857-string-accumulator-stringbuilder.md @@ -0,0 +1,122 @@ +# Plan: append-only string-local promotion to `StringBuilder` (#857 follow-on / new #856 child) + +## STATUS: Phase 1 implemented (2026-06-20) + +Implemented and validated. Files: new `Compilation/StringAccumulatorPromotionAnalyzer.cs`; `TypeSystem/TypeMap.cs`, `Compilation/CompilationContext.cs` (`TryGetPromotedStringAccumulator`), `ILCompiler.cs` (wired at both single- and multi-module sites), `ILEmitter.Statements.cs` (StringBuilder slot), `ILEmitter.Expressions.cs` (`=` append), `ILEmitter.Operators.cs` (`+=` append), `ILEmitter.Properties.cs` (`.length` + `charCodeAt` helper), `ILEmitter.Calls.MethodDispatch.cs` (`charCodeAt` hook); tests `SharpTS.Tests/SharedTests/StringAccumulatorPromotionTests.cs` (16 cases, both modes, green). + +**Result:** bundled `strings.ts` @10k **17.3 ms → 0.265 ms (65×)**, O(n²)→O(n), now ~2.3× Node (was 149×). IL verifies. Full xUnit suite green except pre-existing flaky network + the 2 documented stale Test262 baselines (drift is `Array.isArray`/`proxy`, present in interpreter mode too → unrelated to this compile-only change). + +**One critical deviation from the template below:** candidacy is keyed **per function scope**, NOT whole-program-per-lexeme like `ArrayLocalPromotionAnalyzer`. The array analyzer's whole-program lexeme guard silently fails for common names in bundles — a clean `s` in one function is poisoned by an unrelated escaping `s` in another module (e.g. `perf_hooks`'s `const s = findMark(...)`). The string analyzer enters a new scope per `Stmt.Function`/`Expr.ArrowFunction`; cross-scope references are captures, caught by the `IsVariableCaptured` guard. **Follow-up: apply the same per-scope keying to `ArrayLocalPromotionAnalyzer`** — it has the same latent bug, dodged only by uncommon array names. + +--- + + +## Why + +Re-measured **warm, content-forced** (2026-06, `dotnet build -c Release`), compiled vs Node: + +| strings @10k phase | Compiled | Node | Slowdown | +|---|---|---|---| +| concat-only (`s = s + "ab"` ×n, return `s.length`) | 18.0 ms | 0.05 ms | **337×** | +| scan-only (`charCodeAt` sweep) | 0.96 ms | 0.05 ms | 21× | + +The concat is ~95% of the strings benchmark and is **O(n²)**: content-forced scaling 5k/10k/20k/40k = 6.5 / 17.8 / 74 / **351 ms** (≈4× per 2× n), vs Node O(n) (cons-strings) at 0.26 ms @40k → **1340× @40k and growing without bound**. The IL emits `String.Concat(string,string)` per iteration, copying the whole accumulator each time (`ILEmitter.Operators.cs`, the 2-operand both-string fast path from `fed6fc0a`). + +`Compilation/StringConcatOptimizer.cs` only flattens **intra-expression** chains (`"a"+b+c`, `MinPartsForOptimization=3`). Loop-carried accumulation is unhandled. **No StringBuilder accumulator was ever committed** (checked git history; `fed6fc0a` = typed-string concat + `ArrayLocalPromotionAnalyzer`, nothing else). We build it fresh. + +Goal: detect a provably **append-only** string local and back it with a `System.Text.StringBuilder`, turning O(n²) → O(n). Target: strings @10k from ~17 ms toward sub-millisecond. + +## Template + +This is the direct analog of the existing, shipped `ArrayLocalPromotionAnalyzer` (#857/#860). Follow it exactly: +- whole-program AST visitor, conservative non-escaping first cut; +- permitted-use overrides that consume the receiver variable **without** recursing into it; +- catch-all `VisitVariable` disqualifies any other bare occurrence (escape detection); +- results written to `TypeMap`; the IL emitter's fast paths key off the **slot's CLR type** via `LocalsManager`, so a captured/un-promoted name can never accidentally hit them. + +Key correctness shortcut: **.NET `StringBuilder.Length` == JS `.length`** and **`sb[i]` (char) == `charCodeAt(i)`** — both are UTF-16 code units. So `length` and `charCodeAt` read the builder **directly, with no materialization**. `stringWork` (build + `.length` + `.charCodeAt`) therefore runs end-to-end on the builder with zero `ToString()`. + +## Phase 1 — append + length + charCodeAt (covers the benchmark, fully correct, no materialization) + +### 1. `Compilation/StringAccumulatorPromotionAnalyzer.cs` (new, mirror `ArrayLocalPromotionAnalyzer`) + +A candidate `s` qualifies iff ALL hold: +1. declared `let`/`const` exactly once in the whole program (`DeclCount == 1`), statically `string`, with a **string-literal** initializer (`""` or any `"…"`); +2. not captured by any closure (`closures.IsVariableCaptured(name) == false`); +3. every use is one of the permitted shapes below — anything else disqualifies (the `VisitVariable` catch-all). + +Permitted shapes (each consumes the receiver/target `s` without visiting it as a bare variable): +- **append**: `s = s + E` (`VisitAssign` where `Value` is `Binary(+, Variable(s), E)`) **or** `s += E` (`VisitCompoundAssign`, `+=`), where `E` is **statically `string`** and does **not** reference `s`. Visit `E` only. + - Restricting `E` to statically-`string` sidesteps the `StringifyCoerce` string-hint-vs-default-hint divergence noted at `ILEmitter.Operators.cs:52`. Non-string appends are a Phase-2 concern (route through the exact `+` coercion). +- **`s.length`** (`VisitGet`, name `length`, non-optional). +- **`s.charCodeAt(i)`** (`VisitCall`, callee `Get(Variable(s), "charCodeAt")`, non-optional). Visit the index arg only. + +Reuse verbatim from the array analyzer: `VisitAssign`/`VisitCompoundAssign` override structure, the single-decl `DeclCount` guard, the not-captured guard, the `VisitVariable` catch-all, the "permitted overrides don't recurse into the receiver" discipline. + +### 2. `TypeSystem/TypeMap.cs` (mirror `_promotableArrayLocals`) + +```csharp +private readonly HashSet _promotableStringAccumulators = new(ReferenceEqualityComparer.Instance); +public void MarkPromotableStringAccumulator(Token nameToken) => _promotableStringAccumulators.Add(nameToken); +public bool IsPromotableStringAccumulator(Token nameToken) => _promotableStringAccumulators.Contains(nameToken); +``` + +### 3. Wire into the pipeline — `Compilation/ILCompiler.cs` + +Add `StringAccumulatorPromotionAnalyzer.Analyze(statements, _typeMap, _closures.Analyzer);` next to the existing array/object analyzer calls at **both** sites: `ILCompiler.cs:427/429` and `ILCompiler.cs:957/959`. + +### 4. Declare the slot — `Compilation/ILEmitter.Statements.cs` `EmitVarDeclaration` + +New branch alongside the array branch (~line 233), **before** the generic `CanUseUnboxedLocal` fall-through: + +```csharp +if (_ctx.TypeMap != null && _ctx.TypeMap.IsPromotableStringAccumulator(v.Name)) +{ + var sbType = _ctx.Types.StringBuilder; // add to TypeProvider if absent + var sbLocal = _ctx.Locals.DeclareLocal(v.Name.Lexeme, sbType); + // initializer is a string literal (analyzer guarantee): seed the builder + EmitExpression(v.Initializer!); // pushes the seed string + IL.Emit(OpCodes.Newobj, ctor(StringBuilder, string)); // new StringBuilder(seed) + IL.Emit(OpCodes.Stloc, sbLocal); + return; +} +``` + +### 5. Append lowering — `Compilation/ILEmitter.Operators.cs` + +At the assignment / `+=` emission for a target whose slot is the promoted `StringBuilder` (check `LocalsManager` slot CLR type, same as the array fast paths), emit `sb.Append(E)` instead of `String.Concat` + store: +- `s = s + E`: load `sbLocal`, `EmitExpression(E)` (statically string ⇒ already a `string` on stack), `callvirt StringBuilder StringBuilder::Append(string)`, `pop` the returned builder. +- `s += E`: identical. + +This replaces the per-iteration `Convert::ToString(object)` + `String.Concat` with one amortized-O(1) `Append`. + +### 6. `length` / `charCodeAt` lowering on a promoted slot + +- **`s.length`** — `Compilation/ILEmitter.Properties.cs` (where the array `.length` fast path lives): `ldloc sbLocal; callvirt get_Length; conv.r8`. +- **`s.charCodeAt(i)`** — `Compilation/ILEmitter.Calls.MethodDispatch.cs` (where the array `push` fast path lives): bounds-check `i` against `sb.Length`; in range ⇒ `ldloc sbLocal; ldloc i; conv.i4; callvirt char get_Chars(int32); conv.r8`; out of range ⇒ `ldc.r8 NaN` (JS `charCodeAt` OOB ⇒ `NaN`). + +`TypeProvider.cs`: add `StringBuilder` `Type` + cached `MethodInfo`s (`Append(string)`, `get_Length`, `get_Chars`) and a `(string)` ctor, mirroring how `List` members are cached. **Standalone-DLL note:** `StringBuilder` is pure BCL (`System.Text`), so this is NOT a SharpTS soft-dependency — do **not** call `RequireSharpTSRuntime` (CLAUDE.md). + +## Phase 2 — materialize-on-escape (broadens to real-world `return s` / pass / index / other methods) + +A string you only measure (length/charCodeAt) but never use as a string is rare in real code. Phase 2 permits escapes by materializing: +- Add a companion **cached `string?` slot** per promoted accumulator. +- Append: `sb.Append(...)`; set `cached = null`. +- Any escaping use (`return s`, pass as arg, `s[i]`, `s.`, template literal, comparison, non-string append operand): `if (cached == null) cached = sb.ToString(); use cached;`. +- Total cost stays O(n) when escapes don't interleave with appends (the common build-then-use shape); if they do, it degrades gracefully toward the current behavior. + +Keep Phase 2 a separate PR — Phase 1 ships the headline win and is the minimal correct slice. + +## Correctness & gates (non-negotiable, per epic #856) + +- **Evaluation order**: `s = s + f()` ⇒ load builder, eval `f()`, `Append` — RHS side effects preserved. Disqualify appends whose operand references `s` (`s = s + s`) in Phase 1. +- **`undefined` sentinel**: sidestepped — string-literal init + string-only appends + single decl + no reassignment means `$Undefined` can never enter the slot (this is *why* the generic string-slot typing is blocked at `ILEmitter.Statements.cs:~1324`, and why this narrow promotion is safe where that isn't). +- **Capture/escape**: not-captured guard + `VisitVariable` catch-all (verbatim from the array analyzer). +- **Tests**: new `StringAccumulatorPromotionTests.cs` mirroring `ArrayLocalPromotionTests.cs` — promotion fires on the safe shapes; does NOT fire on captured / reassigned / escaping / non-string-append / shadowed / multi-decl cases; semantic equivalence (interpreter vs compiled) on each. +- **Suites**: green on `dotnet test`, `SharpTS.Test262` (interp + compiled), `SharpTS.TypeScriptConformance` — no baseline regression. Run `--compile … --verify` on the promoted output (IL-verify the new fast paths). +- **Measurement protocol**: content-forced only (return/print a guard) — a build-only loop gets DCE'd and reports a fake sub-ms O(1) number (this is exactly how the prior "concat is O(n), StringBuilder useless" conclusion was wrong). Confirm O(n) via the 5k/10k/20k/40k scaling sweep and compare to Node. + +## Out of scope (tracked separately) + +- **array-methods @100k (2.79× warm)**: `arr: number[]` is NOT promoted because it's consumed by `.map()` — the array analyzer disqualifies any receiver used by a non-`push` method, so `arr` stays `$Array`-backed `object` (isinst ladder on `push`, boxed-double `List` map/filter/reduce intermediates). Extending promotion to typed-list HOF receivers is a separate #860/#861 follow-on. +- **factorial-class tight loops**: per-iteration `$Runtime::CheckCancellation()` is a small constant tax on every numeric loop (~4× warm but in µs). Separate investigation.