Improve memory consumption by cleaning up garbage references

clue · Jun 13, 2018 · 80d38c4 · 80d38c4
1 parent b712068
commit 80d38c4
Show file tree

Hide file tree

Showing 5 changed files with 206 additions and 10 deletions.
diff --git a/composer.json b/composer.json
@@ -16,8 +16,8 @@
     "require": {
     	"php": ">=5.3",
         "react/event-loop": "^1.0 || ^0.5 || ^0.4 || ^0.3.5",
-        "react/promise": "~2.1|~1.2",
-        "react/promise-timer": "~1.0"
+        "react/promise": "^2.7 || ^1.2.1",
+        "react/promise-timer": "^1.5"
     },
     "require-dev": {
         "phpunit/phpunit": "^6.4 || ^5.7 || ^4.8.35"

diff --git a/src/functions.php b/src/functions.php
@@ -76,6 +76,10 @@ function ($error) use (&$exception, &$rejected, &$wait, $loop) {
         }
     );
 
+    // Explicitly overwrite argument with null value. This ensure that this
+    // argument does not show up in the stack trace in PHP 7+ only.
+    $promise = null;
+
     while ($wait) {
         $loop->run();
     }
@@ -120,33 +124,38 @@ function ($error) use (&$exception, &$rejected, &$wait, $loop) {
  */
 function awaitAny(array $promises, LoopInterface $loop, $timeout = null)
 {
+    // Explicitly overwrite argument with null value. This ensure that this
+    // argument does not show up in the stack trace in PHP 7+ only.
+    $all = $promises;
+    $promises = null;
+
     try {
         // Promise\any() does not cope with an empty input array, so reject this here
-        if (!$promises) {
+        if (!$all) {
             throw new UnderflowException('Empty input array');
         }
 
-        $ret = await(Promise\any($promises)->then(null, function () {
+        $ret = await(Promise\any($all)->then(null, function () {
             // rejects with an array of rejection reasons => reject with Exception instead
             throw new Exception('All promises rejected');
         }), $loop, $timeout);
     } catch (TimeoutException $e) {
         // the timeout fired
         // => try to cancel all promises (rejected ones will be ignored anyway)
-        _cancelAllPromises($promises);
+        _cancelAllPromises($all);
 
         throw $e;
     } catch (Exception $e) {
         // if the above throws, then ALL promises are already rejected
         // => try to cancel all promises (rejected ones will be ignored anyway)
-        _cancelAllPromises($promises);
+        _cancelAllPromises($all);
 
         throw new UnderflowException('No promise could resolve', 0, $e);
     }
 
     // if we reach this, then ANY of the given promises resolved
     // => try to cancel all promises (settled ones will be ignored anyway)
-    _cancelAllPromises($promises);
+    _cancelAllPromises($all);
 
     return $ret;
 }
@@ -180,12 +189,17 @@ function awaitAny(array $promises, LoopInterface $loop, $timeout = null)
  */
 function awaitAll(array $promises, LoopInterface $loop, $timeout = null)
 {
+    // Explicitly overwrite argument with null value. This ensure that this
+    // argument does not show up in the stack trace in PHP 7+ only.
+    $all = $promises;
+    $promises = null;
+
     try {
-        return await(Promise\all($promises), $loop, $timeout);
+        return await(Promise\all($all), $loop, $timeout);
     } catch (Exception $e) {
         // ANY of the given promises rejected or the timeout fired
         // => try to cancel all promises (rejected ones will be ignored anyway)
-        _cancelAllPromises($promises);
+        _cancelAllPromises($all);
 
         throw $e;
     }

diff --git a/tests/FunctionAwaitAllTest.php b/tests/FunctionAwaitAllTest.php
@@ -104,4 +104,28 @@ public function testAwaitAllPendingWillThrowAndCallCancellerOnTimeout()
             $this->assertTrue($cancelled);
         }
     }
+
+    /**
+     * @requires PHP 7
+     */
+    public function testAwaitAllPendingPromiseWithTimeoutAndCancellerShouldNotCreateAnyGarbageReferences()
+    {
+        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();
+        });
+        try {
+            Block\awaitAll(array($promise), $this->loop, 0.001);
+        } catch (Exception $e) {
+            // no-op
+        }
+        unset($promise, $e);
+
+        $this->assertEquals(0, gc_collect_cycles());
+    }
 }
diff --git a/tests/FunctionAwaitAnyTest.php b/tests/FunctionAwaitAnyTest.php
@@ -48,7 +48,7 @@ public function testAwaitAnyFirstResolvedConcurrently()
     }
 
     /**
-     * @expectedException UnderflowException     
+     * @expectedException UnderflowException
      */
     public function testAwaitAnyAllRejected()
     {
@@ -97,4 +97,28 @@ public function testAwaitAnyPendingWillThrowAndCallCancellerOnTimeout()
             $this->assertTrue($cancelled);
         }
     }
