Enforce timer interval as minimum time to execution (#319)

Co-authored-by: Aaron Piotrowski <aaron@trowski.com>
amphp · Jul 14, 2020 · 05483cd · 05483cd
1 parent 0b802a5
commit 05483cd
Show file tree

Hide file tree

Showing 12 changed files with 162 additions and 64 deletions.
diff --git a/.travis.yml b/.travis.yml
@@ -31,7 +31,7 @@ install:
   - travis/install-ev.sh
   - travis/install-event.sh
 
-  - if [[ ${TRAVIS_PHP_VERSION:0:3} == "7.0" ]]; then composer remove --dev vimeo/psalm; fi
+  - if [[ ${TRAVIS_PHP_VERSION:0:3} == "7.0" ]]; then composer remove --dev psalm/phar; fi
   - composer update -n --prefer-dist
 
   - mkdir -p coverage/cov coverage/bin
@@ -45,7 +45,7 @@ script:
   - php vendor/bin/phpunit --verbose --group memoryleak
   - php vendor/bin/phpunit --verbose --exclude-group memoryleak --coverage-php coverage/cov/main.cov
   - PHP_CS_FIXER_IGNORE_ENV=1 php vendor/bin/php-cs-fixer --diff --dry-run -v fix
-  - if [[ ${TRAVIS_PHP_VERSION:0:3} == "7.0" ]]; then echo "Skipped psalm static analysis"; else vendor/bin/psalm; fi
+  - if [[ ${TRAVIS_PHP_VERSION:0:3} == "7.0" ]]; then echo "Skipped psalm static analysis"; else vendor/bin/psalm.phar; fi
 
 after_script:
   - curl -OL https://github.com/php-coveralls/php-coveralls/releases/download/v1.0.0/coveralls.phar

diff --git a/composer.json b/composer.json
@@ -41,7 +41,7 @@
         "amphp/php-cs-fixer-config": "dev-master",
         "react/promise": "^2",
         "phpunit/phpunit": "^6.0.9 | ^7",
-        "vimeo/psalm": "^3.11@dev",
+        "psalm/phar": "^3.11@dev",
         "jetbrains/phpstorm-stubs": "^2019.3"
     },
     "autoload": {

diff --git a/lib/Loop/Driver.php b/lib/Loop/Driver.php
@@ -227,6 +227,7 @@ public function delay(int $delay, callable $callback, $data = null): string
         $watcher->id = $this->nextId++;
         $watcher->callback = $callback;
         $watcher->value = $delay;
+        $watcher->expiration = $this->now() + $delay;
         $watcher->data = $data;
 
         $this->watchers[$watcher->id] = $watcher;
@@ -263,6 +264,7 @@ public function repeat(int $interval, callable $callback, $data = null): string
         $watcher->id = $this->nextId++;
         $watcher->callback = $callback;
         $watcher->value = $interval;
+        $watcher->expiration = $this->now() + $interval;
         $watcher->data = $data;
 
         $this->watchers[$watcher->id] = $watcher;
@@ -408,6 +410,14 @@ public function enable(string $watcherId)
                 $this->nextTickQueue[$watcher->id] = $watcher;
                 break;
 
+            case Watcher::REPEAT:
+            case Watcher::DELAY:
+                \assert(\is_int($watcher->value));
+
+                $watcher->expiration = $this->now() + $watcher->value;
+                $this->enableQueue[$watcher->id] = $watcher;
+                break;
+
             default:
                 $this->enableQueue[$watcher->id] = $watcher;
                 break;

diff --git a/lib/Loop/EvDriver.php b/lib/Loop/EvDriver.php
@@ -32,8 +32,6 @@ public static function isSupported(): bool
     private $signals = [];
     /** @var int Internal timestamp for now. */
     private $now;
-    /** @var bool */
-    private $nowUpdateNeeded = false;
     /** @var int Loop time offset */
     private $nowOffset;
 
@@ -213,10 +211,7 @@ public function stop()
      */
     public function now(): int
     {
-        if ($this->nowUpdateNeeded) {
-            $this->now = getCurrentTime() - $this->nowOffset;
-            $this->nowUpdateNeeded = false;
-        }
+        $this->now = getCurrentTime() - $this->nowOffset;
 
         return $this->now;
     }
@@ -236,7 +231,6 @@ public function getHandle(): \EvLoop
      */
     protected function dispatch(bool $blocking)
     {
-        $this->nowUpdateNeeded = true;
         $this->handle->run($blocking ? \Ev::RUN_ONCE : \Ev::RUN_ONCE | \Ev::RUN_NOWAIT);
     }
 
@@ -247,6 +241,9 @@ protected function dispatch(bool $blocking)
      */
     protected function activate(array $watchers)
     {
+        $this->handle->nowUpdate();
+        $now = $this->now();
+
         foreach ($watchers as $watcher) {
             if (!isset($this->events[$id = $watcher->id])) {
                 switch ($watcher->type) {
@@ -273,7 +270,7 @@ protected function activate(array $watchers)
 
                         $interval = $watcher->value / self::MILLISEC_PER_SEC;
                         $this->events[$id] = $this->handle->timer(
-                            $interval,
+                            \max(0, ($watcher->expiration - $now) / self::MILLISEC_PER_SEC),
                             ($watcher->type & Watcher::REPEAT) ? $interval : 0,
                             $this->timerCallback,
                             $watcher

diff --git a/lib/Loop/EventDriver.php b/lib/Loop/EventDriver.php
@@ -31,9 +31,6 @@ class EventDriver extends Driver
     /** @var \Event[] */
     private $signals = [];
 
-    /** @var bool */
-    private $nowUpdateNeeded = false;
-
     /** @var int Internal timestamp for now. */
     private $now;
 
@@ -235,10 +232,7 @@ public function stop()
      */
     public function now(): int
     {
-        if ($this->nowUpdateNeeded) {
-            $this->now = getCurrentTime() - $this->nowOffset;
-            $this->nowUpdateNeeded = false;
-        }
+        $this->now = getCurrentTime() - $this->nowOffset;
 
         return $this->now;
     }
@@ -258,7 +252,6 @@ public function getHandle(): \EventBase
      */
     protected function dispatch(bool $blocking)
     {
-        $this->nowUpdateNeeded = true;
         $this->handle->loop($blocking ? \EventBase::LOOP_ONCE : \EventBase::LOOP_ONCE | \EventBase::LOOP_NONBLOCK);
     }
 
@@ -269,7 +262,7 @@ protected function dispatch(bool $blocking)
      */
     protected function activate(array $watchers)
     {
-        $now = getCurrentTime() - $this->nowOffset;
+        $now = $this->now();
 
         foreach ($watchers as $watcher) {
             if (!isset($this->events[$id = $watcher->id])) {
@@ -335,7 +328,7 @@ protected function activate(array $watchers)
                 case Watcher::REPEAT:
                     \assert(\is_int($watcher->value));
 
-                    $interval = $watcher->value - ($now - $this->now());
+                    $interval = \max(0, $watcher->expiration - $now);
                     $this->events[$id]->add($interval > 0 ? $interval / self::MILLISEC_PER_SEC : 0);
                     break;
 

diff --git a/lib/Loop/Internal/TimerQueue.php b/lib/Loop/Internal/TimerQueue.php
@@ -19,15 +19,17 @@ final class TimerQueue
      * Inserts the watcher into the queue. Time complexity: O(log(n)).
      *
      * @param Watcher $watcher
-     * @param int     $expiration
      *
      * @psalm-param Watcher<int> $watcher
      *
      * @return void
      */
-    public function insert(Watcher $watcher, int $expiration)
+    public function insert(Watcher $watcher)
     {
-        $entry = new TimerQueueEntry($watcher, $expiration);
+        \assert($watcher->expiration !== null);
+        \assert(!isset($this->pointers[$watcher->id]));
+
+        $entry = new TimerQueueEntry($watcher, $watcher->expiration);
 
         $node = \count($this->data);
         $this->data[$node] = $entry;

diff --git a/lib/Loop/NativeDriver.php b/lib/Loop/NativeDriver.php
@@ -31,9 +31,6 @@ class NativeDriver extends Driver
     /** @var Watcher[][] */
     private $signalWatchers = [];
 
-    /** @var bool */
-    private $nowUpdateNeeded = false;
-
     /** @var int Internal timestamp for now. */
     private $now;
 
@@ -71,10 +68,7 @@ public function onSignal(int $signo, callable $callback, $data = null): string
      */
     public function now(): int
     {
-        if ($this->nowUpdateNeeded) {
-            $this->now = getCurrentTime() - $this->nowOffset;
-            $this->nowUpdateNeeded = false;
-        }
+        $this->now = getCurrentTime() - $this->nowOffset;
 
         return $this->now;
     }
@@ -96,8 +90,6 @@ public function getHandle()
      */
     protected function dispatch(bool $blocking)
     {
-        $this->nowUpdateNeeded = true;
-
         $this->selectStreams(
             $this->readStreams,
             $this->writeStreams,
@@ -292,9 +284,7 @@ protected function activate(array $watchers)
                 case Watcher::DELAY:
                 case Watcher::REPEAT:
                     \assert(\is_int($watcher->value));
-
-                    $expiration = $this->now() + $watcher->value;
-                    $this->timerQueue->insert($watcher, $expiration);
+                    $this->timerQueue->insert($watcher);
                     break;
 
                 case Watcher::SIGNAL:

diff --git a/lib/Loop/UvDriver.php b/lib/Loop/UvDriver.php
@@ -190,6 +190,8 @@ public static function isSupported(): bool
      */
     public function now(): int
     {
+        \uv_update_time($this->handle);
+
         /** @psalm-suppress TooManyArguments */
         return \uv_now($this->handle);
     }
@@ -220,6 +222,8 @@ protected function dispatch(bool $blocking)
      */
     protected function activate(array $watchers)
     {
+        $now = $this->now();
+
         foreach ($watchers as $watcher) {
             $id = $watcher->id;
 
@@ -264,7 +268,7 @@ protected function activate(array $watchers)
 
                     \uv_timer_start(
                         $event,
-                        $watcher->value,
+                        \max(0, $watcher->expiration - $now),
                         ($watcher->type & Watcher::REPEAT) ? $watcher->value : 0,
                         $this->timerCallback
                     );

diff --git a/lib/Loop/Watcher.php b/lib/Loop/Watcher.php
@@ -51,4 +51,7 @@ class Watcher
      * @psalm-var TValue
      */
     public $value;
+
+    /** @var int|null */
+    public $expiration;
 }