Two of the four caches cited as precedent no longer make the point - #3570
Merged
lahma merged 1 commit intoSep 1, 2026
Merged
Conversation
…no longer fits The comment above `MethodDescriptor`'s shared invoker tables justifies pinning a `MethodBase` for the life of the process by naming four caches that already do it. Two of the four no longer say what it needs. `TypeReference._memberAccessors` does not exist and has not for some time — `TypeReference`'s only static is its `JsString` name, and its static members go through `TypeResolver.GetStaticAccessor`, whose `StaticAccessorCacheKey` carries the `InteropResolutionProfile` and passes the same `IsShareable` / `IsConverterNeutral` gates as the instance lane. A reader following the citation finds nothing. `JintBinaryExpression._knownOperators` is still there but is now the wrong example: since sebastienros#3526 it is the process-wide half of a pair, and an engine with its own `ClrTypeConverter` gets a table of its own precisely so that a process-lived static does not pin that converter and whatever engine it captured. Citing it as precedent for "pin it for the process" points at the case that decided not to. Replaced with two that hold today and are the same shape as the tables being justified — `CompiledMemberAccessor`'s four `MemberInfo`-keyed delegate tables, and `ReflectionExtensions._operatorOverloadMethodCache`, a `Type`-keyed method list that is the pure-metadata part `_knownOperators` sits on top of. Comment-only; no code change. Found while auditing process-wide caches for sebastienros#3426. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014W5mbjGhyvgAS4pivXoc4S
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
1 participant
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.
The comment above
MethodDescriptor's shared invoker tables justifies pinning aMethodBasefor the life of the process by naming four caches that already do it. Two of the four no longer say what it needs.TypeReference._memberAccessorsdoes not exist.TypeReference's only static is itsJsStringname; its static members go throughTypeResolver.GetStaticAccessor, whoseStaticAccessorCacheKeycarries theInteropResolutionProfileand which passes the sameIsShareable/IsConverterNeutralgates as the instance lane. A reader following the citation finds nothing at all.JintBinaryExpression._knownOperatorsis still there but is now the wrong example. Since #3526 it is the process-wide half of a pair: an engine with its ownClrTypeConvertergets a table of its own, precisely so that a process-lived static does not pin that converter and whatever engine it captured. Citing it as precedent for "pin it for the process" points at the case that decided not to.Replaced with two that hold today and are the same shape as the tables being justified —
CompiledMemberAccessor's fourMemberInfo-keyed delegate tables, andReflectionExtensions._operatorOverloadMethodCache, aType-keyed method list which is the pure-metadata part_knownOperatorssits on top of.Comment-only; no code change.
dotnet build -c Release Jint/Jint.csprojclean, 0 warnings.Found while auditing process-wide caches for #3426. The same dead name was in
Jint/Runtime/Interpreter/AGENTS.md; #3522/#3563 already rewrote that paragraph, and this comment is the last place it survived.🤖 Generated with Claude Code
https://claude.ai/code/session_014W5mbjGhyvgAS4pivXoc4S