Improve memory consumption by cleaning up garbage references

reactphp · Jun 11, 2018 · 9efaccd · 9efaccd
1 parent 604513e
commit 9efaccd
Show file tree

Hide file tree

Showing 5 changed files with 154 additions and 9 deletions.
diff --git a/composer.json b/composer.json
@@ -20,7 +20,7 @@
     "require": {
         "php": ">=5.3",
         "react/event-loop": "^1.0 || ^0.5 || ^0.4 || ^0.3.5",
-        "react/promise": "~2.1|~1.2"
+        "react/promise": "2.x-dev as 2.6.0 || ^2.6.0 || ^1.2.1"
     },
     "require-dev": {
         "phpunit/phpunit": "^6.4 || ^5.7 || ^4.8.35"

diff --git a/src/functions.php b/src/functions.php
@@ -13,7 +13,12 @@ function timeout(PromiseInterface $promise, $time, LoopInterface $loop)
     // thus leaving responsibility to the input promise.
     $canceller = null;
     if ($promise instanceof CancellablePromiseInterface) {
-        $canceller = array($promise, 'cancel');
+        // pass promise by reference to clean reference after cancellation handler
+        // has been invoked once in order to avoid garbage references in call stack.
+        $canceller = function () use (&$promise) {
+            $promise->cancel();
+            $promise = null;
+        };
     }
 
     return new Promise(function ($resolve, $reject) use ($loop, $time, $promise) {
@@ -38,12 +43,15 @@ function timeout(PromiseInterface $promise, $time, LoopInterface $loop)
         }
 
         // start timeout timer which will cancel the input promise
-        $timer = $loop->addTimer($time, function () use ($time, $promise, $reject) {
+        $timer = $loop->addTimer($time, function () use ($time, &$promise, $reject) {
             $reject(new TimeoutException($time, 'Timed out after ' . $time . ' seconds'));
 
+            // try to invoke cancellation handler of input promise and then clean
+            // reference in order to avoid garbage references in call stack.
             if ($promise instanceof CancellablePromiseInterface) {
                 $promise->cancel();
             }
+            $promise = null;
         });
     }, $canceller);
 }
@@ -55,11 +63,12 @@ function resolve($time, LoopInterface $loop)
         $timer = $loop->addTimer($time, function () use ($time, $resolve) {
             $resolve($time);
         });
-    }, function ($resolve, $reject, $notify) use (&$timer, $loop) {
-        // cancelling this promise will cancel the timer and reject
+    }, function () use (&$timer, $loop) {
+        // cancelling this promise will cancel the timer, clean the reference
+        // in order to avoid garbage references in call stack and then reject.
         $loop->cancelTimer($timer);
+        $timer = null;
 
-        $resolve = $reject = $notify = $timer = $loop = null;
         throw new \RuntimeException('Timer cancelled');
     });
 }

diff --git a/tests/FunctionRejectTest.php b/tests/FunctionRejectTest.php
@@ -38,12 +38,42 @@ public function testPromiseExpiredWillBeRejectedOnTimeout()
         $this->expectPromiseRejected($promise);
     }
 
-    public function testCancelingPromiseWillRejectTimer()
+    public function testCancellingPromiseWillRejectTimer()
     {
         $promise = Timer\reject(0.01, $this->loop);
 
         $promise->cancel();
 
         $this->expectPromiseRejected($promise);
     }
+
+    public function testWaitingForPromiseToRejectDoesNotLeaveGarbageCycles()
+    {
+        if (class_exists('React\Promise\When')) {
+            $this->markTestSkipped('Not supported on legacy Promise v1 API');
+        }
+
+        gc_collect_cycles();
+
+        $promise = Timer\reject(0.01, $this->loop);
+        $this->loop->run();
+        unset($promise);
+
+        $this->assertEquals(0, gc_collect_cycles());
+    }
+
+    public function testCancellingPromiseDoesNotLeaveGarbageCycles()
+    {
+        if (class_exists('React\Promise\When')) {
+            $this->markTestSkipped('Not supported on legacy Promise v1 API');
+        }
+
+        gc_collect_cycles();
+
+        $promise = Timer\reject(0.01, $this->loop);
+        $promise->cancel();
+        unset($promise);
+
+        $this->assertEquals(0, gc_collect_cycles());
+    }
 }
