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

Ditch async_trait in favor of the official async_fn_in_trait feature #959

Closed
wvwwvwwv opened this issue Dec 7, 2023 · 6 comments
Closed

Comments

@wvwwvwwv
Copy link
Contributor

wvwwvwwv commented Dec 7, 2023

Hello @drmingdrmer,

I guess you may already be aware of it, but the async_fn_in_trait feature has recently been stabilized in the nightly Rust channel and will be generally available in Rust 1.75.0. Currently, openraft heavily relies on the use of async_trait in order to provide asynchronous methods in traits, but obviously, async_trait has a negative effect on overall performance in many cases due to heap memory allocation.

So, I suggest that we try to get rid of async_trait incrementally.

  • I didn't try it yet, but it would be possible since the current toolchain (nightly-10-17) already includes Stabilize async fn and return-position impl Trait in trait rust-lang/rust#115822
  • Unfortunately, a complete removal will not be easy due to many known issues with async_fn_in_trait and related language features, e.g., Send bound errors.
  • Due to the notorious Send bound error, I think we should firstly focus on the !OptionalSend case.
Copy link

github-actions bot commented Dec 7, 2023

👋 Thanks for opening this issue!

Get help or engage by:

  • /help : to print help messages.
  • /assignme : to assign this issue to you.

@wvwwvwwv
Copy link
Contributor Author

wvwwvwwv commented Dec 7, 2023

Update: just tried to remove async_trait by slightly modifying macros/lib.rs to not emit async_trait, and it works pretty well.

@drmingdrmer
Copy link
Member

Make sense.

Since you've already provided an entry in macros/lib.rs thus it would be easy to switch off async_trait, when rust 1.75.0 becomes stable :)

Before that, I think I should add a single threaded application example as a test for singlethreaded feature 🤔

@wvwwvwwv
Copy link
Contributor Author

wvwwvwwv commented Dec 7, 2023

Great! Then I'll try removing async_trait in my forked repository, and then make a pull request once 1.75 is out.

@drmingdrmer
Copy link
Member

drmingdrmer commented Dec 29, 2023

@wvwwvwwv
Is this issue fixed by?:

Is there any additional action required?

@wvwwvwwv
Copy link
Contributor Author

Nope, we can close this issue for now since we cannot currently get rid of every async_trait usage due to some limitations in the Rust compiler.

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

2 participants