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

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

Closed
minghuaw opened this issue Feb 18, 2024 · 8 comments

Comments

@minghuaw
Copy link
Owner

minghuaw commented Feb 18, 2024

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

@minghuaw
Copy link
Owner Author

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

@minghuaw
Copy link
Owner Author

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

@minghuaw
Copy link
Owner Author

Because of rust-lang/rust#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
Copy link
Owner Author

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
Copy link
Owner Author

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
Copy link
Owner Author

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
Copy link
Owner Author

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
Copy link
Owner Author

Implemented 0.9.4 and 0.8.27

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant