launchbadge / sqlx

🧰 The Rust SQL Toolkit. An async, pure Rust SQL crate featuring compile-time checked queries without a DSL. Supports PostgreSQL, MySQL, and SQLite.
Apache License 2.0
12.48k stars 1.19k forks source link

Moving away from `async_trait` and into Rust's own async traits #3059

Open Aandreba opened 5 months ago

Aandreba commented 5 months ago

Given the recent stabilization of async fn and return impl Trait in traits on Rust 1.75.0, perhaps it should be time to consider moving away from the async_trait crate.

This would probably allow the compiler to optimize async blocks more easily, and would most likely result in a decrease in memory usage (given that async_trait boxes the futures returned by the trait functions). It would also mean one dependency less to maintain, which is always nice.

Things to consider

If you see any other downsides (or benefits) to this change, please feel free to comment them.

saiintbrisson commented 5 months ago

Though removing unnecessary boxing is always nice, I don't feel bumping the MSRV to 1.75 is worth the change. We would also really just swap async_trait for trait_variant (or do it by hand anyway). I say we wait until RPITIT is more mature and not discouraged by the async team.

FSMaxB commented 5 months ago

We would also really just swap async_trait for trait_variant (or do it by hand anyway).

I don't think trait_variant is required, because sqlx requires the futures to be Send anyways, so no need (and no possiblity either) to support both Send and !Send. This also means that the current limitation doesn't apply from what I understand.

See also the discussion in #3016

abonander commented 5 months ago

Re. MSRV, I'll remind readers of our official policy: https://github.com/launchbadge/sqlx/blob/main/FAQ.md#what-versions-of-rust-does-sqlx-support-what-is-sqlxs-msrv

If we do this refactor alongside #3016 it would have to be a major release anyway as that's a breaking change.