minghuaw / fe2o3-amqp

A rust implementation of the AMQP1.0 protocol based on serde and tokio.
MIT License
58 stars 7 forks source link

Return `impl Future + Send + ‘lifetime` in traits instead of using `async_trait` macro #220

Closed minghuaw closed 6 months ago

minghuaw commented 6 months ago

Other crates have observed performance improvement after adopting this strategy (eg. async-graphql reports almost 20% performance gain).

minghuaw commented 6 months ago

This is only stabilized in 1.75 (rust-lang/rust#115822), would this be considered a breaking change?

minghuaw commented 6 months ago

Because of https://github.com/rust-lang/rust/issues/100013, a breaking release might be needed

minghuaw commented 6 months ago

Because of https://github.com/rust-lang/rust/issues/100013, a breaking release might be needed

Removing Send bound for txn related traits kinda solves this problem, but this also means that the futures returned by those methods are not Send, which should probably be considered a breaking change.

minghuaw commented 6 months ago

Removing Send bound for txn related traits kinda solves this problem, but this also means that the futures returned by those methods are not Send, which should probably be considered a breaking change.

When the issue is fixed, this will simply allow the future to be Send again, which is more general and should not be considered breaking then.

minghuaw commented 6 months ago

The two traits that are affected by this are:

  1. TransactionDischarge in fe2o3-amqp. The three async methods are not Send
  2. AsyncCbsTokenProvider in fe2o3-amqp-cbs. The associated type Fut is removed and get_token_async returns impl Future
minghuaw commented 6 months ago

Because of rust-lang/rust#100013, a breaking release might be needed

Sender is also affected. A breaking change is probably not enough, and v0.10.0 might need to wait for the issue

minghuaw commented 6 months ago

A breaking release seems not necessary at all now. The issue is (sort of) resolved in a weird way. Essentially, return position impl trait is used in trait declaration but async fn in trait is used in the trait implementation. ie.

pub trait AsyncTrait {
    fn some_async_fn() -> impl Future<Output = ()> + Send;
}

struct Foo;

impl AsyncTrait for Foo {
    async fn some_async_fn() -> () {
        // ....
    }
}
minghuaw commented 6 months ago

Implemented 0.9.4 and 0.8.27