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

Feature Request: Provide a way to find out if a calbackId is valid #91

Open
Kingdutch opened this issue Mar 12, 2024 · 4 comments
Open

Comments

@Kingdutch
Copy link

Problem Description

I want to run code like the following:

// Perform eager tasks as often as possible but never more than one at a time.
$callbackId = EventLoop::repeat(0, function ($callbackId) {
  EventLoop::disable($callbackId);
  $this->performHalfSecondTask();
  EventLoop::enable($callbackId);
});

// Simulate some long task that mostly suspends.
EventLoop::delay(1.25, function() use ($callbackId) {
  echo "Cancelling repeat\n";
  EventLoop::cancel($callbackId);
});

EventLoop::run();

The problem with this is that there's no efficient way to check if $callbackId is still valid when running EventLoop::enable($callbackId); which means that there's a guaranteed exception somewhere in the program.

Workarounds

There's two possible ways around this, neither of which are great:

  1. Check all the identifiers, this is potentially expensive if the list of identifiers is large due to a high number of async tasks.
if (in_array($callbackId, EventLoop::getIdentifiers, TRUE)) {}
  1. Catch the exception. This feels quite verbose and it's unclear whether this is intentional or whether a developer unintentionally hides a bug here.
try {
    EventLoop::enable($callbackId);
}
catch (EventLoop\InvalidCallbackError $e) {}

Feature Request

It would be great if there is a method saying EventLoop::isValidIdentifier which does an isset behind the scenes.

if (EventLoop::isValidIdentifier($callbackId)) {
   EventLoop::enable($callbackId);
}

Alternatively a second parameter to enable/disable to indicate it may fail silently would work too:

EventLoop::enable($callbackId, TRUE);
@kelunik
Copy link
Member

kelunik commented Mar 12, 2024

What is $this->performHalfSecondTask() doing? repeat(0) is a good indicator that there's something solved the wrong way and better options exist.

@Kingdutch
Copy link
Author

In this case performHalfSecondTask is pre-warming one of N caches in an arbitrary order. It's essentially an optional background task: if we're waiting for some file or network I/O, pre-warm the cache to speed up future work, but otherwise just stick with what we're doing.

If our main task (handling a web request) is complete then we want to stop pre-warming caches and end the request.

@Kingdutch
Copy link
Author

I've pushed a more complete example in Kingdutch/revolt-playground@528a931 :)

@trowski
Copy link
Member

trowski commented Mar 15, 2024

One suggestion is to use a reference in the closures to set a flag that the callback has been cancelled.

$cancelled = false;
$callbackId = EventLoop::repeat(0, function (string $callbackId) use (&$cancelled): void {
    EventLoop::disable($callbackId);
    $this->performHalfSecondTask();
    if (!$cancelled) {
        EventLoop::enable($callbackId);
    }
});

EventLoop::delay(1.25, function () use (&$cancelled, $callbackId): void {
    EventLoop::cancel($callbackId);
    $cancelled = true;
});

If you use object properties instead you can completely avoid the by-reference use, but you will be creating a circular reference to the object. This may or may not matter for your application.

As @kelunik pointed out, this is a bit of an odd use of event-loop callbacks. Other async abstractions would be useful here, such as Promises/Futures, cancellations, or async iterators (pipelines as we call them in amphp/pipeline).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants