I/O resources lazily bind to reactor. #160
Conversation
This patch makes a significant change to how I/O resources bind to a reactor. Currently, an I/O resource (TCP, UDP, PollEvented) will bind itself with a reactor upon creation. First, some history. Originally, tokio-core required that I/O resources be explicitly associated with a reactor upon creation by passing in a `&Handle`. Tokio reform introduced a default reactor. If I/O resources do not specify a reactor upon creation, then the default reactor is used. However, futures tend to favor being lazy. Creating a future should do no work, instead it is defining a computation to be performed once the future is executed. Binding an I/O resource with a reactor on creation goes against this pattern. This patch fixes this by allowing I/O resources to lazily bind to a reactor. An explicit `&Handle` can still be used on creation, but if no reactor is specified, then the default reactor is used. However, this binding happens during execution time (read / write) and not creation.
| /// idea. Concurrent calls to `register` will attempt to register different | ||
| /// tasks to be notified. One of the callers will win and have its task set, | ||
| /// but there is no guarantee as to which caller will succeed. | ||
| pub fn register_task(&self, task: Task) { |
There was a problem hiding this comment.
This file was copied from the futures crate in order to get this fn.
A PR has been submitted.
| @@ -117,6 +118,8 @@ impl Drop for Background { | |||
| None => return, | |||
| }; | |||
|
|
|||
| inner.shutdown_now(); | |||
There was a problem hiding this comment.
This fixes an unrelated bug found while testing this change.
| /// Try to get a handle to the current reactor. | ||
| /// | ||
| /// Returns `Err` if no handle is found. | ||
| pub(crate) fn try_current() -> io::Result<Handle> { |
There was a problem hiding this comment.
It might be useful to expose this as a public API, but for now it is kept private.
| /// Essentially a good rule of thumb is that if you're using the `poll_ready` | ||
| /// method you want to also use `need_read` to signal blocking and you should | ||
| /// otherwise probably avoid using two tasks on the same `PollEvented`. | ||
| pub struct PollEvented<E> { |
There was a problem hiding this comment.
The existing PollEvented type is not compatible with lazy binding due to how the trait bounds are setup. Because of this, a new type must be introduced and the existing PollEvented deprecated.
This also reverts some aspects of PollEvented back to what they were originally in tokio-core. Adding lazy binding reduced some of the benefits of limiting fns to &mut self.
| /// The difficulty is due to the fact that a single registration drives two | ||
| /// separate tasks -- A read half and a write half. | ||
| #[derive(Debug)] | ||
| pub struct Registration { |
There was a problem hiding this comment.
Registration is introduced as the most minimal API that is required to integrate with the reactor. It is only pair of readiness streams (one for read, one for write).
| @@ -530,7 +552,7 @@ impl Inner { | |||
| Direction::Write => (&sched.writer, mio::Ready::writable()), | |||
| }; | |||
|
|
|||
| task.register(); | |||
| task.register_task(t); | |||
There was a problem hiding this comment.
task is an AtomicTask? Not a big deal, just reads a little funny.
There was a problem hiding this comment.
I agree, but I also didn't want to have to re-run CI :P I'll try to bundle this up w/ another PR.
This patch makes a significant change to how I/O resources bind to a
reactor. Currently, an I/O resource (TCP, UDP, PollEvented) will bind
itself with a reactor upon creation.
First, some history.
Originally, tokio-core required that I/O resources be explicitly
associated with a reactor upon creation by passing in a
&Handle. Tokioreform introduced a default reactor. If I/O resources do not specify a
reactor upon creation, then the default reactor is used.
However, futures tend to favor being lazy. Creating a future should do
no work, instead it is defining a computation to be performed once the
future is executed. Binding an I/O resource with a reactor on creation
goes against this pattern.
This patch fixes this by allowing I/O resources to lazily bind to a
reactor. An explicit
&Handlecan still be used on creation, but if noreactor is specified, then the default reactor is used. However, this
binding happens during execution time (read / write) and not creation.