Repository navigation
Backport #4227 to 4.x: Let a reused engine discard the host objects its warmed caches hold - #4229
Merged
Merged
Conversation
…st objects its warmed caches hold A host that reuses an engine through CaptureGlobalSnapshot/RestoreGlobalSnapshot keeps the warmed handler trees on purpose, but a warmed call site keeps its last callee and a member-read site its last receiver, and nothing cleared them. A global method built per request from an IServiceProvider therefore kept a finished request's services alive on an idle pooled engine. Engine.Advanced.DiscardInterpreterCaches() is the opt-in remedy. It drops the engine-owned handler trees (_functionDefinitions, _scriptStatementLists), releases the body tree of every definition it can reach - the cached ones, the realm's new Function cache, and functions bound on the global surface (with their parked call environment) - and empties the ReferencePool. _evaluatedScripts stays so the next run re-caches at once, and _propertyKeyExpressions stays because a suspended computed key finds its parked state by handler identity. The async concise-arrow resume delegate now captures the body handler it started on instead of re-reading the field, in place of capturing `this`, so the closure is no larger. No other evaluation path changed. 4.x adaptations: no ThrowIfRetired/EnterHostCall guard (neither exists on 4.x; the refusal matches 4.x's RestoreGlobalSnapshot), no EngineMemoryReport doc change (no memory report on 4.x), tests in xUnit v3 reading the caches by reflection as GlobalSnapshotInternalsTests does, docs in README.md and the root AGENTS.md instead of docs/guide/performance.md and the interpreter AGENTS.md, and API baselines include net462. (cherry picked from commit fa0ee13) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This was referenced Oct 7, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Backport of #4227 (
fa0ee1354) to4.x. Merge this only after #4227 has merged onmain.What it adds
Engine.Advanced.DiscardInterpreterCaches()is a new opt-in call for hosts that reuse an engine throughCaptureGlobalSnapshotandRestoreGlobalSnapshot. A warmed call site keeps its last callee, and a warmed member-read site keeps its last receiver. On a pooled engine, that kept a finished request's host objects alive. One example is a delegate closing over the request's wholeIServiceProvider, which is the Orchard Core engine-pool case that stays on 4.x.The call drops the engine-owned handler trees (
_functionDefinitions,_scriptStatementLists). It also releases the body tree of every definition it can reach:new Functioncache, including the parked_dynamicCachedEnv_envReuseFinally, it empties the
ReferencePool. Two caches stay on purpose:_evaluatedScripts, so the next run re-caches at once, and_propertyKeyExpressions, which needs handler identity for #3142. The call refuses while an evaluation is in progress, the same refusalRestoreGlobalSnapshotmakes. The design rationale and the list of what it does not reach are in #4227.The only evaluation-path change is in
JintFunctionDefinition.EvaluateConciseBodyAsync(async expression-bodied arrow). Its resume delegate now captures the body handler it started on instead ofthis, so the closure is the same size.This is additive public API, so the 4.x release that carries it should be a minor version (4.17.0), not a patch.
4.x adaptations
ThrowIfRetiredorEnterHostCall, because neither exists on 4.x. Its guard is exactly 4.x'sRestoreGlobalSnapshotguard (IsEvaluationInProgress || HasPendingAsyncOperations).EngineMemoryReport/HandlerTreeCacheReport. That XML-doc change and the main-only cache-count properties andObjectPool.PooledCountthat came along as context were dropped.ObjectPool.Clear()andReferencePool.Clear()are ported as they are.[Theory]/[InlineData], and aDisableParallelizationcollection in place of[NonParallelizable].Tasks.*becomesAdvanced.RegisterPromise/ProcessTasks. The cache counts are read by reflection, as the existingGlobalSnapshotInternalsTestsdoes, instead of through the memory report.docs/guide/performance.mdor interpreterAGENTS.md. The pooling paragraph went into the README's "Reusing a configured engine" section. The gotcha edits went into the rootAGENTS.md, minus theHandlerTreeCacheReportreference. TheRestoreGlobalSnapshotandResetCallStackXML docs are ported as they are.Evidence
Unfixed 4.x. For this run, the product change was reverted and replaced by a no-op
DiscardInterpreterCaches()stub so that the tests compile.InterpreterCacheDiscardTests: 8 of 16 fail on net10.0 and 8 of 16 fail on net472. That is everydiscard: trueretention case (6), the re-cache test and the refusal test. All 6 no-discard controls pass, which shows the retention is real on 4.x.HostInterpreterCacheDiscardTests: 2 of 2 fail on each TFM.With the fix, all of the above pass on both TFMs. As an ablation, I reverted only the concise-arrow capture:
ASuspendedAsyncArrowResumesAcrossADiscardthen fails with aNullReferenceException.Full suites on this branch, Release:
No benchmarks were run. As noted on #4227, only the async concise-arrow path changed.
🤖 Generated with Claude Code