Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 48 additions & 28 deletions Compilation/ArrayLocalPromotionAnalyzer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,9 @@ namespace SharpTS.Compilation;
/// <c>x.push(...)</c> — any other appearance of the bare variable (argument pass, return, store,
/// spread, <c>for…of</c>, <c>===</c>, reassignment, <c>delete x[i]</c>, <c>pop</c>/any other
/// method or property) disqualifies it;</item>
/// <item>the name is declared exactly once in the whole program (conservative guard against scope
/// ambiguity / shadowing without full scope resolution);</item>
/// <item>the name is declared exactly once within its function scope (conservative guard against
/// shadowing without full scope resolution; candidacy is keyed per scope, so a same-named
/// array in a different function/module never interferes);</item>
/// <item>the name is not captured by any closure (a captured local is routed to an <c>object</c>
/// display-class field, never a typed slot).</item>
/// </list>
Expand All @@ -42,12 +43,12 @@ public static void Analyze(List<Stmt> program, TypeMap? typeMap, ClosureAnalyzer
foreach (var stmt in program)
visitor.Visit(stmt);

foreach (var (name, nameToken) in visitor.Candidates)
foreach (var (key, nameToken) in visitor.Candidates)
{
if (visitor.Disqualified.Contains(name)) continue;
if (visitor.DeclCount.GetValueOrDefault(name) != 1) continue;
if (!visitor.ElementToken.TryGetValue(name, out var token)) continue; // no Double/Bool use seen
if (closures?.IsVariableCaptured(name) == true) continue;
if (visitor.Disqualified.Contains(key)) continue;
if (visitor.DeclCount.GetValueOrDefault(key) != 1) continue;
if (!visitor.ElementToken.TryGetValue(key, out var token)) continue; // no Double/Bool use seen
if (closures?.IsVariableCaptured(key.Name) == true) continue;
typeMap.MarkPromotableArrayLocal(nameToken, token);
}
}
Expand All @@ -56,17 +57,36 @@ private sealed class Visitor(TypeMap typeMap) : AstVisitorBase
{
private readonly TypeMap _typeMap = typeMap;

/// <summary>name → candidate declaration's name token (empty-array-literal local).</summary>
public Dictionary<string, Token> Candidates { get; } = new();
// Candidacy/disqualification is keyed by (function scope, lexeme), NOT whole-program lexeme: a
// common array name like `arr`/`data` in one function must not be poisoned by an unrelated,
// escaping same-name array in another (e.g. across bundled modules). 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. Mirrors StringAccumulatorPromotionAnalyzer.
private int _scope;
private int _nextScope;

/// <summary>How many times each name is declared anywhere (any kind of binding).</summary>
public Dictionary<string, int> DeclCount { get; } = new();
/// <summary>(scope, name) → candidate declaration's name token (empty-array-literal local).</summary>
public Dictionary<(int Scope, string Name), Token> Candidates { get; } = new();

/// <summary>name → element primitive token (TYPE_NUMBER / TYPE_BOOLEAN), from its first typed use.</summary>
public Dictionary<string, TokenType> ElementToken { get; } = new();
/// <summary>How many times each (scope, name) is declared.</summary>
public Dictionary<(int Scope, string Name), int> DeclCount { get; } = new();

/// <summary>Names with at least one disqualifying occurrence.</summary>
public HashSet<string> Disqualified { get; } = new();
/// <summary>(scope, name) → element primitive token (TYPE_NUMBER / TYPE_BOOLEAN), from its first typed use.</summary>
public Dictionary<(int Scope, string Name), TokenType> ElementToken { get; } = new();

/// <summary>(scope, name) pairs with at least one disqualifying occurrence.</summary>
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);
Expand All @@ -76,11 +96,11 @@ protected override void VisitConst(Stmt.Const stmt) =>

private void HandleDeclaration(Token name, Expr? initializer)
{
var lexeme = name.Lexeme;
DeclCount[lexeme] = DeclCount.GetValueOrDefault(lexeme) + 1;
var key = (_scope, name.Lexeme);
DeclCount[key] = DeclCount.GetValueOrDefault(key) + 1;

if (initializer is Expr.ArrayLiteral { Elements.Count: 0 } && !Candidates.ContainsKey(lexeme))
Candidates[lexeme] = name;
if (initializer is Expr.ArrayLiteral { Elements.Count: 0 } && !Candidates.ContainsKey(key))
Candidates[key] = name;

// Visit the initializer for completeness ([] has no sub-uses, but a
// non-candidate initializer may reference other arrays).
Expand Down Expand Up @@ -110,7 +130,7 @@ protected override void VisitSetIndex(Expr.SetIndex expr)
// diverging from the general path that preserves it. Disqualify — the
// array analogue of the #367/#372 numeric-slot taint guard.
if (ValueAdmitsUndefinedSentinel(expr.Value))
Disqualified.Add(v.Name.Lexeme);
Disqualified.Add((_scope, v.Name.Lexeme));
}
else
Visit(expr.Object);
Expand Down Expand Up @@ -147,7 +167,7 @@ protected override void VisitCall(Expr.Call expr)
foreach (var arg in expr.Arguments)
{
if (ValueAdmitsUndefinedSentinel(arg))
Disqualified.Add(v.Name.Lexeme);
Disqualified.Add((_scope, v.Name.Lexeme));
Visit(arg);
}
return;
Expand All @@ -160,13 +180,13 @@ protected override void VisitAssign(Expr.Assign expr)
// `x = ...` rebinds the slot — disqualify. (Only the empty-literal
// initializer is supported; any later store could be a $Array or a
// value the typed slot can't hold.)
Disqualified.Add(expr.Name.Lexeme);
Disqualified.Add((_scope, expr.Name.Lexeme));
base.VisitAssign(expr);
}

