-
Notifications
You must be signed in to change notification settings - Fork 806
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 <gue@korporal.at>
@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 <gue@korporal.at>
GrpcChannel
has aConcurrentDictionary
to cache method details. However, if the channel never has more than a handful of methods thenConcurrentDictionary
uses a large amount of memory to store just a few instances.This PR adds a
ThreadSafeLookup
type that uses a replacing array for a small number of items and upgrades to a dictionary beyond a threashold.