Skip to content

Commit 28d52ba

Browse files
jsorclue
jsor
and
clue
committed
Improve memory consumption
This incorporates the work done by @clue in the following PR's for reactphp/promise: * reactphp/promise#115 * reactphp/promise#116 * reactphp/promise#117 * reactphp/promise#118 * reactphp/promise#119 Co-authored-by: Christian Lück <[email protected]>
1 parent 18b7514 commit 28d52ba

File tree

3 files changed

+293
-39
lines changed

3 files changed

+293
-39
lines changed

‎src/Promise.php

Lines changed: 121 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,12 @@ final class Promise
1010
const STATE_FULFILLED = 3;
1111
const STATE_REJECTED = 4;
1212

13+
/**
14+
* Constant used to explicitly overwrite arguments and references.
15+
* This ensures that they do not show up in stack traces in PHP 7+.
16+
*/
17+
const GC_CLEANUP = '[Pact\Promise::GC_CLEANUP]';
18+
1319
/**
1420
* @internal
1521
*/
@@ -85,8 +91,14 @@ public function __construct($resolver = null, $canceller = null)
8591

8692
$this->canceller = $canceller;
8793

94+
if (null !== $canceller) {
95+
$canceller = self::GC_CLEANUP;
96+
}
97+
8898
if (null !== $resolver) {
89-
$this->_resolveFromCallback($resolver);
99+
$cb = $resolver;
100+
$resolver = self::GC_CLEANUP;
101+
$this->_resolveFromCallback($cb);
90102
}
91103
}
92104

@@ -258,12 +270,12 @@ public function cancel()
258270
$parent->requiredCancelRequests--;
259271

260272
if ($parent->requiredCancelRequests <= 0) {
261-
$parentCanceller = array($parent, 'cancel');
273+
$parentCanceller = array(&$parent, 'cancel');
262274
}
263275
} else {
264276
// Parent is a foreign promise, check for cancel() is already
265277
// done in _resolveCallback()
266-
$parentCanceller = array($parent, 'cancel');
278+
$parentCanceller = array(&$parent, 'cancel');
267279
}
268280
}
269281

@@ -276,6 +288,8 @@ public function cancel()
276288
\call_user_func($parentCanceller);
277289
}
278290

291+
$parent = self::GC_CLEANUP;
292+
279293
// Must be set after cancellation chain is run
280294
$this->isCancelled = true;
281295
}
@@ -504,49 +518,53 @@ private function _target()
504518
return $target;
505519
}
506520

