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

New poll_immediate functions to immediately return from a poll #2452

Merged
merged 4 commits into from
Jul 29, 2021

Conversation

tylerhawkes
Copy link
Contributor

@tylerhawkes tylerhawkes commented Jun 17, 2021

I can open an issue as well if needed, but since I've already done the work I thought it would be just as easy to actually talk about the additions proposed in the pull request.

The reasons for these new functions are because I'm using a tokio::sync::mpsc::Receiver and sometimes I need to just wait until a value is ready and sometimes I need to check if one is ready and if not do some other processing. This can be accomplished by doing

    futures_util::select_biased! {
        msg = receiver.recv().await => {...}
        _ = futures_util::future::ready(()) => {...}
    }

but it seems like it would be nice to have a dedicated Future for this that works better with IDE's.

I could probably use the now_or_never() function from FuturesExt, but that consumes the future and these new functions and futures allow for polling repeatedly (due to the Stream implementation) to see if they are done without blocking and they can also get the future back if it is Unpin which is useful if the futures are expensive to construct or if you want to just wait for them to be done at a later time.

Closes #2257

Copy link
Member

@taiki-e taiki-e left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. Almost all of unsafe code used in this PR is not correct. Please use pin-project-lite as existing code does.

futures-util/src/future/poll_immediate.rs Outdated Show resolved Hide resolved
futures-util/src/stream/poll_immediate.rs Outdated Show resolved Hide resolved
futures-util/src/future/poll_immediate.rs Outdated Show resolved Hide resolved
@taiki-e taiki-e added A-future Area: futures::future A-stream Area: futures::stream S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author labels Jun 18, 2021
#1)

New poll_immediate(_unpin) functions to immediately return from a poll on futures and streams
@tylerhawkes
Copy link
Contributor Author

tylerhawkes commented Jun 22, 2021

@taiki-e I think all the checks should pass now, if you can approve the waiting workflow.

@taiki-e taiki-e removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author label Jun 23, 2021
@tylerhawkes tylerhawkes requested a review from taiki-e June 23, 2021 21:33
Copy link
Member

@taiki-e taiki-e left a comment

Choose a reason for hiding this comment

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

Some futures expect it will be polled until complete, and such usage may cause problems. (see smol-rs/async-io#37 (comment))
Also, creating futures frequently and then dropping them may cause performance problems. (see #2173)

I am not strongly opposed to adding such features, but I think it would be preferable to mention these issues in the documentation.

futures-util/src/stream/poll_immediate.rs Outdated Show resolved Hide resolved
futures-util/src/future/poll_immediate.rs Outdated Show resolved Hide resolved
…re. Removing the poll_immediate_reuse function.
@tylerhawkes tylerhawkes requested a review from taiki-e June 25, 2021 17:28
@tylerhawkes tylerhawkes changed the title New poll_immediate(_reuse) functions to immediately return from a poll New poll_immediate functions to immediately return from a poll Jun 28, 2021
Copy link
Member

@taiki-e taiki-e left a comment

Choose a reason for hiding this comment

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

Thanks. LGTM aside from a nit.

futures-util/src/future/poll_immediate.rs Outdated Show resolved Hide resolved
Co-authored-by: Taiki Endo <te316e89@gmail.com>
Copy link
Member

@taiki-e taiki-e left a comment

Choose a reason for hiding this comment

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

Thanks!

@taiki-e taiki-e merged commit ea07b4b into rust-lang:master Jul 29, 2021
@taiki-e taiki-e added the 0.3-backport: pending The maintainer accepted to backport this to the 0.3 branch, but backport has not been done yet. label Jul 31, 2021
@taiki-e taiki-e mentioned this pull request Aug 28, 2021
@taiki-e taiki-e added 0.3-backport: completed and removed 0.3-backport: pending The maintainer accepted to backport this to the 0.3 branch, but backport has not been done yet. labels Aug 28, 2021
@taiki-e taiki-e mentioned this pull request Aug 30, 2021
@taiki-e
Copy link
Member

taiki-e commented Aug 30, 2021

Published in 0.3.17

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0.3-backport: completed A-future Area: futures::future A-stream Area: futures::stream
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Utility for polling a future from an async function
2 participants