Why Loop is a static class?

[khelle]

Why Loop interop has been implemented as static class, therefore forcing people implementing this to follow wrong programming patterns? The interop class should be standard, instantiable class allowing proper dependency injection. The static access can be easily added via facades, but no the other way around.

[kelunik]

See #14. Passing around the event loop everywhere you need it prone to errors, because it usually leads to lazy injection like $this->eventLoop = $eventLoop ?: new Loop; and thus multiple loops while there should usually only be one. (Note that this lazy injection would even be bound to a specific loop implementation.)

Just have a look at JavaScript. The loop is just there, you don't even notice it until you have a blocking event handler. You can just write setTimeout(...) and it works.

[khelle]

Still, using it as static class breaks encapsulation, is OOP anti-pattern and disallows usage of multiple loops, which might be crucial for applications that works on multiple processing flows depending on application state. Interop interface should never be about forcing developer to apply given architecture, but to create set of methods for libraries to work together, and leave them as much implementation freedom as possible.

[sagebind]

@khelle Static classes are not automatically an anti-pattern (very few things are). This has already been discussed extensively, and we as a group decided on a static class as the best solution we could think of.

Here's some of the reasons we decided on the current design:

• It is useless to create a design that supports multiple event loop instances being used simultaneously. By definition only one loop can run in a single thread.
• The event loop is intended to be as generic as possible; applications can use the same loop, regardless of their internal state. Application state does not, and should not, leak into the event loop implementation.
• The static class is simply a static wrapper to a proper interface. That way we can still allow any application to use whatever loop implementation they desire, and the rest of the code does not need to know what loop implementation they are using.

Some additional thoughts:

[...] disallows usage of multiple loops [...]

Multiple loops are allowed, but only to the extent of running one at a time. Again, it is impossible to run more than one at once, so this design choice makes sense. For example:

Loop::execute(function() {
    // Do some async stuff...

    // Run a specialized or alternate loop in place. Again by def.n, this pauses the current loop and runs a different one.
    Loop::execute(function() {
        // Do something and then stop this loop so the outer one can continue.
        Loop::stop();
    }, new DifferentLoopDriverImpl());
}, new MyLoopDriverImpl());

Interop interface should never be about forcing developer to apply given architecture, [...]

We're just modeling the architecture already imposed. The OS is forcing certain architecture; there's no point in pretending there aren't any real restrictions on how event loops can be used.

[khelle]

I am aware that only one loop can be run at the same time, but please tell me how with current static interface can I create components, that know about different loops? Let's call them normal-state-loop and failed-state-loop, which as name suggests, works only one at a time? Do you tell me, I have to add to each component custom logic, just to differentatiate if global scope loop points to either one of them? Or do you tell me this is not something you are desigining interop for? If yes, then what's the point of doing interop interface which is in fact not interoperable?

[sagebind]

I'm not sure that I understand what you're trying to do. Components shouldn't know about any event loops at all. Isn't that the point of an interoperable interface? Once a component begins inspecting the loop or loops and tries to test for specific implementations, that component is no longer interoperable, because it expects a specific implementation.

Remember that when any event loop is running, regardless of who sets the implementation, every single piece of async code being used is going to be using the same loop instance to get their async work done. If that was not the case, only the methods tied to the event loop that is running would work and every other async function call would never complete.

[khelle]

Component should not expect specific loop implementation, but specific loop interface, which in this case would mean, it should expect this interop-loop-interface.

I have two loops - one for normal app state and one for emergency. These loops have their own , separate listeners. When application boots, it starts with normal state, and their evens are being processed etc. etc., if some error happens, then this loop stops, switches to emergency one, starts and leaves it until supervising system decides problem is solved. These loops do not share resources. How can I add to something like this static and global interface for loop? I don't see a way. On the other hand, if the interface was designed for instantiable loops there won't be problem in case like this, nor it would be a problem for people that want to keep their implementation accessible from global state (facade). In future, when there are more async projects and frameworks for PHP, the technical debt of making it static will deepen further, as more use cases and implementation emerges.

[kelunik]

@khelle There's no problem running a second loop after an error occurred in the first one, even easier if they don't share any state.

$app = function () { … }
$err = function (Throwable $e) { … }

try {
    Loop::execute($app);
} catch (Throwable $e) {
    Loop::execute($err($e));
}

[khelle]

@kelunik Running loop is not a problem here. The problem is that, by passing loop interface to component, I automatically decide whether it connects to normal or emergency loop. With global reference to loop, this cannot be done, without really dirty hacks.

$component = new AsyncComponent($normalLoop); // this will be run in normal state
$component = new AsyncComponent($errorLoop); // this will be run in emergency state

[kelunik]

The components do not have to know anything about the currently running loop. They just use the accessor functions directly as well.

[khelle]

They do not know about the running loop, they know about given interface that provides them Loop API. They do not care what kind of loop this is and when it is invoked. I can create many separate computing flows using multiple loops with something like:

$component = new AsyncComponent($loopA); // A
$component = new AsyncComponent($loopB); // B
$component = new AsyncComponent($loopC); // C

Then, only by switching the control between these loops, I am able to decide which code actually runs and when. If I can not pass the loop, I would be forced to do some dirty hacks, like actually switching the loop before registering component, so It would apply to right flow, or flood my code with additional ifs that determine which loop is actually pointed by global state, which breaks encapsulation. The whole TDD would also be a nightmare.

[WyriHaximus]

@khelle The Loop class is an accessor on steroids for the Driver. Which is the actual loop implementation. Now you can just happily pass the Driver around, though we recommend you use the Loop as there can only be one loop active at a given time. Components shouldn't even be created when you're not going to use them. Instead creating and invoking them when desired keeps program cleaner then when you create all flows for when you might need them.

[khelle]

@WyriHaximus This does not solve the issue, when you yourself say that it is not recommended to use. There is not point of using interop implemention details instead of actual interfaces, because in practice, it would probably mean that it won't be interoperable. Please, don't go on the way of "this shouldn't be done this way, because ..." as the given examples are always simplistic. You don't like the idea of creation all these components, nor do I, but in real world they would be delegated for factory methods registered to DI container on application boot, and then your point would not apply, but the problem would still occur.

From what I have seen from your arguments, the current implementation is designed only for really simplistic architecturally applications. I hope, I do not come around as too much of negative person, as I try not to and really, really would like to implement interop interfaces in my framework, but as it stands now, I don't see any positives. It's like fixing non-existant problem introducing many more. I am not convinced. I hope you will evalute my feedback a little and think if it is really a good choice, when more applications will surely come in time with other, but similiar, problems.

[WyriHaximus]

@khelle yes the examples are always simplified to the point where they don't make sense anymore. There is no need to tell me about DIC's, factory methods, lazy instantiation etc etc. I'm well aware of them and extensively using them. With very simplefied examples it is hard to completely get what you're trying to do and provide a different perspective. So yes I do go down that road, but not without giving you a solution in the first two sentences: passing multiple drivers around like you want to do with loops in your examples.

[khelle]

@WyriHaximus I appreciate that, the Driver looks nice and it is exactly what I believe should be the main and only loop interop interface. It allows interoparability and does not force developer to anything on the othe end. The whole global static concept should be dropped as it does not solve anything, just bring more problems. There is nothing I can say more here. Thank you for all of your answers! :)

[kelunik]

The whole global static concept should be dropped as it does not solve anything

It solves the there should only be one global loop problem. Otherwise we end up with an accessor for every library which you have to set which is even worse. We had that. It didn't work well.

[khelle]

@kelunik Maybe, but you are trying to solve not interop-related problem here. The same point could be stated about many other design patterns.

[sagebind]

@khelle

Maybe, but you are trying to solve not interop-related problem here.

It is an interop problem. If every library is either using a different accessor or or creating their own loop instance, you can no longer guarantee a single instance without a huge mess of boilerplate code. Any library wanting to adhere to this standard must have a non-implementation-specific way to access the single event loop instance currently in use, otherwise the interfaces are for naught.

[khelle]

@sagebind But if dev wants to use a set of loops, he has either way to create instances of them in running application. If he creates these instances then it is up to him to know how and where to pass them.

[assertchris]

Hey folks,

This looks like an interesting discussion, and perhaps worth more thought. It's been 7 hours and 19 comments so far. I suggest we give each other time to think a bit about what's been said here and come back in a day or two with (what I hope will be) clearer ideas about what (if anything) needs to change.

It'll also give others a chance to absorb and chime in if they have something insightful to add.

Thanks!

[bwoebi]

@khelle And the issue is? You can just simply switch loops like:

Loop::execute(function() {
    /* setup loop tasks for MyLoop */
    Loop::stop();
}, $loop1 = new MyLoop);
Loop::execute(function() use ($loop1) {
    /* start task */

    /* in some task when necessary, just continue the other loop for 1second (or whatever conditions you need) */
    Loop::execute(function() {
        Loop::once(1000, function() {
            Loop::stop();
        });
    }, $loop1);

});

The design is explicitly very explicitly requiring to scope when which loop runs. This coupling of objects to the loop (instead of injecting the loop into the objects) puts the caller into explicit control of the loop and makes it easier to avoid mistakes like running things in the wrong loop.

Do you have maybe an actual more concrete scenario where it's needed? Then I can better illustrate how the Loop accessor nicely solves this scenario you propose?

[khelle]

I have two loops, I am bootstraping my application, I want to create instances of both of them, then create two services, might be lazy creation, the first one should be attached and work only with the first loop, the second one should be attached to second. I cannot call execute or anything like that in bootstrap, since starting loop is done in another call.

[bwoebi]

@khelle as shown in the comment, just immediately run Loop::stop() after setup inside a Loop::execute() call - this prevents the loop from being actually run.

[khelle]

@bwoebi How does it answer my question? Maybe I will show what I mean with code

public function boot(ContainerInterface $container) 
{
    $loopA = $container->make('Loop.A');
    $loopB = $container->make('Loop.B');

    $container->factory(ServiceA::class, function() use($loopA) {
        return new ServiceA($loopA); // this service on creation will automatically attach itself to A loop
    });
    $container->factory(ServiceB::class, function() use($loopB) {
        return new ServiceB($loopB); // this service on creation will automatically attach itself to B loop
    });       
}

