refactor(lib): drop futures-util except in ffi #3890
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Make hyper usable for h1/h2 and client/server without this heavyweight dependency. It's about 17k lines of code and takes up to 1.7 seconds to compile on my machine, but hyper is only using a tiny fraction of it. Larger applications probably still pull in futures-util by other means, but it's no longer as unavoidable as in the early days of the ecosystem. To remove futures-util without raising MSRV, I took these steps: * When futures-util just re-exports something from its dependencies, use it directly from the source. * Inline trivial helpers like `poll_unpin` that "only" communicate intent a little better but don't save any significant amount of code. * Refactor the h2 client code to avoid `StreamFuture` for the "Client has been dropped" detection -- just poll the mpsc channel directly. * Implement a couple of small helpers from scratch when they're straightforward and fit on one screen each. The majority of this is polyfills for standard library APIs that would require a higher MSRV. * Use `AtomicWaker` from the `atomic-waker` crate, a separately published copy of the futures-util type of the same name. While the two crates are owned by different organizations (smol-rs vs. rust-lang), it's mostly the same people maintaining both copies. The uses of future-util in hyper's tests/benches/examples and in the `ffi` module seem much harder to remove entirely, so I did not touch those modules at all.
hanna-kruppe
commented
May 17, 2025
There was a problem hiding this comment.
I'm not a fan of all these polyfills but I included them for now to show it's possible (and what it costs) to do without MSRV bump. I'd also be happy to bump the MSRV if that's acceptable.
seanmonstar
approved these changes
May 19, 2025
|
If you're happy with it then I'm happy as well :) Thanks for the quick review! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Make hyper usable for h1/h2 and client/server without this heavyweight dependency. It's about 17k lines of code and takes up to 1.7 seconds to compile on my machine, but hyper is only using a tiny fraction of it. Larger applications probably still pull in futures-util by other means, but it's no longer as unavoidable as in the early days of the ecosystem.
To remove futures-util without raising MSRV, I took these steps:
poll_unpinthat "only" communicate intent a little better but don't save any significant amount of code.StreamFuturefor the "Client has been dropped" detection -- just poll the mpsc channel directly.AtomicWakerfrom theatomic-wakercrate, a separately published copy of the futures-util type of the same name. While the two crates are owned by different organizations (smol-rs vs. rust-lang), it's mostly the same people maintaining both copies.The uses of future-util in hyper's tests/benches/examples and in the
ffimodule seem much harder to remove entirely, so I did not touch those modules at all.