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

Park trait, ParkThread implementation and using it in LocalPool #1668

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

stbuehler
Copy link
Contributor

Hi,

I think it would be good to have a Park abstration (like the one in tokio) in futures-executor (this has already been proposed and merged in #665 for the tokio-reform branch, which has been abonded in #645).

In #665 Park was renamed to Sleep, but I think Park is the better choice:

  • std lib provides thread::park for a sleeping mechanism that can be cancelled ("unparked").
  • tokio users already know it by that name
  • it is not just a "sleep" mechanism due to the "wakeup" part

Apart from the names Sleep from #665 is basically the same as tokio Park; further changes to the API (compared to tokio):

  • merge Park::park_timeout into Park::park, now taking a ParkDuration enum: most implementations I've seen used shared code anyway.
  • Park::park now also includes a &mut Enter argument
  • Removed Unpark trait, Park no longer has an associated Unpark type. Using futures_util::task::WakerRef instead for the result of the waker function (renamed from unpark).

I think reusing the Waker abstraction is rather obvious; but there are probably two open questions:

  • Use Waker instead of WakerRef?
  • Remove ParkDuration::Poll and interpret a zero duration in ParkDuration::Duration(..) as "poll only but don't yield"?

This pull request also contains a ParkThread implementation of Park (based on std::thread::park and similar to what LocalPool was using) and supports using LocalPool with custom Park implentations (defaulting to ParkThread). This way LocalPool can be combined into a "local-thread runtime" with a timer and a reactor.

References:

@stbuehler
Copy link
Contributor Author

Just rebased to current master. Is there anything I can do to help moving this (and maybe other pull requests) forward?

- split all state but park into separate struct
- various "poll" methods need to call park(Poll) as it might change the
  readiness of futures (expired timers, IO ready, ...) or spawn new
  futures (although ParkThread::park(Poll) won't)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-executor Area: futures::executor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants