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

Manual "tick" execution #216

Closed
SerafimArts opened this issue Sep 24, 2020 · 11 comments
Closed

Manual "tick" execution #216

SerafimArts opened this issue Sep 24, 2020 · 11 comments

Comments

@SerafimArts
Copy link

I suggest adding a manual tick() call to the loop:

interface LoopInterface
{
    // ...

    public function tick(): void;
}

In this case, the implementation of the run() method will be as follows:

public function run()
{
    $this->running = true;

    while ($this->running) {
        $this->tick();
    }
}  

Such functionality is extremely necessary if the implementation uses its own event loop.
Such cases are needed when you need to implement additional functionality, for example, add support for high-performance tick handlers. For example:

class MyLoop
{
     public function __construct(LoopInterface $parent) { ... }

    public function run(): void
    {
        while(true) {
            $now = \microtime(true);
            $delta = $now - $this->updated;

            // custom operations
            foreach ($this->ticks as $handler) {
                $handler($delta);
            }

            $this->parent->tick();

            $this->updated = $now;
        }
    } 
}
@itnelo
Copy link

itnelo commented Jan 21, 2021

We already have a futureTick queue, which runs callback functions in the same way..isn't it a similar feature?

/**
 * Flush the callback queue.
 */
public function tick()
{
    // Only invoke as many callbacks as were on the queue when tick() was called.
    $count = $this->queue->count();

    while ($count--) {
        \call_user_func(
            $this->queue->dequeue()
        );
    }
}

We can do something like that:

public function run()
{
    $this->baseEventLoop->futureTick(fn () => $this->tick());

    $this->baseEventLoop->run();
}

// will be executed on each while(isRunning) iteration of the baseEventLoop
private function tick(): void
{
    // exception bubbling is not allowed here
    try {
        // our work
    } catch (Throwable $exception) {
        // handle exceptions and errors
    }

    // repeat
    $this->baseEventLoop->futureTick(fn () => $this->tick());
}

Or call_user_func and queue push/pop operations are "too slow" things for the original task and it is not acceptable?

Still learning reactphp, but i think the core concept was.. If any performance-heavy component wants to become "async" in our reactphp environment, it should start to request execution time from the centralized (single) event loop, not command event loop internals directly (as a separate "loop"). Am i correct here? So, we have timers and tick queue api.

Furthermore, we can try to hide loop existence (timers, ticks) on the user-side / business layer (if we cooking a "native" reactphp app). I would suggest something like getTime(): PromiseInterface sugar alongside existing levers, in the same manner, but it will be, probably, a bad move, because the EventLoop is designed as a low-level unit.. :)

(it looks like this):

/**
 * Returns a promise that resolves to a list of hardware items, found on the website using a remote browser.
 *
 * The resulting collection represents a Traversable<Item> or Item[].
 *
 * @param BrowserContext $browserContext Holds browser state and a driver reference to perform actions
 *
 * @return PromiseInterface<iterable>
 */
public function readItems(BrowserContext $browserContext): PromiseInterface
{
    $browsingThread = $browserContext->getBrowsingThread();

    $lookupDeferred = new Deferred();

    $tabReadyPromise = $browsingThread
        // acquiring a time frame in the shared event loop.
        ->getTime()
        // opening a tab in the remote browser, where we will analyse a web resource with hardware list.
        ->then(fn () => $this->openBrowserTab($browserContext))
    ;

    $resourceAccessPromise = $tabReadyPromise
        // accessing a page with hardware list.
        ->then(fn () => $this->accessResource($browserContext))
    ;

    $resourceAccessPromise
        // loading a page source.
        ->then(fn () => $this->readSourceCode($browserContext))
        // extracting a hardware list from the page.
        ->then(fn (string $rawData) => $this->itemParser->parse($rawData))
        // handling errors and releasing a thread lock.
        ->then(
            function (iterable $hardwareItems) use ($browsingThread, $lookupDeferred) {
                $browsingThread->release();

                $lookupDeferred->resolve($hardwareItems);
            },
            function (Throwable $rejectionReason) use ($browsingThread, $lookupDeferred) {
                $browsingThread->release();

                $reason = new RuntimeException('Unable to lookup hardware items.', 0, $rejectionReason);
                $lookupDeferred->reject($reason);
            }
        )
    ;

    $itemListPromise = $lookupDeferred->promise();

    return $itemListPromise;
}

