carllerche/mio — pull request #3 by rrichardson (2014-09-06 18:58:26+00:00)
Title: Bufs

In https://github.com/rrichardson/mio/tree/refactor/src you will find my proposed refactoring of the Selector and Reactor interfaces along with ancillary supporting traits.

This set of changes was born out of a couple API design philosophies. I have been coding c++ for 15 years, but quite a bit of Haskell and Clojure as of late, so I have a strong functional bent which certainly influences my design decisions.

But some of my rules are:
- Don't make design decisions for the user, except where it will simplify everyone's code.
- Decouple all the things.
- Don't use a class where a function will do.
- Don't mix stack layers.
- Only include from lower layers.
- No mutual dependencies
- Simply. Simplify. Simplify.

I'm not listing these because I think other people should follow them. I don't expect anyone to agree with any of these things, or that my implementation even accurately reflects these beliefs perfectly :)

So with that as the background, let me try to explain what I did:

The first thing you might notice is that I reduced the size of the Reactor implementation by about 50%. Not only because I removed connect and listen, but because I changed the logic by which the reactor loops. It now loops conditionally on whether the handler says to loop or not (boolean). I think it puts more power in the hands of the reactor user without making the interface more complex.

In addition, I brought the ability to decide which events to subscribe, and also timeouts up to the top level of the Reactor interfaces. This puts even more power in the hands of the user.

I removed a couple circular dependencies by moving the IoEvent trait and Token out into their own modules.

The high level goal was to enable the user to define the Events in which they were interested. So the Reactor interface now supports the registration of any event that is part of the IoEventKind type, which has been pulled out into its own module, events.rs

Note that this is now completely decoupled from the notion of a Token. I don't know if this was a design choice or just a technical artifact of working with epoll. Either way, I don't think that epoll's specific implementation should be influencing the design of the high level interface.

I removed connect and listen from Reactor. IMO they have no business there because Reactor is for Files, Pipes and many future things that have no notion of connect or listen. Those should absolutely be part of the customer's domain.

I removed the handler trait entirely. All that's needed here is a callback. If a user wants to manage their IO with a struct, let them, but that should be no business of ours. The callback function leverages this new notion of decoupling the event subscription/alert (as much as possible) from the reactor interface.

The os/epoll implementation has been modified to reflect the new IoEvent trait. This tacks the IoEvent (generic) functionality directly into the low-level nix::sys::epoll EpollEventKind. No need for an intermediate structure here.

I moved the EpollEvent array that is passed to select() to be a member of the Selector struct.
I am not particularly happy with this. The reason I did it was to hide the guts of the array (which are EpollEvent, and the user should know nothing about that) . This breaks the model slightly, because if someone wanted to run reactor.run simultaneously in multiple threads, which is possible but not common, this would break. If we wanted to let the reactor create it while hiding the EpollEvent guts (or doing nasty casts) we would have to do it in a Box. This would not a big deal, IMO, as the alloc is still out of the hot-path of event processing.

I modified Yehuda's reactor unit test to reflect the way I envision people using this interface. As you'll now notice, run_once is simply accomplished by returning false from the handler callback.

I rather like it, but let me know your thoughts. Thanks.