perf: opt-in reflection cache via ApplicationContext; promote internal API to public#4
Merged
baal2000 merged 6 commits intoperformance/reflection-cachefrom Mar 9, 2026
Conversation
Co-authored-by: baal2000 <22180333+baal2000@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Add ReflectionCacheSettings class for opt-in caching
Make reflection cache opt-in via ReflectionCacheSettings
Mar 8, 2026
Copilot
AI
changed the title
Make reflection cache opt-in via ReflectionCacheSettings
Verify new tests pass with no regressions
Mar 9, 2026
…ive XML docs Co-authored-by: baal2000 <22180333+baal2000@users.noreply.github.com>
Copilot
AI
changed the title
Verify new tests pass with no regressions
Make ReflectionCache and PropertyWithAttribute public with comprehensive XML docs
Mar 9, 2026
…ibute Co-authored-by: baal2000 <22180333+baal2000@users.noreply.github.com>
Copilot
AI
changed the title
Make ReflectionCache and PropertyWithAttribute public with comprehensive XML docs
Make ReflectionCache and PropertyWithAttribute public with full XML docs
Mar 9, 2026
…nsion to public Co-authored-by: baal2000 <22180333+baal2000@users.noreply.github.com>
Copilot
AI
changed the title
Make ReflectionCache and PropertyWithAttribute public with full XML docs
Remove InternalsVisibleTo(Google.Apis) and make previously-internal API surface public
Mar 9, 2026
…lectionCacheSettings Co-authored-by: baal2000 <22180333+baal2000@users.noreply.github.com>
Copilot
AI
changed the title
perf: make reflection cache opt-in via ApplicationContext and publicize cache API surface
Consolidate reflection cache opt-in into ApplicationContext; remove ReflectionCacheSettings
Mar 9, 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.
Addresses the opt-in request from googleapis/google-api-dotnet-client#3118 review.
Motivation
The parent PR (googleapis#3118) caches
PropertyInforesults to fix theDestroyScoutfinalizer pathology documented in #3112. The maintainer requested that caching be opt-in rather than always-on, citing concerns about unbounded cache growth andConcurrentDictionaryoverhead.Design decision: reuse
ApplicationContextinstead of a new classRather than introducing a new
ReflectionCacheSettingsclass (adding permanent public API surface), this PR addsEnableReflectionCacheto the existingApplicationContext— the library's established pattern for app-wide startup settings (cf.ApplicationContext.RegisterLogger). This:[Obsolete]-d without removing an entire class (consistent with how this library already handles API evolution, e.g.IClock.Now,GoogleClientSecrets.Load,ConfigurableMessageHandler.UnsuccessfulResponseHandlers)ApplicationContextis already referenced byParameterUtils,RequestBuilder,ConfigurableMessageHandler, andBackOffHandlerfor exactly this "set once at startup" patternA note on the reviewer's concerns
For the record on the technical points raised in the review:
Type. A CI testing libraries in separate processes would never accumulate all types in one cache. Even in the extreme single-process case, ~20K entries is trivial memory. The current no-cache behavior is actually "unbounded" in a worse way — it creates unbounded transientPropertyInfoarrays and IL stubs per request, which is exactly what triggers the GC/finalizer pathology in Performance: Reflection in ResumableUpload and ParameterUtils causes intermittent long response delays. googleapis/google-api-dotnet-client#3112.ConcurrentDictionarybottleneck:GetOrAddon a cache hit is aVolatile.Read— no lock is taken. Cost is ~20–50 ns. Without the cache,Type.GetProperties()+GetCustomAttribute()runs per request at 10–100 µs. The cache is strictly faster on every code path after the first call. This is the standard pattern recommended by Microsoft and used internally in ASP.NET Core, EF Core, and System.Text.Json for reflection caching.Changes
ApplicationContext.csEnableReflectionCacheproperty (defaultfalse); updated class summary;Reset()now clears the flagReflectionCache.csApplicationContext.EnableReflectionCache;public; extractedComputeProperties; conditional cache pathPropertyWithAttribute.cspublic; comprehensive XML docsParameterUtils.csInitParametersWithExpansionpromotedinternal→public; staleInternalsVisibleToremark removedAssemblyInfo.csInternalsVisibleTo("Google.Apis", …)Usage
Default behavior is unchanged — no caching, no new overhead.