Skip to content

Commit 26718a5

Browse files
cluejsor
authored andcommitted
Only pass args to resolver and canceller if callback requires them
1 parent 21b667c commit 26718a5

3 files changed

Lines changed: 58 additions & 14 deletions

File tree

src/Promise.php

Lines changed: 26 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -176,15 +176,33 @@ private function unwrap($promise)
176176

177177
private function call(callable $callback)
178178
{
179+
// Use reflection to inspect number of arguments expected by this callback.
180+
// We did some careful benchmarking here: Using reflection to avoid unneeded
181+
// function arguments is actually faster than blindly passing them.
182+
// Also, this helps avoiding unnecessary function arguments in the call stack
183+
// if the callback creates an Exception (creating garbage cycles).
184+
if (is_array($callback)) {
185+
$ref = new \ReflectionMethod($callback[0], $callback[1]);
186+
} elseif (is_object($callback) && !$callback instanceof \Closure) {
187+
$ref = new \ReflectionMethod($callback, '__invoke');
188+
} else {
189+
$ref = new \ReflectionFunction($callback);
190+
}
191+
$args = $ref->getNumberOfParameters();
192+
179193
try {
180-
$callback(
181-
function ($value = null) {
182-
$this->resolve($value);
183-
},
184-
function ($reason = null) {
185-
$this->reject($reason);
186-
}
187-
);
194+
if ($args === 0) {
195+
$callback();
196+
} else {
197+
$callback(
198+
function ($value = null) {
199+
$this->resolve($value);
200+
},
201+
function ($reason = null) {
202+
$this->reject($reason);
203+
}
204+
);
205+
}
188206
} catch (\Throwable $e) {
189207
$this->reject($e);
190208
} catch (\Exception $e) {

tests/PromiseTest.php

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,4 +45,16 @@ public function shouldRejectIfResolverThrowsException()
4545
$promise
4646
->then($this->expectCallableNever(), $mock);
4747
}
48+
49+
/** @test */
50+
public function shouldRejectWithoutCreatingGarbageCyclesIfResolverThrowsException()
51+
{
52+
gc_collect_cycles();
53+
$promise = new Promise(function () {
54+
throw new \Exception('foo');
55+
});
56+
unset($promise);
57+
58+
$this->assertSame(0, gc_collect_cycles());
59+
}
4860
}

tests/PromiseTest/CancelTestTrait.php

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,15 +14,29 @@ abstract public function getPromiseTestAdapter(callable $canceller = null);
1414
/** @test */
1515
public function cancelShouldCallCancellerWithResolverArguments()
1616
{
17-
$mock = $this->createCallableMock();
18-
$mock
19-
->expects($this->once())
20-
->method('__invoke')
21-
->with($this->isType('callable'), $this->isType('callable'));
17+
$args = null;
18+
$adapter = $this->getPromiseTestAdapter(function ($resolve, $reject) use (&$args) {
19+
$args = func_get_args();
20+
});
2221

23-
$adapter = $this->getPromiseTestAdapter($mock);
22+
$adapter->promise()->cancel();
23+
24+
$this->assertCount(2, $args);
25+
$this->assertTrue(is_callable($args[0]));
26+
$this->assertTrue(is_callable($args[1]));
27+
}
28+
29+
/** @test */
30+
public function cancelShouldCallCancellerWithoutArgumentsIfNotAccessed()
31+
{
32+
$args = null;
33+
$adapter = $this->getPromiseTestAdapter(function () use (&$args) {
34+
$args = func_num_args();
35+
});
2436

2537
$adapter->promise()->cancel();
38+
39+
$this->assertSame(0, $args);
2640
}
2741

2842
/** @test */

0 commit comments

Comments
 (0)