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

Remove ordered mailbox and rename priority to unicast #207

Merged
merged 11 commits into from
Sep 29, 2023

Conversation

thomaseizinger
Copy link
Collaborator

@thomaseizinger thomaseizinger commented Sep 26, 2023

This aims to resolve #94 by renaming split_receiver to detach. 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.

@thomaseizinger thomaseizinger added this to the 0.6.0 milestone Sep 26, 2023
@thomaseizinger thomaseizinger changed the title Remove priority mailbox Remove ordered mailbox and rename priority to unicast Sep 28, 2023
@Restioson
Copy link
Owner

Restioson commented Sep 28, 2023

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?

@thomaseizinger
Copy link
Collaborator Author

thomaseizinger commented Sep 29, 2023

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.

I am glad that we arrived at this! I agree with the atomics view. The similarity is not surprising when you think about it: Address is a way of writing to an actor without requiring &mut self and thus requires some kind of interior mutability.

@thomaseizinger
Copy link
Collaborator Author

thomaseizinger commented Sep 29, 2023

@Restioson I had to adapt a few tests, please have a look!

Comment on lines 7 to +9
use futures_util::FutureExt;
use smol_timeout::TimeoutExt;
use tokio::task::JoinSet;
Copy link
Owner

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

@thomaseizinger thomaseizinger merged commit 472a065 into master Sep 29, 2023
6 checks passed
@thomaseizinger thomaseizinger deleted the rename-detach branch September 29, 2023 13:05
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.

Re-consider separate, ordered mailbox
2 participants