diff --git a/tests/FunctionResolveTest.php b/tests/FunctionResolveTest.php
@@ -60,12 +60,42 @@ public function testCancellingPromiseWillCancelLoopTimer()
         $promise->cancel();
     }
 
-    public function testCancelingPromiseWillRejectTimer()
+    public function testCancellingPromiseWillRejectTimer()
     {
         $promise = Timer\resolve(0.01, $this->loop);
 
         $promise->cancel();
 
         $this->expectPromiseRejected($promise);
     }
+
+    public function testWaitingForPromiseToResolveDoesNotLeaveGarbageCycles()
+    {
+        if (class_exists('React\Promise\When')) {
+            $this->markTestSkipped('Not supported on legacy Promise v1 API');
+        }
+
+        gc_collect_cycles();
+
+        $promise = Timer\resolve(0.01, $this->loop);
+        $this->loop->run();
+        unset($promise);
+
+        $this->assertEquals(0, gc_collect_cycles());
+    }
+
+    public function testCancellingPromiseDoesNotLeaveGarbageCycles()
+    {
+        if (class_exists('React\Promise\When')) {
+            $this->markTestSkipped('Not supported on legacy Promise v1 API');
+        }
+
+        gc_collect_cycles();
+
+        $promise = Timer\resolve(0.01, $this->loop);
+        $promise->cancel();
+        unset($promise);
+
+        $this->assertEquals(0, gc_collect_cycles());
+    }
 }
diff --git a/tests/FunctionTimeoutTest.php b/tests/FunctionTimeoutTest.php
@@ -5,7 +5,7 @@
 use React\Promise\Timer;
 use React\Promise;
 
-class FunctionTimerTest extends TestCase
+class FunctionTimeoutTest extends TestCase
 {
     public function testResolvedWillResolveRightAway()
     {
@@ -166,4 +166,80 @@ public function testCancelTimeoutWillResolveIfGivenPromiseWillResolve()
         $this->expectPromiseResolved($promise);
         $this->expectPromiseResolved($timeout);
     }
+
+    public function testWaitingForPromiseToResolveBeforeTimeoutDoesNotLeaveGarbageCycles()
+    {
+        if (class_exists('React\Promise\When')) {
+            $this->markTestSkipped('Not supported on legacy Promise v1 API');
+        }
+
+        gc_collect_cycles();
+
+        $promise = Timer\resolve(0.01, $this->loop);
+
+        $promise = Timer\timeout($promise, 1.0, $this->loop);
+
+        $this->loop->run();
+        unset($promise);
+
+        $this->assertEquals(0, gc_collect_cycles());
+    }
+
+    public function testWaitingForPromiseToRejectBeforeTimeoutDoesNotLeaveGarbageCycles()
+    {
+        if (class_exists('React\Promise\When')) {
+            $this->markTestSkipped('Not supported on legacy Promise v1 API');
+        }
+
+        gc_collect_cycles();
+
+        $promise = Timer\reject(0.01, $this->loop);
+
+        $promise = Timer\timeout($promise, 1.0, $this->loop);
+
+        $this->loop->run();
+        unset($promise);
+
+        $this->assertEquals(0, gc_collect_cycles());
+    }
+
+    public function testWaitingForPromiseToTimeoutDoesNotLeaveGarbageCycles()
+    {
+        if (class_exists('React\Promise\When')) {
+            $this->markTestSkipped('Not supported on legacy Promise v1 API');
+        }
+
+        gc_collect_cycles();
+
+        $promise = new \React\Promise\Promise(function () { }, function () {
+            throw new \RuntimeException();
+        });
+
+        $promise = Timer\timeout($promise, 0.01, $this->loop);
+
+        $this->loop->run();
+        unset($promise);
+
+        $this->assertEquals(0, gc_collect_cycles());
+    }
+
+    public function testCancellingPromiseDoesNotLeaveGarbageCycles()
+    {
+        if (class_exists('React\Promise\When')) {
+            $this->markTestSkipped('Not supported on legacy Promise v1 API');
+        }
+
+        gc_collect_cycles();
+
+        $promise = new \React\Promise\Promise(function () { }, function () {
+            throw new \RuntimeException();
+        });
+
+        $loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock();
+        $promise = Timer\timeout($promise, 0.01, $loop);
+        $promise->cancel();
+        unset($promise);
+
+        $this->assertEquals(0, gc_collect_cycles());
+    }
 }