-
Notifications
You must be signed in to change notification settings - Fork 815
Avoid using ConcurrentDictionary for channels with few methods #2597
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (1)
src/Grpc.Net.Client/Internal/ThreadSafeLookup.cs:24
- The comment should be corrected to 'Looking up a key in an array is as fast as a dictionary for small collections and uses much less memory.'
// Looking up a key in an array is as fast as a dictionary for small collections and uses much less memory.
Co-authored-by: Günther Foidl <[email protected]>
|
@BrennanConroy Good feedback. I don't think the type would have broken and caused errors, but it could have gotten into a weird state (using the dictionary but still having some values in the array, duplicate values in the array). I've placed all updates in the lock. And added a comment about the value that's created. @gfoidl The main goal here is to optimize idle memory usage. Making the array only as big as it needs to be, plus some array allocations while a channel is first used is an ok tradeoff. |
Co-authored-by: Günther Foidl <[email protected]>
GrpcChannelhas aConcurrentDictionaryto cache method details. However, if the channel never has more than a handful of methods thenConcurrentDictionaryuses a large amount of memory to store just a few instances.This PR adds a
ThreadSafeLookuptype that uses a replacing array for a small number of items and upgrades to a dictionary beyond a threashold.