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

periodic timer with the interval of zero #236

Closed
amir-khoshbakht opened this issue Jul 18, 2021 · 4 comments · Fixed by #243
Closed

periodic timer with the interval of zero #236

amir-khoshbakht opened this issue Jul 18, 2021 · 4 comments · Fixed by #243

Comments

@amir-khoshbakht
Copy link

amir-khoshbakht commented Jul 18, 2021

Is this normal? periodic timer with zero value is executed only once . and never executes again !

    $timer = $loop->addPeriodicTimer(0,function ()use (&$timer){
       dump($timer); 
    });

the out put :

React\EventLoop\Timer\Timer^ {#2409
  -interval: 1.0E-6
  -callback: Closure()^ {#2407
    class: "App\Console\Commands\Example"
    this: App\Console\Commands\Example {#2277 …}
    use: {
      $timer: React\EventLoop\Timer\Timer^ {#2409}
    }
    file: "./app/Console/Commands/Example.php"
    line: "91 to 93"
  }
  -periodic: true
}
@WyriHaximus
Copy link
Member

To be honest, I'm not sure, adding a periodic timer with an interval of 0 is not an expected thing to do. Theoretically, it should trigger every loop iteration, creating a very busy loop, and completely utilizing a full CPU core.

I'm assuming you want to have that function called a lot, and guessing 100 times a second is enough then using 0.01 should do the trick.

@amir-khoshbakht
Copy link
Author

To be honest, I'm not sure, adding a periodic timer with an interval of 0 is not an expected thing to do. Theoretically, it should trigger every loop iteration, creating a very busy loop, and completely utilizing a full CPU core.

I'm assuming you want to have that function called a lot, and guessing 100 times a second is enough then using 0.01 should do the trick.

in the path : React\EventLoop\Timer\Timer

const MIN_INTERVAL = 0.000001;
if ($interval < self::MIN_INTERVAL) {
    $interval = self::MIN_INTERVAL;
}

There is a piece of logic in the timer class constructor that if the time is less than the minimum time, the minimum time is selected. But by selecting zero time, the timer no longer works periodically. And calls the callback only once

Even if I choose very small number instead of zero, the timer works periodically

$this->loop->addPeriodicTimer(0.0000000000000000000001, function () {
   echo "time is ticking...";   // this works periodically 
});

This only happens when ExtEvLoop is used and StreamSelectLoop does not have this problem.

@clue
Copy link
Member

clue commented Oct 23, 2021

@my-random-username Thanks for reporting what sounds like a bug in the ExtEvLoop event loop implementation.

I agree that using a zero interval is rarely useful but may still happen in practice. As a counter measure, it looks like this is easy to work around by using a non-zero interval as suggested above.

I don't currently have access to a system with ext-ev installed, but if what you're saying is correct than I would consider this to be a (minor) defect. If anybody feels like picking this up, this sounds like a rather small fix, so PRs to address this are much appreciated! :shipit:

@lucasnetau
Copy link
Contributor

The ExtEvLoop implementation did not take into account the adjusted min bounded $interval stored in Timer. EvTimer was being initialised with a repeat value of 0, completing after the first invocation of the timer. A simple patch is proposed in PR #243

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

Successfully merging a pull request may close this issue.

4 participants