$browsingThread is just a wrapper under loop->addTimer call and some other execution flow control (ticks, delays).

btw: "fn () => ..." is pretty ok for debugging (tested with xdebug 3) and rejections/retvals are well forwarded.

@SerafimArts
Copy link
Author

isn't it a similar feature?

Looks like nope.

In your example, you are just subscribing to "ticks". In my example, this is a manual call of the "tick" method which can already execute the necessary logic (for example, call "tick subscribers").

$loop->futureTick(fn() => null); // << Subscription (onTick)
$loop->tick(); // << Imperative tick execution

That is, I propose to move this code into a separate method: https://github.com/reactphp/event-loop/blob/master/src/StreamSelectLoop.php#L182-L212

@gitneko
Copy link

gitneko commented Jan 21, 2021

If you have your own event loop, why not wrap the event loop in a class that implements the reactphp event loop interface?

I think that would be the better way than make an event loop compatible with an outside event loop. Going down the tick route would break encapsulation and isolation and provide great surface for races and hard to track down bugs.

@SerafimArts
Copy link
Author

SerafimArts commented Jan 22, 2021

If you have your own event loop, why not wrap the event loop in a class that implements the reactphp event loop interface?

Because the implementation of custom event loop may not be compatible with reactphp interface. For example, there may be no support for signals, timers and resource streams... Like this: https://github.com/SerafimArts/opengl-demo/blob/master/engine/src/EventLoop/OrderedEventLoop.php#L51-L63

And even if we used standard implementations of reactphp via delegation, it would not work due to the lack of a mechanism for imperatively calling ticks:

class CustomLoop implements LoopInterface
{
    public function __construct(private LoopInterface $parent) { ... }
    
    public function addReadStream($stream, $listener)
    {
        $this->parent->addReadStream($stream, $listener);
    } 
    
    // etc...

    public function run(): void
    {
        while($this->running) {
            // My own operations
            
            $this->parent->tick(); // << I cant do that
        }
    }
}

Therefore, I propose to add a tick method so that reactphp loop delegation can be used correctly.

@WyriHaximus
Copy link
Member

FYI here is the PR where we removed the tick method and why: #72

@SerafimArts
Copy link
Author

SerafimArts commented Jan 24, 2021

Oh, there it is like...

This method was specially removed so that no one could get into the internal behavior of the event loop.

Perhaps this is logical, but it makes it impossible to use reactphp solutions as auxiliary =(

In any case, if this functionality was specifically removed, then no one will return it back. So this issue is no longer relevant. @WyriHaximus thank you!

@WyriHaximus
Copy link
Member

@SerafimArts let me look into this thread, not sure how I missed it. There might be another way

@WyriHaximus
Copy link
Member

@SerafimArts So am I assuming correctly that you want to be able to run this loop and https://github.com/SerafimArts/opengl-demo/blob/master/engine/src/EventLoop/OrderedEventLoop.php#L51-L63 a reactphp event loop?

@clue
Copy link
Member

clue commented Jan 25, 2021

@SerafimArts Love the project you're working on, this is a really interesting use case! ❤️

As @WyriHaximus has pointed out, we used to have a tick() method that was intentionally removed as per #72. There's some good discussion about this in the ticket, but if we feel there's value in bringing it back in the future, we're open to discuss this further 👍

That being said, I've checked the internals and the following piece of code perform the exact same logic as a hypothetical tick() method:

$loop->futureTick(function () use ($loop) {
    $loop->stop();
});
$loop->run();

I agree this looks a bit more expensive, but this shouldn't have noticeable more overhead than directly invoking a tick() method.

Either way, keep us posted with your OpenGL/SDL demo 👍

@SerafimArts
Copy link
Author

This is not the only example where this use case was required, it was just the most obvious.

@WyriHaximus @clue thanks for help. I think that case with futureTick + stop + run is a perfectly valid substitute.

@itnelo sorry, the first time I didn't understand what you meant in ur code =)

@WyriHaximus
Copy link
Member

@SerafimArts Bear in mind that your CPU usage will rise when the event loop isn't busy. But I'm not sure how that will work the other way around

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

5 participants