Add `Sender::{send, try_send}` methods

[yoshuawuyts]

In addition to broadcasting a T: Clone message to all receivers, we should also support sending a single T message to one receiver. That would enable async-broadcast to be used in all cases where async-channel is currently used as well, but be strictly more flexible because it supports sending messages to all receivers as well.

The primary concern this introduces is that it might cost us a bit in terms of performance, especially when compared to async-channel. But this can be optimized later on. Specifically, we could apply some of the same tricks from std::sync::mpsc which uses an atomic pointer to switch backing implementations based on the workload.

API Overview

impl Sender {
    /// Sends a message into the channel.
    pub fn send(&self, msg: T) -> Send<'_, T> { ... }
    /// Attempts to send a message into the channel.
    pub fn try_send(&self, msg: T) -> Result<(), TrySendError<T>> { ... }
}
/// A future returned by `Sender::send()`.
pub struct Send;
impl Future for Send { ... }

[zeenix]

Hey Yosh,

You've discussed this before with me but I'm afraid I'm still not 100% clear on the use case. Would it be to combine this crate with async-channel so that folks don't have to pick and choose or is it that we allow the same channel instance to be used for both broadcast and unicast? If it's the latter, that sounds nice at first but given that sender can't really decide which receiver gets the message, I can't really imagine someone explicitly creating a broadcast channel and switch between broadcasting and unicasting but maybe I'm missing something here?

In either case, we'll need to somehow make the Receiver::{recv, try_recv} work w/o the T: Clone requirement.

[yoshuawuyts]

Would it be to combine this crate with async-channel so that folks don't have to pick and choose or is it that we allow the same channel instance to be used for both broadcast and unicast? If it's the latter, that sounds nice at first but given that sender can't really decide which receiver gets the message, I can't really imagine someone explicitly creating a broadcast channel and switch between broadcasting and unicasting but maybe I'm missing something here?

It's mostly about the first case, but that requires the second case to work too. Most folks will likely either want to unicast or broadcast. But if for whatever reason both are used interchangeably, that shouldn't fail. The primary point is that folks don't need to match a channel type to the topology they want to implement. A single channel type should be flexible and performant enough for all topologies, removing the need to spend time figuring out which channel type matches your exact use case.

In either case, we'll need to somehow make the Receiver::{recv, try_recv} work w/o the T: Clone requirement.

Yes that's right. A Sender only needs to know whether a type is T: Clone, on the receiving side there doesn't need to be a distinction. I'm pretty sure this should be possible to express.

[zeenix]

But if for whatever reason both are used interchangeably, that shouldn't fail.

As I wrote,

given that sender can't really decide which receiver gets the message, I can't really imagine someone explicitly creating a broadcast channel and switch between broadcasting and unicasting

so I don't see any reason for allowing this but if this can be made to work w/o complicating the code/API, then it's fine with me even if we don't know any specific use cases for this flexibility. :smile:

A single channel type should be flexible and performant enough for all topologies, removing the need to spend time figuring out which channel type matches your exact use case.

I agree 100% here. I'm not sure the effort involved is worth the benefit though but if you'll be doing the work, I will have no objections. :smile:

In either case, we'll need to somehow make the Receiver::{recv, try_recv} work w/o the T: Clone requirement.

Yes that's right. A Sender only needs to know whether a type is T: Clone, on the receiving side there doesn't need to be a distinction. I'm pretty sure this should be possible to express.

I would think it's the other way around so you only clone when/if needed.

[zeenix]

@yoshuawuyts Hey, do you think you'll be getting around to implementing this? If not, please close it as I won't have the time or motivation for this.

[yoshuawuyts]

@zeenix I don't have the time currently, but I also do still think this is functionality we're missing. It's okay if you don't want to have it in this library, but imo it should definitely exist somewhere. I don't know, do you think it would make sense to be part of this library?

[zeenix]

don't know, do you think it would make sense to be part of this library?

TBH, I don't have any strong opinions either way but if we were to add this functionality here, would it make sense for async-channel crate to still exist or should it be deprecated? If it should be deprecated, this becomes a bit more complex of a task, as it's no longer just about implementing it. We'll need to convince other smol-rs maintainers that it's a good idea.

[yoshuawuyts]

I was thinking we’d do it more the other way around: add the functionality to async-broadcast, then optimize it. And once that’s done we can merge it back into async-channel and deprecate this crate.

async-channel is more widely used, and so it would make more sense probably to just extend its functionality with (if we do it right) what are essentially just a few new methods

[zeenix]

I was thinking we’d do it more the other way around: add the functionality to async-broadcast, then optimize it. And once that’s done we can merge it back into async-channel and deprecate this crate.

That sounds good to me. :+1: Will still need to sync w/ rest of @smol-rs/admins team though but with this approach, I'm sure it'll be much easier to convince everyone. :)

[notgull]

Honestly, I'm a fan of the approach of "optimize the internal representation and then merge the functionality into async-channel". I'd like async-channel to be the one-stop shop for channels, and having a decent function for "send this to all receivers" would be nice.

I'd be opposed to structuring channels such that it starts with the normal ConcurrentQueue strategy and then "upgrades" to a broadcast channel when the functionality is demanded. I'm of the understanding that that strategy caused a lot of pain in libstd's mpsc implementation.

Maybe the API could be something like this?

trait ChannelType { /* ... */ }
struct CanBroadcast { /* ... */ }
struct NoBroadcast;
impl ChannelType for CanBroadcast { /* ... */ }
impl ChannelType for NoBroadcast { /* ... */ }

struct Sender<T, Type: ChannelType = NoBroadcast> { /* ... */ }
struct Receiver<T, Type: ChannelType = NoBroadcast> { /* ... */ }

impl<T, Type: ChannelType> Sender<T, Type> {
    /* normal api */
}

impl<T: Clone> Sender<T, CanBroadcast> {
    /* broadcast api */
}

Maybe that's too overcomplicated. I'd just like to avoid being locked into an inefficient channel implementation.

[fogti]

@notgull But that sounds like the internal representations are so vastly different that it won't buy us anything to try to mold it into async-channel...

[notgull]

You may be right. I think we should explore a more efficient implementation of this primitive before we move forwards with this.

[zeenix]

This has been pending for 1.5 years now. @yoshuawuyts (or anyone here) do you still intend to push forward with this? If not, let's close it. Issues open forever, don't help anyone and only are hurtful to my autism. :)