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

MultiSemaphore #16

Closed
wants to merge 2 commits into from
Closed

MultiSemaphore #16

wants to merge 2 commits into from

Conversation

sgaluza
Copy link

@sgaluza sgaluza commented Apr 14, 2015

Hello, Stephen

Please check if it's worth to have such a primitive (MultiSemaphore). Basically I need it to synchronize only specific objects in the same loop/method.

For example I may have code which works with aggregates and I care about synchronization of writes to the same aggregate only, like if I had a list of users and I would synchronize writes to the same user only, but it's ok to write changes to different users without synchronization.

If it's ok I could propogate the changes to other folders...

@niemyjski
Copy link

Really cool idea, I can't believe I didn't think of that :). Do you have any tests for this?

@StephenCleary
Copy link
Owner

While this is definitely an interesting concept, I'm going to decline to include it in the AsyncEx library.

The original purpose behind the coordination primitives was to make the same primitives available for async code as the .NET framework provides for sync code. Usually, when developers want a semaphore (or AsyncSemaphore) for each object, they just include it in the object itself. As such, there is no synchronous equivalent of a MultiSemaphore.

Also, I have a goal of simplifying the next version of AsyncEx coordination primitives. In particular, AsyncReaderWriterLock and AsyncBarrier didn't make the cut, and will be removed.

That's not to say this isn't useful, though. If you want to release it in a library of your own, I'd recommend allowing the users to specify how the keys are compared, and also (possibly) some way of clearing individual entries.

@sgaluza
Copy link
Author

sgaluza commented Apr 15, 2015

ok, got it. Thank you for the explanation. I actually needed that for in a few places in my project. And I had another project which required the same. It's a nice feature for an Event Sourcing pattern, because we can write asynchronously to different streams which is ok. So that's why I sent you the idea for a review.

But I agree, probably it is a simple thing to implement just in a project separately and not to create too many primitives in the base library

@niemyjski
Copy link

Why not have an AsyncExtensions project? We could move the ones that
are being removed as well as add this.. I would find it useful and
others can benefit.

On 4/15/15, sgaluza notifications@github.com wrote:

ok, got it. Thank you for the explanation. I actually needed that for in a
few places in my project. And I had another project which required the same.
It's a nice feature for an Event Sourcing pattern, because we can write
asynchronously to different streams which is ok. So that's why I sent you
the idea for a review.

But I agree, probably it is a simple thing to implement just in your project
separately and not to create too many primitives in the base library


Reply to this email directly or view it on GitHub:
#16 (comment)

Thanks
-Blake Niemyjski

@sgaluza
Copy link
Author

sgaluza commented Apr 15, 2015

Yes, something like, AsyncEx.Extra would make sense to highlight that AsyncEx will be the dependency. @StephenCleary, I've got another request for you. Is it possible to add WaitersCount property to AsyncSemaphore (there is an example in the pull request I made). The absence of the property is the only thing which doesn't allow me using your original library code version as a dependency for my project. I could implement it in a another pull request and test.

@IanYates
Copy link

👍 for AsyncEx.Extra. Out of curiosity, is there a reason why AsyncReaderWriterLock isn't making the cut? If it's time, that's completely understandable. I'm more asking from the point of view of someone trying to see if there are good reasons to avoid such a lock when writing async code (and as someone who uses AsyncReaderWriterLock in a good chunk of my code!)

@StephenCleary
Copy link
Owner

I think adding a WaitersCount should be doable. Go ahead and open a pull request (or issue) and I'll get around to it. :)

AsyncReaderWriterLock isn't making the cut for several reasons. First, it's extremely complex code, and thus has a much higher likelihood of lurking bugs. Second, the vast majority of people don't need it; the much simpler AsyncLock easily handles the 99% use case. Third, the implementation - as it currently is - depends on assumptions around how the wait queues work, which are no longer true with the new wait queue implementation using the .NET 4.6 APIs.

@StephenCleary StephenCleary mentioned this pull request Dec 29, 2018
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.

None yet

4 participants