Merge pull request #124 from clue-labs/static

Improve memory consumption for pending promises by using static internal callbacks without binding to self
reactphp · Jun 13, 2018 · 9bd9d48 · 9bd9d48
2 parents 5e60e55 + ef228a9
commit 9bd9d48
Show file tree

Hide file tree

Showing 5 changed files with 157 additions and 6 deletions.
diff --git a/src/Promise.php b/src/Promise.php
@@ -61,7 +61,7 @@ public function done(callable $onFulfilled = null, callable $onRejected = null,
             return $this->result->done($onFulfilled, $onRejected, $onProgress);
         }
 
-        $this->handlers[] = function (ExtendedPromiseInterface $promise) use ($onFulfilled, $onRejected) {
+        $this->handlers[] = static function (ExtendedPromiseInterface $promise) use ($onFulfilled, $onRejected) {
             $promise
                 ->done($onFulfilled, $onRejected);
         };
@@ -73,7 +73,7 @@ public function done(callable $onFulfilled = null, callable $onRejected = null,
 
     public function otherwise(callable $onRejected)
     {
-        return $this->then(null, function ($reason) use ($onRejected) {
+        return $this->then(null, static function ($reason) use ($onRejected) {
             if (!_checkTypehint($onRejected, $reason)) {
                 return new RejectedPromise($reason);
             }
@@ -84,11 +84,11 @@ public function otherwise(callable $onRejected)
 
     public function always(callable $onFulfilledOrRejected)
     {
-        return $this->then(function ($value) use ($onFulfilledOrRejected) {
+        return $this->then(static function ($value) use ($onFulfilledOrRejected) {
             return resolve($onFulfilledOrRejected())->then(function () use ($value) {
                 return $value;
             });
-        }, function ($reason) use ($onFulfilledOrRejected) {
+        }, static function ($reason) use ($onFulfilledOrRejected) {
             return resolve($onFulfilledOrRejected())->then(function () use ($reason) {
                 return new RejectedPromise($reason);
             });
@@ -116,7 +116,7 @@ private function resolver(callable $onFulfilled = null, callable $onRejected = n
     {
         return function ($resolve, $reject, $notify) use ($onFulfilled, $onRejected, $onProgress) {
             if ($onProgress) {
-                $progressHandler = function ($update) use ($notify, $onProgress) {
+                $progressHandler = static function ($update) use ($notify, $onProgress) {
                     try {
                         $notify($onProgress($update));
                     } catch (\Throwable $e) {
@@ -129,7 +129,7 @@ private function resolver(callable $onFulfilled = null, callable $onRejected = n
                 $progressHandler = $notify;
             }
 
-            $this->handlers[] = function (ExtendedPromiseInterface $promise) use ($onFulfilled, $onRejected, $resolve, $reject, $progressHandler) {
+            $this->handlers[] = static function (ExtendedPromiseInterface $promise) use ($onFulfilled, $onRejected, $resolve, $reject, $progressHandler) {
                 $promise
                     ->then($onFulfilled, $onRejected)
                     ->done($resolve, $reject, $progressHandler);

diff --git a/tests/DeferredTest.php b/tests/DeferredTest.php
@@ -76,4 +76,37 @@ public function shouldRejectWithoutCreatingGarbageCyclesIfCancellerHoldsReferenc
 
         $this->assertSame(0, gc_collect_cycles());
     }
+
+    /** @test */
+    public function shouldNotLeaveGarbageCyclesWhenRemovingLastReferenceToPendingDeferred()
+    {
+        gc_collect_cycles();
+        $deferred = new Deferred();
+        $deferred->promise();
+        unset($deferred);
+
+        $this->assertSame(0, gc_collect_cycles());
+    }
+
+    /** @test */
+    public function shouldNotLeaveGarbageCyclesWhenRemovingLastReferenceToPendingDeferredWithUnusedCanceller()
+    {
+        gc_collect_cycles();
+        $deferred = new Deferred(function () { });
+        $deferred->promise();
+        unset($deferred);
+
+        $this->assertSame(0, gc_collect_cycles());
+    }
+
+    /** @test */
+    public function shouldNotLeaveGarbageCyclesWhenRemovingLastReferenceToPendingDeferredWithNoopCanceller()
+    {
+        gc_collect_cycles();
+        $deferred = new Deferred(function () { });
+        $deferred->promise()->cancel();
+        unset($deferred);
+
+        $this->assertSame(0, gc_collect_cycles());
+    }
 }
diff --git a/tests/FulfilledPromiseTest.php b/tests/FulfilledPromiseTest.php
@@ -47,4 +47,30 @@ public function shouldThrowExceptionIfConstructedWithAPromise()
 
         return new FulfilledPromise(new FulfilledPromise());
     }
+
+    /** @test */
+    public function shouldNotLeaveGarbageCyclesWhenRemovingLastReferenceToFulfilledPromiseWithAlwaysFollowers()
+    {
+        gc_collect_cycles();
+        $promise = new FulfilledPromise(1);
+        $promise->always(function () {
+            throw new \RuntimeException();
+        });
+        unset($promise);
+
+        $this->assertSame(0, gc_collect_cycles());
+    }
+
+    /** @test */
+    public function shouldNotLeaveGarbageCyclesWhenRemovingLastReferenceToFulfilledPromiseWithThenFollowers()
+    {
+        gc_collect_cycles();
+        $promise = new FulfilledPromise(1);
+        $promise = $promise->then(function () {
+            throw new \RuntimeException();
+        });
+        unset($promise);
+
+        $this->assertSame(0, gc_collect_cycles());
+    }
 }
diff --git a/tests/PromiseTest.php b/tests/PromiseTest.php
@@ -191,6 +191,72 @@ public function shouldIgnoreNotifyAfterReject()
         $promise->cancel();
     }
 
+
+    /** @test */
+    public function shouldNotLeaveGarbageCyclesWhenRemovingLastReferenceToPendingPromise()
+    {
+        gc_collect_cycles();
+        $promise = new Promise(function () { });
+        unset($promise);
+
+        $this->assertSame(0, gc_collect_cycles());
+    }
+
+    /** @test */
+    public function shouldNotLeaveGarbageCyclesWhenRemovingLastReferenceToPendingPromiseWithThenFollowers()
+    {
+        gc_collect_cycles();
+        $promise = new Promise(function () { });
+        $promise->then()->then()->then();
+        unset($promise);
+
+        $this->assertSame(0, gc_collect_cycles());
+    }
+
+    /** @test */
+    public function shouldNotLeaveGarbageCyclesWhenRemovingLastReferenceToPendingPromiseWithDoneFollowers()
+    {
+        gc_collect_cycles();
+        $promise = new Promise(function () { });
+        $promise->done();
+        unset($promise);
+
+        $this->assertSame(0, gc_collect_cycles());
+    }
+
+    /** @test */
+    public function shouldNotLeaveGarbageCyclesWhenRemovingLastReferenceToPendingPromiseWithOtherwiseFollowers()
+    {
+        gc_collect_cycles();
+        $promise = new Promise(function () { });
+        $promise->otherwise(function () { });
+        unset($promise);
+
+        $this->assertSame(0, gc_collect_cycles());
+    }
+
+    /** @test */
+    public function shouldNotLeaveGarbageCyclesWhenRemovingLastReferenceToPendingPromiseWithAlwaysFollowers()
+    {
+        gc_collect_cycles();
+        $promise = new Promise(function () { });
+        $promise->always(function () { });
+        unset($promise);
+
+        $this->assertSame(0, gc_collect_cycles());
+    }
+
+    /** @test */
+    public function shouldNotLeaveGarbageCyclesWhenRemovingLastReferenceToPendingPromiseWithProgressFollowers()
+    {
+        gc_collect_cycles();
+        $promise = new Promise(function () { });
+        $promise->then(null, null, function () { });
+        unset($promise);
+
+        $this->assertSame(0, gc_collect_cycles());
+    }
+
     /** @test */
     public function shouldFulfillIfFullfilledWithSimplePromise()
     {

diff --git a/tests/RejectedPromiseTest.php b/tests/RejectedPromiseTest.php
@@ -47,4 +47,30 @@ public function shouldThrowExceptionIfConstructedWithAPromise()
 
         return new RejectedPromise(new RejectedPromise());
     }
+
+    /** @test */
+    public function shouldNotLeaveGarbageCyclesWhenRemovingLastReferenceToRejectedPromiseWithAlwaysFollowers()
+    {
+        gc_collect_cycles();
+        $promise = new RejectedPromise(1);
+        $promise->always(function () {
+            throw new \RuntimeException();
+        });
+        unset($promise);
+
+        $this->assertSame(0, gc_collect_cycles());
+    }
+
+    /** @test */
+    public function shouldNotLeaveGarbageCyclesWhenRemovingLastReferenceToRejectedPromiseWithThenFollowers()
+    {
+        gc_collect_cycles();
+        $promise = new RejectedPromise(1);
+        $promise = $promise->then(null, function () {
+            throw new \RuntimeException();
+        });
+        unset($promise);
+
+        $this->assertSame(0, gc_collect_cycles());
+    }
 }