Skip to content

Conversation

JamesNK
Copy link
Member

@JamesNK JamesNK commented Feb 16, 2025

GrpcChannel has a ConcurrentDictionary to cache method details. However, if the channel never has more than a handful of methods then ConcurrentDictionary 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.

Copy link

@Copilot Copilot AI left a 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>
@JamesNK
Copy link
Member Author

JamesNK commented Feb 18, 2025

@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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants