Skip to content

Commit 55ef568

Browse files
clueclue
committed
Improve memory consumption by cleaning up garbage references
1 parent b712068 commit 55ef568

File tree

5 files changed

+210
-10
lines changed

5 files changed

+210
-10
lines changed

‎composer.json‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,8 @@
1616
"require": {
1717
"php": ">=5.3",
1818
"react/event-loop": "^1.0 || ^0.5 || ^0.4 || ^0.3.5",
19-
"react/promise": "~2.1|~1.2",
20-
"react/promise-timer": "~1.0"
19+
"react/promise": "2.x-dev as 2.7.0 || ^2.7 || ^1.2.1",
20+
"react/promise-timer": "dev-master as 1.5.0 || ^1.5"
2121
},
2222
"require-dev": {
2323
"phpunit/phpunit": "^6.4 || ^5.7 || ^4.8.35"

‎src/functions.php‎

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,10 @@ function ($error) use (&$exception, &$rejected, &$wait, $loop) {
7676
}
7777
);
7878

79+
// Explicitly overwrite argument with null value. This ensure that this
80+
// argument does not show up in the stack trace in PHP 7+ only.
81+
$promise = null;
82+
7983
while ($wait) {
8084
$loop->run();
8185
}
@@ -120,33 +124,38 @@ function ($error) use (&$exception, &$rejected, &$wait, $loop) {
120124
*/
121125
function awaitAny(array $promises, LoopInterface $loop, $timeout = null)
122126
{
127+
// Explicitly overwrite argument with null value. This ensure that this
128+
// argument does not show up in the stack trace in PHP 7+ only.
129+
$all = $promises;
130+
$promises = null;
131+
123132
try {
124133
// Promise\any() does not cope with an empty input array, so reject this here
125-
if (!$promises) {
134+
if (!$all) {
126135
throw new UnderflowException('Empty input array');
127136
}
128137

129-
$ret = await(Promise\any($promises)->then(null, function () {
138+
$ret = await(Promise\any($all)->then(null, function () {
130139
// rejects with an array of rejection reasons => reject with Exception instead
131140
throw new Exception('All promises rejected');
132141
}), $loop, $timeout);
133142
} catch (TimeoutException $e) {
134143
// the timeout fired
135144
// => try to cancel all promises (rejected ones will be ignored anyway)
136-
_cancelAllPromises($promises);
145+
_cancelAllPromises($all);
137146

138147
throw $e;
139148
} catch (Exception $e) {
140149
// if the above throws, then ALL promises are already rejected
141150
// => try to cancel all promises (rejected ones will be ignored anyway)
142-
_cancelAllPromises($promises);
151+
_cancelAllPromises($all);
143152

144153
throw new UnderflowException('No promise could resolve', 0, $e);
145154
}
146155

147156
// if we reach this, then ANY of the given promises resolved
148157
// => try to cancel all promises (settled ones will be ignored anyway)
149-
_cancelAllPromises($promises);
158+
_cancelAllPromises($all);
150159

151160
return $ret;
152161
}
@@ -180,12 +189,17 @@ function awaitAny(array $promises, LoopInterface $loop, $timeout = null)
180189
*/
181190
function awaitAll(array $promises, LoopInterface $loop, $timeout = null)
182191
{
192+
// Explicitly overwrite argument with null value. This ensure that this
193+
// argument does not show up in the stack trace in PHP 7+ only.
194+
$all = $promises;
195+
$promises = null;
196+
183197
try {
184-
return await(Promise\all($promises), $loop, $timeout);
198+
return await(Promise\all($all), $loop, $timeout);
185199
} catch (Exception $e) {
186200
// ANY of the given promises rejected or the timeout fired
187201
// => try to cancel all promises (rejected ones will be ignored anyway)
188-
_cancelAllPromises($promises);
202+
_cancelAllPromises($all);
189203

190204
throw $e;
191205
}

‎tests/FunctionAwaitAllTest.php‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,4 +104,28 @@ public function testAwaitAllPendingWillThrowAndCallCancellerOnTimeout()
104104
$this->assertTrue($cancelled);
105105
}
106106
}
107+
108+
/**
109+
* @requires PHP 7
110+
*/
111+
public function testAwaitAllPendingPromiseWithTimeoutAndCancellerShouldNotCreateAnyGarbageReferences()
112+
{
113+
if (class_exists('React\Promise\When')) {
114+
$this->markTestSkipped('Not supported on legacy Promise v1 API');
115+
}
116+
117+
gc_collect_cycles();
118+
119+
$promise = new \React\Promise\Promise(function () { }, function () {
120+
throw new RuntimeException();
121+
});
122+
try {
123+
Block\awaitAll(array($promise), $this->loop, 0.001);
124+
} catch (Exception $e) {
125+
// no-op
126+
}
127+
unset($promise, $e);
128+
129+
$this->assertEquals(0, gc_collect_cycles());
130+
}
107131
}

‎tests/FunctionAwaitAnyTest.php‎

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ public function testAwaitAnyFirstResolvedConcurrently()
4848
}
4949

5050
/**
51-
* @expectedException UnderflowException
51+
* @expectedException UnderflowException
5252
*/
5353
public function testAwaitAnyAllRejected()
5454
{
@@ -97,4 +97,28 @@ public function testAwaitAnyPendingWillThrowAndCallCancellerOnTimeout()
9797
$this->assertTrue($cancelled);
9898
}
9999
}
100+
101+
/**
102+
* @requires PHP 7
103+
*/
104+
public function testAwaitAnyPendingPromiseWithTimeoutAndCancellerShouldNotCreateAnyGarbageReferences()
105+
{
106+
if (class_exists('React\Promise\When')) {
107+
$this->markTestSkipped('Not supported on legacy Promise v1 API');
108+
}
109+
110+
gc_collect_cycles();
111+
112+
$promise = new \React\Promise\Promise(function () { }, function () {
113+
throw new RuntimeException();
114+
});
115+
try {
116+
Block\awaitAny(array($promise), $this->loop, 0.001);
117+
} catch (Exception $e) {
118+
// no-op
119+
}
120+
unset($promise, $e);
121+
122+
$this->assertEquals(0, gc_collect_cycles());
123+
}
100124
}