This bootstraps two services, one works in normal state, second one works only in emergency state. How can I do this with one global static reference to loop?

[bwoebi]

$container->factory(ServiceA::class, function() use($loopA) {
    Loop::execute(function() use (&$service) {
        $service = new ServiceA;
        Loop::stop();
    }, $loopA);
    return $service;
});

?

Do you have an existing case with a real event loop which you can show? I feel like that code needs a redesign then. Please show me if you have one, then I'll either acknowledge the issue or be able to tell you how to solve it with the Loop class.

[khelle]

It would probably work the way you are describing, but compare the code complexity of:

$container->factory(ServiceA::class, function() use($loopA) {
    return new ServiceA($loopA); // this service on creation will automatically attach itself to A loop
});
$container->factory(ServiceB::class, function() use($loopB) {
    return new ServiceB($loopB); // this service on creation will automatically attach itself to B loop
});

vs

$container->factory(ServiceA::class, function() use($loopA) {
    Loop::execute(function() use (&$service) {
        $service = new ServiceA;
        Loop::stop();
    }, $loopA);
    return $service;
});
$container->factory(ServiceB::class, function() use($loopB) {
    Loop::execute(function() use (&$service) {
        $service = new ServiceB;
        Loop::stop();
    }, $loopB);
    return $service;
});

Not to mention the problem of mixing static and variadic referencing. You claim that the static interface is done to avoid problem with creating multiple accessors for each library, yet on the other hand in the example I gave you I still have to implement accessors to set of drivers. Doesn't it contradict itself?

What do you mean by existing case? This example I have given is practical issue of what I am saying. There's nothing that can be shown more, as it already illustrates the problem in my understanding. Do yo want an example of application using this kind of two loop flows?

[bwoebi]

One can though simply write a wrapper function for that:

function wrapInLoopContext($loop, $cb) {
    return function() use ($loop, $cb) {
        Loop::execute(function() use ($cb) {
            $cb();
            Loop::stop();
        }, $loop);
    });
}

And use it as:

$container->factory(ServiceA::class, wrapInLoopContext($loopA, function() {
    return new ServiceA; // this service on creation will automatically attach itself to A loop
}));
$container->factory(ServiceB::class, wrapInLoopContext($loopB, function() {
    return new ServiceB; // this service on creation will automatically attach itself to A loop
}));

which is of pretty equal simplicity then.

And yes, I meant an example of an application using this kind of two loop flows.

[khelle]

So then, the solution is to wrap this method in another abstraction layer, which would need to be additionaly tested with tests itsef that would not follow proper TDD principles, and results might be obscured by app global state? The answer is to add more boilerplate? Wasn't one of the pros of static referencing to minimze need of creating additional code (what kelunik wrot about at the beginning).

Supervising system in my framework uses these two flows. I believe the best way to describe it would be to post a link to documentation on this topic http://kraken-php.com/docs/0.3/supervision

[joelwurtz]

I don't know if this is the right approach, but as a person that create a library using this interface i mainly use the driver and only use the loop as a factory to get the driver in my service constructor (so i'm sure to have the correct instance), then you can use DIC pattern, easily test your code, etc ... :

class ServiceA
{
    public function __construct(Driver $eventLoop = null)
    {
        $this->eventLoop = null === $eventLoop ? Loop::getDriver() : $eventLoop;
    }

    public function doSomething()
    {
        $this->eventLoop->defer(function () {
            echo 'foo';
        });
    }
}

For me the Loop is only used by end user and in a "entrypoint" script to init it and run it (but maybe, i'm wrong not an expert on the subject)

[kelunik]

@joelwurtz This makes me think whether we should really have Loop::get and even allow this. Testability isn't harmed by using a global accessor here, as it's clearly scoped using Loop::execute. It's pretty uncommon to mock the driver itself, I have never done that so far.

After all, Loop is your runtime. You don't mock it, just like you don't mock PHP for your tests. Did you ever mock the runtime in JavaScript in your tests?

[sagebind]

@kelunik Hmm... it does seem like the point of Loop::getDriver() is misinterpreted. The event loop isn't a dependency by the common interpretation of the word. Like you said, it is a runtime.

We could remove it, but I would be concerned about preventing the possibility of using custom loop features in certain applications. E.g., it would no longer be possible to do

function customLoopFeature()
{
    $driver = Loop::getDriver();
    if ($driver instanceof SomeDriverImpl) {
        ((SomeDriverImpl)$driver)->someFeature();
    } else {
        throw new Exception();
    }
}

I'm not sure that I can come up with a use case for this at the moment, but it isn't inconceivable that someone may need to do this in an application (not a library of course).

[kelunik]

@sagebind It's called Loop::get(), we don't have Loop::getDriver(). But I just remembered we need it for the wait implementation, that continues with the old loop. Without wait we could drop it.

[kelunik]

Closing, as the original issue "Why Loop is a static class?" has been explained. If anyone wants to discuss anything other discussed within this thread, please open another issue.