Account cache optimization #5792
Merged
Conversation
This file contains 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
…tion-library-for-js into account-cache-optimization
github-actions
bot
added
msal-browser
peterzenz
reviewed
Mar 15, 2023
…tion-library-for-js into account-cache-optimization
Codecov Report
*This pull request uses carry forward flags. Click here to find out more.
|
tnorling
requested review from
jennyf19,
jmprieur,
bgavrilMS,
sameerag,
hectormmg,
Robbie-Microsoft,
jo-arroyo and
konstantin-msft
as code owners
github-actions
bot
added
the
samples
peterzenz
approved these changes
Mar 20, 2023
derisen
reviewed
Mar 21, 2023
derisen
reviewed
Mar 21, 2023
derisen
reviewed
Mar 21, 2023
Co-authored-by: Doğan Erişen <v-derisen@microsoft.com>
sameerag
reviewed
Mar 21, 2023
sameerag
reviewed
Mar 21, 2023
sameerag
reviewed
Mar 21, 2023
tnorling
added a commit
that referenced
this pull request
Mar 29, 2023
Follow up to #5792, addressing the same problem but for token lookups. Today on every lookup of tokens in the cache we iterate through every entry in the cache (including cache entries not belonging to MSAL), attempt to parse it into JSON and then validate that it matches our credential shape. This is incredibly inefficient and becomes more inefficient with each additional thing stored in browser storage. This PR adds a new cache entry which contains an object of keys to all of the tokens known to MSAL for a given clientId. Now when the tokens need to be enumerated the worst case number of cache entries touched and parsed is 1 + number of tokens of the given type. Example: ```js key: msal.token.keys.<clientId> value: { idToken: ["idToken-key-1", "idToken-key-2"], accessToken: ["accessToken-key-1", "accessToken-key-2"], refreshToken: ["refreshToken-key-1"] } ``` --------- Co-authored-by: Thomas Norling <thnorlin@microsoft.com>
konstantin-msft
pushed a commit
that referenced
this pull request
Apr 3, 2023
Today on every lookup of an account in the cache we iterate through every entry in the cache (including cache entries not belonging to MSAL), attempt to parse it into JSON and then validate that it matches our account shape. This is incredibly inefficient and becomes more inefficient with each additional thing stored in browser storage. This PR adds a new cache entry (with a static key) which contains an array of keys to all of the accounts known to MSAL. Now when the accounts need to be enumerated the worst case number of cache entries touched and parsed is 1 + number of accounts. A new configuration option is added which determines whether or not this new cache entry should be created on initialization. This is to help migrate apps using localStorage from versions before this change. Follow up PR will address the same issue for id, access and refresh tokens. --------- Co-authored-by: Thomas Norling <thnorlin@microsoft.com> Co-authored-by: Doğan Erişen <v-derisen@microsoft.com>
konstantin-msft
pushed a commit
that referenced
this pull request
Apr 3, 2023
Follow up to #5792, addressing the same problem but for token lookups. Today on every lookup of tokens in the cache we iterate through every entry in the cache (including cache entries not belonging to MSAL), attempt to parse it into JSON and then validate that it matches our credential shape. This is incredibly inefficient and becomes more inefficient with each additional thing stored in browser storage. This PR adds a new cache entry which contains an object of keys to all of the tokens known to MSAL for a given clientId. Now when the tokens need to be enumerated the worst case number of cache entries touched and parsed is 1 + number of tokens of the given type. Example: ```js key: msal.token.keys.<clientId> value: { idToken: ["idToken-key-1", "idToken-key-2"], accessToken: ["accessToken-key-1", "accessToken-key-2"], refreshToken: ["refreshToken-key-1"] } ``` --------- Co-authored-by: Thomas Norling <thnorlin@microsoft.com>
|
🎉 We recommend upgrading to the latest version of Handy links: |
|
🎉 Handy links: |
|
🎉 Handy links: |
hectormmg
pushed a commit
that referenced
this pull request
Apr 5, 2023
Today on every lookup of an account in the cache we iterate through every entry in the cache (including cache entries not belonging to MSAL), attempt to parse it into JSON and then validate that it matches our account shape. This is incredibly inefficient and becomes more inefficient with each additional thing stored in browser storage. This PR adds a new cache entry (with a static key) which contains an array of keys to all of the accounts known to MSAL. Now when the accounts need to be enumerated the worst case number of cache entries touched and parsed is 1 + number of accounts. A new configuration option is added which determines whether or not this new cache entry should be created on initialization. This is to help migrate apps using localStorage from versions before this change. Follow up PR will address the same issue for id, access and refresh tokens. --------- Co-authored-by: Thomas Norling <thnorlin@microsoft.com> Co-authored-by: Doğan Erişen <v-derisen@microsoft.com>
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.
Today on every lookup of an account in the cache we iterate through every entry in the cache (including cache entries not belonging to MSAL), attempt to parse it into JSON and then validate that it matches our account shape. This is incredibly inefficient and becomes more inefficient with each additional thing stored in browser storage.
This PR adds a new cache entry (with a static key) which contains an array of keys to all of the accounts known to MSAL. Now when the accounts need to be enumerated the worst case number of cache entries touched and parsed is 1 + number of accounts.
A new configuration option is added which determines whether or not this new cache entry should be created on initialization. This is to help migrate apps using localStorage from versions before this change.
Follow up PR will address the same issue for id, access and refresh tokens.