Skip to content

Pool FunctionEnvironment per JintFunctionDefinition.State#2413

Merged
lahma merged 1 commit into
sebastienros:mainfrom
lahma:perf/function-env-pool
Apr 25, 2026
Merged

Pool FunctionEnvironment per JintFunctionDefinition.State#2413
lahma merged 1 commit into
sebastienros:mainfrom
lahma:perf/function-env-pool

Conversation

@lahma

@lahma lahma commented Apr 25, 2026

Copy link
Copy Markdown
Collaborator

Summary

Slot arrays were already pooled per State, but the FunctionEnvironment object itself was allocated fresh on every call. For functions whose bindings cannot escape — no closure capture, not a generator/async, which the existing EnvironmentMayEscape flag already tracks — the env is trivially safe to reuse: at end-of-call we reset _thisValue, _thisBindingStatus, _outerEnv, _functionObject, NewTarget, and clear _dictionary / _disposeCapability before returning it to the pool.

The slot-pooling gate widens from {UseFixedSlots: true, EnvironmentMayEscape: false} to {EnvironmentMayEscape: false} alone — slot-less envs (zero params/vars, common for tight inner closures like sw.Start() in the benchmark) now also pool, which is where the bulk of the allocation reduction comes from.

Implementation notes

  • New FunctionEnvironment.Reset(...) method centralizes the per-call cleanup (this-binding, outer env, function ref, dispose capability, dictionary clear). Slot reset is left to the caller, mirroring the existing slot-pool pattern.
  • _functionObject becomes mutable so a pooled env can be re-bound to a different Function instance that shares the same State (function literals re-evaluated in loops produce distinct Function instances over the same AST).
  • Pool storage is a single-slot cache on JintFunctionDefinition.State._cachedEnv, mirroring _cachedSlots. Interlocked.Exchange and ReferenceEquals(_engine, engine) are used the same way.
  • Construct path is unchanged — its env is consumed (GetThisBinding) after LeaveExecutionContext, so it can't be returned to the pool until that's restructured.

Benchmark (stopwatch.js, clean machine)

Ryzen 9 5950X / .NET 10.0.7. Both runs back-to-back on an idle machine; std-dev is ≤1.6ms.

Method Modern main PR Δ time Δ alloc
Execute var 197.4 ms 197.5 ms flat -47%
Execute_ParsedScript var 222.5 ms 215.3 ms -3.2% -47%
Execute let 255.3 ms 255.5 ms flat -47%
Execute_ParsedScript let 264.2 ms 241.8 ms -8.5% -47%
Allocated all ~72.3 MB ~38.5 MB -46.7%

The headline number is the allocation reduction: 72 MB → 38.5 MB on the same workload (391k function calls). Time is mostly flat on the var (re-parsed each iteration) cases and improves 3-8% on the parsed-script (cached AST, warm pool) cases — which is the realistic deployment pattern when reusing Prepared<Script> across executions.

Validation

  • Jint.Tests (net10.0): 2861 passed, 0 failed.
  • Jint.Tests.Test262 (net10.0): 98567 passed, 506 skipped, 0 failed (matches upstream/main).

