Skip to content

Commit 1e7460b

Browse files
SimonFringsSimonFrings
authored
Merge pull request #266 from clue-labs/perf
Improve performance of `Loop` by avoiding unneeded method calls
2 parents a78ae0b + 2f6d3cd commit 1e7460b

File tree

2 files changed

+213
-13
lines changed

2 files changed

+213
-13
lines changed

‎src/Loop.php‎

Lines changed: 54 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
final class Loop
99
{
1010
/**
11-
* @var LoopInterface
11+
* @var ?LoopInterface
1212
*/
1313
private static $instance;
1414

@@ -83,7 +83,11 @@ public static function set(LoopInterface $loop)
8383
*/
8484
public static function addReadStream($stream, $listener)
8585
{
86-
self::get()->addReadStream($stream, $listener);
86+
// create loop instance on demand (legacy PHP < 7 doesn't like ternaries in method calls)
87+
if (self::$instance === null) {
88+
self::get();
89+
}
90+
self::$instance->addReadStream($stream, $listener);
8791
}
8892

8993
/**
@@ -97,7 +101,11 @@ public static function addReadStream($stream, $listener)
97101
*/
98102
public static function addWriteStream($stream, $listener)
99103
{
100-
self::get()->addWriteStream($stream, $listener);
104+
// create loop instance on demand (legacy PHP < 7 doesn't like ternaries in method calls)
105+
if (self::$instance === null) {
106+
self::get();
107+
}
108+
self::$instance->addWriteStream($stream, $listener);
101109
}
102110

103111
/**
@@ -109,7 +117,9 @@ public static function addWriteStream($stream, $listener)
109117
*/
110118
public static function removeReadStream($stream)
111119
{
112-
self::get()->removeReadStream($stream);
120+
if (self::$instance !== null) {
121+
self::$instance->removeReadStream($stream);
122+
}
113123
}
114124

115125
/**
@@ -121,7 +131,9 @@ public static function removeReadStream($stream)
121131
*/
122132
public static function removeWriteStream($stream)
123133
{
124-
self::get()->removeWriteStream($stream);
134+
if (self::$instance !== null) {
135+
self::$instance->removeWriteStream($stream);
136+
}
125137
}
126138

127139
/**
@@ -134,7 +146,11 @@ public static function removeWriteStream($stream)
134146
*/
135147
public static function addTimer($interval, $callback)
136148
{
137-
return self::get()->addTimer($interval, $callback);
149+
// create loop instance on demand (legacy PHP < 7 doesn't like ternaries in method calls)
150+
if (self::$instance === null) {
151+
self::get();
152+
}
153+
return self::$instance->addTimer($interval, $callback);
138154
}
139155

140156
/**
@@ -147,7 +163,11 @@ public static function addTimer($interval, $callback)
147163
*/
148164
public static function addPeriodicTimer($interval, $callback)
149165
{
150-
return self::get()->addPeriodicTimer($interval, $callback);
166+
// create loop instance on demand (legacy PHP < 7 doesn't like ternaries in method calls)
167+
if (self::$instance === null) {
168+
self::get();
169+
}
170+
return self::$instance->addPeriodicTimer($interval, $callback);
151171
}
152172

153173
/**
@@ -159,7 +179,9 @@ public static function addPeriodicTimer($interval, $callback)
159179
*/
160180
public static function cancelTimer(TimerInterface $timer)
161181
{
162-
return self::get()->cancelTimer($timer);
182+
if (self::$instance !== null) {
183+
self::$instance->cancelTimer($timer);
184+
}
163185
}
164186

165187
/**
@@ -171,7 +193,12 @@ public static function cancelTimer(TimerInterface $timer)
171193
*/
172194
public static function futureTick($listener)
173195
{
174-
self::get()->futureTick($listener);
196+
// create loop instance on demand (legacy PHP < 7 doesn't like ternaries in method calls)
197+
if (self::$instance === null) {
198+
self::get();
199+
}
200+
201+
self::$instance->futureTick($listener);
175202
}
176203

177204
/**
@@ -184,7 +211,12 @@ public static function futureTick($listener)
184211
*/
185212
public static function addSignal($signal, $listener)
186213
{
187-
self::get()->addSignal($signal, $listener);
214+
// create loop instance on demand (legacy PHP < 7 doesn't like ternaries in method calls)
215+
if (self::$instance === null) {
216+
self::get();
217+
}
218+
219+
self::$instance->addSignal($signal, $listener);
188220
}
189221

190222
/**
@@ -197,7 +229,9 @@ public static function addSignal($signal, $listener)
197229
*/
198230
public static function removeSignal($signal, $listener)
199231
{
200-
self::get()->removeSignal($signal, $listener);
232+
if (self::$instance !== null) {
233+
self::$instance->removeSignal($signal, $listener);
234+
}
201235
}
202236

203237
/**
@@ -208,7 +242,12 @@ public static function removeSignal($signal, $listener)
208242
*/
209243
public static function run()
210244
{
211-
self::get()->run();
245+
// create loop instance on demand (legacy PHP < 7 doesn't like ternaries in method calls)
246+
if (self::$instance === null) {
247+
self::get();
248+
}
249+
250+
self::$instance->run();
212251
}
213252

214253
/**
@@ -220,6 +259,8 @@ public static function run()
220259
public static function stop()
221260
{
222261
self::$stopped = true;
223-
self::get()->stop();
262+
if (self::$instance !== null) {
263+
self::$instance->stop();
264+
}
224265
}
225266
}

‎tests/LoopTest.php‎

Lines changed: 159 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,19 @@ public function testStaticAddReadStreamCallsAddReadStreamOnLoopInstance()
6262
Loop::addReadStream($stream, $listener);
6363
}
6464

65+
public function testStaticAddReadStreamWithNoDefaultLoopCallsAddReadStreamOnNewLoopInstance()
66+
{
67+
$ref = new \ReflectionProperty('React\EventLoop\Loop', 'instance');
68+
$ref->setAccessible(true);
69+
$ref->setValue(null);
70+
71+
$stream = stream_socket_server('127.0.0.1:0');
72+
$listener = function () { };
73+
Loop::addReadStream($stream, $listener);
74+
75+
$this->assertInstanceOf('React\EventLoop\LoopInterface', $ref->getValue());
76+
}
77+
6578
public function testStaticAddWriteStreamCallsAddWriteStreamOnLoopInstance()
6679
{
6780
$stream = tmpfile();
@@ -75,6 +88,19 @@ public function testStaticAddWriteStreamCallsAddWriteStreamOnLoopInstance()
7588
Loop::addWriteStream($stream, $listener);
7689
}
7790

91+
public function testStaticAddWriteStreamWithNoDefaultLoopCallsAddWriteStreamOnNewLoopInstance()
92+
{
93+
$ref = new \ReflectionProperty('React\EventLoop\Loop', 'instance');
94+
$ref->setAccessible(true);
95+
$ref->setValue(null);
96+
97+
$stream = stream_socket_server('127.0.0.1:0');
98+
$listener = function () { };
99+
Loop::addWriteStream($stream, $listener);
100+
101+
$this->assertInstanceOf('React\EventLoop\LoopInterface', $ref->getValue());
102+
}
103+
78104
public function testStaticRemoveReadStreamCallsRemoveReadStreamOnLoopInstance()
79105
{
80106
$stream = tmpfile();
@@ -87,6 +113,18 @@ public function testStaticRemoveReadStreamCallsRemoveReadStreamOnLoopInstance()
87113
Loop::removeReadStream($stream);
88114
}
89115

116+
public function testStaticRemoveReadStreamWithNoDefaultLoopIsNoOp()
117+
{
118+
$ref = new \ReflectionProperty('React\EventLoop\Loop', 'instance');
119+
$ref->setAccessible(true);
120+
$ref->setValue(null);
121+
122+
$stream = tmpfile();
123+
Loop::removeReadStream($stream);
124+
125+
$this->assertNull($ref->getValue());
126+
}
127+
90128
public function testStaticRemoveWriteStreamCallsRemoveWriteStreamOnLoopInstance()
91129
{
92130
$stream = tmpfile();
@@ -99,6 +137,18 @@ public function testStaticRemoveWriteStreamCallsRemoveWriteStreamOnLoopInstance(
99137
Loop::removeWriteStream($stream);
100138
}
101139

140+
public function testStaticRemoveWriteStreamWithNoDefaultLoopIsNoOp()
141+
{
142+
$ref = new \ReflectionProperty('React\EventLoop\Loop', 'instance');
143+
$ref->setAccessible(true);
144+
$ref->setValue(null);
145+
146+
$stream = tmpfile();
147+
Loop::removeWriteStream($stream);
148+
149+
$this->assertNull($ref->getValue());
150+
}
151+
102152
public function testStaticAddTimerCallsAddTimerOnLoopInstanceAndReturnsTimerInstance()
103153
{
104154
$interval = 1.0;
@@ -115,6 +165,20 @@ public function testStaticAddTimerCallsAddTimerOnLoopInstanceAndReturnsTimerInst
115165
$this->assertSame($timer, $ret);
116166
}
117167

168+
public function testStaticAddTimerWithNoDefaultLoopCallsAddTimerOnNewLoopInstance()
169+
{
170+
$ref = new \ReflectionProperty('React\EventLoop\Loop', 'instance');
171+
$ref->setAccessible(true);
172+
$ref->setValue(null);
173+
174+
$interval = 1.0;
175+
$callback = function () { };
176+
$ret = Loop::addTimer($interval, $callback);
177+
178+
$this->assertInstanceOf('React\EventLoop\TimerInterface', $ret);
179+
$this->assertInstanceOf('React\EventLoop\LoopInterface', $ref->getValue());
180+
}
181+
118182
public function testStaticAddPeriodicTimerCallsAddPeriodicTimerOnLoopInstanceAndReturnsTimerInstance()
119183
{
120184
$interval = 1.0;
@@ -131,6 +195,21 @@ public function testStaticAddPeriodicTimerCallsAddPeriodicTimerOnLoopInstanceAnd
131195
$this->assertSame($timer, $ret);
132196
}
133197

198+
public function testStaticAddPeriodicTimerWithNoDefaultLoopCallsAddPeriodicTimerOnNewLoopInstance()
199+
{
200+
$ref = new \ReflectionProperty('React\EventLoop\Loop', 'instance');
201+
$ref->setAccessible(true);
202+
$ref->setValue(null);
203+
204+
$interval = 1.0;
205+
$callback = function () { };
206+
$ret = Loop::addPeriodicTimer($interval, $callback);
207+
208+
$this->assertInstanceOf('React\EventLoop\TimerInterface', $ret);
209+
$this->assertInstanceOf('React\EventLoop\LoopInterface', $ref->getValue());
210+
}
211+
212+
134213
public function testStaticCancelTimerCallsCancelTimerOnLoopInstance()
135214
{
136215
$timer = $this->getMockBuilder('React\EventLoop\TimerInterface')->getMock();
@@ -143,6 +222,18 @@ public function testStaticCancelTimerCallsCancelTimerOnLoopInstance()
143222
Loop::cancelTimer($timer);
144223
}
145224

225+
public function testStaticCancelTimerWithNoDefaultLoopIsNoOp()
226+
{
227+
$ref = new \ReflectionProperty('React\EventLoop\Loop', 'instance');
228+
$ref->setAccessible(true);
229+
$ref->setValue(null);
230+
231+
$timer = $this->getMockBuilder('React\EventLoop\TimerInterface')->getMock();
232+
Loop::cancelTimer($timer);
233+
234+
$this->assertNull($ref->getValue());
235+
}
236+
146237
public function testStaticFutureTickCallsFutureTickOnLoopInstance()
147238
{
148239
$listener = function () { };
@@ -155,6 +246,18 @@ public function testStaticFutureTickCallsFutureTickOnLoopInstance()
155246
Loop::futureTick($listener);
156247
}
157248

249+
public function testStaticFutureTickWithNoDefaultLoopCallsFutureTickOnNewLoopInstance()
250+
{
251+
$ref = new \ReflectionProperty('React\EventLoop\Loop', 'instance');
252+
$ref->setAccessible(true);
253+
$ref->setValue(null);
254+
255+
$listener = function () { };
256+
Loop::futureTick($listener);
257+
258+
$this->assertInstanceOf('React\EventLoop\LoopInterface', $ref->getValue());
259+
}
260+
158261
public function testStaticAddSignalCallsAddSignalOnLoopInstance()
159262
{
160263
$signal = 1;
@@ -168,6 +271,27 @@ public function testStaticAddSignalCallsAddSignalOnLoopInstance()
168271
Loop::addSignal($signal, $listener);
169272
}
170273

274+
public function testStaticAddSignalWithNoDefaultLoopCallsAddSignalOnNewLoopInstance()
275+
{
276+
if (DIRECTORY_SEPARATOR === '\\') {
277+
$this->markTestSkipped('Not supported on Windows');
278+
}
279+
280+
$ref = new \ReflectionProperty('React\EventLoop\Loop', 'instance');
281+
$ref->setAccessible(true);
282+
$ref->setValue(null);
283+
284+
$signal = 1;
285+
$listener = function () { };
286+
try {
287+
Loop::addSignal($signal, $listener);
288+
} catch (\BadMethodCallException $e) {
289+
$this->markTestSkipped('Skipped: ' . $e->getMessage());
290+
}
291+
292+
$this->assertInstanceOf('React\EventLoop\LoopInterface', $ref->getValue());
293+
}
294+
171295
public function testStaticRemoveSignalCallsRemoveSignalOnLoopInstance()
172296
{
173297
$signal = 1;
@@ -181,6 +305,19 @@ public function testStaticRemoveSignalCallsRemoveSignalOnLoopInstance()
181305
Loop::removeSignal($signal, $listener);
182306
}
183307

308+
public function testStaticRemoveSignalWithNoDefaultLoopIsNoOp()
309+
{
310+
$ref = new \ReflectionProperty('React\EventLoop\Loop', 'instance');
311+
$ref->setAccessible(true);
312+
$ref->setValue(null);
313+
314+
$signal = 1;
315+
$listener = function () { };
316+
Loop::removeSignal($signal, $listener);
317+
318+
$this->assertNull($ref->getValue());
319+
}
320+
184321
public function testStaticRunCallsRunOnLoopInstance()
185322
{
186323
$loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock();
@@ -191,6 +328,17 @@ public function testStaticRunCallsRunOnLoopInstance()
191328
Loop::run();
192329
}
193330

331+
public function testStaticRunWithNoDefaultLoopCallsRunsOnNewLoopInstance()
332+
{
333+
$ref = new \ReflectionProperty('React\EventLoop\Loop', 'instance');
334+
$ref->setAccessible(true);
335+
$ref->setValue(null);
336+
337+
Loop::run();
338+
339+
$this->assertInstanceOf('React\EventLoop\LoopInterface', $ref->getValue());
340+
}
341+
194342
public function testStaticStopCallsStopOnLoopInstance()
195343
{
196344
$loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock();
@@ -201,6 +349,17 @@ public function testStaticStopCallsStopOnLoopInstance()
201349
Loop::stop();
202350
}
203351

352+
public function testStaticStopCallWithNoDefaultLoopIsNoOp()
353+
{
354+
$ref = new \ReflectionProperty('React\EventLoop\Loop', 'instance');
355+
$ref->setAccessible(true);
356+
$ref->setValue(null);
357+
358+
Loop::stop();
359+
360+
$this->assertNull($ref->getValue());
361+
}
362+
204363
/**
205364
* @after
206365
* @before

0 commit comments

Comments
 (0)