protected override void VisitCompoundAssign(Expr.CompoundAssign expr)
{
Disqualified.Add(expr.Name.Lexeme);
Disqualified.Add((_scope, expr.Name.Lexeme));
base.VisitCompoundAssign(expr);
}

Expand All @@ -182,31 +202,31 @@ protected override void VisitDelete(Expr.Delete expr)
break;
}
if (target is Expr.Variable v)
Disqualified.Add(v.Name.Lexeme);
Disqualified.Add((_scope, v.Name.Lexeme));
base.VisitDelete(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, spread, compared, etc.).
Disqualified.Add(expr.Name.Lexeme);
Disqualified.Add((_scope, expr.Name.Lexeme));
}

private void NotePermittedReceiver(Expr.Variable v)
{
// Resolve the element kind from the receiver's static array type (as
// ArrayHoistAnalyzer does). Only Double/Bool arrays are promotable; an
// Object-kind array (string[]/union[]) disqualifies the name.
if (ElementToken.ContainsKey(v.Name.Lexeme)) return;
if (ElementToken.ContainsKey((_scope, v.Name.Lexeme))) return;
var desc = ArrayElements.Resolve(_typeMap.Get(v));
if (desc == null) return; // not statically an array here — leave undecided
if (desc.Kind == ArrayElementsKind.Object || desc.ElementTokenType is not { } tok)
{
Disqualified.Add(v.Name.Lexeme);
Disqualified.Add((_scope, v.Name.Lexeme));
return;
}
ElementToken[v.Name.Lexeme] = tok;
ElementToken[(_scope, v.Name.Lexeme)] = tok;
}

/// <summary>
Expand Down
29 changes: 29 additions & 0 deletions SharpTS.Tests/SharedTests/ArrayLocalPromotionTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -244,4 +244,33 @@ function f(): number {

Assert.Equal("6\n", TestHarness.Run(source, mode));
}

[Theory]
[MemberData(nameof(ExecutionModes.All), MemberType = typeof(ExecutionModes))]
public void Promoted_PerScope_NameCollisionDoesNotPoison(ExecutionMode mode)
{
// `xs` in build() is a clean push/index/length array and must promote even though an
// unrelated, escaping `xs` (returned) exists in leak() — candidacy is keyed per function
// scope, so a common array name shared across functions/modules no longer poisons the
// whole program. Both must produce correct results.
var source = """
function leak(): number[] {
const xs: number[] = [];
xs.push(9);
return xs;
}
function build(): number {
const xs: number[] = [];
for (let i: number = 0; i < 5; i++) { xs.push(i); }
let sum: number = 0;
for (let i: number = 0; i < xs.length; i++) { sum = sum + xs[i]; }
return sum;
}
console.log(build());
console.log(leak()[0]);
""";

// 0+1+2+3+4 = 10 ; leak()[0] = 9
Assert.Equal("10\n9\n", TestHarness.Run(source, mode));
}
}
71 changes: 71 additions & 0 deletions docs/plans/issue-861-array-methods-typed-pipeline.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
# Plan: typed `List<double>` HOF pipeline for monomorphic number arrays (#861 follow-on / #856 child)

## Why

