Implement proper tail calls for strict functions - #2975
Conversation
Add strict-mode tail-position analysis and trampoline interpreted calls without growing the execution or call stacks. Preserve cleanup, constructor, callback, debugger, and recursion-limit semantics, and add Test262, regression, and benchmark coverage. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 06db877c-0e12-407f-b360-69341c261890
lahma
left a comment
There was a problem hiding this comment.
Great to see proper tail calls land — the mechanism (prepare-time marking + pooled request + trampoline after the caller unwinds) is sound, and the recursion win is real and reproduced below. I ran an independent benchmark gate against the merge-base alongside the review, and it surfaced one hard blocker plus two reproducible regressions that the sloppy-mode suites structurally cannot see. Numbers and methodology at the bottom.
Blocking
1. LimitRecursion no longer terminates infinite strict tail recursion — the host thread hangs. (ScriptFunction.cs#L441)
"use strict";
function f() { return f(); }
f();With options.LimitRecursion(8) this used to throw RecursionDepthOverflowException immediately; on this branch Evaluate never returns (reproduced empirically — still running after 5 s, and the loop has no exit). ReplaceTop pops and re-pushes the same function each iteration, so the per-function statistics count never grows and the depth check at line 442 can never trip. Your own OptimizesStrictDirectTailRecursion test (10k-deep under LimitRecursion(1)) demonstrates the mechanism. For an embedder whose only runaway-script guard is LimitRecursion, this converts a caught exception into a permanent hang. The spec requires constant stack, not that the engine forget how many tail transfers happened — a monotonic transfer counter checked against the limit (or documenting + pinning the new contract and pointing embedders at MaxStatements/TimeoutInterval) is needed before this ships.
2. Marked strict tail sites lose both inline-cache dispatch lanes: +13–24% measured on plain delegation. (JintCallExpression.cs#L248, #L283)
tailCall has no recursion requirement, so the everyday shape "use strict"; function wrapper(x) { return helper(x); } — depth 1, never deep — abandons the fast-call and register lanes for rented-array argument evaluation, a pooled TailCallRequest round-trip, and a TryPeek+ReplaceTop frame replace per call. Measured (guard benchmark, two independent A/B pairs, engagement proven via Error().stack frame replacement):
| Row (strict, depth-1, 10k calls/op) | base | PR | Δ pair 1 | Δ pair 2 |
|---|---|---|---|---|
return inner(x) |
3.155 ms | 3.904 ms | +23.7% | +12.7% |
return helper.compute(x) |
3.147 ms | 3.816 ms | +21.3% | +15.6% |
return inner(x) | 0 (non-tail control) |
3.229 ms | 3.431 ms | +6.3% | −4.2% (sign flip → envelope) |
That is ~+50–75 ns per delegation call on the shape the repo tells embedders to prefer ("prefer strict mode, which executes faster"). SunSpider/Dromaeo run sloppy engines here, so the standing gate never exercises this path — which is why your 12-script comparison saw nothing. A cheap availability heuristic (e.g. only route through the trampoline when the callee could actually recurse, or keep the register lane and let it hand back the request) would preserve the win without taxing every strict return f(x).
3. The using cleanup barrier is order-insensitive; spec-guaranteed tail calls are silently not optimized. (JintFunctionDefinition.cs#L926)
Per ECMA-262 HasCallInTailPosition, ContainsUsing counts only preceding statements — a return f(n - 1) before a using in the same block is a guaranteed proper tail call. HasUsingDeclaration checks all of the block's children, so:
"use strict";
function f(n) {
if (n !== 0) return f(n - 1); // spec: tail position; Jint: not marked
using r = acquire();
return 0;
}
f(1000000);grows the stack by a million frames (or dies on LimitRecursion) — exactly the code PTC exists to make safe. The walk needs to track position within the statement list, not mere presence.
Contract changes to decide deliberately
error.stackloses intermediate frames for strict tail calls, with no opt-out (JintCallStack.cs#L113). Spec-conformant — but it's the devtools breakage that made V8 unship PTC, and it hitsBuildCallStackHandlerconsumers and stack-based telemetry on upgrade. Worth an explicit release-note entry, and possibly an option.- The
LimitRecursionsemantics change is embedder-visible but pinned only inJint.Tests; the public-API-only tests belong inJint.Tests.PublicInterfaceper AGENTS.md (theCallStack.Countassertions stay behind, of course). - README.md line 48 still says "❌ Tail calls" — flip it in this PR.
Hardening (constructible, not observed)
- Shared-AST marker publication: markers are stamped on sibling nodes as a side effect of
BuildState, but an engine that observes the memoizedStateskipsBuildStateand readscall.UserDatawith no ordering — on ARM64 a second engine can bake_isTailPosition = falseinto its handler tree for the same shared parse-onlyPrepared<Script>the repo pins as supported. Publishing theStatewith a release store (or marking before theStatepublication with a fence) closes it. - An escaped
TailCallRequestself-identifies as genuineundefined(base(Types.Undefined), ScriptFunction.cs#L777) — any future dispatch path missing theis TailCallRequestcheck hands script a live pooled object that passes every undefined test and is later mutated byRent. A distinct internal type flag or a fail-fast in conversion paths would surface the first escape instead of corrupting silently. - No test pins the generator/async/debug gates (
suspendable is null && !_isDebugMode) — the only barriers keeping the sentinel out of the generator/async completion machinery.
Efficiency and duplication
TailCallAstVisitor.Markruns for every function including sloppy/generator/async bodies whose markers can never be consumed (JintFunctionDefinition.cs#L551). This is measurable: the Dromaeo modern unprepared rows (re-parse per op, strict class bodies) regressed +4.5%/+3.2% (Cube) and +3.1%/+2.4% (CoreEval) across two pairs, while their prepared variants stayed in the envelope. Gating on!Generator && !Async && IsStrict()skips the walk where it provably cannot matter.TailCallRequestPoolis process-wide (ConcurrentObjectPool, capacity 4) though ≤1 request is ever live per engine — a per-engine slot (theReferencePoolprecedent) drops the interlocked op and shared cache line from the tail path.ReplaceTopdoes a full Pop+Push with two statistics-dictionary round-trips per iteration even for self-recursion where the key is unchanged — and this is the same accounting that must change for blocker #1 anyway.- The guarded
if (CallStack.Count > 0) Pop()pattern now exists at ~6 sites; aJintCallStack.TryPop()keeps the reset-tolerance rule in one place. - The tail-call branch of
Constructduplicates the return-coercion block from 27 lines above (ScriptFunction.cs#L654); the 4-condition eligibility gate is copy-pasted betweenJintCallExpressionandJintTaggedTemplateExpression— both want a shared helper.
Benchmark evidence
Setup: pinned A/B worktrees — base = merge-base 20bbf87, PR = cc50eb6 — default BenchmarkDotNet jobs, .NET 10.0.10, Ryzen 9 5950X, serial runs on an idle machine; every suspect row re-run for a second pair, and only same-direction-twice movements are reported as real.
The win (depth 500 — see portability note below):
| Row | base | PR | Δ (pair 1 / pair 2) | Alloc |
|---|---|---|---|---|
| DirectTailRecursion | 135.5 µs | 92.9 µs | −31.4% / −31.9% | 104.8 KB → 15.6 KB (−85%) |
| MutualTailRecursion | 111.9 µs | 83.3 µs | −25.5% / −30.4% | 80.0 KB → 208 B (−99.7%) |
| IterativeLoop (control) | 11.24 µs | 11.21 µs | flat / flat | identical |
Clean: full 26-row SunSpider — two-sided scatter within the historical ±3–8% envelope, every suspect sign-flipped on re-run (crypto-aes +3.8% → −4.4%, date-format-tofte +5.6% → −1.1%); Dromaeo classic rows likewise; strict call micros (MethodCall/ClosureCall) mixed ±3% with ArrayPushPop +15.8% → −3.2% (bimodal row, exonerated); allocation columns byte-identical on all non-tail paths. So the per-call is TailCallRequest test and CallOnce split are below the measurable noise floor on this hardware — the two regressions above are the only reproducible ones, and both have identified mechanisms.
Portability note on the committed TailCallBenchmark: its [GlobalSetup] warms depth-1000 recursion, which on any pre-PTC build under Windows' 1 MB default stack dies with a CLR StackOverflowException — all three rows report NA on the baseline, loop control included, so the benchmark can't produce the A/B quoted in the PR description on Windows (depth 700 survives here; 1000 does not — the 2.16×/1.94× figures presumably come from an 8 MB-stack Linux environment). At depth 500 both sides run, giving the table above. Worth lowering the depth or noting the constraint in the doc comment.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 06db877c-0e12-407f-b360-69341c261890
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 06db877c-0e12-407f-b360-69341c261890
|
Addressed the review in
Validation: 4,998 Jint tests, 1,466 public-interface tests, all 35 tail-call Test262 cases, and the multi-target Release build pass. Default BenchmarkDotNet runs remain allocation-neutral for shallow strict delegation (296 B per 10,000 calls, with and without |
lahma
left a comment
There was a problem hiding this comment.
Re-ran the full gate against the same pinned merge-base (20bbf87) with the two fix commits (f264b91). Every item from the previous review is addressed — the recursion-limit hang, the using ordering, the marker publication race, the sentinel typing, the pool, the stack helpers, the tests, the README — and the register-dispatch rework pays off:
| Row (depth 500) | base | fixed PR | Δ |
|---|---|---|---|
| DirectTailRecursion | 137 µs | 88 µs | −35.8% (was −31%) |
| MutualTailRecursion | 115 µs | 84 µs | −27.0% |
Strict return inner(x) delegation |
3.37 ms | 3.76 ms | +11.7% (was +13…24%) |
Strict return helper.compute(x) |
3.41 ms | 3.80 ms | +11.5% (was +16…21%) |
The residual ~+11.5% on shallow delegation looks like the irreducible frame-replacement mechanics of spec-required PTC — I'd accept it. Dromaeo suspects re-paired within the envelope, and the recursion-limit tests all pass locally on net10.0 and net472.
One new regression slipped in with the fix, though. f264b91 added _functionDefinition.EnsureTailCallMarkers(state, _strict) to CallFromRegisters, i.e. once per warm register-lane call. EnsureTailCallMarkers contains a lock block, and methods with EH blocks are never inlined by RyuJIT — so every strict register-lane call now pays a real method call whose fast path still evaluates Function.Generator and Function.Async (interface dispatches on the AST node) plus a Volatile.Read, all to decide "already done, return". Measured twice, same direction both pairs:
| Row (strict, 1M/300k calls) | Δ pair 1 | Δ pair 2 |
|---|---|---|
FreeFunctionCall (s = f(s)) |
+9.3% | +8.4% |
ParamLocalCall (add(acc, 1)) |
+8.5% | +5.5% |
| ManyLocalCall | +4.0% | +5.2% |
| Sloppy variants | flat | flat |
Member-call rows (o.tick()) |
flip | flip (noise — different lane) |
The attribution is clean: sloppy rows short-circuit at !strict and stay flat, member calls don't route through CallFromRegisters and stay flat, and Initialize() isn't on the register path at all — only the strict free-identifier rows move, and they move together.
Suggested shape of the fix: make the guard a single inlinable field test and push everything else into a separate slow-path method —
[MethodImpl(MethodImplOptions.AggressiveInlining)]
internal void EnsureTailCallMarkers(State state, bool strict)
{
if (state.TailCallMarkersInitialized == 0 && strict)
{
EnsureTailCallMarkersSlow(state); // not inlined; owns the generator/async test and the lock
}
}with the Function.Generator/Function.Async interface reads either moved into the slow path (they only matter before the first marking) or precomputed once per definition. A plain read of TailCallMarkersInitialized is sufficient here on the fast path — the Volatile.Read/Volatile.Write pair only matters for the transition, which the slow path can own.
Two optional nits while this file is warm: the reader-side Thread.MemoryBarrier() at the end of Initialize() runs on every non-register call and is redundant under .NET's documented memory model (reference stores are release; all State field reads are dependent loads through the just-read reference, which even ARM64 orders without a barrier) — the lock plus the writer-side fence already cover the build race. And with it gone, Initialize() is back to exactly its pre-PR cost.
Numbers methodology unchanged from the previous review: pinned merge-base A/B, default BDN jobs, serial on an idle machine, second agreeing pair required before calling a movement real.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 06db877c-0e12-407f-b360-69341c261890
|
Applied the suggested hot-path split in
The focused tail-call and shared parse-only AST race suite passes (30 tests), and the multi-target Release build completes without warnings. The default BenchmarkDotNet
That is a 3.9% improvement with non-overlapping 99.9% confidence intervals, restoring the fast path while preserving marker publication safety. |
lahma
left a comment
There was a problem hiding this comment.
Re-gated after the hot-path fix: the +5-9% strict register-lane regression is gone (FreeFunctionCall flipped to its best-ever absolute, ParamLocalCall decayed to +1.9% = envelope), the recursion-limit, using-order, publication and sentinel fixes all verified with tests green on net10.0/net472, and the remaining ~+11% on strict tail-position delegation is the inherent frame-replacement cost of spec-required PTC, documented in the README. Deep tail recursion measures −27...−36% time and −85...−99.7% allocation against the merge-base. LGTM.
Full re-measure of both suites (script + interop, one session, default job, idle machine: 96 script rows and 20 interop rows) on the 4.16.0 release candidate 7b56c83. Every figure in the narrative sections is recomputed from the new reports; no number is carried forward from the 4.15.0 tables. Where the table stands now: - Jint is fastest outright on 5 of 12 scripts (minimal ~345x V8's compiled lane, evaluation-modern ~80x, linq-js ~6.7x, dromaeo-core-eval-modern ~5%, dromaeo-object-regexp-modern 1.25x ahead of V8's fresh-context lane and 1.46x ahead of its compiled lane), fastest managed engine on 10 of 12, and fastest interpreter on all 12 - V8 keeps the tight-loop rows: base64 9.8x, object-string 6.6x, stopwatch 6.0x, 3d-cube 3.4x, json-parse 2.2x, plus narrow leads on object-array (1.08x) and array-stress (1.09x) - array-stress being the one script row that changes hands, out of the rank-1 tie it held at 4.15.0 - Allocation: Jint is lowest of the managed engines on 10 of 12 scripts (Okojo on object-array, NiL.JS on minimal) and on all four interop rows, 3.9x-12.4x under the nearest managed competitor there - Interop: rank 1 on string-passing, and back into a rank-1 tie with NiL.JS on collection-traversal (1,251.0 vs 1,242.7 us, 0.7% apart); rank 2 on method-calls (NiL.JS ahead) and property-access (YantraJS by 2.6%). Plain ClearScript costs 8.6x-11.2x against Jint, FastProxy 3.4x-7.0x Adds a "What changed for 4.16.0" section: proper tail calls (#2975, which measured -15.6% time and -40.4% allocation on the Jint-only controlflow-recursive row against 4.15.3), the fast-call lane's growth (#2968, #2980, #2984), the wrapped-dictionary probe lane (#2969), and the disclosed cost of the join-hole re-read (#3003, +3.4% on hole-heavy joins). The comparison against the 4.15.0 tables is stated as directional only - the two sessions ran on .NET 10.0.10 -> 10.0.11 with YantraJS 1.2.419 -> 1.2.422 in between, so no row-for-row delta is claimed - and the two stale prose claims naming 4.15.0 outside the history sections are refreshed from this session's data. Environment: AMD Ryzen 9 5950X, .NET 10.0.11 (SDK 10.0.400), BenchmarkDotNet 0.15.8, default job, otherwise idle machine. ClearScript's V8 lanes land within ~3% of the 2026-07-28 session on the identical package. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
ECMAScript requires proper tail calls in strict code, but Jint previously let eligible recursive calls grow the .NET and JavaScript call stacks until the recursion limit was reached. This adds stack-safe interpreted tail calls and enables the corresponding Test262 feature coverage.
Implementation
LimitRecursioneffective for direct, mutual, and multi-function tail cycles without counting one-way delegation between distinct functions as recursion.usingdeclarations in source order so only preceding resources block a tail transfer; retain ordinary calls wherecatch/finally, iterator closing, or resource disposal still has deferred work.super(), async, generator, and debugger behavior.undefined.Proper tail calls intentionally replace intermediate strict-function frames. Those callers are therefore absent from
error.stackand host stack telemetry, as required by ECMAScript.Examples
Deep accumulator recursion can now run without growing the call stack:
Mutually recursive state machines benefit as well:
Fibonacci benefits when expressed in tail-recursive accumulator form:
Classic
fibonacci(n - 1) + fibonacci(n - 2)remains non-tail-recursive because addition must occur after both calls return. Automatically rewriting that form would require a different algorithm or an explicit work stack, rather than proper tail-call elimination.Validation
tail-call-optimizationtests passed.