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

@tylerhawkes tylerhawkes requested a review from taiki-e as a code owner Jun 17, 2021
Copy link
Member

@taiki-e taiki-e left a comment

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 A-stream S-waiting-on-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 label Jun 23, 2021
@tylerhawkes tylerhawkes requested a review from taiki-e Jun 23, 2021
Copy link
Member

@taiki-e taiki-e left a comment

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 Jun 25, 2021
@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

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

Thanks!

@taiki-e taiki-e merged commit ea07b4b into rust-lang:master Jul 29, 2021
22 checks passed
@taiki-e taiki-e added the 0.3-backport: pending 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 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 A-stream
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants