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

add async <--> blocking "bridge" channels #26

Open
hawkw opened this issue Jan 3, 2022 · 1 comment
Open

add async <--> blocking "bridge" channels #26

hawkw opened this issue Jan 3, 2022 · 1 comment
Labels
enhancement New feature or request

Comments

@hawkw
Copy link
Owner

hawkw commented Jan 3, 2022

In some cases, it's desirable to have a channel where one end of the channel is async, and another end is blocking. For example, we might want to send messages to an async task from a context where we can't yield, or we might want to send messages from an async task to a dedicated background thread that can wait by blocking.

It should be quite easy to add support for async <-> blocking communication due to the overall modular design of thingbuf's MPSC channels. We could add something like this:

enum EitherWaiter {
   Async(core::task::Waker),
   Blocking(std::thread::Thread),
}

impl Notify for EitherWaiter {
    fn notify(self) {
        match self {
            Self::Async(waker), => waker.notify(),
            Self::Blocking(thread) => thread.notify(),
        }
    }

    #[inline]
    fn same(&self, other: &Self) -> bool {
        match (self, other) {
            (Self::Async(ref this), Self::Async(ref that)), => that.will_wake(this),
            (Self::Blocking(ref this), Self::Blocking(ref that)) => this.id() == that.id(),
             _ => false,
        }
    }
}

and now, suddenly, a WaitQueue or WaitCell can contain both async Wakers and blocking Threads!

Design Questions

The main design question is whether this should be added to the existing channel types, or whether we should add a new channel type specifically for asynchronous <--> blocking communication. As I see it, this is roughly the tradeoff:

Pros of adding bridging to the existing channels

  • The API would be much simpler. Currently we have separate async Sender/Receiver, async StaticSender/StaticReceiver, synchronous sync::Sender/sync::Receiver, and synchronous sync::StaticSender/sync::StaticReceiver...which is a lot of types. Adding a third bridge channel type would introduce a new sender, new receiver, and (presumably) also static variants, for four new types.

    On the other hand, if we decided to combine everything, we could even get rid of the separate synchronous channel types, and just have one channel with a receiver with async fn recv() and fn recv_blocking(), and a sender with async fn send() and fn send_blocking(). This would simplify the API significantly.

  • Fewer API types also means less code duplication. Adding a separate bridging channel would probably require duplicating some code from both the existing async and sync channel implementations.

  • More flexible user code. If users want to write code that (for example) takes a receiver capable of doing an async recv operation, but the senders might be sync or async, they would have to wrap the separate Receiver and BridgeReceiver types or be generic over them. If we just combined everything into one channel type, this wouldn't be necessary.

Cons of the combined API

  • Enum dispatch overhead. AFAIK, this is the one really big downside of combining everything: whenever you wake something, you have to match on the enum and call the appropriate notify method. This is a small amount of additional runtime work you have to do on every wakeup. I don't know how severe a performance impact this would have, but it's impossible for it to not have some ooverhead over not having to do it. It's possible that the overhead is so small that it doesn't meaningfully effect our benchmarks, though. We should test this. If there's a performance impact that's noticeable, we might want to have a separate bridge API so that you only pay that cost if you're using it.

  • Enum discriminants. Similarly, adding the enum introduces a bit of space overhead; at least a byte per waiter (possibly more depending on padding & alignment). I severely doubt that niche optimization can find anywhere in an enum of Waker/Thread` to put the discriminant, so it's almost certainly making waiters a bit bigger. I'm not sure how much this matters, but users might not want to pay the space cost if they're not using it.

  • Maybe people actually want to have a type-level distinction between the different kinds of channels? I'm not really sure why, though.

@hawkw hawkw added the enhancement New feature or request label Jan 3, 2022
@hawkw
Copy link
Owner Author

hawkw commented Jan 3, 2022

@Darksonn pointed out to me on Discord that Tokio just sticks the Thread inside a Waker to implement async/sync bridging: https://github.com/tokio-rs/tokio/blob/12dd06336d2af8c2d735d4d9e3dc0454ad7942a0/tokio/src/park/thread.rs#L252-L269

Because a Waker always does dynamic dispatch anyway, using that dynamic dispatch for bridging doesn't add any additional overhead in the async case (which an enum would).

For the blocking send/recv case, it does have additional overhead over just sticking Threads in the waitlist directly, because it adds dynamic dispatch through Waker. So, I think what we probably want to do is add separate send_blocking/recv_blocking methods to the main (async) channel API, and keep the separate blocking-only API for use cases that are purely blocking.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant