Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve memory consumption by cleaning up garbage references #33

Merged
merged 1 commit into from
Jun 11, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -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.6.0 || ^1.2.1"
},
"require-dev": {
"phpunit/phpunit": "^6.4 || ^5.7 || ^4.8.35"
Expand Down
19 changes: 14 additions & 5 deletions src/functions.php
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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);
}
Expand All @@ -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');
});
}
Expand Down
32 changes: 31 additions & 1 deletion tests/FunctionRejectTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
}
32 changes: 31 additions & 1 deletion tests/FunctionResolveTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
}
78 changes: 77 additions & 1 deletion tests/FunctionTimeoutTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
use React\Promise\Timer;
use React\Promise;

class FunctionTimerTest extends TestCase
class FunctionTimeoutTest extends TestCase
{
public function testResolvedWillResolveRightAway()
{
Expand Down Expand Up @@ -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());
}
}