reactphp/react — pull request #191 by clue (2013-04-26 21:05:46+00:00)
Title: WIP: [EventLoop] Add SocketSelectLoop

Currently, there's no _easy_ way to add socket resources (those from `ext-sockets`) to the event loop. Each one of the current event loop implementations is a bit unclear about which type of resources it actually supports for either reading and writing. Due to a lack of interop between `ext-sockets` and the otherwise ubiquitous stream resources, they all _appear_ to be focused around stream resources exclusively and provide no support (or at least no _documented_ support) for any other type of resources.

With the event loop's main target striving for "async based libraries to be interoperable", this bias towards stream resources severely limits this interop.

I understand the initial thoughts and reasoning about this current limitation and agree that it's probably the one approach that's just _good enough_ for most people. But seeing the limitations `stream_socket_*()`-family of functions impose (such as no access to low level socket options and functions), I deem it necessary to support socket resources (and in the future possibly also other resource types) just as well.

We've already had a fair bit of discussion going on in IRC, but I'd like to see this discussed by a broader audience here. If we settle to support _just_ stream resources, this should be _okay_ for now IMHO. At the very least, this could result in clearly documented commitment to support only a limited subset of resources (and the reasoning about this decision).

Anyways, this quite simple patch splits the current `StreamSelectLoop` into a base `AbstractSelectLoop` and the actual `StreamSelectLoop` and introduces a new `SocketSelectLoop` which overwrites just a single abstract method. Otherwise it shares exactly the same code base. I also made it run the default set of unit tests which - for obvious reasons - fail due to not being able to handle file streams at all.

Given its controversial nature, I understand it's unlikely to be applied as-is, but I'd like to get some feedback on this anyway. Considering interop the main concern here, I have a few thoughts and suggestions in my mind that could possibly rectify this situation.

- Current event loop implementations do not support *all* types of streams anyway: pecl/ev does not support `php://temp` streams? Possibly some other streams aren't supported either in some of the loop implementations?
- Users should not have to care about which event loop backend is being used and should be able to rely on a _good_ automatic selection. Think of a complex async application with several external dependencies that all share the same main loop... IMHO users should usually not have to (and likely won't even be _able_ to) manually select the _right_ event loop, and that's why `Loop\Factory::create()` exists in the first place afaikt.
- Currently, there's no _sane_ way to check which type of resources are actually supported by the selected event loop. We should probably throws `Exception`s for invalid/unsupported streams in `addReadStream()` and `addWriteStream()` separately (because some resources *could* possibly be read from, but not written to). This should already have been done in the current implementation anyway.
- So the current situation is that users can't really rely on interop given the current event loop implementations.
- Admittedly, adding a new `SocketSelectLoop` has *even worse* interop and should absolutely *not* be selected as a default in `Loop\Factory` - but it may still be useful for users who *explicitly* want to use socket resources. This is similar to the current approach where users *explicitly* have to instantiate `LibEvLoop` if they're aware of its limitations but want to use it for its improved performance anyway.

So summing it all up, my current conclusion is that interop is the main issue here and that while the `SocketSelectLoop` provides very little interop, it still doesn't change the fact that interop between the current event loop implementation is already slightly limited. Providing a new `SocketSelectLoop` does not change this in any way, but could at least provide interop between users who *explicitly* wish to use socket resources.

So ultimately some thoughts on how to achieve true interop between different event loop implementations (despite this probably better being handled in a separate ticket):
- Rename `LoopInterface`s methods `addReadStream()` and `addWriteStream()` to `addReadListener()` and `addWriteListener()` (or whatever we can come up with without the word "stream" in it) just for the sake of exactness (this is the only BC break, but we should _consider_ it anyway).
- The new `addReadListener()` and `addWriteListener()` should throw an `Exception` if the given resource can not be handled by the respective loop implementation. In most cases a `is_resource()` or `get_resource_type()` check should be sufficient here.
- Add new `TheOneMainLoop` class implementing the same `LoopInterface` that acts as kind of a decorator for any number of backend loop implementation. So the first time you add a read listener to it, it automatically tries to create an appropriate backend loop (e.g. `StreamSelectLoop`), checks if the given resource can be added to it by checking its return value and otherwise tries the next available backend. As long as the provided resources can all be handled by the same backend loop, nothing particularly interesting happens and the new frontend loop implementation acts as a mere decorator/wrapper for the backend loop. Once a backend can *not* handle a given resource, the frontend should try the next available backend. If it can not find any available backend at all, it should throw an `Exception`. Once it finds a second backend is needed to handle this type of resource, this is where things get interesting:
The frontend `tick()` method now has to poll on each backends `tick()` method and make sure to set a really short timeout for each backend in order to iteratively poll each backend rapidly. Quite obviously, this is not an _optimal_ solution and will result in increased CPU usage. Similarly, the `run()` method has to repeatedly iterate over each backend.
So while this should indeed work, it's quite obvious that checking multiple backends _should_ be avoided when possible. So in the future, we should consider trying to re-allocate the previously registered listeners to the newly instantiated backend loop (different re-allocation strategies). E.g. the default backend loop could be pecl/ev, and severals stream listeners could already be added to it, but once you add a `php://temp` stream, the frontend will find that `StreamSelectLoop` is the only available backend loop that supports this type of resource and now also re-allocates all listeners from pecl/ev as well and thus can avoid the need for checking two independent backend loops. Admittedly this won't work in all cases (e.g. checking stream and socket resources), and implementing several re-allocation strategies would be desired.
- (Daring the impossible:) Consider Adding a `ThreadedMainLoop` that uses a thread pool provided by `pthreads` in order to block on multiple loop backend loops at the same time. Think of calling `socket_select()` and `stream_select()` in separate threads so each one could block on its set of resources independently. It should use internal notifications for the frontend loop to make sure the actual userland code only runs in a single threaded environment. To re-iterate: I understand the risks of introducing multiple threads, but what I'm suggesting is to only use them for each backend loop and *not* to expose them to the main loop's interface!
- Make sure `Factory::create()` creates either `TheOneMainLoop` or `ThreadedMainLoop` instead of a limited backend loop implementation.

Alright, now looking forward for your feedback :+1: