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

What about adding a ConcurrentSet<T>? #16443

Closed
JohanLarsson opened this issue Feb 22, 2016 · 31 comments
Closed

What about adding a ConcurrentSet<T>? #16443

JohanLarsson opened this issue Feb 22, 2016 · 31 comments
Labels
api-needs-work API needs work before it is approved, it is NOT ready for implementation area-System.Collections
Milestone

Comments

@JohanLarsson
Copy link

The need for it comes up every now and then. To me it would make sense next to ConcurrentDictionary<TKey, TValue>

@ayende
Copy link
Contributor

ayende commented Feb 23, 2016

What does this gives you that ConcurrentDictionary<T, object> (and just not using the value) ?

@JohanLarsson
Copy link
Author

JohanLarsson commented Feb 23, 2016

Nothing but communicates intent better imo. I usually do stuff like this (not a proper set).
Set operations are perhaps tricky to get right with concurrency.

@svick
Copy link
Contributor

svick commented Feb 23, 2016

@ayende Apart from the "it's clearer what this code does" reason and the precedent of having HashSet<T>, there is a small, but measurable, difference in performance and memory consumption.

A very simple test: when I add a million values to Dictionary<int, int>, it takes 42 ms and consumes 26.6 MB of memory, while with HashSet<int>, it takes 38 ms and consumes 21.3 MB.

I assume the difference with concurrent collections would be similar.

@JonHanna
Copy link
Contributor

If anything more so, since the concurrency consideration means there are a few spots that want to be particularly efficient, so not having to do anything with values would be an even greater gain there than with HashSet<T>.

@svick
Copy link
Contributor

svick commented Feb 23, 2016

@JohanLarsson

Set operations are perhaps tricky to get right with concurrency.

I think a ConcurrentSet would be useful even without set operations.

@JonHanna
Copy link
Contributor

I know of one C# concurrent hash set and one in Java and neither offer concurrency on set operations, only on the add, remove, contains.

@hermitdave
Copy link

Is there's plan to add this? If there is and it's up for grabs, I can look into it

@svick
Copy link
Contributor

svick commented Feb 23, 2016

@hermitdave You might want to have a look at how the API review process works.

@hermitdave
Copy link

Thanks @svick will keep an eye on this one

@JohanLarsson
Copy link
Author

@svick

I think a ConcurrentSet would be useful even without set operations.

Agreed, perhaps it should not be called Set? IDK.

@Priya91
Copy link
Contributor

Priya91 commented Sep 30, 2016

@hermitdave Do you have a proposal for this ?

@hermitdave
Copy link

@Priya91 oops forgot about this one.. I'll get started asap

@hermitdave
Copy link

Starting work on this

@karelz
Copy link
Member

karelz commented Nov 18, 2016

@hermitdave are you working on it?

@danmoseley
Copy link
Member

@hermitdave do you still have interest in writing up the formal proposal here? If we review okays it promply and someone is interested in implementation, there may still be time to get this into 2.0.

Possibly the implementation could begin by wrapping ConcurrentDictionary, so the API is quickly available to use, then rewritten to have the space/time improvements of a custom implementation.

@daveaglick
Copy link

FWIW, @i3arnon has a really nice implementation at https://github.com/i3arnon/ConcurrentHashSet

@hermitdave
Copy link

hermitdave commented Jan 4, 2017

@i3arnon maybe you should do a PR instead.

@danmoseley
Copy link
Member

Just to be clear folks -- feel free to make a PR, but we can't take any change without an API review approval. That needs a proposal written up as above. @hermitdave were you going to do that?

@hermitdave
Copy link

@danmosemsft I had a look at what @i3arnon did and it is which is decent. if he isn't interested then I will do a PR later this week

@karelz
Copy link
Member

karelz commented Jan 4, 2017

@hermitdave we first need API proposal (see API review process) - i.e. review the API surface of the collection (with motivation and relation to 'classic' HashSet and other concurrent collections).

Just to set expectations: Based on recent discussions around other collections, we will likely have to find the right place for new collections (CoreFX repo might not be the desired destination). That may take even longer.

@danmoseley
Copy link
Member

@hermitdave apologies, I wasn't clear in my comment. I should have said -- feel free to prototype in a fork, but a PR against CoreFX would be noise at this point without API approval

@vancem
Copy link
Contributor

vancem commented Apr 11, 2018

Just FYI The implementation Dan refers to above is very specialized (e.g. there is no remove), and is probably not representative.

The C# compiler community wrote a simple wrapper for themselves, that is probably a better representation of what would be done if we wanted to add this class.

http://source.roslyn.io/#microsoft.codeanalysis/InternalUtilities/ConcurrentSet.cs

@eltiare
Copy link

eltiare commented Jul 24, 2018

I find myself wanting this very much on occasion. Seems like something that shouldn't be overlooked.

@LeaFrock
Copy link
Contributor

LeaFrock commented Feb 13, 2019

@vancem I can't agree more! But three years passed, and no ConcurrentHashSet yet =.=
I think most of us want a kind of collections which is similar to ConcurrentBag, but its elements are never duplicated and its 'Contains()' is an O(1) operation like HashSet.
We can make it using ConcurrentDictionary, but that implementation is too ugly.

@reggaeguitar

This comment has been minimized.

@JohanLarsson
Copy link
Author

@reggaeguitar fixed the link.

@markusschaber
Copy link

I think most of us want a kind of collections which is similar to ConcurrentBag, but its elements are never duplicated and its 'Contains()' is an O(1) operation like HashSet.

And which allows to remove a specific element, instead of just a random one (I just needed this today, and see https://stackoverflow.com/questions/3029818/how-to-remove-a-single-specific-object-from-a-concurrentbag for another example).

@msftgits msftgits transferred this issue from dotnet/corefx Jan 31, 2020
@msftgits msftgits added this to the 5.0 milestone Jan 31, 2020
@maryamariyan maryamariyan added the untriaged New issue has not been triaged by the area owner label Feb 23, 2020
@eiriktsarpalis eiriktsarpalis removed the untriaged New issue has not been triaged by the area owner label May 6, 2020
@eiriktsarpalis eiriktsarpalis modified the milestones: 5.0.0, Future Jun 24, 2020
@MgSam
Copy link

MgSam commented Jul 15, 2020

So no one ever made an API proposal for this? Maybe you guys should add the up-for-grabs label.

@LeaFrock
Copy link
Contributor

LeaFrock commented Jul 25, 2020

@MgSam I can't stand it anymore, so I've spent some time on the API proposal.

Details see #39919 . Let's promote it and finally make it.

@jkotas
Copy link
Member

jkotas commented Jul 25, 2020

Duplicate of #39919 that has actual API proposal

@jkotas jkotas closed this as completed Jul 25, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Jan 2, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-needs-work API needs work before it is approved, it is NOT ready for implementation area-System.Collections
Projects
None yet
Development

No branches or pull requests