+
+    /**
+     * @requires PHP 7
+     */
+    public function testAwaitAnyPendingPromiseWithTimeoutAndCancellerShouldNotCreateAnyGarbageReferences()
+    {
+        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();
+        });
+        try {
+            Block\awaitAny(array($promise), $this->loop, 0.001);
+        } catch (Exception $e) {
+            // no-op
+        }
+        unset($promise, $e);
+
+        $this->assertEquals(0, gc_collect_cycles());
+    }
 }
diff --git a/tests/FunctionAwaitTest.php b/tests/FunctionAwaitTest.php
@@ -103,4 +103,138 @@ public function testAwaitOnceWithTimeoutWillResolvemmediatelyAndCleanUpTimeout()
 
         $this->assertLessThan(0.1, $time);
     }
+
+    public function testAwaitOneResolvesShouldNotCreateAnyGarbageReferences()
+    {
+        if (class_exists('React\Promise\When') && PHP_VERSION_ID >= 50400) {
+            $this->markTestSkipped('Not supported on legacy Promise v1 API with PHP 5.4+');
+        }
+
+        gc_collect_cycles();
+
+        $promise = Promise\resolve(1);
+        Block\await($promise, $this->loop);
+        unset($promise);
+
+        $this->assertEquals(0, gc_collect_cycles());
+    }
+
+    public function testAwaitOneRejectedShouldNotCreateAnyGarbageReferences()
+    {
+        if (class_exists('React\Promise\When') && PHP_VERSION_ID >= 50400) {
+            $this->markTestSkipped('Not supported on legacy Promise v1 API with PHP 5.4+');
+        }
+
+        gc_collect_cycles();
+
+        $promise = Promise\reject(new RuntimeException());
+        try {
+            Block\await($promise, $this->loop);
+        } catch (Exception $e) {
+            // no-op
+        }
+        unset($promise, $e);
+
+        $this->assertEquals(0, gc_collect_cycles());
+    }
+
+    public function testAwaitOneRejectedWithTimeoutShouldNotCreateAnyGarbageReferences()
+    {
+        if (class_exists('React\Promise\When') && PHP_VERSION_ID >= 50400) {
+            $this->markTestSkipped('Not supported on legacy Promise v1 API with PHP 5.4+');
+        }
+
+        gc_collect_cycles();
+
+        $promise = Promise\reject(new RuntimeException());
+        try {
+            Block\await($promise, $this->loop, 0.001);
+        } catch (Exception $e) {
+            // no-op
+        }
+        unset($promise, $e);
+
+        $this->assertEquals(0, gc_collect_cycles());
+    }
+
+    public function testAwaitNullValueShouldNotCreateAnyGarbageReferences()
+    {
+        if (class_exists('React\Promise\When') && PHP_VERSION_ID >= 50400) {
+            $this->markTestSkipped('Not supported on legacy Promise v1 API with PHP 5.4+');
+        }
+
+        gc_collect_cycles();
+
+        $promise = Promise\reject(null);
+        try {
+            Block\await($promise, $this->loop);
+        } catch (Exception $e) {
+            // no-op
+        }
+        unset($promise, $e);
+
+        $this->assertEquals(0, gc_collect_cycles());
+    }
+
+    /**
+     * @requires PHP 7
+     */
+    public function testAwaitPendingPromiseWithTimeoutAndCancellerShouldNotCreateAnyGarbageReferences()
+    {
+        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();
+        });
+        try {
+            Block\await($promise, $this->loop, 0.001);
+        } catch (Exception $e) {
+            // no-op
+        }
+        unset($promise, $e);
+
+        $this->assertEquals(0, gc_collect_cycles());
+    }
+
+    /**
+     * @requires PHP 7
+     */
+    public function testAwaitPendingPromiseWithTimeoutAndWithoutCancellerShouldNotCreateAnyGarbageReferences()
+    {
+        gc_collect_cycles();
+
+        $promise = new \React\Promise\Promise(function () { });
+        try {
+            Block\await($promise, $this->loop, 0.001);
+        } catch (Exception $e) {
+            // no-op
+        }
+        unset($promise, $e);
+
+        $this->assertEquals(0, gc_collect_cycles());
+    }
+
+    /**
+     * @requires PHP 7
+     */
+    public function testAwaitPendingPromiseWithTimeoutAndNoOpCancellerShouldNotCreateAnyGarbageReferences()
+    {
+        gc_collect_cycles();
+
+        $promise = new \React\Promise\Promise(function () { }, function () {
+            // no-op
+        });
+        try {
+            Block\await($promise, $this->loop, 0.001);
+        } catch (Exception $e) {
+            // no-op
+        }
+        unset($promise, $e);
+
+        $this->assertEquals(0, gc_collect_cycles());
+    }
 }