Skip to content

Fix prepared script performance regression from shared identifier cache thrashing#2324

Merged
lahma merged 2 commits into
sebastienros:mainfrom
lahma:prepared-regression
Mar 11, 2026
Merged

Fix prepared script performance regression from shared identifier cache thrashing#2324
lahma merged 2 commits into
sebastienros:mainfrom
lahma:prepared-regression

Conversation

@lahma

@lahma lahma commented Mar 11, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Fix performance regression where Engine.PrepareScript() + Execute(Prepared<Script>) was slower than executing from a raw string
  • Root cause: shared mutable JintIdentifierExpression instances caused environment cache thrashing across scopes and engine instances
  • Add PreparedScriptBenchmark that replicates the exact JavaScriptEngineSwitcher JsExecutionHeavyBenchmark pattern: Handlebars template rendering on 4 separate engine instances

Problem

During PrepareScript(), the AstAnalyzer stored fully-constructed JintIdentifierExpression instances in each Identifier node's UserData. The JintExpression.Build() fast path returned these cached instances directly, meaning all references to the same identifier — across different scopes and different engine instances — shared a single JintIdentifierExpression with its mutable _cachedEnvironment field.

When the same identifier appeared in multiple scopes (e.g. x in functionA and x in functionB), each scope's execution overwrote _cachedEnvironment, causing the fast-path lookup (ReferenceEquals(env, _cachedEnvironment)) to always miss. This turned the O(1) cached lookup into an O(n) scope chain walk on every access.

The regression was visible in the JavaScriptEngineSwitcher benchmark where prepared Jint was 45% slower than unprepared (251ms vs 173ms), while every other engine showed preparation as faster:

Engine withPrecompilation=False withPrecompilation=True Delta
Jint (before fix) 172.53 ms 251.02 ms +45% slower
ChakraCore 424.24 ms 366.05 ms -14% faster
Jurassic 5,384.26 ms 1,578.40 ms -71% faster
V8 146.61 ms 131.12 ms -11% faster

Fix

Store only the immutable BindingName in UserData instead of the full JintIdentifierExpression. Since BindingName is not a JintExpression, the Build() shortcut no longer matches — each engine creates fresh JintIdentifierExpression instances with independent environment caches. The pre-computed BindingName (containing the cached JsString and Key) is still reused from UserData, preserving the allocation benefits of preparation.

Also fixes a minor existing bug where the single-parameter JintIdentifierExpression constructor created two redundant BindingName instances (one in the this(...) chain, one in the body).

PreparedScriptBenchmark (Handlebars template rendering, 4 engines)

This benchmark replicates the exact JavaScriptEngineSwitcher pattern: load handlebars.js, then call renderTemplate() across 4 engine instances with different content items.

Before fix (baseline):

Method Mean Allocated
ExecuteStringOnMultipleEngines 96.40 ms 34.92 MB
ExecutePreparedOnMultipleEngines 62.92 ms 18.44 MB

After fix:

Method Mean Allocated
ExecuteStringOnMultipleEngines 103.66 ms 34.08 MB
ExecutePreparedOnMultipleEngines 51.48 ms 18.91 MB

The prepared path improved from 62.92 ms → 51.48 ms (18% faster) due to the elimination of environment cache thrashing. The prepared-to-string ratio improved from 0.69x to 0.52x.

Verification

  • Jint.Tests: 0 failures (2,757 passed on net10.0, 2,710 on net472)
  • Test262: 0 failures (92,896 passed)
  • DromaeoBenchmark: No performance regressions across all 24 test configurations

Test plan

  • dotnet test --configuration Release Jint.Tests/Jint.Tests.csproj — all pass
  • dotnet test --configuration Release Jint.Tests.Test262/Jint.Tests.Test262.csproj — all pass
  • PreparedScriptBenchmark — prepared path 18% faster after fix
  • DromaeoBenchmark — no regressions in any configuration
  • Verify with JavaScriptEngineSwitcher benchmark that prepared Jint is now faster than unprepared

🤖 Generated with Claude Code

lahma and others added 2 commits March 11, 2026 21:34
…ifierExpression cache thrashing

When using Engine.PrepareScript(), the AstAnalyzer stored JintIdentifierExpression instances
in AST node UserData. Since JintExpression.Build() returns these cached instances directly,
ALL references to the same identifier across all scopes and all engine instances shared a single
JintIdentifierExpression with its mutable _cachedEnvironment field. Different scopes continuously
overwrote each other's cached environment, causing the fast-path lookup to always miss.

The fix stores only the immutable BindingName in UserData instead. Each engine execution now
creates fresh JintIdentifierExpression instances (with independent environment caches) while
still reusing the pre-computed BindingName to avoid repeated JsString/BindingName allocation.

This also fixes a minor bug in the single-parameter JintIdentifierExpression constructor that
created two redundant BindingName instances (one in the this() chain, one in the body).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…pattern

Use the exact Handlebars template-rendering workload from JsExecutionHeavyBenchmark:
4 engine instances each loading the library and calling renderTemplate() with
different content items (hello-world, contacts, js-engines, web-browser-family-tree).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@lahma
lahma merged commit 8e44385 into sebastienros:main Mar 11, 2026
7 of 8 checks passed
@lahma
lahma deleted the prepared-regression branch March 11, 2026 20:20
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