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

Tokio reform changes #645

Closed
wants to merge 8 commits into from
Closed

Tokio reform changes #645

wants to merge 8 commits into from

Conversation

carllerche
Copy link
Member

@carllerche carllerche commented Nov 16, 2017

Work in progress do not merge.

This PR represents the changes initially discussed in the Tokio reform RFC. Changes are not directly applied here but go through PRs that then get merged into this one. This enables granular changes to be reviewed / discussed in a more focused context.

Progress

Unanswered questions

Some believe that spawn represents better terminology for submitting a future to an executor. However, the library already contains an Executor trait that provides an execute function. Using disjoint terminology is not ideal either.

This commit deprecates `Future::wait`. Instead of having a trait
function, a wrapper is provided at `future::Blocking`. `Blocking` wraps
a `Future` value and provides thread-blocking operation on it.

The same treatment is applied to `Sink` and `Stream`.

Related: tokio-rs/tokio-rfcs#3.
@carllerche carllerche mentioned this pull request Nov 16, 2017
7 tasks
@carllerche
Copy link
Member Author

Incidentally, I also don't love how future and task is often used interchangeably when talking about "spawning/executing a [X]".

The `CurrentThread` executor is an event loop that allows the user to
spawn tasks that are guaranteed to run on the current thread.

This is done by extracting the scheduling logic from FuturesUnordered
and moving it to a `scheduler` module that can be used by both
`FuturesUnordered` and `CurrentThread` executor.

This is based on the Tokio reform RFC, but also includes some API
tweaks.
This helps prevent accidentally blocking an executor thread.
This enables customizing executor sleeping strategies.

Currently, it is not possible to run a Tokio reactor and a 
`CurrentThread` executor on the same thread. The executor is
hard coded to use condvars for sleeping and the Tokio reactor
requires calling `epoll_wait` (or equivalent) for blocking the
current thread.

The `Sleep` trait is added to abstract over this sleeping strategy.

Beyond just supporting the Tokio reactor, adding a `Sleep` trait is
useful for integrating any logic that requires hooking into the sleep
strategy (e.g. timers).

`executor::CurrentThread` is then modified to accept a `Sleep` value
that allows specifying a custom sleep strategy.
alexcrichton and others added 4 commits January 23, 2018 11:52
This commit moves the `CurrentThread` type to a top-level `current_thread`
module. Along the way a number of changes were made as well:

* Functions with "execute" in the name are tweaked to have "spawn" instead
  (e.g. `execute_daemon` becomes `spawn_daemon`).
* The `CurrentThread` type was renamed to `TaskExecutor` and
  `CurrentThread::current` was renamed to `task_executor`.
* The `run` method was renamed to `block_with_init`.

This commit is currently following the [proposed design][rfc] and will make room
soon for a `block_on_all` method as well. For now though it's primarily just
restruturing without any other deeper changes.

[rfc]: https://github.com/aturon/tokio-rfcs/blob/tokio-reform/tokio-reform.md#changes-to-the-futures-crate
Move `executor::CurrentThread` to `current_thread`
This patch removes the concept of daemon tasks from the current thread
executor as well as the global function to cancel all spawned tasks.

If appropriate, these APIs can be brought back at a later time, but in
an effort to initially be conservative and ship a release, they are
being held back.

The main hesitation being that these APIs are related to shutting down
an executor and do not seem appropriate to be accessible from a global
context. If they remain accessible, then once the executor
implementation is pluggable, it will require that all executors
implement the same logic.

Before making this commitment, an analysis of real use cases should be
done.
@alexcrichton
Copy link
Member

Work is now happening on the 0.2 branch, so I'm going to close this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants