refactor: replace async-trait with native async functions in traits - #441
Merged
Conversation
Rust stabilized async functions in traits in 1.75 and the MSRV is already 1.88, so the macro carries its own weight for nothing. Removing it drops a proc-macro from the build graph and the `Box<dyn Future>` that each of the 30 annotated blocks allocated on every call, which the state machine paid on every transition. All but one of those blocks asked for `?Send`, and that is what a native async function gives on its own. `CommunicationState` used the `Send` variant, but nothing ever needed its future to be `Send`, so the bound goes away with no other change. Two things the expansion was hiding: `StateChangeImpl::handle` now needs `where Self: Sized`. A native async function makes a trait dyn-incompatible, and `State::inner_state` returns `&dyn StateChangeImpl`. The bound keeps `handle` out of the vtable and costs nothing, because `inner_state` only reaches `name`, `is_handling_download` and `is_preemptive_state`. `Download::handle` took `mut self` and never mutated it. The macro moved the binding somewhere `unused_mut` could not see it. async-trait stays in Cargo.lock as an indirect dependency of other crates.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Rust stabilized async functions in traits in 1.75 and the MSRV of this workspace is already 1.88, so
async-traitno longer earns its place. This drops the dependency and removes all 30#[async_trait]attributes.What it buys
Box<dyn Future>allocation per call. The state machine paid that on every transition.Two things the expansion was hiding
StateChangeImpl::handleneedswhere Self: Sized. A native async function makes a trait dyn-incompatible, andState::inner_statereturns&dyn StateChangeImpl. The bound keepshandleout of the vtable and costs nothing, becauseinner_stateonly reachesname,is_handling_downloadandis_preemptive_state.Download::handletookmut selfand never mutated it. The macro moved the binding somewhereunused_mutcould not see it.Notes
?Send, which is what a native async function gives on its own.CommunicationStateused theSendvariant, but nothing needed its future to beSend.async-traitstays inCargo.lockas an indirect dependency of other crates.Verification
cargo clippy --workspace --all-targets --all-features -- -D warningsaftercargo cleanof every workspace crate: no errors, no warnings.cargo test: 162 pass, 0 fail.