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

Consider allowing futures to require specialized executors? #1196

Closed
AlphaModder opened this issue Aug 12, 2018 · 25 comments
Closed

Consider allowing futures to require specialized executors? #1196

AlphaModder opened this issue Aug 12, 2018 · 25 comments

Comments

@AlphaModder
Copy link

Forgive me if this is not the appropriate place to be discussing things like this, but has the possibility of allowing futures to specify what sort of executors they may be spawned on been examined at all? This would allow libraries to support extra features like task thread-affinity or priority.

The way I'm thinking of doing this would be to make the following changes to Future and Context:

pub trait Future<S: Spawn + ?Sized = dyn Spawn> {
    type Output;

    fn poll(self: PinMut<Self>, cx: &mut Context<S>) -> Poll<Self::Output>;
}
pub struct Context<'a, S: Spawn + 'a + ?Sized = dyn Spawn> {
    local_waker: &'a LocalWaker,
    spawner: &'a mut S,
}
// And appropriate changes to method signatures in Context::new and Context::with_spawner

Existing futures would not need to change because of the defaulted type parameter, but futures could require additional features by changing S to something else. For instance, if a future has a non-send subfuture one could create a new trait SpawnLocal: Spawn which takes LocalFutureObj instead of FutureObj, and implement Future<dyn SpawnLocal>.

If anyone is interested, I have a sort of proof-of-concept for the changes in libcore here. (It's not an actual fork of libcore though, since I'm not quite sure how to do that.)

@jkozlowski
Copy link

I think this was discussed recently quite a lot on the discord channel, I wonder if this would be an interesting alternative. But it was mentioned that this was experimented with but quickly became quite unergonomic, can you put some examples into your repo?

@Nemo157
Copy link
Member

Nemo157 commented Aug 12, 2018

Is there some reason you want to just have Futures generic over the spawner instead of the whole context? The place I imagine using something like this is to be able to use alternative wake systems, where the lowest level futures need to be able to verify they're running on an executor that supports the out-of-band wake system they're using.

@jkozlowski
Copy link

Is there some reason you want to just have Futures generic over the spawner instead of the whole context?

That was my thought too, wasn't sure if that was possible off the bat. But doesn't look too onerous

@AlphaModder
Copy link
Author

@Nemo157 I did consider that but wasn't sure if there was enough of a use-case for alternate wake-systems. If there is, then I'd be equally happy with that proposal, since it seems like a pure generalization of this one.

@jkozlowski I can try and adapt some of the futures-core code for this system and see if it becomes an issue. Do you remember at all what sort of ergonomic issues they ran into?

@jkozlowski
Copy link

Sorry, I don't :( I just remember that this came up recently on discord (come join us: https://discord.gg/VHe9nG), but I wasn't involved in that effort.

@AlphaModder
Copy link
Author

AlphaModder commented Aug 14, 2018

@jkozlowski So far I've been able to convert futures-core and futures-channel to a generic-spawner (not generic-context as of the moment) system. The only ergonomic concessions so far have been:

  • Somewhat less pretty implementations for fully-generalized futures:
    impl Future for ... becomes impl<S: Spawn + ?Sized> Future<S> for ....
    However, I'm not sure if these fully-generalized impls are necessary for non-combinator futures. I think that when everything shakes out, an implementation for dyn Spawn (which will be the default) will be able to run on more specialized executors without knowing it just by virtue of the type system, but I'm not 100% sure yet.
  • Taking Option<&mut Context<S>> requires type inference when passing None. For now, I've only seen instances of this that ignore S entirely, and so I created a type WakeContext which erases the type of Spawn and just keeps the waker, and made Context deref to it. This way such cases can just take Option<&mut WakeContext>. Maybe the fully-generic-context solution would be more elegant here.

Here's the branch I'm working on, and the commit I did this in.

@AlphaModder
Copy link
Author

AlphaModder commented Aug 14, 2018

Hm, actually it looks like there may be a more fundamental issue here. I don't think it's at all possible to run less specialized futures on more specialized executors specifically because it's not possible to coerce something like &dyn SpawnLocal into &dyn Spawn even if SpawnLocal: Spawn. :/ The only workaround I can think of would be to use static dispatch, which would be an ergonomic nightmare, if it is even feasible.

@Ekleog
Copy link

Ekleog commented Nov 15, 2018

I've come here from the perspective of task-local- and timeout-able executors rustasync/team#7 (comment). These comments are, I think, what you call using static dispatch (which IMO would be the best option as it's truly 0-cost).

To add to the two messages from the link above, I just noticed this would even allow libraries to be generic over eg. TcpAbleExecutor, which would avoid libraries like ldap3 to be generic over its executor so long as it is able to do TCP connections.

Basically, I believe the current design forces libraries to depend on tokio as soon as they are doing non-trivial stuff (even timeouts make one need to depend on tokio, right now!), while the choice of the executor should be the end-user's choice, not the library/ecosystem's choice. Making Future generic over Executor would, I think, fix this :)

@Ralith
Copy link
Contributor

Ralith commented Nov 15, 2018

I believe the current design forces libraries to depend on tokio as soon as they are doing non-trivial stuff

I don't think this is true. You can write e.g. protocol implementations to be generic over an underlying I/O primitive, which might be tokio's TcpStream or some other executor's, and pass that primitive in during construction. Futures don't need awareness of executors to accomplish this.

@Ekleog
Copy link

Ekleog commented Nov 16, 2018 via email

@Ralith
Copy link
Contributor

Ralith commented Nov 16, 2018

There is nothing stopping you from being generic over your timeout primitives, just like you can be generic over your I/O primitives. It would be convenient if there was a standard trait for timeout primitives, just like there are a standard traits for asynchronous byte streams, but there is absolutely nothing stopping you from defining such a trait in your library and moving on with your now-generic code. The same applies for any form of primitive asynchronous event.

@Ekleog
Copy link

Ekleog commented Nov 16, 2018 via email

@AlphaModder
Copy link
Author

@Ekleog I see in the other thread you suggested a design where Future is generic over an Executor parameter. In my experiments, that seemed like adding an unnecessary level of indirection compared to just making the future generic over its context directly. After all, it could just use a context type that includes a way to get the executor. Would you agree, or am I missing a use case for the executor parameter?

Second, both those approaches have one limitation: the type of the context cannot depend on the lifetime of self in Future::poll, due to the lack of associated type constructors (i.e. higher-kinded types). When I tried to solve this problem initially, poll was defined as fn poll<'a>(Pin<&'a mut Self>, cx: Context<'a>), and so I had to abandon my efforts when I realized it was impossible to express that with a generic context. However, now cx is just a LocalWaker, so I'm not sure how much of a problem this limitation is anymore.

@Ralith
Copy link
Contributor

Ralith commented Nov 16, 2018

For instance, I may want a no_std executor that handles timeouts with a hook in the platform clock interruptions. In which case I don't have thread_local, and I can't do timeouts unless I'm passing something in-band to each future.

Again, timeouts are no different than any other primitive; sockets require support from the executor for exactly the same reasons. Hence, the same solutions apply to implementing them, without need for thread locals or universally propagated polymorphic contexts. When you pass in an executor-specific socket type to a network library, you're not just passing a file descriptor; you're also passing whatever information and handles that executor needs to bundle into the socket to process it. Similarly, when you pass in a timer factory obtained from your executor implementation, you are passing in whatever information and handles the executor deems necessary to implement timers.

@Ekleog
Copy link

Ekleog commented Nov 16, 2018 via email

@Ekleog
Copy link

Ekleog commented Nov 16, 2018

Actually, thinking about it again, passing arguments in-band has an additional drawback: it means it must be explicitly passed by every async/await-using function. While passing them as part of an executor-specific context (which allows lightweight executors not to support the feature and for support for it to be checked at compile-time) means that it's transparent to all functions but the ones that actually rely on the timeout / task-local / etc., it only appears in their signature as a + TimeoutExecutor that hopefully can be inferred by the compiler for async/await functions.

