Add Sender::try_send_cow

[WilliamVenner]

This function's main purpose is to minimize memory allocations whilst using Sender::try_send in a loop and to make any code doing this cleaner, simpler and more idiomatic.

The function takes a Cow<'_, T> instead of T as the message argument. This means you can provide try_send_cow with either a borrowed reference to some data, or simply owned data for codepath convenience.

For example, in this snippet, bytes will only be cloned and sent to the channel if it is not full and it is not disconnected. If all the channels are disconnected, this will not allocate any unnecessary memory.

let bytes: Vec<u8> = vec![...];
let data = Cow::Borrowed(&bytes);
let channels: Vec<Sender<T>> = vec![...];
for ch in channels {
    ch.try_send_cow(data.clone());
}

[cynecx]

Can't we just use &impl Clone instead of Cow?

[WilliamVenner]

Definitely, but that's going to be a difficult function to name 😁

I'm happy to change it up if we can come up with a self-descriptive name. try_send_clonable doesn't really explain much.

[cynecx]

try_send_cloned, try_send_owned?

I am not really against using Cow here, it's just a tad redundant imo.

[WilliamVenner]

There is a very slight QoL benefit to using a Cow here, which I think should have virtually no overhead:

fn send_with_extra_processing(ch: Sender<...>, data: Cow<'_, ...>, extra arguments...) -> ... {
    // process the extra arguments or something that we dont want to duplicate the code of in our project :D
    ch.try_send_cow(data)
}

fn send_in_loop() {
    let data = Cow::Borrowed(&...);
    for ch in ... {
        send_with_extra_processing(ch, data.clone(), ...);
    }
}

fn send_to_one() {
    let data = Cow::Owned(...);
    for ch in ... {
        send_with_extra_processing(ch, data, ...);
    }
}

I say very slight because you can just do this instead, but this seems more unclear from a high-level API perspective, and delegates handling codepaths like this to the end user:

fn send_with_extra_processing(ch: Sender<...>, data: Cow<'_, ...>, extra arguments...) -> ... {
    // process the extra arguments or something that we dont want to duplicate the code of in our project :D
    match data {
        Cow::Borrowed(data) => ch.try_send_cloned(data),
        Cow::Owned(data) => ch.try_send(data)
    }
}

[WilliamVenner]

As in, changing try_send to try_send_owned? This is probably the sensical thing to do, but that leaves us with no choice to either mark try_send as deprecated (which would kind of smell imo) or release a new major version (I don't think this is necessary :D) as to not break anything which depends on a minor semver version of crossbeam.

[WilliamVenner]

One thing to consider is that avoiding Cow may help with supporting no_std - not sure if that is a goal of the channels part of crossbeam.

[cynecx]

You meant to say this, right?:

Cow::Borrowed(data) => ch.try_send_cloned(data), // there is no clone here

As in, changing try_send to try_send_owned?

Personally, I'd like to keep the current try_send semantics (owned semantics) because this is really just the most common case.

I was referring to try_send_owned as a replacement name for try_send_cow, however it seems like a bad choice. try_send_cloned would be a better choice imo.

One thing to consider is that avoiding Cow may help with supporting no_std - not sure if that is a goal of the channels part of crossbeam.

crossbeam-channel doesn't target `no_std' iiuc because it inherently relies on dynamic allocation.

@taiki-e?

[WilliamVenner]

Yes I did, thank you :D

I would have no problem with try_send_cloned if it weren't for the fact that it's not exactly something you look at and can infer roughly what it does without looking it up.

What about try_send_lazy? Maybe that implies something about the function's behavior on the lower level, and not its Clone-On-Send behaviour.

There's also try_clone_on_send?

[cynecx]

Ah nice it looks more readable. However, I'd really just use Clone instead of ToOwned<Owned = T> because I think it's more concise and readable :)

[WilliamVenner]

It may be, but Cow's trait bound is ToOwned; something that implements ToOwned does not necessarily implement Clone!

https://github.com/rust-lang/rust/blob/d620ae10709ca3669cda89db8b29afcd9accc188/library/alloc/src/borrow.rs#L180-L182

[cynecx]

Every type that implements Clone also implements ToOwned: https://doc.rust-lang.org/stable/alloc/borrow/trait.ToOwned.html#impl-ToOwned-1. So Clone basically always implies ToOwned.

[WilliamVenner]

Yes - but ToOwned does not implement Clone and the trait bounds of Cow use ToOwned

[taiki-e]

Thanks for the PR! I haven't reviewed the code yet, but I have a few questions.

• Is there any reason why this is not a method that takes a closure as an argument (something like try_send_with)?

• How does this function work in the following cases?

  • If into_owned takes a long time. (And if a message was sent from another thread before the into_owned completed.)
  • If into_owned panics.

[WilliamVenner]

Is there any reason why this is not a method that takes a closure as an argument (something like try_send_with)?

Interesting idea - I will actually make a separate PR with this function since it sounds very useful.

It would be awesome if crossbeam could actually provide both functions. The user can pick whatever they want to use :D

If into_owned takes a long time. (And if a message was sent from another thread before the into_owned completed.)

I'll have to draw up a test for this.

If into_owned panics.

It would panic just like any normal panic - is there something to be concerned about here? Are there situations in crossbeam channels where panicking is bad? I suppose in a multi threaded context this could be a concern. I'll have to draw up a test for this too.

[cynecx]

Panicking here would be bad because it would stall the channel because write_unchecked is never called. If we'd want to support panicking here, we'd have to include a Guard here which when a panic occurs "cancels" the to be written slot.

[WilliamVenner]

I'm not too familiar on catching/handling panicking in Rust yet. Should we use these?

https://doc.rust-lang.org/std/panic/trait.UnwindSafe.html

https://doc.rust-lang.org/std/panic/fn.catch_unwind.html

[taiki-e]

A better pattern for this case is RAII scope guard, such as scopeguard (because it has no additional cost).

[taiki-e]

It would be awesome if crossbeam could actually provide both functions. The user can pick whatever they want to use :D

IIRC there are not many types that can actually use Cow, so I feel providing only a method that takes a closure would probably be sufficient.