‎tests/FunctionAwaitTest.php‎

Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,4 +103,142 @@ public function testAwaitOnceWithTimeoutWillResolvemmediatelyAndCleanUpTimeout()
103103

104104
$this->assertLessThan(0.1, $time);
105105
}
106+
107+
public function testAwaitOneResolvesShouldNotCreateAnyGarbageReferences()
108+
{
109+
if (class_exists('React\Promise\When') && PHP_VERSION_ID >= 50400) {
110+
$this->markTestSkipped('Not supported on legacy Promise v1 API with PHP 5.4+');
111+
}
112+
113+
gc_collect_cycles();
114+
115+
$promise = Promise\resolve(1);
116+
Block\await($promise, $this->loop);
117+
unset($promise);
118+
119+
$this->assertEquals(0, gc_collect_cycles());
120+
}
121+
122+
public function testAwaitOneRejectedShouldNotCreateAnyGarbageReferences()
123+
{
124+
if (class_exists('React\Promise\When') && PHP_VERSION_ID >= 50400) {
125+
$this->markTestSkipped('Not supported on legacy Promise v1 API with PHP 5.4+');
126+
}
127+
128+
gc_collect_cycles();
129+
130+
$promise = Promise\reject(new RuntimeException());
131+
try {
132+
Block\await($promise, $this->loop);
133+
} catch (Exception $e) {
134+
// no-op
135+
}
136+
unset($promise, $e);
137+
138+
$this->assertEquals(0, gc_collect_cycles());
139+
}
140+
141+
public function testAwaitOneRejectedWithTimeoutShouldNotCreateAnyGarbageReferences()
142+
{
143+
if (class_exists('React\Promise\When') && PHP_VERSION_ID >= 50400) {
144+
$this->markTestSkipped('Not supported on legacy Promise v1 API with PHP 5.4+');
145+
}
146+
147+
gc_collect_cycles();
148+
149+
$promise = Promise\reject(new RuntimeException());
150+
try {
151+
Block\await($promise, $this->loop, 0.001);
152+
} catch (Exception $e) {
153+
// no-op
154+
}
155+
unset($promise, $e);
156+
157+
$this->assertEquals(0, gc_collect_cycles());
158+
}
159+
160+
public function testAwaitNullValueShouldNotCreateAnyGarbageReferences()
161+
{
162+
if (class_exists('React\Promise\When') && PHP_VERSION_ID >= 50400) {
163+
$this->markTestSkipped('Not supported on legacy Promise v1 API with PHP 5.4+');
164+
}
165+
166+
gc_collect_cycles();
167+
168+
$promise = Promise\reject(null);
169+
try {
170+
Block\await($promise, $this->loop);
171+
} catch (Exception $e) {
172+
// no-op
173+
}
174+
unset($promise, $e);
175+
176+
$this->assertEquals(0, gc_collect_cycles());
177+
}
178+
179+
/**
180+
* @requires PHP 7
181+
*/
182+
public function testAwaitPendingPromiseWithTimeoutAndCancellerShouldNotCreateAnyGarbageReferences()
183+
{
184+
if (class_exists('React\Promise\When')) {
185+
$this->markTestSkipped('Not supported on legacy Promise v1 API');
186+
}
187+
188+
gc_collect_cycles();
189+
190+
$promise = new \React\Promise\Promise(function () { }, function () {
191+
throw new RuntimeException();
192+
});
193+
try {
194+
Block\await($promise, $this->loop, 0.001);
195+
} catch (Exception $e) {
196+
// no-op
197+
}
198+
unset($promise, $e);
199+
200+
$this->assertEquals(0, gc_collect_cycles());
201+
}
202+
203+
/**
204+
* @requires PHP 7
205+
*/
206+
public function testAwaitPendingPromiseWithTimeoutAndWithoutCancellerShouldNotCreateAnyGarbageReferences()
207+
{
208+
//$this->markTestIncomplete('Root promise has no canceller, so cancelling will not settle and leave garbage references');
209+
210+
gc_collect_cycles();
211+
212+
$promise = new \React\Promise\Promise(function () { });
213+
try {
214+
Block\await($promise, $this->loop, 0.001);
215+
} catch (Exception $e) {
216+
// no-op
217+
}
218+
unset($promise, $e);
219+
220+
$this->assertEquals(0, gc_collect_cycles());
221+
}
222+
223+
/**
224+
* @requires PHP 7
225+
*/
226+
public function testAwaitPendingPromiseWithTimeoutAndNoOpCancellerShouldNotCreateAnyGarbageReferences()
227+
{
228+
//$this->markTestIncomplete('Root promise canceller does not settle, so this will leave garbage references');
229+
230+
gc_collect_cycles();
231+
232+
$promise = new \React\Promise\Promise(function () { }, function () {
233+
// no-op
234+
});
235+
try {
236+
Block\await($promise, $this->loop, 0.001);
237+
} catch (Exception $e) {
238+
// no-op
239+
}
240+
unset($promise, $e);
241+
242+
$this->assertEquals(0, gc_collect_cycles());
243+
}
106244
}

0 commit comments

Comments
 (0)