Skip to content
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

Memory allocation documentation for ImmutableDictionary Keys and Values #13927

Closed
sharwell opened this issue Dec 14, 2014 · 5 comments
Closed
Labels
area-System.Collections documentation Documentation bug or enhancement, does not impact product or test code help wanted [up-for-grabs] Good issue for external contributors
Milestone

Comments

@sharwell
Copy link
Member

Background

As described in dotnet/corefx#246 (and previously in dotnet/corefx#147), the Keys and Values properties of ImmutableDictionary<TKey, TValue> cannot be used without incurring memory allocations on the heap. However, a slight modification of user code provides equivalent behavior without these allocations.

With allocations:

foreach (var key in dictionary.Keys)
{
    ...
}

Without allocations:

foreach (var pair in dictionary)
{
    var key = pair.Key;
}

Suggestion

Since updating the Keys and Values properties to eliminate the allocations would be a breaking change (#246), the only recourse is to update the documentation for these properties to describe steps a user can take to avoid allocations if these properties are used heavily within a particular application.

Caveat

Care must be taken to ensure that the documentation does not read in a way that suggests the Keys and Values properties should not be used. The properties behave properly and would not be even a measurable source of time spent in the application in nearly all cases. This notice is only meant for users making heavy use of these properties who are also actively working to reduce the memory allocations performed within an especially performance-sensitive section of code.

@safern
Copy link
Member

safern commented Mar 6, 2017

Does anyone want to suggest how should the docs be updated if this issue is still relevant?

@AArnott
Copy link
Contributor

AArnott commented Mar 6, 2017

I don't see why this is such an important issue. The docs never promise that enumeration of collections or dictionary keys is alloc-free. And as you point out, the API makes this pretty obvious. I've never seen a doc that mentions "this allocates, but this other API doesn't".

@safern
Copy link
Member

safern commented Mar 6, 2017

+1 with @AArnott comment. So then we should probably close this issue. right?

@danmoseley
Copy link
Member

+1

@sharwell
Copy link
Member Author

sharwell commented May 3, 2017

If you can use C# 7, one handy way to address this can be seen in dotnet/roslyn#19126. 👍

@msftgits msftgits transferred this issue from dotnet/corefx Jan 31, 2020
@msftgits msftgits added this to the 2.0.0 milestone Jan 31, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Jan 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Collections documentation Documentation bug or enhancement, does not impact product or test code help wanted [up-for-grabs] Good issue for external contributors
Projects
None yet
Development

No branches or pull requests

5 participants