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

Drop tokio dependency #3767

Open
jbaum98 opened this issue Oct 20, 2024 · 9 comments
Open

Drop tokio dependency #3767

jbaum98 opened this issue Oct 20, 2024 · 9 comments
Labels
C-feature Category: feature. This is adding a new feature.

Comments

@jbaum98
Copy link

jbaum98 commented Oct 20, 2024

Now that hyper uses its own traits instead of tokios (#3110, #3230), the only place that tokio is used is to provide tokio::sync::oneshot::Receiver in src/upgarde.rs. If we could find a way to implement this functionality without tokio's oneshot, we could drop the dependency entirely. This would have a pretty huge impact on the transitive dependencies we pull in everywhere hyper is used: from this:

hyper v1.5.0 (/home/jake/tmp/hyper)
├── bytes v1.7.2
├── http v1.1.0
│   ├── bytes v1.7.2
│   ├── fnv v1.0.7
│   └── itoa v1.0.11
├── http-body v1.0.1
│   ├── bytes v1.7.2
│   └── http v1.1.0 (*)
└── tokio v1.40.0
    ├── bytes v1.7.2
    ├── libc v0.2.161
    ├── mio v1.0.2
    │   └── libc v0.2.161
    ├── pin-project-lite v0.2.14
    ├── socket2 v0.5.7
    │   └── libc v0.2.161
    └── tokio-macros v2.4.0 (proc-macro)
        ├── proc-macro2 v1.0.88
        │   └── unicode-ident v1.0.13
        ├── quote v1.0.37
        │   └── proc-macro2 v1.0.88 (*)
        └── syn v2.0.80
            ├── proc-macro2 v1.0.88 (*)
            ├── quote v1.0.37 (*)
            └── unicode-ident v1.0.13

to this:

hyper v1.5.0
├── bytes v1.7.2
├── http v1.1.0
│   ├── bytes v1.7.2
│   ├── fnv v1.0.7
│   └── itoa v1.0.11
└── http-body v1.0.1
    ├── bytes v1.7.2
    └── http v1.1.0 (*)

I also think this would make hyper more appealing to use with other executors like smol @notgull.

@jbaum98 jbaum98 added the C-feature Category: feature. This is adding a new feature. label Oct 20, 2024
@jbaum98
Copy link
Author

jbaum98 commented Oct 20, 2024

It seems to me that some options are:

  • Implement the functionality without a oneshot channel.
  • Use or vendor the implementation in futures-channel.
  • Use or vendor the implementation in async-oneshot. hyper compiles using the latest version in git, but not the latest released version.
  • Pull out the implementation from tokio.

@notgull
Copy link

notgull commented Oct 20, 2024

It's pretty easy to implement a oneshot channel with a mutex, especially if there is only one receiver. I'd say just implement it quickly in hyper itself.

The main issue is that hyper usually pulls in h2, which depends on tokio for a lot more than synchronization primitives. So I'm not sure what the practical effect of this would be.

@jbaum98
Copy link
Author

jbaum98 commented Oct 20, 2024

My understanding is that right now, there isn't any http implementation with few dependencies that can be integrated into many executors. Even async_h1 pulls in async-std. With this change, hyper with http2 disabled would fill that gap.

@jbaum98
Copy link
Author

jbaum98 commented Oct 20, 2024

And, from a quick look, perhaps the goal of dropping tokio as a dependency for h2 also isn't too crazy:

$ rg tokio --glob *.rs --glob '!tests' --glob '!benches' --glob '!examples' --glob '!fuzz' | rg -v '//'
src/proto/go_away.rs:use tokio::io::AsyncWrite;
src/proto/mod.rs:use tokio::io::AsyncWrite;
src/proto/connection.rs:use tokio::io::AsyncRead;
src/proto/ping_pong.rs:use tokio::io::AsyncWrite;
src/proto/streams/streams.rs:use tokio::io::AsyncWrite;
src/proto/streams/send.rs:use tokio::io::AsyncWrite;
src/server.rs:use tokio::io::{AsyncRead, AsyncWrite, ReadBuf};
src/client.rs:use tokio::io::{AsyncRead, AsyncWrite, AsyncWriteExt};
src/codec/mod.rs:use tokio::io::{AsyncRead, AsyncWrite};
src/codec/mod.rs:use tokio_util::codec::length_delimited;
src/codec/framed_write.rs:use tokio::io::{AsyncRead, AsyncWrite, ReadBuf};
src/codec/framed_write.rs:use tokio_util::io::poll_write_buf;
src/codec/framed_read.rs:use tokio::io::AsyncRead;
src/codec/framed_read.rs:use tokio_util::codec::FramedRead as InnerFramedRead;
src/codec/framed_read.rs:use tokio_util::codec::{LengthDelimitedCodec, LengthDelimitedCodecError};

Everything is AsyncRead, AsyncWrite, AsyncWriteExt, ReadBuf, or tokio_util.

@notgull
Copy link

notgull commented Oct 20, 2024

Agreed. Even just having an HTTP/1 implementation would be nice.

cc hyperium/h2#720

@carloskiki
Copy link

Just a stranger randomly landing on this issue, but you could also consider the oneshot crate. It has no dependencies, supports async, and is really simple.

@seanmonstar
Copy link
Member

Thanks for looking into this.

This would have a pretty huge impact on the transitive dependencies we pull in everywhere hyper is used.

This change likely would not have as big an impact as it appears. hyper only enables the sync feature of Tokio. That doesn't enable all the dependencies you listed. For example, if you're checking without enabling any of hyper's optional features, this is what an actual compile looks like:

    Checking fnv v1.0.7
    Checking itoa v1.0.11
    Checking bytes v1.7.2
    Checking http v1.1.0
    Checking pin-project-lite v0.2.14
    Checking tokio v1.40.0
    Checking http-body v1.0.1
    Checking hyper v1.5.0

(Depend on another crate, or vendor)

I'm not likely to be inclined to do that. The implementation in Tokio is very well tested and optimized. Part of keeping hyper safe is vetting its dependencies. I wouldn't want to switch to another dependency that doesn't have the same level of testing, and more importantly, active maintainers able to fix things if a problem is found.

My understanding is that right now, there isn't any http implementation with few dependencies that can be integrated into many executors.

I'd like to clear that up. hyper is indeed a good choice for any runtime. It's been able to support bringing your own runtime for a while, and with hyper v1, we made that even more supported. That it happens to use Tokio for some channels should not affect working with any other runtime. That's because Tokio's sync implementations don't rely on the runtime module. You can use hyper with any runtime you'd like. :)