The pool-return is gated on state.EnvironmentMayEscape == false, so closures that capture the env stay un-pooled (otherwise we'd corrupt their captured reference). Generators and async functions also force EnvironmentMayEscape = true and never enter the pool. The existing Interlocked + engine-identity checks make cross-engine reuse safe.

Stacked work

This is the second of six related performance changes; the rest sit on lahma/jint as perf/identifier-slot-cache, perf/drop-interlocked, perf/skip-empty-callee-env, perf/block-env-reset, all rebased on top of this branch and ready to PR after this lands.

🤖 Generated with Claude Code

@lahma
lahma enabled auto-merge (squash) April 25, 2026 19:40
@lahma
lahma disabled auto-merge April 25, 2026 19:40
Slot arrays were already pooled per State, but the FunctionEnvironment
object itself was allocated fresh on every call. For functions whose
bindings cannot escape (no closure capture, not a generator/async — the
existing EnvironmentMayEscape flag already tracks this), the env is
trivially safe to reuse: at end-of-call we reset _thisValue,
_thisBindingStatus, _outerEnv, _functionObject, NewTarget, and clear
_dictionary / _disposeCapability before returning it to the pool.

Mirror existing slot pool: per-State single-slot via Interlocked.Exchange,
gated on engine identity to avoid cross-engine reuse. The slot pooling
gate widens from {UseFixedSlots: true, EnvironmentMayEscape: false} to
{EnvironmentMayEscape: false} alone — slot-less envs (zero params/vars,
common for tight closures like Stopwatch.Start) now also pool.

stopwatch benchmark (preliminary, machine under contention):
  Execute (var):     260 ms -> 248 ms,  72 MB -> 38 MB  (-47% alloc)
  Execute (let):     335 ms -> 308 ms,  72 MB -> 38 MB  (-47% alloc)

98567 test262 + 2861 Jint.Tests pass; 0 regressions.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@lahma
lahma force-pushed the perf/function-env-pool branch from f6c146f to d7c831c Compare April 25, 2026 19:50
@lahma
lahma enabled auto-merge (squash) April 25, 2026 19:52
@lahma
lahma merged commit b481f33 into sebastienros:main Apr 25, 2026
4 checks passed
@lahma
lahma deleted the perf/function-env-pool branch April 25, 2026 23:41
lahma added a commit to lahma/jint that referenced this pull request May 3, 2026
The pool added by sebastienros#2413 has a single slot per JintFunctionDefinition.State
guarded by Interlocked.Exchange. For tight self-recursion (ack/fib/tak), only
the topmost frame can ever be cached — every recursive call beyond that
allocates anyway, so the 4 atomic ops per call become pure overhead.

Detect direct named self-call at AST analysis (SelfCallAstVisitor walks the
function body for CallExpression with callee Identifier matching the
function's own name) and gate the pool path on the new IsDirectRecursive
flag in JintEnvironment.NewFunctionEnvironment and ScriptFunction.Call.

SunSpider controlflow-recursive: 63.56 ms -> 62.28 ms on a noisy
measurement (separate isolated run measured 60.79 ms; v4.8.0 baseline
58.71 ms). Trade-off: allocations on the affected functions return to
fresh-per-call, since the pool is intentionally bypassed.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
lahma added a commit that referenced this pull request May 3, 2026
… read alloc (#2450)

* Skip FunctionEnvironment pool for direct-recursive functions

The pool added by #2413 has a single slot per JintFunctionDefinition.State
guarded by Interlocked.Exchange. For tight self-recursion (ack/fib/tak), only
the topmost frame can ever be cached — every recursive call beyond that
allocates anyway, so the 4 atomic ops per call become pure overhead.

Detect direct named self-call at AST analysis (SelfCallAstVisitor walks the
function body for CallExpression with callee Identifier matching the
function's own name) and gate the pool path on the new IsDirectRecursive
flag in JintEnvironment.NewFunctionEnvironment and ScriptFunction.Call.

SunSpider controlflow-recursive: 63.56 ms -> 62.28 ms on a noisy
measurement (separate isolated run measured 60.79 ms; v4.8.0 baseline
58.71 ms). Trade-off: allocations on the affected functions return to
fresh-per-call, since the pool is intentionally bypassed.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* Avoid StringInstance alloc per primitive property read

PR #2412's inline-cache fast path added `baseValue.GetV(...)` for
non-ObjectInstance receivers. For JsString primitives this routes through
TypeConverter.ToObject, which calls intrinsics.String.Construct(jsString)
and allocates a fresh StringInstance per read. Hot path: any
`for (var i = 0; i < s.length; i++)` re-reads `s.length` every iteration —
the slot the script picks for the loop bound doesn't help.

Mirror Engine.TryHandleStringValue's primitive-aware lookup inline:
length is intrinsic, everything else lives on String.prototype and accepts
the primitive as `this` without wrapping.

Dromaeo StringBase64 allocations: ~6.12 MB -> ~3.62 MB across all 4 cases
(byte-for-byte match with v4.8.0). SunSpider string-base64: 8.83 MB ->
5.07 MB. Knock-on alloc reductions of 4-14% on crypto-md5/sha1,
string-fasta, string-validate-input, date-format-xparb, ObjectRegExp.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* Use AddDangerous for hand-rolled additions in source-gen prototypes

Mirrors the pattern in GlobalObject.Properties.cs: after
CreateProperties_Generated() returns, _properties is the HybridDictionary
wrapper around the generator-built StringDictionarySlim. AddDangerous
skips both duplicate-key probing and SetOwnProperty's validation pipeline.

For the four prototypes that add known-new keys after the generator runs —
StringPrototype (trimLeft/trimRight aliases), DatePrototype (toGMTString
alias), RegExpPrototype (exec — hand-registered to keep
HasDefaultRegExpExec's reference identity), IntrinsicTypedArrayPrototype
(toString alias to Array.prototype.toString) — switch to
_properties!.AddDangerous(...) and presize the dict via
[JsObject(ExtraCapacity = N)] so the additions don't trigger a resize.

Symbol-key additions stay as SetOwnProperty since they go to the separate
_symbols dict (DictionarySlim) which has no AddDangerous.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
lahma added a commit to lahma/jint that referenced this pull request Jul 3, 2026
… instance (sebastienros#2560)

Since sebastienros#2413 a call's FunctionEnvironment is pooled on the shared
JintFunctionDefinition.State. An environment roots its creating engine
(Environment._engine plus its outer chain to the global), and a prepared
script's State is shared by every engine that runs it — so the pool slot
pins the last-caller engine (and its realm) until the function is next
invoked by any engine. For idle functions in long-lived cached prepared
scripts the engine is pinned indefinitely (issue sebastienros#2560, secondary cause).

Move the pool (single cached env, cached slot array, and the bounded
recursive pool from sebastienros#2549) onto the ScriptFunction instance. Function
instances are created per engine, so the pool is per-engine by construction
and dies with the engine — while same-instance call reuse and recursive
frame reuse, the workloads the pools exist for, are unchanged. Because a
function instance is single-threaded like its engine, the Interlocked
handshakes and cross-engine identity checks the shared-State pool needed
are no longer necessary.

Includes a WeakReference regression test: 20 throwaway engines running one
shared prepared script must all be collectable (fails before this change —
the last engine stayed pinned).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bf1LESD4X8dMNPwLKJCAh9
lahma added a commit to lahma/jint that referenced this pull request Jul 4, 2026
… instance (sebastienros#2560)

Since sebastienros#2413 a call's FunctionEnvironment is pooled on the shared
JintFunctionDefinition.State. An environment roots its creating engine
(Environment._engine plus its outer chain to the global), and a prepared
script's State is shared by every engine that runs it — so the pool slot
pins the last-caller engine (and its realm) until the function is next
invoked by any engine. For idle functions in long-lived cached prepared
scripts the engine is pinned indefinitely (issue sebastienros#2560, secondary cause).

Move the pool (single cached env, cached slot array, and the bounded
recursive pool from sebastienros#2549) onto the ScriptFunction instance. Function
instances are created per engine, so the pool is per-engine by construction
and dies with the engine — while same-instance call reuse and recursive
frame reuse, the workloads the pools exist for, are unchanged. Because a
function instance is single-threaded like its engine, the Interlocked
handshakes and cross-engine identity checks the shared-State pool needed
are no longer necessary.

Includes a WeakReference regression test: 20 throwaway engines running one
shared prepared script must all be collectable (fails before this change —
the last engine stayed pinned).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bf1LESD4X8dMNPwLKJCAh9
lahma added a commit to lahma/jint that referenced this pull request Jul 4, 2026
… instance (sebastienros#2560)

Since sebastienros#2413 a call's FunctionEnvironment is pooled on the shared
JintFunctionDefinition.State. An environment roots its creating engine
(Environment._engine plus its outer chain to the global), and a prepared
script's State is shared by every engine that runs it — so the pool slot
pins the last-caller engine (and its realm) until the function is next
invoked by any engine. For idle functions in long-lived cached prepared
scripts the engine is pinned indefinitely (issue sebastienros#2560, secondary cause).

Move the pool (single cached env, cached slot array, and the bounded
recursive pool from sebastienros#2549) onto the ScriptFunction instance. Function
instances are created per engine, so the pool is per-engine by construction
and dies with the engine — while same-instance call reuse and recursive
frame reuse, the workloads the pools exist for, are unchanged. Because a
function instance is single-threaded like its engine, the Interlocked
handshakes and cross-engine identity checks the shared-State pool needed
are no longer necessary.

Includes a WeakReference regression test: 20 throwaway engines running one
shared prepared script must all be collectable (fails before this change —
the last engine stayed pinned).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bf1LESD4X8dMNPwLKJCAh9
lahma added a commit that referenced this pull request Jul 4, 2026
…; computed-key and escape-analysis fixes (#2568)

* Run deep call-stack test on a dedicated large-stack thread

ShouldAllowReasonableCallStackDepth nests 990 calls (Release) on a default
engine, which has no stack guard (MaxExecutionStackCount is disabled), so
the depth runs directly against the native stack. The xUnit worker thread's
remaining stack varies per run, making the test crash the whole process
intermittently with STACK_OVERFLOW (0xC00000FD) — reproducible on unmodified
main on Windows/net10.0.

Run the body on a dedicated thread with an explicit 16 MB stack so the
available stack is deterministic.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bf1LESD4X8dMNPwLKJCAh9

* Move the function-env reuse pool off the shared State to the function instance (#2560)

Since #2413 a call's FunctionEnvironment is pooled on the shared
JintFunctionDefinition.State. An environment roots its creating engine
(Environment._engine plus its outer chain to the global), and a prepared
script's State is shared by every engine that runs it — so the pool slot
pins the last-caller engine (and its realm) until the function is next
invoked by any engine. For idle functions in long-lived cached prepared
scripts the engine is pinned indefinitely (issue #2560, secondary cause).

Move the pool (single cached env, cached slot array, and the bounded
recursive pool from #2549) onto the ScriptFunction instance. Function
instances are created per engine, so the pool is per-engine by construction
and dies with the engine — while same-instance call reuse and recursive
frame reuse, the workloads the pools exist for, are unchanged. Because a
function instance is single-threaded like its engine, the Interlocked
handshakes and cross-engine identity checks the shared-State pool needed
are no longer necessary.

Includes a WeakReference regression test: 20 throwaway engines running one
shared prepared script must all be collectable (fails before this change —
the last engine stayed pinned).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bf1LESD4X8dMNPwLKJCAh9

* Keep slot arrays shared on State; cache the env directly on the instance

Refine the per-instance env cache based on A/B benchmarking:

- Cleared fixed-slot Binding[] hold no engine references, so restore them to
  the shared State cache (exactly as before) — freshly created instances of
  the same function (e.g. a prepared script re-evaluated, or a function
  expression re-created per loop iteration) reuse the slot array again.
- Store the cached FunctionEnvironment directly in a single object field on
  ScriptFunction, interpreted via State.IsDirectRecursive (direct-recursive
  functions store the bounded RecursiveEnvPool there instead). This drops
  the pool wrapper allocation and keeps the instance-size cost at one field.

vs main: recursion and closure benchmarks are allocation-identical and
3-13% faster (the env-side Interlocked handshake is gone); the remaining
deltas are the leak fix itself — one env allocation per re-created-and-
called function instance (~0.1 KB) and 8 bytes per ScriptFunction.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bf1LESD4X8dMNPwLKJCAh9

* Apply code-review findings: scan parameter expressions in escape analysis

The environment-escape analysis (EnvironmentEscapeAstVisitor.MayEscape)
scanned only the function body, so a closure created in a parameter default
expression escaped undetected and the call's environment was cached for
reuse while still reachable from the closure — the next call's Reset then
rebound the bindings the escaped closure resolves through:

  function f(a, get = function() { return a; }) { return get; }
  var g1 = f(1); f(2);
  g1(); // returned 2 instead of 1 (pre-existing before this branch too)

Scan function.Params as well; the slot-aware MayEscapeWithReferences path
needs no change because fixed slots require !HasParameterExpressions.

Also from the review:
- RecursiveEnvPool: plain LIFO stack pointer instead of the O(16) slot scans
  inherited from the lock-free shared-State pool (single-threaded now);
  fib -9.5% vs main (was -5.0%)
- GC engine-retention test: cover the direct-recursive pool path too, and
  NoInlining on the helper so the engine can't be stack-rooted; document
  that block/loop environments cached on shared AST state can still pin the
  last-caller engine (pre-existing, tracked separately)
- EngineLimitTests: 4 MB dedicated stack instead of 16 MB so a large
  per-frame native-stack regression still fails the test, and drop the
  macOS skip (the explicit stack is platform-independent)
- hoist the duplicate (ScriptFunction) cast; fix stale comments describing
  the deleted shared-State pool

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bf1LESD4X8dMNPwLKJCAh9

* Apply second-review findings: this/new.target capture, construct rent gate

- ClosureReferencesAny (the slot-aware escape analysis) only matched
  identifiers against slot names, so an escaping closure whose only lexical
  dependency is `this`/`new.target`/`super` was classified non-capturing and
  its FunctionEnvironment was cached and rebound by the next call:

    function f(a) { var g = function() { return 1; }; var h = () => this; return h; }
    var h1 = f.call({name:'o1'}, 1); f.call({name:'o2'}, 2);
    h1().name; // returned 'o2' — also reproducible on main with the old State cache

  The environment carries the this-binding, new.target and super base, so
  any such node inside a closure now counts as an environment reference
  (conservative). Regression tests for the this and new.target variants.

- The construct path rents from the reuse cache via PrepareForOrdinaryCall
  but never returns its environment, so every `new f()` drained the cache
  and starved the next call's reuse (upstream parity). Rent only for plain
  calls (newTarget undefined).

- EngineLimitTests: back to a generous 16 MB dedicated stack —
  StackOverflowException is uncatchable and would kill the whole test
  process on platforms with larger native frames; the test is a functional
  smoke test of deep call chains, not a per-frame stack-budget test.

Benchmarks after these fixes (vs main): Fib -10.3%, ParamLocalCall -12.7%,
closures -6..7%, Warm_* -3..6%, allocations unchanged.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bf1LESD4X8dMNPwLKJCAh9

* Cache the block environment on the statement handler, not the shared BlockState

Same #2560 engine-retention family as the function-env cache: BlockState
lives on the AST node's UserData and is shared by every engine running a
prepared script, and the DeclarativeEnvironment cached on it roots its
creating engine — so any prepared script containing a let/const block kept
the last-caller engine (and its realm) alive. Reproduced: 1 of 20 throwaway
engines stayed pinned.

Move the cache to the JintBlockStatement handler instance, which is built
per statement list and therefore per engine; the Interlocked handshake and
engine-identity check become unnecessary. BlockState keeps only the
immutable, engine-agnostic data (declarations, slot names/templates).

JintForStatement._cachedLoopEnv needed no change: statement handlers are
never cached on AST UserData (only BlockState is), so the loop-env cache
was already per engine — confirmed by the extended regression test, which
covers an ordinary function, a direct-recursive one, a let/const block and
a for-let loop and requires all 20 engines to be collectable.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bf1LESD4X8dMNPwLKJCAh9

* Evaluate any expression type in computed property keys

AstExtensions.TryGetComputedPropertyKey had a node-type allowlist with a
silent JsValue.Undefined fallback: computed keys whose expression type was
missing from the list (SequenceExpression, NewExpression,
TaggedTemplateExpression, ChainExpression, ClassExpression, ...) bound the
property "undefined" and never evaluated the key expression at all — side
effects were silently skipped:

  var calls = 0;
  function fb(x, { [(calls++, "k")]: v }) { return v; }
  fb(1, { k: 6 }); // returned undefined with calls === 0

Every expression type is buildable by JintExpression.Build and both call
sites already handle abrupt completions and generator suspension after
evaluating the key, so drop the allowlist and always evaluate.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bf1LESD4X8dMNPwLKJCAh9

* Apply third-review findings: evaluate decorated computed keys exactly once

- Decorator bookkeeping re-called GetKey after the class element was already
  defined, re-running computed key side effects and — when the key changed
  between evaluations — silently applying the decorator to the wrong
  (nonexistent) property. Thread the defined key out of
  MethodDefinitionEvaluation/ClassElementEvaluation, reuse
  ClassFieldDefinition.Name for fields and carry the declared key in
  AutoAccessorResult (FieldDefinition.Name is the private backing name).
  Pre-existing for previously-allowlisted key types; became more visible
  once computed keys evaluate all expression types.

- Object literals: check for generator suspension after evaluating a
  computed method key (mirrors the non-method path) so the build loop
  doesn't keep executing properties while the generator is suspended.

- Block env cache: park only after leaving the block, and reset slots +
  detach the outer chain at park time so the cached env doesn't root the
  completed call's scope chain and last binding values until reuse.

- Remove the write-only BlockState.CanReuseEnvironment; document the
  handlers-must-not-be-shared invariant at JintStatement.Build, where a
  future UserData caching change would violate it.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bf1LESD4X8dMNPwLKJCAh9

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant