Add Fuchsia-specific reactor and net impls

[cramertj]

This change splits out the reactor into separate mio-based and Fuchsia-ports-based implementations. This is greatly beneficial to Fuchsia's uses of tokio-core, as it allows us to schedule and receive custom notifications via the PacketReceiver interface. After this change, the Fuchsia backend for mio will no longer be used by tokio-core.

I apologize this is so much code-- I wanted to do this all at once so that I could see how all the different pieces came together. I hope that the Fuchsia components are isolated enough that it doesn't impose too great of a maintenance burden.

cc @raggi, @raphlinus, @alexcrichton, @carllerche

[carllerche]

If this mio issue is resolved, is this PR required?

[cramertj]

@carllerche Theoretically we could solve some of the same problems by allowing arbitrary signal data. However, I think this approach allows for a much more flexible and performant implementation, and will require less churn in the future. It allows other libraries to declare their own PacketReceiver types which understand how to handle incoming readiness/signal data. Without this change, that code would have to be baked into mio.

This change also opens the door for Fuchsia-specific optimizations of reactor primitives, such as the Timer/Interval implementation here. I'm also interested in adding multithreading support for the Fuchsia reactor as we move over into the new "tokio" crate.

Finally, this PR makes it much simpler to debug and diagnose issues in Fuchsia's tokio support. There are now far fewer layers to dig through when searching for bugs, and the PacketReceiver structure makes it easier to track down missed or excess wakeups.

[carllerche]

I'm pretty hesitant at this point to start adding significant platform specific code to tokio and would rather consider alternate strategies.

[aturon]

@carllerche would you be open to having a separate Fuchsia "back end" playing a role similar to mio for Tokio, with most of the code living there? (NB I haven't reviewed this patch carefully so this may not make sense.)

[cramertj]

@carllerche Can you say more about your specific concerns? I'm not expecting the Tokio authors to maintain this code, or ever make sure it compiles. This structure opens up a lot of flexibility for us in terms of how we can use Tokio.

If you're just worried about the code structure being muddled by shifting things into mio-specific and non-mio-specific directories, I'm happy to restructure things as you'd prefer. For that matter, we could even have a completely separate tokio-fuchsia crate which tokio-core pub uses and cfgs out all of its own modules. I'd prefer not to do that as it will make my own life slightly more difficult, but I'm greatly invested in this plan and would like to find a way to make it work.

[carllerche]

@cramertj

This is a pretty big PR w/ not much accompanied documentation. It doesn't explain what PacketReceiver is in this case and what the benefits are.

This change also opens the door for Fuchsia-specific optimizations of reactor primitives, such as the Timer/Interval implementation here.

Timer / Interval are being removed from Tokio.

Can you say more about your specific concerns?

My main concern is that I just don't see the point in having this in Tokio, which aims to be portable. This seems like it should live in a completely different library.

[carllerche]

@aturon Tokio is converging to be a light futures + mio shim. The amount of work needed to make such a change makes me think it belongs in a separate library.

[carllerche]

@cramertj

And yes, there are maintenance concerns. Unlike Mio, which has a very limited integration surface, this would almost definitely break a lot more often.

[cramertj]

@aturon Tokio is converging to be a light futures + mio shim. The amount of work needed to make such a change makes me think it belongs in a separate library.

FWIW I suspect that the amount of code will be much smaller in the new tokio library, since the overall structure of that reactor is much more similar to the Fuchsia one I have here.

[cramertj]

This is a pretty big PR w/ not much accompanied documentation. It doesn't explain what PacketReceiver is in this case and what the benefits are.

There is fairly extensive documentation for the PacketReceiver trait here.

[carllerche]

@cramertj why can't this be part of mio?

[aturon]

@carllerche The problem with not integrating with Tokio is that it becomes difficult to integrate with any part of the async ecosystem that assumes Tokio (rather than just AsyncRead etc). Ideally we would continue to be able to offer a portable API to non-mainstream platforms like Fuchsia.

FWIW, std is set up in a similar manner, with the surface-level APIs hooking into sys modules that are each expected to export a common API. We've allowed non-1st-tier platforms to include their own sys modules, but do not gate on them in CI, which means there's effectively no maintenance burden on the Rust team (we ask the Redox folks to do their own reviews as well).

If plausible, it does seem good to move as much of the support code as possible into a separate crate, like I said above -- that way Tokio itself is effectively able to work with multiple back-ends, each of which is a distinct crate.

[carllerche]

@aturon

The problem with not integrating with Tokio is that it becomes difficult to integrate with any part of the async ecosystem that assumes Tokio

This PR adds non-portable extensions. Tokio is only providing tcp & udp types. AFAIK fuschia already provides tcp / udp w/ tokio.

[carllerche]

@aturon it would probably be helpful to focus on specifics. We both agree that "tokio should work on fuschia".

[cramertj]

@carllerche If you're interested in pulling PacketReceiver into mio and reimplementing PollEvented in and IoToken in terms of the PacketReceiver structure, I bet we could figure something out. However, that seems like a much bigger change to the existing mio/tokio interaction surface than the one I'm proposing in this PR. That change would be predicated on moving to the tokio crate since the PacketReceiver interface requires access to the scheduled_io Slab in order to deregister.

[aturon]

@cramertj is it possible to provide a non-portable small hook, akin to Evented, that would allow incorporating other non-portable functionality through a separate add-on crate?

[cramertj]

@aturon

@cramertj is it possible to provide a non-portable small hook, akin to Evented, that would allow incorporating other non-portable functionality through a separate add-on crate?

Sure, but you also have to remember all of the other parts of tokio-core that currently know about mio, including all of the net impls, the reactor awakeners and readiness indicators, etc. The API surface there is relatively large, and all of that would have to be stubbed out into a trait somehow.

[cramertj]

@aturon @carllerche Should we move this chat to Gitter or IRC (or vidyo/hangouts) so that we can have a bit more bandwidth for communicating?

[carllerche]

@cramertj

Can you step back and explain, within the context of only TcpStream, what the problem is with the current setup.

[cramertj]

@carllerche TcpStream isn't really an issue-- TcpStream is mostly fine the way it is (modulo the annoying bit about it not being deregistered and leaking, but that could be fixed via the new tokio trait). The real win here is getting direct access to the packets coming from zx::Port and being able to preprocess them as part of the reactor loop, rather than trying to bubble them through some kind of mio readiness type, which will be very challenging since they can contain things like pointers into data buffers and other non-signal information.

[carllerche]

@cramertj So why don't you add some way to get the fuschia selector impl out of Poll and then let users pass in PacketReceiver impls there?

[cramertj]

@carllerche

@cramertj So why don't you add some way to get the fuschia selector impl out of Poll and then let users pass in PacketReceiver impls there?

In order for that to work, I'd have to have a custom reactor built into the Fuchsia reactor inside of mio which stored all of the Arc<PacketReceiver>s, and bubble access to that reactor up through tokio-core::Handle. When polling the reactor, I'd have to poll my internal multiple times, dispatching to the PacketReceivers and only returning when a token meant for a tokio-core task or PollEvented is received. On top of all that, we'd still have to deal with the extra layer of abstraction mio introduces, and the performance penalty of mio's reimplementaiton of features that Fuchsia's reactor already provides. None of the additional reactor facilities mio provides are necessary for our reactor-- they already exist and are implemented natively. Plus, that still wouldn't solve the deregistration problem of PollEventeds, and wouldn't allow for us to make platform-specific optimizations to the tokio-core reactor (such as multi-threading).

[carllerche]

In order for that to work, I'd have to have a custom reactor built into the Fuchsia reactor inside of mio

This doesn't sound bad (a "reactor" is only a loop).

which stored all of the Arc<PacketReceiver>s

I'm not sure why this would be required when otherwise not.

and bubble access to that reactor up through tokio-core::Handle

This is already done via PollEvented (currently public) and something like IoToken could probably be made public too.

When polling the reactor, I'd have to poll my internal multiple times, dispatching to the PacketReceivers and only returning when a token meant for a tokio-core task or PollEvented is received.

I don't see why this is true. All the work would happen in your fuschia specific reactor and the mio interface is via an Events, which is a batch interface. Your description sounds like you need to do one off work.

On top of all that, we'd still have to deal with the extra layer of abstraction mio introduces

What I am suggesting bypasses mio for all non portable fuschia behavior, so afaik you wouldn't pay any additional cost.

None of the additional reactor facilities mio provides are necessary for our reactor-- they already exist and are implemented natively

Could you be specific? I am not familiar with fuschia, so statements like this doesn't help me understand.

Plus, that still wouldn't solve the deregistration problem of PollEventeds

Could you refresh my memory what the issue is?

nd wouldn't allow for us to make platform-specific optimizations to the tokio-core reactor (such as multi-threading)

Supporting multi-threading is generally a mio TODO and should be tackled separately, I'm not sure what other platform-specific optimizations you would not be able to do, again, please be specific.

[carllerche]

Something that may not be obvious. In this path that I am suggesting, you would end up not returning mio Events for resources that go via your PacketReceiver interface. You would pull the task and notify it yourself.

[cramertj]

Yes, I understand that. I'll respond in more detail tomorrow.