@Ekleog
Copy link

Ekleog commented Nov 16, 2018

@AlphaModder Good point for the lifetime. But I think poll could be defined this way, still allowing for lifetime-limited contexts, which would be useful for SpawnExecutor / SpawnContext that'd allow spawning from arbitrary points in the future hierarchy:

fn poll<'a, Ctxt: Context + 'a>(self: Pin<&'a mut Self>, cx: &'a mut Ctxt) -> Poll<...>

And then Ctxt could be bound to eg. TokioContext<'a> if need be, that'd implement all nice traits that define tokio.

On the other hand, this formulation pushes Ctxt back to poll instead of being on Future. Which means that Output can no longer be generic over Ctxt unless we have GAT. Now, is that actually an issue… Maybe, for futures that return futures when polled?

Now, if we think backwards, when does Ctxt need to be bound by a lifetime? I think that executors, with the help of a bit of unsafe (similar to what's in UnsafeWake currently), should be able to not make it unsafe for the user and use the lifetime of the &mut in &mut Ctxt to limit its context lifetime in practice. So, with some unsafe in the executor, I think this is actually a non-issue :)

As for my reason for being generic over Executor instead of over Context was mostly one of semantics (it's really the tokio executor that's able to open a TCP connection and return a TCP stream to it, not the tokio context), but I guess the boilerplate becomes too big when manually writing futures? TBD, I guess, no strong opinions here :)

@Ralith
Copy link
Contributor

Ralith commented Nov 16, 2018

Actually, thinking about it again, passing arguments in-band has an additional drawback: it means it must be explicitly passed by every async/await-using function

You don't need to pass a timer factory to every single async/await function any more than you need to pass a TCP socket to every single async/await function. All primitive foreign async events have the same requirements, and can be supported generically in the same way: by passing the resource you need only to the exact locations that need it.

Overall, the idea I'm going with is that the ability to do timeouts, task-locals or to open a tcp connections are properties of the executor, and that this should be reflected in the type system if we don't want to hit roadblocks (as we currently are for at least task-locals).

What roadblocks? Task locals can be implemented in a straightforward fashion by hooking or wrapping an executor, as we've discussed at length. There's no blocker beyond nobody having cared enough to spec an API.

@AlphaModder
Copy link
Author

@Ekleog

  • Unfortunately that signature would require futures to be executable on any context type. Note that there is no way to place additional bounds on Ctxt.
  • You don't actually need unsafe to use that &mut lifetime for the context, it's possible to erase inner lifetimes using a trait object (i.e, Future<Ctx = dyn TokioContext>). The issues I ran into with that solution were mainly problems getting futures with different types of context to interoperate (dyn T does not impl U even if T: U), but I think that can be solved with more boilerplate in various places.

@Ekleog
Copy link

Ekleog commented Nov 16, 2018

@Ralith

All primitive foreign async events have the same requirements, and can be supported generically in the same way: by passing the resource you need only to the exact locations that need it.

I think my point is: If we transparently pass a Waker everywhere, why can we not pass other things too? The point against passing other things is that said other things are executor-dependent. To which I reply, let's make it actually executor-dependent.

What you propose appears to be, basically: along with every Stream we should pass a timer factory (because every part of the code handling a stream will need to handle timeouts as well, for fear the stream will come from the network -- and it likely often will).

Which means either multiplying by two the number of arguments of each function that either uses a stream or forwards it to another function, or have a trait StreamWithTimerFactory, at which point semantics are out of the window.

Task locals can be implemented in a straightforward fashion by hooking or wrapping an executor, as we've discussed at length. There's no blocker beyond nobody having cared enough to spec an API.

There must have been discussions outside of rustasync/team#7, then, because re-reading it again doesn't help me see how to do that in practice, even considering both hooking and wrapping as possible: it'd require access to the executor at places it isn't currently accessible. Do you know where I can find those discussions?

@Ekleog
Copy link

Ekleog commented Nov 16, 2018

