Auth - avoid repeated account lookups#301712
Open
xingsy97 wants to merge 1 commit intomicrosoft:mainfrom
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Adds an in-memory, per-auth-provider cache for AuthenticationService.getAccounts() to avoid re-querying all sessions on every call, with cache invalidation wired into existing session/access lifecycle events.
Changes:
- Introduces
_accountsCachekeyed by provider id and populates it on firstgetAccounts()call. - Invalidates cached entries when sessions change, when extension session access changes, and when a provider is unregistered.
src/vs/workbench/services/authentication/browser/authenticationService.ts
Show resolved
Hide resolved
src/vs/workbench/services/authentication/browser/authenticationService.ts
Show resolved
Hide resolved
830eeb4 to
eab87db
Compare
Add a per-provider cache for getAccounts() to avoid redundantly querying all sessions on every call. The cache is invalidated when: - The provider fires onDidChangeSessions - Extension session access changes for the provider - The provider is unregistered This addresses the TODO comment by TylerLeonhardt.
eab87db to
40b29b4
Compare
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.
Problem
AuthenticationService.getAccounts()queries all sessions viagetSessions()on every call without caching, as noted by a TODO comment from @TylerLeonhardt (// TODO: Cache this).Fix
Add a per-provider
Map<string, ReadonlyArray<AuthenticationSessionAccount>>cache that is populated on first call and invalidated when:onDidChangeSessions(cache is cleared before the event is re-fired to consumers, ensuring any consumer callinggetAccounts()in the event handler sees fresh data)onDidChangeExtensionSessionAccess)unregisterAuthenticationProvider)Validation