507-
private function _resolveFromCallback($callback, $unblock = false)
521+
private function _resolveFromCallback($cb, $unblock = false)
508522
{
509-
$that = $this;
523+
$callback = $cb;
524+
$cb = self::GC_CLEANUP;
525+
526+
// Use reflection to inspect number of arguments expected by this callback.
527+
// We did some careful benchmarking here: Using reflection to avoid unneeded
528+
// function arguments is actually faster than blindly passing them.
529+
// Also, this helps avoiding unnecessary function arguments in the call stack
530+
// if the callback creates an Exception (creating garbage cycles).
531+
if (is_array($callback)) {
532+
$ref = new \ReflectionMethod($callback[0], $callback[1]);
533+
} elseif (is_object($callback) && !$callback instanceof \Closure) {
534+
$ref = new \ReflectionMethod($callback, '__invoke');
535+
} else {
536+
$ref = new \ReflectionFunction($callback);
537+
}
538+
539+
$args = $ref->getNumberOfParameters();
510540

511541
try {
542+
if ($args === 0) {
543+
$callback();
544+
return;
545+
}
546+
547+
// Keep a reference to this promise instance for the static
548+
// resolve/reject functions.
549+
// See also resolveFunction() and rejectFunction() for more details.
550+
$target = &$this;
551+
512552
\call_user_func(
513553
$callback,
514-
function ($value = null) use ($that, $unblock) {
515-
if ($unblock) {
516-
$that->state = Promise::STATE_PENDING;
517-
}
518-
519-
$that->_resolve($value);
520-
},
521-
// Allow rejecting with non-throwable reasons to ensure
522-
// interoperability with foreign promise implementations which
523-
// may allow arbitrary reason types or even rejecting without
524-
// a reason.
525-
function ($reason = null) use ($that, $unblock) {
526-
if (null === $reason) {
527-
if (0 === \func_num_args()) {
528-
$reason = ReasonException::createWithoutReason();
529-
} else {
530-
$reason = ReasonException::createForReason(null);
531-
}
532-
} elseif (!$reason instanceof \Throwable && !$reason instanceof \Exception) {
533-
$reason = ReasonException::createForReason($reason);
534-
}
535-
536-
if ($unblock) {
537-
$that->state = Promise::STATE_PENDING;
538-
}
539-
540-
$that->_reject($reason);
541-
}
554+
self::resolveFunction($target, $unblock),
555+
self::rejectFunction($target, $unblock)
542556
);
543557
} catch (\Exception $e) {
558+
$target = self::GC_CLEANUP;;
559+
544560
if ($unblock) {
545561
$this->state = Promise::STATE_PENDING;
546562
}
547563

548564
$this->_reject($e);
549565
} catch (\Throwable $e) {
566+
$target = self::GC_CLEANUP;
567+
550568
if ($unblock) {
551569
$this->state = Promise::STATE_PENDING;
552570
}
@@ -555,6 +573,76 @@ function ($reason = null) use ($that, $unblock) {
555573
}
556574
}
557575

576+
/**
577+
* Creates a static resolver callback that is not bound to a promise instance.
578+
*
579+
* Moving the closure creation to a static method allows us to create a
580+
* callback that is not bound to a promise instance. By passing the target
581+
* promise instance by reference, we can still execute its resolving logic
582+
* and still clear this reference when settling the promise. This helps
583+
* avoiding garbage cycles if any callback creates an Exception.
584+
*
585+
* These assumptions are covered by the test suite, so if you ever feel like
586+
* refactoring this, go ahead, any alternative suggestions are welcome!
587+
*/
588+
private static function resolveFunction(self &$target, $unblock)
589+
{
590+
return function ($value = null) use (&$target, $unblock) {
591+
if (Promise::GC_CLEANUP === $target) {
592+
return;
593+
}
594+
595+
if ($unblock) {
596+
$target->state = Promise::STATE_PENDING;
597+
}
598+
599+
$target->_resolve($value);
600+
$target = Promise::GC_CLEANUP;
601+
};
602+
}
603+
604+
/**
605+
* Creates a static rejection callback that is not bound to a promise instance.
606+
*
607+
* Moving the closure creation to a static method allows us to create a
608+
* callback that is not bound to a promise instance. By passing the target
609+
* promise instance by reference, we can still execute its rejection logic
610+
* and still clear this reference when settling the promise. This helps
611+
* avoiding garbage cycles if any callback creates an Exception.
612+
*
613+
* These assumptions are covered by the test suite, so if you ever feel like
614+
* refactoring this, go ahead, any alternative suggestions are welcome!
615+
*/
616+
private static function rejectFunction(self &$target, $unblock)
617+
{
618+
// Allow rejecting with non-throwable reasons to ensure
619+
// interoperability with foreign promise implementations which
620+
// may allow arbitrary reason types or even rejecting without
621+
// a reason.
622+
return function ($reason = null) use (&$target, $unblock) {
623+
if (Promise::GC_CLEANUP === $target) {
624+
return;
625+
}
626+
627+
if (null === $reason) {
628+
if (0 === \func_num_args()) {
629+
$reason = ReasonException::createWithoutReason();
630+
} else {
631+
$reason = ReasonException::createForReason(null);
632+
}
633+
} elseif (!$reason instanceof \Throwable && !$reason instanceof \Exception) {
634+
$reason = ReasonException::createForReason($reason);
635+
}
636+
637+
if ($unblock) {
638+
$target->state = Promise::STATE_PENDING;
639+
}
640+
641+
$target->_reject($reason);
642+
$target = Promise::GC_CLEANUP;
643+
};
644+
}
645+
558646
private static function enqueue($task)
559647
{
560648
if (!self::$queue) {

‎tests/PromiseCancelTest.php

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -53,15 +53,29 @@ function ($resolve, $reject) {
5353
/** @test */
5454
public function it_invokes_canceller_with_resolver_arguments()
5555
{
56-
$mock = $this->createCallableMock();
57-
$mock
58-
->expects($this->once())
59-
->method('__invoke')
60-
->with($this->isType('callable'), $this->isType('callable'));
56+
$args = null;
57+
$promise = new Promise(function () {}, function ($resolve, $reject) use (&$args) {
58+
$args = func_get_args();
59+
});
6160

62-
$promise = new Promise(function () {}, $mock);
61+
$promise->cancel();
62+
63+
$this->assertCount(2, $args);
64+
$this->assertInternalType('callable', $args[0]);
65+
$this->assertInternalType('callable', $args[1]);
66+
}
67+
68+
/** @test */
69+
public function it_invokes_canceller_without_arguments_if_not_accessed()
70+
{
71+
$args = null;
72+
$promise = new Promise(function () {}, function () use (&$args) {
73+
$args = func_num_args();
74+
});
6375

6476
$promise->cancel();
77+
78+
$this->assertSame(0, $args);
6579
}
6680

6781
/** @test */

0 commit comments

Comments
 (0)