make tokio dependency optional

[FlorianUekermann]

The h3 crate depends on tokio, but only uses it for the unbounded channel, which signals request completion. I would like to replace the use of tokio::sync::mpsc with a lighter alternative, or at least make it optional.

This PR adds an optional dependency on async-channel and makes tokio optional. Alternatively any other mpsc implementation or even a Arc<Mutex<(VecDeque, Waker)>> should be sufficient.

[seanmonstar]

Thanks for sending us the PR!

Does the channel you suggested actually "weigh" less? Currently, h3 only depends on the sync feature of Tokio, so it's not compiling the runtime part. I would assume they are similar amounts of code, and so then would tend to lean toward depending on Tokio's because it's usage by other is extensive.

[FlorianUekermann]

Does the channel you suggested actually "weigh" less?

Depends by what metric. In terms of code being downloaded from crate.io it's roughly 50 times lighter. In terms of performance, and compiled size I have no idea (but I can check...).

A motivation was to make it obvious that the crate is runtime independent and doesn't spawn tasks. With tokio as a dependency that is tricky to tell for most people, since some things in tokio require running inside a tokio runtime, while others don't. It's not the case here, but I almost decided not to use h3 because I didn't look close enough at first.

Currently, h3 only depends on the sync feature of Tokio, so it's not compiling the runtime part. I would assume they are similar amounts of code, and so then would tend to lean toward depending on Tokio's because it's usage by other is extensive.

Same with async-channel. That's the one async-std and smol use. That was my reasoning behind making it optional.

Maybe I should have used Arc<Mutex<(VecDeque, Option<Waker>)>>, I just wanted to send something that works to start the conversation.

[FlorianUekermann]

I did a quick benchmark that seemed reasonable for this case and tokio was about 70% slower than futures-channel. It's also a much smaller download and as frequently used (according to crates.io) as tokio. Compiled sizes didn't differ.

I would propose to change the PR and get rid of both tokio and async-channel in favor of futures-channel with no feature flags (or make it default and hide the other two behind features).

use futures_util::StreamExt;

fn main() {
    let w = noop_waker::noop_waker();
    let mut cx = std::task::Context::from_waker(&w);
    let cx = &mut cx;

    #[cfg(feature = "async-channel")]
    let (s, mut r) = async_channel::unbounded();
    #[cfg(feature = "tokio")]
    let (s, mut r) = tokio::sync::mpsc::unbounded_channel();
    #[cfg(feature = "futures-channel")]
    let (s, mut r) = futures_channel::mpsc::unbounded();

    for i in 0..10_000_000u64 {
        #[cfg(feature = "async-channel")]
        let x = {
            let _ = r.poll_next_unpin(cx);
            s.try_send(i).unwrap();
            r.poll_next_unpin(cx)
        };
        #[cfg(feature = "tokio")]
        let x = {
            let _ = r.poll_recv(cx);
            s.send(i).unwrap();
            r.poll_recv(cx)
        };
        #[cfg(feature = "futures-channel")]
        let x = {
            let _ = r.poll_next_unpin(cx);
            s.unbounded_send(i).unwrap();
            r.poll_next_unpin(cx)
        };
        std::hint::black_box(x);
    }
}

[seanmonstar]

Performance characteristics for channels really depends on usage. They all chose different pros and cons, such as using a linked list versus arrays and blocks, instrusive collections, locks vs spinning, preferring allocating in send or recv or reserve, etc. Depending on how contended they'll be, how big the message type is, how often an item is sent, etc.

For that reason, I tend to find Tokio's channel preferable because it sees more usage, and there are several active maintainers available to fix any bugs that are reported.

[Ruben2424]

A motivation was to make it obvious that the crate is runtime independent and doesn't spawn tasks. With tokio as a dependency that is tricky to tell for most people, since some things in tokio require running inside a tokio runtime, while others don't. It's not the case here, but I almost decided not to use h3 because I didn't look close enough at first.

Independent of the channel, I think it is a good idea to address in the readme that h3 is runtime independent. I will submit a PR for this.

[FlorianUekermann]

Independent of the channel, I think it is a good idea to address in the readme that h3 is runtime independent. I will submit a PR for this.

Thx, that helps.

Performance characteristics for channels really depends on usage.

That's why the benchmark looks the way it does. I suspect the channel signalling request ends will pretty much always be empty and at least half off the polls result in Poll::Pending.

I tend to find Tokio's channel preferable because it sees more usage, and there are several active maintainers available to fix any bugs that are reported.

I would have assumed that anything in futures-rs project is quite actively maintained as well and the recent download numbers on crates.io are practically identical. But as a contributor to both of them, you are obviously more qualified to comment on that.

To be clear, I only sent this PR because of the size and the "looks like depends on tokio runtime" concern, not performance. I just wanted to alleviate potential performance concerns with the benchmark. I personally would still prefer the smaller dependency, but it is pretty subjective, so feel free to close. I'm happy to send a PR for any variant mentioned in the thread otherwise.