-
Notifications
You must be signed in to change notification settings - Fork 38
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
Remove ordered mailbox and rename priority to unicast #207
Conversation
Happy to merge when passing CI - I assume will need rebase from master. Thinking about this again with fresh eyes, I think it really does make a lot of sense. I'm not sure if this was said by either of us before, but this kind of mimics atomics in some way (?). At least, that's how my mental model for this is now. If you have two threads which are doing atomic operations on the same thing, the order of their atomic operations relative to eachother, if they are both using relaxed orderings, will be unspecified. They will happen, but they could interleave arbitrarily. The compiler is also free to rearrange operations on that thread. Previously, ordered sends worked the same, except that operations on one thread would still be ordered. This is nice, but as we've discussed can lead to backpressure footguns and is also harder to support. Plus, it's possible to emulate previous behaviour with new behaviour: you could always // look ma, like `detach` but with multiple messages
tokio::spawn(async {
addr.send(1).await;
addr.send(2).await;
}); though the above wouldn't wait for the first to be sent before queuing the next (wonder if this is an issue)? Do you still even care about the back-pressure of mailbox size being exerted on the sending task if using detach? |
I am glad that we arrived at this! I agree with the atomics view. The similarity is not surprising when you think about it: |
@Restioson I had to adapt a few tests, please have a look! |
use futures_util::FutureExt; | ||
use smol_timeout::TimeoutExt; | ||
use tokio::task::JoinSet; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
side note: it's funny how many async crates we're mixing and matching here, haha
This aims to resolve #94 by renaming
split_receiver
todetach
. This wording has prior art in the ecosystem, for example:Both of these are about "running a task in the background".
As a 2nd step and with the terminology resolved, we remove the ordered mailbox and rename the priority one to unicast as a counterpart to broadcast.