Skip to content

Re-resolve slot locations when a shared handler tree changes function instance#2616

Merged
lahma merged 1 commit into
sebastienros:mainfrom
lahma:fix-slot-cache-shared-trees
Jul 10, 2026
Merged

Re-resolve slot locations when a shared handler tree changes function instance#2616
lahma merged 1 commit into
sebastienros:mainfrom
lahma:fix-slot-cache-shared-trees

Conversation

@lahma

@lahma lahma commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator

#2613 shares one handler tree across the fresh function instances that a re-evaluated prepared script creates — but SlotLocationCache still assumed a node lives and dies with a single function instance. The cache stored the first instance's environment, and once that environment was no longer reachable from the current chain, TryResolve returned a permanent miss: it never fell through to re-resolution.

Net effect on main today: from the second re-evaluation of a prepared script onward (i.e. the second use of the #2613-cached tree), every slot-backed fast lane in that script is silently dead — unboxed loop counters, the comparison/equality lanes, fused modulo, sum-of-products — and every identifier read in a tight loop materializes a JsNumber through the generic path. The engine-reuse embedding scenario #2613 targeted is exactly the scenario that hits it.

How it surfaced: an A/B for a new lane showed its target row allocation-unchanged on both sides. A per-evaluation allocation probe with lane fire counters showed fires = 100k on evaluations 1–2, then 0 forever, with the allocation cliff at evaluation 3; decline-site counters pointed at the left slot resolve; the permanent-miss return in TryResolve was the mechanism.

The fix

When the hit-walk exhausts without reaching the cached environment (and without meeting a shadowing binding or a with-object — those still decline this read), fall through to ResolveAndPopulate and re-cache against the current chain — exactly what the existing cross-engine Prepared<Script> path already does on an engine-identity mismatch. Cost: one bounded, pure walk per instance switch; subsequent reads hit the re-populated cache. The fall-through is also cheaper than the old behavior on the miss path itself, which re-walked and declined on every read.

Benchmarks

LoopDispatchBenchmarks (one engine re-evaluating each prepared loop; every measured iteration past the first is a re-evaluation, so main's numbers are the degraded-lane floor):

Method main fix Δ time main alloc fix alloc
EmptyLoop 4.114 ms 1.143 ms −72.2% 2.74 MB 1.63 KB
VariableBoundLoop 4.809 ms 1.336 ms −72.2% 2.74 MB 1.63 KB
CounterAdd 7.796 ms 2.058 ms −73.6% 5.48 MB 1.63 KB
LocalCopy 7.315 ms 2.917 ms −60.1% 2.74 MB 1.59 KB
StringAppend 6.984 ms 2.653 ms −62.0% 2.94 MB 206.94 KB¹
ComparisonOnly 9.250 ms 5.066 ms −45.2% 2.74 MB 1.59 KB
StrictEqualTest 8.392 ms 4.156 ms −50.5% 2.74 MB 1.59 KB
LooseEqualTest 8.406 ms 4.076 ms −51.5% 2.74 MB 1.59 KB
ModuloEqualTest 11.096 ms 5.651 ms −49.1% 2.74 MB 1.59 KB

EmptyLoop returns to ~11 ns/iteration — the floor these lanes were built to deliver; on main every row was paying a JsNumber materialization per identifier read from the second evaluation on.

¹ StringAppend's remaining allocation is the rope builder for the 100k-char result string, not lane churn.

PreparedAnomalyBenchmarks guard (the #2613 gate):

Method main fix Δ time Allocated
SourcePerOp 13.93 ms 13.74 ms −1.4% 45.25 KB (=)
PreparedShared 13.77 ms 14.08 ms +2.3%² 23.69 KB (=)
PreparedPerOp 14.21 ms 13.96 ms −1.8% 46.81 KB (=)
PreparedReusedEngine 15.64 ms 15.13 ms −3.3% 9.99 KB (=)

² Within this row's run-to-run band; allocations byte-identical across all four rows.

Tests

New SlotLocationCacheTests: correctness pins for repeated prepared evaluations and for two function instances alternating over one shared tree, plus an allocation pin (net10.0) that fails on main (626 KB/evaluation for a 20k-iteration counter loop) and passes with the fix.

Full gate: Jint/Jint.csproj all TFMs, Jint.Tests (net10.0 + net472), Jint.Tests.PublicInterface (net10.0 + net472), full Test262 (single failure = Atomics/waitAsync/bigint/no-spurious-wakeup-no-operation.js, the documented wall-clock flake family — 8/8 green re-run in isolation, 88 tests per run).

🤖 Generated with Claude Code

https://claude.ai/code/session_01BY8HoKam5H8vTPn1bMY2Hv

… instance

Per-engine definition reuse (sebastienros#2613) shares one handler tree across the fresh
function instances a re-evaluated prepared script creates. SlotLocationCache
cached the first instance's environment and returned a permanent miss once that
environment became unreachable, silently disabling every slot-backed fast lane
- unboxed loop counters, comparison/equality lanes, sum-of-products - from the
second re-evaluation onward (a JsNumber materialization per identifier read in
tight loops). On an unreachable cached environment, fall through and re-resolve
against the current chain exactly like the cross-engine Prepared<Script> path;
one bounded walk per instance switch, subsequent reads hit the re-populated
cache.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BY8HoKam5H8vTPn1bMY2Hv
@lahma
lahma force-pushed the fix-slot-cache-shared-trees branch from 4869229 to 4f23f0f Compare July 10, 2026 06:55
@lahma
lahma merged commit c68ffcf into sebastienros:main Jul 10, 2026
4 checks passed
@lahma
lahma deleted the fix-slot-cache-shared-trees branch July 10, 2026 11:09
lahma added a commit that referenced this pull request Jul 10, 2026
Full re-run after the 2026-07-10 batch (#2613, #2615-#2620). Highlights vs
the 2026-07-09 refresh: dromaeo-string-base64 allocation -30% (2.30 -> 1.62
MB/op) and -modern -4.3% time, the classic row now ranked first outright
(previously tied with NiL.JS) - loop tests of the form i < s.length read the
length unboxed through the comparison lane (#2617). The batch's engine-reuse
wins (#2613/#2615 definition reuse, #2616 slot-cache lane restoration) target
long-lived-engine embeddings and are not visible in this fresh-engine-per-op
table. Everything else moved within documented noise bands; competitor rows
served as thermal canaries (times within +/-3%, allocations byte-identical).


Claude-Session: https://claude.ai/code/session_01BY8HoKam5H8vTPn1bMY2Hv

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
lahma added a commit that referenced this pull request Jul 10, 2026
… epoch (#2625)

Global reads from nested lexical scopes (loop bodies, closures) validate
through TryGetValidatedGlobalDescriptorNested, which re-walks up to four
environments with per-hop shadow probes on EVERY read - the dominant
cost difference between the stopwatch-modern and classic variants, whose
most-referenced binding (sw) is a global reached from two levels of loop
scope, ~3 reads x 391k iterations per op.

The walk's outcome is now memoized per identifier node: the start
environment and every intermediate link are pinned by identity, and a
repeat read from the identical chain revalidates with reference
compares instead of HasBinding scans. Chain-link identity (not just the
start) is required because pooled loop environments are re-attached
under different outers across entries. The one mutation identity cannot
see - a binding injected into a pre-existing pinned environment - is
covered by a new Engine._envBindingInjectionEpoch, bumped only at the
injection sites: sloppy direct eval instantiation and the AnnexB
block-function var-scope copies. Creates into fresh environments never
bump (a fresh environment cannot appear in a previously pinned chain).
The existing global shape/lexical version checks run before the memo
and cover all global-side mutations; a miss falls through to the walk
and re-publishes, never permanently declining (the #2616 pattern).
Serves reads, writes and the unboxed numeric lanes through the shared
validator.


Claude-Session: https://claude.ai/code/session_01BY8HoKam5H8vTPn1bMY2Hv

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