@AlphaModder I was thinking of using the lifetime of the &mut because it'd allow to not use trait objects, which are not really 0-cost and have that issue you hit that dyn T: U isn't a fact. Now, I must say I didn't really try implementing it to see what impact it'd have on executors yet (well, minimal executors could always only provide Waker with no additional cost, but more powerful executors): I'd rather wait for a few more people to chip in and either say they're in favor of it or say they can see a blocker in it before I spend too much time on it.

@Ralith
Copy link
Contributor

Ralith commented Nov 16, 2018

If we transparently pass a Waker everywhere, why can we not pass other things too?

Because other things are not necessary for the execution of every single non-degenerate future.

along with every Stream we should pass a timer factory

It is not in any way true that every single fragment of code that processes a Stream needs to create timers (or, for that matter, that code that processes Futures never does). I'm not sure how to address this concern because I have difficulty understanding where it's even coming from. It is not productive to construct new timers on every stage of every possible operation; not only would this be incredibly tedious and verbose no matter what the interface for constructing timers is, it is also unclear what it would accomplish.

Maybe there is some confusion on what a timer factory does. A timer factory is a tool to construct timers. For an example, see tokio_timer::Handle. You do not need it to poll the futures/streams it constructs; they contain all necessary information internally. I don't expect it to be contentious that most async code is not spending most of its time constructing new timers.

There must have been discussions outside of rustasync/team#7, then, because re-reading it again doesn't help me see how to do that in practice, even considering both hooking and wrapping as possible: it'd require access to the executor at places it isn't currently accessible. Do you know where I can find those discussions?

I don't see any serious problems with the proof-of-concept approach presented on that very issue.

@Ekleog
Copy link

Ekleog commented Nov 16, 2018

Because other things are not necessary for the execution of every single non-degenerate future.

Timeouts are necessary for the execution of almost every single non-degenerate futures.

I don't know if you've read the SMTP spec (what I'm currently interested on), but they point out that timeouts need not be the same at every stage of the SMTP session. Meaning, basically, that you need to intersperse .timeout() throughout the SMTP stack.

I must say I'm not familiar with the networking code other people write, but mine is in very much in need for it. Even though I do think the task-local is the only one that really needs fixing, because all the other ones can be emulated by stashing more stuff into task-local, however ugly that'll be forced to be if it can't be handled properly through the type system.

I don't see any serious problems with the proof-of-concept approach presented on that very issue.

This proof of concept requires to wrap each future into a wrapper future before passing it to the executor. It's also the approach I adopted before the removal of Spawn from the context, because I could just provide my own spawn function. Now, it requires the user to manually wrap each spawned future in the wrapper.

A way to make this wrapping automatic is what is, as far as I understand, under discussion at rustasync/team#7. And making this automatic is precisely the thing for which I can't think of any good API, as it'd require to basically wrap the Executor and propagate the information that the Executor is wrapped to the child futures somehow. And this second requirement is the reason for parameterizing Future on Executor (or Context), so that it needs not rely on dynamically checking it, risking panic and having more non-zero-cost abstractions. (and that's disregarding the issues with double-wrapping the executor, because I'm not sure these really need to be addressed)

@Nemo157
Copy link
Member

Nemo157 commented Nov 16, 2018

By my understanding IO and timers should be fully independent of "executor". An "executor" just provides the capability to run futures with an associated [Local]Waker, IO and timers should be built on top of that abstract wake system and link whatever notification channel they use to any arbitrary executor's Wakers (tokio calls these things "drivers").

I believe (but still have not yet tried implementing) that it should be possible to take tokio's Reactor and Timer, combine them with futures(0.3)'s ThreadPool (along with a generous sprinkling of compat) and have tokio based IO running on a non-tokio executor.

If you want some more complete "runtime" that is accessible alongside the wake system on the context, that could make sense. I think that should be a property of the Context, not an Executor to avoid misunderstandings about what the terms mean.

@cramertj
Copy link
Member

The Future trait is now defined in std and is not generic over executor types.

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

No branches or pull requests

6 participants