After strings (#857), **array-methods is the last benchmark workload meaningfully off Node** (everything else is at parity or we win):

| array-methods @100k (warm) | Compiled | Node | Slowdown |
|---|---|---|---|
| full pipeline | 11.2 | 4.0 | 2.79× |

Stage decomposition (mean ms @100k, content-forced) localizes the gap:

| stage | Compiled | Node | Slowdown |
|---|---|---|---|
| build (`push` loop) | 6.0 | 1.5 | 3.9× |
| `map` | 4.9 | 1.1 | 4.4× |
| `filter` | 9.4 | 1.2 | **7.9×** |
| `reduce` | 1.1 | 1.0 | ~parity ✅ |

**Root cause (confirmed from IL of `filterOnly`):** the whole pipeline is **boxed `double` in `List<object>`**. `arr` is `$Array`-backed (not promoted to `List<double>` — it's consumed by `.map`, which the `ArrayLocalPromotionAnalyzer` escape rule disqualifies). `filter` calls `ArrayFilterDirectBool(List<object>, Func<object,bool>)`: every element is unboxed inside the adapter and the kept ones re-boxed into a fresh `List<object>`. `map` is analogous. #861 already removed the callback-dispatch overhead (bool-returning adapter, no per-stage `List↔$Array` round-trip) — the **residual gap is purely per-element boxing**, so there is no surgical fix; the representation must become typed.

`reduce` is already at parity because it folds to a scalar (no result array to box).

## Goal

When a number array flows through `map`/`filter`/`reduce` with monomorphic `number`→`number` / `number`→`bool` callbacks, keep it as a concrete `List<double>` end-to-end: no per-element box/unbox, no `$Array` indirection.

## Design

Three coordinated pieces (analyzer → emitter → runtime helpers):

### 1. Promotion analyzer — permit terminal HOF consumers

Extend the typed-array-local promotion (currently `push`/`[i]`/`.length` only) so a `number[]`/`boolean[]` local may also be the **receiver of `map`/`filter`/`reduce`** and still stay `List<double>`/`List<bool>` — provided the array identity itself doesn't escape (the HOF consumes it and returns a *new* array/scalar; the original isn't returned/passed/aliased). `forEach` can join later.

- Reuse the same conservative escape discipline as `ArrayLocalPromotionAnalyzer` (permitted-use overrides that consume the receiver without recursing into it).
- **Prerequisite:** port the **per-function-scope candidacy** fix from `StringAccumulatorPromotionAnalyzer` to `ArrayLocalPromotionAnalyzer` first (see its own note) — without it, common array names (`arr`, `data`, `items`) are poisoned across bundled modules and never promote anyway.

### 2. Typed HOF result locals

`const doubled = arr.map(...)` / `const evens = doubled.filter(...)` must themselves be `List<double>` slots, not `object`. Mark the HOF-result local as promoted when (a) the receiver is a typed `List<double>`/`List<bool>` and (b) the callback's return type is statically `number`/`boolean`. This chains: `arr`(List<double>) → `doubled`(List<double>) → `evens`(List<double>) → `reduce`→`double`.

### 3. Typed runtime HOF helpers + emitter dispatch

Add typed helpers alongside the existing `ArrayFilterDirectBool` etc. (in `RuntimeEmitter.Arrays.*` / the `$Runtime` HOF surface):
- `ArrayMapDouble(List<double>, Func<double,double>) -> List<double>`
- `ArrayFilterDouble(List<double>, Func<double,bool>) -> List<double>`
- `ArrayReduceDouble(List<double>, Func<double,double,double>, double) -> double`
- (`bool` variants as needed)

Emitter (`ILEmitter.Calls.MethodDispatch.cs`, the #861 HOF path): when the receiver slot is `List<double>` and the callback is a monomorphic `double`-typed arrow, bind a `Func<double,double>`/`Func<double,bool>` (the #858/#861 de-virtualized arrow already gives a direct delegate) and call the typed helper. The callback bodies are already numeric (the #861 typed-adapter work) — here the adapter signature becomes `double`-based instead of `object`-based, eliminating the box at the call boundary and the unbox inside.

Keep the existing `object`/`$Array` helpers as the fallback for polymorphic / non-promoted arrays.

## Expected impact

Eliminating per-element boxing across build+map+filter should pull each from 4–8× toward `reduce`'s ~parity, i.e. array-methods @100k from ~11 ms toward ~5 ms (≈Node). Bounded by the inherent intermediate-array allocations (which Node also pays).

## Correctness & gates

- `$Array` semantics (holes, sparseness, identity) must never be observed for a promoted `List<double>` — same non-escaping guard as the existing array promotion. OOB and `undefined`-sentinel handling per the #857/#860 landmines (bounds-checked reads; a `double` slot can't carry `$Undefined`).
- Callback return-type monomorphism must be statically proven; a callback returning `number | undefined` etc. disqualifies (falls back to the boxed path).
- Green on `dotnet test`, `SharpTS.Test262` (interp + compiled), `SharpTS.TypeScriptConformance`; `--verify` the emitted HOF chains.
- Measure content-forced, with the stage decomposition above as before/after.

## Scope / sequencing

High value (last benchmark gap), **medium-high risk** (touches the HOF hot path + the promotion analyzer). Larger than #857. Suggested order:
1. Port per-scope candidacy to `ArrayLocalPromotionAnalyzer` (small, low-risk, independently shippable; also hardens existing bundled-program promotion).
2. Typed `map`→`reduce` (no `filter`) as the first vertical slice (proves the typed-delegate + helper plumbing on the simplest chain).
3. Add `filter` (the worst stage) and result-local chaining.
Loading