Skip to content

Definition-level environment reuse for Function-constructor instances#2579

Merged
lahma merged 1 commit into
sebastienros:mainfrom
lahma:fix-dynamic-function-env-reuse
Jul 4, 2026
Merged

Definition-level environment reuse for Function-constructor instances#2579
lahma merged 1 commit into
sebastienros:mainfrom
lahma:fix-dynamic-function-env-reuse

Conversation

@lahma

@lahma lahma commented Jul 4, 2026

Copy link
Copy Markdown
Collaborator

Fixes the regression bisected in #2568 (comment): repeated new Function(...) on one engine went 1.05 → 1.39 ms (+32%) because every call creates a fresh ScriptFunction, so the per-instance _envReuse cache introduced there never warms — and worse, the shared statement tree's per-node slot caches (identifier reads, unboxed update/compound/comparison lanes) keep pointing at the previous instance's dead environment and miss on every iteration.

What this does

Function-constructor definitions get definition-level environment reuse: JintFunctionDefinition.IsDynamic marks definitions created by CreateDynamicFunction, and their call environment parks on State._dynamicCachedEnv instead of the instance (outer env nulled while parked; engine-identity checked on rent; only pool-eligible calls park, same gates as before).

This is a deliberate, documented exception to #2568's "no environments on State" rule, and it cannot reintroduce the engine-retention problem that rule solves: a dynamic definition lives in the per-realm dynamic-function cache (Realm._dynamicFunctionCache) and its AST is parsed per realm — unlike prepared scripts, the definition and its State are never shared across engines, so the parked environment can only root its own engine. Stable environment identity across the one-shot instances is exactly what the per-node slot caches key on.

Measurements (same-window stash A/B, BenchmarkDotNet default job)

Row regressed main fixed Δ
NewFunctionHotReusedEngine 1,487.9 µs 777.7 µs −47.7%
PlainFunctionLoopReference (structural floor) 778.8 µs 776.8 µs dynamic hot path now equals it
NewFunctionFreshEngine 810.7 µs 803.2 µs flat
Eval rows / ParseOnly flat

The fixed number beats even the pre-#2568 1.05 ms: with stable env identity the newer unboxed lanes stay warm across instances, which the old per-State pooling would also have delivered.

Gates

🤖 Generated with Claude Code

Repeated new Function(...) on one engine regressed 1.05 -> 1.39 ms
(+32%, bisected to the per-instance environment reuse change): every
call creates a fresh ScriptFunction, so an instance-level cache never
warms - and the shared statement tree''s per-node slot caches keep
pointing at the previous instance''s dead environment, missing on every
iteration.

JintFunctionDefinition.IsDynamic marks definitions created by
CreateDynamicFunction; their call environments park on
State._dynamicCachedEnv instead of the instance (outer env nulled while
parked, engine identity checked on rent, same pool-eligibility gates).
This is a documented exception to the no-environments-on-State rule
and cannot reintroduce the engine retention that rule solves: dynamic
definitions live in the per-realm dynamic-function cache and their ASTs
are parsed per realm, so the definition and its State are never shared
across engines - the parked environment can only root its own engine.
Stable environment identity across one-shot instances is exactly what
the per-node slot caches key on.

Same-window stash A/B: NewFunctionHotReusedEngine 1,487.9 -> 777.7 us
(-47.7%), now equal to the plain-function structural floor (776.8 us)
and faster than before the regression (1.05 ms) because the newer
unboxed lanes stay warm across instances; fresh-engine and eval rows
flat. New DynamicFunctionTests cover fresh bindings per call on the
shared environment, closure-capturing bodies staying independent
(never pooled), and interleaved instances of one source. Jint.Tests
3210/3148 (including the engine-retention tests), PublicInterface
82/82, CommonScripts 28/28, all-TFM build; Test262 99,256 with the four
known annexB RegExp-escape-BMP load-flakes timing out under full-suite
parallelism only (pass in 5 s isolated on this change).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@lahma
lahma enabled auto-merge (squash) July 4, 2026 20:34
@lahma
lahma merged commit 664d7f8 into sebastienros:main Jul 4, 2026
3 of 4 checks passed
@lahma
lahma deleted the fix-dynamic-function-env-reuse branch July 5, 2026 06:26
lahma added a commit to lahma/jint that referenced this pull request Jul 5, 2026
Full-suite EngineComparison re-run for the 4.11.0 release, on the branch
rebased onto main @ 664d7f8 (includes sebastienros#2579). Competitor versions are
unchanged (Jurassic 3.2.9, NiL.JS 2.6.1722, YantraJS.Core 1.2.406), so
their rows act as a thermal canary confirming the window is comparable
to the previous refresh.

No regressions: every Jint row is flat or faster than the previous
refresh, with allocation flat or lower. Headline wins from the loop-lane
and environment work now in main: dromaeo-core-eval -27% (2.10 -> 1.53 ms
prepared), dromaeo-3d-cube -13..15% (allocation also -16%), stopwatch
-10%, dromaeo-string-base64 -5..12%. The core-eval gap to the closest
competitor narrows from ~1.7x to ~1.3x and 3d-cube from ~1.4x to ~1.3x.

Also retunes the prose to frame results against "the closest competitor"
rather than singling out engines by name, and refers to Jint as 4.11.0.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
lahma added a commit that referenced this pull request Jul 6, 2026
#2594)

Analysis of #2587 confirmed that everything a prepared script retains
after engine disposal is engine-independent prepare-time metadata by
design (constant-folded primitives, FDI State, block declaration
caches, binding names) and that no Engine/Realm/Environment can be
reached from the shared AST.

The regression test guarding that invariant predates two environment
cache shapes added since: the for-of/for-in per-iteration environment
cache on the JintForInForOfStatement handler (#2586) and the
Function-constructor definition-level environment parked on the
realm-cached dynamic State (#2579). Cover both so a future refactor
moving either onto shared AST state fails the test.

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