Defer array creation in ConcurrentCache. #72291
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
dotnet-issue-labeler
bot
added
Area-Compilers
untriaged
CyrusNajmabadi
commented
Feb 28, 2024
| // short-lived compilations that do not end up using the cache. As the cache is simple best-effort, it's | ||
| // fine if multiple threads end up creating the backing array at the same time. One thread will be last and | ||
| // will win, and the others will just end up creating a small piece of garbage that will be collected. | ||
| : base(size, createBackingArray: false) |
There was a problem hiding this comment.
i kept this logic only for ConcurrentCache, as it's this cache that says explicitly: expiration policy is "new entry wins over old entry if hashed into the same bucket". So it felt safest to have just for this type. We could move this down to the base if we're ok with this policy for all subclsses.
CyrusNajmabadi
changed the title
|
Speedometer/DDRITs shows no regressions. @dotnet/roslyn-compiler for review here. Thanks! |
jjonescz
approved these changes
Mar 1, 2024
ToddGrun
reviewed
Mar 1, 2024
| } | ||
|
|
||
| public bool TryGetValue(TKey key, [MaybeNullWhen(returnValue: false)] out TValue value) | ||
| { | ||
| int hash = RuntimeHelpers.GetHashCode(key); | ||
| int idx = hash & mask; | ||
|
|
||
| var entries = this.entries; | ||
| var entries = this.Entries; |
There was a problem hiding this comment.
fair. but this keeps things exactly the same otherwise. something we could consider.
|
@dotnet/roslyn-compiler ptal |
333fred
reviewed
Mar 5, 2024
|
@333fred ptal. |
333fred
approved these changes
Mar 5, 2024
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.
Followup to #72290.
I want to also run this through speedometer to ensure it helps out.
--
Build: https://devdiv.visualstudio.com/DefaultCollection/DevDiv/_build/results?buildId=9159598&view=results
https://devdiv.visualstudio.com/DefaultCollection/DevDiv/_build/results?buildId=9160401&view=results