@jbaum98
Copy link
Author

jbaum98 commented Oct 22, 2024

This change likely would not have as big an impact as it appears.

Interesting! Clearly cargo tree's default output didn't mean what I thought. I totally agree that the impact isn't as big as I would have thought. I checked, and it is worth noting that even though only tokio gets compiled, more of its transitive dependencies do get downloaded on a clean build.

  Downloaded itoa v1.0.11
  Downloaded http-body v1.0.1
  Downloaded socket2 v0.5.7
  Downloaded tokio-macros v2.4.0
  Downloaded unicode-ident v1.0.13
  Downloaded proc-macro2 v1.0.88
  Downloaded fnv v1.0.7
  Downloaded quote v1.0.37
  Downloaded pin-project-lite v0.2.14
  Downloaded bytes v1.7.2
  Downloaded mio v1.0.2
  Downloaded http v1.1.0
  Downloaded syn v2.0.80
  Downloaded tokio v1.40.0
  Downloaded libc v0.2.161
  Downloaded 15 crates (2.3 MB) in 2.01s
   Compiling fnv v1.0.7
   Compiling itoa v1.0.11
   Compiling bytes v1.7.2
   Compiling pin-project-lite v0.2.14
   Compiling tokio v1.40.0
   Compiling http v1.1.0
   Compiling http-body v1.0.1
   Compiling hyper v1.5.0 (/home/jake/tmp/hyper)
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 13.82s

compared to without tokio:

  Downloaded fnv v1.0.7
  Downloaded bytes v1.7.2
  Downloaded itoa v1.0.11
  Downloaded http-body v1.0.1
  Downloaded http v1.1.0
  Downloaded 5 crates (195.4 KB) in 0.41s
   Compiling fnv v1.0.7
   Compiling bytes v1.7.2
   Compiling itoa v1.0.11
   Compiling http v1.1.0
   Compiling http-body v1.0.1
   Compiling hyper v1.5.0 (/home/jake/tmp/hyper)

So I think dropping the tokio dependency makes a small difference, but definitely it's not dramatic as I thought.

I'm not likely to be inclined to do that. The implementation in Tokio is very well tested and optimized.

That makes sense. Would you be open to a PR that pulled out the tokio implementation? I'd understand if in your judgement it's not worth it.

I'd like to clear that up. hyper is indeed a good choice for any runtime.

Heard! Thank you for the work that went into making that the case! Seeing the dependency on tokio is kind of a red flag, but clearly that doesn't have the impact on compile-time or on run-time that one might expect from "pulling in a whole 'nother executor".

@dvtkrlbs
Copy link

can we also drop the `Send + 'static' requirement if we were to drop tokio dependency to support thread per core runtimes better.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-feature Category: feature. This is adding a new feature.
Projects
None yet
Development

No branches or pull requests

5 participants