axum
axum copied to clipboard
use afit and remove async-trait
Motivation
https://github.com/rust-lang/rust/pull/115822
1.75.0 will be stable on: 28 December, 2023.
Remove async-trait and use AFIT(Async Fn In Trait) and RPITIT(Return Position Impl Trait In Traits).
Solution
Are you able to make it actually compile?
I'm not totally up to date on the specifics of async fn in traits but last I heard there were some shortcomings around declaring/propagating Send bounds. That probably will be a deal breaker for axum.
The situation is this: You still can't write a generic function like fn foo<T: FromRequest>() where T::foo(): Send to say you need the returned future to be Send. However, async_trait was already enforcing Sendness for all trait implementations, and that is still possible by using fn from_request(...) -> impl Future<Output = Self> + Send in the trait declaration, such that callers can rely on all implementations fulfilling that. Implementations can then use async fn sugar.
As such, I think it makes sense to get this in for the next breaking release. It removes a bunch of indirection, and allows auto traits other than Send to be inferred in non-generic contexts. It should also make the documentation a little easier to read, and might even allow a few projects to get rid of syn & quote in their dependency trees (turbo.fish is really close, other than axum's async-trait dependency there is only tower's pin-project dependency which is still waiting on a breaking-change release though, unless the decision to not merge my PR removing it by introducing a few lines of unsafe is revisited).
As for this PR, it introduces a number of trait declarations that don't use + Send after -> impl Future and I think that should be changed, even if there is no code that relies on the Sendness of the returned futures in generic code inside axum.
Thats great news! 🎉
Are you able to make it actually compile?
I'm not totally up to date on the specifics of
async fnin traits but last I heard there were some shortcomings around declaring/propagatingSendbounds. That probably will be a deal breaker for axum.
It can be compiled on nightly version.
rustup default nightly
As for this PR, it introduces a number of trait declarations that don't use
+ Sendafter-> impl Futureand I think that should be changed, even if there is no code that relies on theSendness of the returned futures in generic code inside axum.
Why is + Send nessesary? It can be compiled without + Send.
Why is
+ Sendnessesary? It can be compiled without+ Send.
Because it's impossible for generic functions to declare that they accept any implementer of a given trait where one or all of its async methods return a Send future.
I have rebased it to axum 0.7
There are still a lot of macros need to be fixed.
::axum::extract::FromRequest::from_request
should be changed to
<#field_ty as ::axum::extract::FromRequest<#trait_generics, _>>::from_request // the second generic type is private::ViaParts
---- debug_handler::ui stdout ----
thread 'debug_handler::ui' panicked at /Users/lz1998/.cargo/registry/src/rsproxy.cn-8f6827c7555bfaf8/trybuild-1.0.86/src/run.rs:101:13:
6 of 29 tests failed
---- from_request::ui stdout ----
thread 'from_request::ui' panicked at /Users/lz1998/.cargo/registry/src/rsproxy.cn-8f6827c7555bfaf8/trybuild-1.0.86/src/run.rs:101:13:
15 of 58 tests failed
error[E0282]: type annotations needed
--> src/main.rs:22:11
|
22 | host: Result<headers::Host, TypedHeaderRejection>,
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ cannot infer type
@davidpdrsn It's hard to solve the problem of macros because of the second generic type private::ViaParts, can you help to solve this?
Sure I'll try and take a look!
Sure I'll try and take a look!
I have rebased it to the main branch.
Thanks for all your work on this by the way ❤️
Just to add on this, if someone was wondering about the performance implications.
(Tested with --release, lto = "thin", and codegen-units = 1)
I am comparing with the following Cargo.toml
[package]
name = "bench"
version = "0.1.0"
edition = "2021"
# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
[profile.release]
lto = "thin"
codegen-units = 1
[dependencies]
tokio = { version = "1", features = [ "full" ] }
# This PR
axum = { git = "https://github.com/lz1998/axum.git", branch = "main" }
# Upstream
#axum = { git = "https://github.com/tokio-rs/axum.git", rev = "c1c917092daeb91456b41510b629c4ba9763b2b6" }
and this main.rs
use axum::{
routing::get,
Router,
};
#[tokio::main]
async fn main() {
// build our application with a single route
let app = Router::new()
.route("/", get(|| async { "Hello, World!" }));
// run our app with hyper, listening globally on port 3000
let listener = tokio::net::TcpListener::bind("0.0.0.0:3000").await.unwrap();
axum::serve(listener, app).await.unwrap();
}
Baseline results
# --release lto = "thin" codegen-units = 1
> wrk -t1 -c1 --latency http://127.0.0.1:3000
Running 10s test @ http://127.0.0.1:3000
1 threads and 1 connections
Thread Stats Avg Stdev Max +/- Stdev
Latency 12.35us 5.98us 330.00us 95.38%
Req/Sec 80.57k 10.08k 88.10k 88.00%
Latency Distribution
50% 11.00us
75% 12.00us
90% 14.00us
99% 33.00us
800650 requests in 10.00s, 99.26MB read
Requests/sec: 80064.18
Transfer/sec: 9.93MB
> wrk -t16 -c1000 --latency http://127.0.0.1:3000
Running 10s test @ http://127.0.0.1:3000
16 threads and 1000 connections
Thread Stats Avg Stdev Max +/- Stdev
Latency 1.04ms 1.16ms 39.22ms 88.95%
Req/Sec 71.86k 8.35k 114.12k 74.25%
Latency Distribution
50% 608.00us
75% 1.18ms
90% 2.35ms
99% 5.55ms
11481372 requests in 10.08s, 1.39GB read
Requests/sec: 1139313.22
Transfer/sec: 141.25MB
> ls -lb target/release
-rwxr-xr-x. 2 giacomo giacomo 8417072 Jan 7 22:40 bench
PR results
# --release lto = "thin" codegen-units = 1
> wrk -t1 -c1 --latency http://127.0.0.1:3000
Running 10s test @ http://127.0.0.1:3000
1 threads and 1 connections
Thread Stats Avg Stdev Max +/- Stdev
Latency 11.34us 4.51us 588.00us 98.88%
Req/Sec 86.17k 5.20k 88.44k 98.02%
Latency Distribution
50% 11.00us
75% 11.00us
90% 12.00us
99% 16.00us
865967 requests in 10.10s, 107.36MB read
Requests/sec: 85737.63
Transfer/sec: 10.63MB
> wrk -t16 -c1000 --latency http://127.0.0.1:3000
Running 10s test @ http://127.0.0.1:3000
16 threads and 1000 connections
Thread Stats Avg Stdev Max +/- Stdev
Latency 1.02ms 1.12ms 18.49ms 88.87%
Req/Sec 72.55k 7.59k 100.27k 67.81%
Latency Distribution
50% 604.00us
75% 1.16ms
90% 2.30ms
99% 5.51ms
11593246 requests in 10.06s, 1.40GB read
Requests/sec: 1152503.62
Transfer/sec: 142.88MB
> ls -lb target/release
-rwxr-xr-x. 2 giacomo giacomo 8417080 Jan 7 22:38 bench
As you can see, latency and throughput improves with almost no difference in binary size
Also interesting that the P99 latency for -c1 is halved
@lz1998 @jplatte RPITIT & AFIT (Rust 1.75) have been stable for more than 6 months now. Is there a chance that this can be merged soon? If theres any work remaining I'd like to help.
There is a chance, though getting a release out is a lot more involved (due to the matchit upgrade, but also a few other planned changes).
Anyways.. @lz1998 I just fixed merge conflicts for you, would you mind taking another look at the current CI failures?
There is a chance, though getting a release out is a lot more involved (due to the matchit upgrade, but also a few other planned changes).
Anyways.. @lz1998 I just fixed merge conflicts for you, would you mind taking another look at the current CI failures?
The commented line will cause an error, because the second generic type private::ViaRequest is private. Can you help to solve this? @jplatte
https://github.com/lz1998/axum/blob/main/axum-macros/src/lib.rs#L162
use axum::extract::{FromRequest, Request};
use axum_extra::{
TypedHeader,
headers::{UserAgent},
};
struct MyExtractor {}
impl<S: Send + Sync> FromRequest<S> for MyExtractor {
type Rejection = ();
async fn from_request(req: Request, state: &S) -> Result<Self, Self::Rejection> {
// let _ = <TypedHeader::<UserAgent> as FromRequest<S>>::from_request(req, state).await;
let _ = TypedHeader::<UserAgent>::from_request(req, state).await;
Ok(MyExtractor {})
}
}
#[tokio::main]
async fn main() {}
error[E0277]: the trait bound `TypedHeader<UserAgent>: FromRequest<S>` is not satisfied
--> src/main.rs:18:18
|
18 | let _ = <TypedHeader::<UserAgent> as FromRequest<S>>::from_request(req, state).await;
| ^^^^^^^^^^^^^^^^^^^^^^^^ the trait `FromRequest<S>` is not implemented for `TypedHeader<UserAgent>`
|
= note: Function argument is not a valid axum extractor.
See `https://docs.rs/axum/0.7/axum/extract/index.html` for details
= help: the following other types implement trait `FromRequest<S, M>`:
(T1, T2)
(T1, T2, T3)
(T1, T2, T3, T4)
(T1, T2, T3, T4, T5)
(T1, T2, T3, T4, T5, T6)
(T1, T2, T3, T4, T5, T6, T7)
(T1, T2, T3, T4, T5, T6, T7, T8)
(T1, T2, T3, T4, T5, T6, T7, T8, T9)
and 20 others
Yeah, in that case you wouldn't use T::from_request, you'd use T::from_request_parts or RequestExt::extract_parts.
Oh, that code isn't what we actually have as an example whatever, it's macro-generated? I don't see how this would be a new problem though.. Will look at what changed in the macro code.
Small tip, instead of accepting every suggestion individually, you can go to the files tab of the PR and batch them up into a single commit 😉
Also, I opened #2943 for the MSRV bump that's needed for this.
Is it possible to get a beta release with this patch included?
I was planning to do a pre-release soon, yes. I'll probably wait until we have merged the matchit upgrade though.
This is out as part of the latest set of alpha releases :)