Skip to content

Manually implement the future for with_timeout #4123

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

Merged
merged 1 commit into from
Apr 23, 2025

Conversation

alexmoon
Copy link
Contributor

Using async fn for these functions causes the memory for fut to be double-allocated due to rust-lang/rust#62958. Switching to a manual implementation of the future prevents the double-allocation which may be significant if the future is large.


/// Future for the [`with_timeout`] and [`with_deadline`] functions.
#[must_use = "futures do nothing unless you `.await` or poll them"]
pub struct UntilTimer<F> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you rename this to TimeoutFuture?

It makes sense to call xxxFuture things that implement Future

@alexmoon alexmoon force-pushed the manual-with-timeout-impl branch from 5ace4c8 to 4d91140 Compare April 23, 2025 17:30
@alexmoon alexmoon force-pushed the manual-with-timeout-impl branch from 4d91140 to 0b8f43b Compare April 23, 2025 17:32
@Dirbaio
Copy link
Member

Dirbaio commented Apr 23, 2025

Thanks!

@Dirbaio Dirbaio added this pull request to the merge queue Apr 23, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Apr 23, 2025
@Dirbaio Dirbaio merged commit 8474e57 into embassy-rs:main Apr 23, 2025
7 checks passed
@ArXen42
Copy link

ArXen42 commented Apr 28, 2025

Noticed that after updating embassy with this PR merged my firmware size dropped by a few % for both flash and RAM.
Neat!

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

Successfully merging this pull request may close these issues.

3 participants