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

Consider using BoundedChannel instead of SemaphoreSlim #322

Open
mtmk opened this issue Jan 10, 2024 · 1 comment
Open

Consider using BoundedChannel instead of SemaphoreSlim #322

mtmk opened this issue Jan 10, 2024 · 1 comment
Labels

Comments

@mtmk
Copy link
Collaborator

mtmk commented Jan 10, 2024

It is also useful and simplifies a lot just to use a BoundedChannel instead of SemaphoreSlim to synchronize writes to the PipeWriter, since SemaphoreSlim, despite it's name, is not really a lightweight synchronization primitive considering it's pretty likely the writes might be happening from multiple threads for a highly concurrent application. Channels also have some nice optimizations for multiple-writers/single-reader scenarios.

That should simplify the CommandWriter code quite a bit, and connection exceptions can be bubbled up in the WriteLoop by completing the ChannelWriter with an exception for example.

With the Channel, you can also in many/most cases avoid the async state machine when the Channel isn't full, by doing a TryWrite and only falling down to WriteAsync when that fails (the bounded channel is full and waiting for the Write loop to pull stuff off it first). That makes PublishAsync very cheap for the cases where the Pipeline buffer can keep up with the channel ingress.

Originally posted by @stebet in #303 (comment)

see also #303 (comment) by @to11mtm

@to11mtm
Copy link
Contributor

to11mtm commented Jan 11, 2024

despite it's name, is not really a lightweight synchronization primitive considering it's pretty likely the writes might be happening from multiple threads for a highly concurrent application.

I should be clear that this statement, I agree in the context of 'lots of WaitAsync calls, for Sync waits it is fine (ish).

However IIRC channel has a little better weak ordering FWIW.

As for how to potentially use a channel, In the past I have used something like this or changed to suit needs appropriately (i.e. if you need a little bit of state).

@mtmk mtmk added the perf label Jan 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants