-
Notifications
You must be signed in to change notification settings - Fork 650
Drastically simplify with ideas from tokio #77
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
Conversation
A more appealing model is actually just automatically inferring what needs to be scheduled based on what actions are done during poll. For example if during a poll you check a oneshot channel, then the current task is registered for being woken up if it's not ready. Similarly this will apply to I/O where if I/O is attempted but we see EAGAIN then we'll schedule the task to get notified when it's ready. This may also have performance benefits in some niche situations because you don't need to recompute where you are in the state machine both during poll and during schedule. Instead, it now happens all at once.
After some re-evaluation it was concluded that this parameter isn't actually needed as part of the `poll` function. An alternative to threading through a task is that there's simply a thread local global which indicates the "current task". This global is then used in the same way `thread::{park, unpark}` are used to block and notify tasks. Additionally, this change unlocks a few further benefits: * The core trait and all combinators are now `#![no_std]` compatible (minus oneshot/channel) * Detaches `Task` from `Future` so different runtime systems can perhaps be used to drive a future. (none planned currently) * It will soon be easier to remove the `'static` bounds from the trait because the `Task` is stored separately and the lifetimes don't need to be connected. * The `poll` signature in general is now much simpler as it takes 0 arguments. A downside of this change is that it is much easier to panic a thread. All I/O or "blocking" operations require a current task to be set in a global context, and if not set these operations will panic. This drawback, however, seems well worth the benefits that it unlocks.
* Auto-register interest whenever we see WouldBlock * Remove implementations of `Stream<Item=Ready>`, no longer needed * Add explicit `poll_{read,write}` methods, if needed * Remove all I/O streams, libstd ones suffice * Update all I/O futures
Also involved yet another round of bug fixes to the timer wheel as well as an unfortunately serious rejiggering of the level-translation into libcurl. Eew.
This optimization only rarely worked anyway and was otherwise a bit of an ugly addition to the `Future` trait. More technically, there are two reasons tailcall is not generally applicable: * It *only* works in chains like `.and_then(..).and_then(..)`. If, for example, a `map` or `map_err` were inserted in the middle then the optimization wouldn't appply. Additionally, it only worked with trait objects in play, which isn't necessarily always the case. * It doesn't handle "back pressure". That is if a stream of events is ready very quickly, there are still many test cases of it blowing the stack. In other words, the problem that `tailcall` was supposed to solve, was still left unsolved in most cases. Removing it also additionally unlocks removing the `'static` bound from the trait, which is another nice benefit!
The core trait and associated types no longer require the `'static` bounds, and all associated methods have also had the `'static` bounds removed. The only location for the `'static` bound is `forget`. While the restriction of `'static` on `forget` is here to stay, we'll soon enable `Loop::run` to take a non-`'static` future, allowing driving a non-`'static` future.
We know that the future will never persist beyond this stack frame, so we can just leave its ownership on the stack frame itself and receive notifications off the event loop that we need to poll it.
Add a macro, `try_nb!`, which was usable in all cases for simplifying handling of `WouldBlock`.
No longer needed!
We see `Interrupted` once the server auth completed and we requested to break at that point. If this happens then we need to turn the loop again, validate some certificates, and then resume the handshake process.
I like the changes. Simplification is good :-) |
👍 looks good to me. |
Sorry for asking after it was merged and without being able to give a deep read of the code, but what about environments that don't have threads? The concepts of this library on a tiny microcontroller still make sense to me, even if I haven't had any opportunity to do anything with it. |
That's a good question! For microcontrollers they're probably using The core assumption, however, is that there is some global state which indicates how to "block the current task" right now. That is, there must be some equivalent for For cases without threads this could just be implemented trivially with a global, so I don't think this excludes those situations (or at least that's our belief) |
This series of commits are the culmination of some discussions with @carllerche
about experiences in tokio and how they could be more generally applicable to
futures. The end result is a drastically simplified
Future
trait, simplifiedinterface to I/O, and a much more ergonomic method of implementing a future.
The two key insights which led to simplification was:
schedule
method ifpoll
did the scheduling for you.That is, if
poll
returnsNotReady
then it's responsible for arranging thefuture's task to get notified.
Task
parameter, but instead use thread local storage for the current task.This enables reusing existing abstractions off the shelf as threading a
Task
parameter around isn't necessary.
When combined together, these end up having large ramifications across the
implementations of the event loop, futures, etc.
Simplified
Future
traitThe trait now looks like:
As mentioned above, the
schedule
method has been removed aspoll
will dothat automatically. Additionally the
Task
argument has been removed as it'snow a piece of thread-local global state indicating what the current task is.
Finally, the
'static
bounds have been removed entirely (more on this in amoment).
Finally, with this definition, it means that the
Future
trait is#![no_std]
compatible! All core combinators and the trait itself can be used in the context
of only libcore. The scheduling (e.g.
task
module) of futures still requiresthe standard library, but it is now easily split off into a separate layer if
needed.
Simplified I/O
In the spirit of
poll
automatically "scheduling" the current task to receive anotification when it's ready to make progress, I/O now operates the same way.
When a
read
orwrite
is performed and it returns "would block", the currenttask is scheduled to receive a notification when the I/O operation is again
available.
This means that the futures-io abstraction of
Stream<Item=Ready>
is no longerneeded, and has now been removed. All I/O now works purely over the
Read
andWrite
traits assuming that it will not block. Most of the I/O objects infutures-io
have been removed as the ones in the standard library can be usedinstead. The futures abstractions here, however, for things like
write_all
etcstill exist (and operate over
Read
andWrite
).This in turn greatly simplifies implementations of state machines as you
generally just perform the I/O as if it were going to succeed, and "would block"
is propagated as
NotReady
. The operation will then get automatically retriedwhen I/O is available again.
Removal of
'static
Another key improvement in this PR is the removal of the
'static
bound fromthe
Future
trait and its associated types. The insight here is that anon-
'static
future, containing stack borrows, can be run like scoped threads.That is, you can borrow data as long as you know the task completes before the
stack frame is popped.
The
Loop::run
method no longer requires the'static
bound and can be used toexecute futures which do not have the
'static
lifetime.