From a5c39673eb92afb6a78680256b3b96aad35328ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20L=C3=BCck?= Date: Sun, 4 Jun 2023 21:15:15 +0200 Subject: [PATCH] Improve performance by only using `FiberHandler` for `ReactiveHandler` --- src/App.php | 6 -- src/Io/ReactiveHandler.php | 3 +- tests/AppMiddlewareTest.php | 11 ---- tests/AppTest.php | 96 -------------------------------- tests/Io/ReactiveHandlerTest.php | 71 +++++++++++++++++++++++ 5 files changed, 73 insertions(+), 114 deletions(-) diff --git a/src/App.php b/src/App.php index 8a4815d..c746ceb 100644 --- a/src/App.php +++ b/src/App.php @@ -2,7 +2,6 @@ namespace FrameworkX; -use FrameworkX\Io\FiberHandler; use FrameworkX\Io\MiddlewareHandler; use FrameworkX\Io\ReactiveHandler; use FrameworkX\Io\RedirectHandler; @@ -103,11 +102,6 @@ public function __construct(...$middleware) \array_unshift($handlers, $needsAccessLog->getAccessLogHandler()); } - // automatically start new fiber for each request on PHP 8.1+ - if (\PHP_VERSION_ID >= 80100) { - \array_unshift($handlers, new FiberHandler()); // @codeCoverageIgnore - } - $this->router = new RouteHandler($container); $handlers[] = $this->router; $this->handler = new MiddlewareHandler($handlers); diff --git a/src/Io/ReactiveHandler.php b/src/Io/ReactiveHandler.php index 1ebd4b7..579eb8a 100644 --- a/src/Io/ReactiveHandler.php +++ b/src/Io/ReactiveHandler.php @@ -40,7 +40,8 @@ public function run(callable $handler): void { $socket = new SocketServer($this->listenAddress); - $http = new HttpServer($handler); + // create HTTP server, automatically start new fiber for each request on PHP 8.1+ + $http = new HttpServer(...(\PHP_VERSION_ID >= 80100 ? [new FiberHandler(), $handler] : [$handler])); $http->listen($socket); $logger = $this->logger; diff --git a/tests/AppMiddlewareTest.php b/tests/AppMiddlewareTest.php index dadd24f..c88115d 100644 --- a/tests/AppMiddlewareTest.php +++ b/tests/AppMiddlewareTest.php @@ -4,7 +4,6 @@ use FrameworkX\AccessLogHandler; use FrameworkX\App; -use FrameworkX\Io\FiberHandler; use FrameworkX\Io\MiddlewareHandler; use FrameworkX\Io\RouteHandler; use PHPUnit\Framework\TestCase; @@ -687,16 +686,6 @@ private function createAppWithoutLogger(...$middleware): App $handlers = $ref->getValue($middleware); assert(is_array($handlers)); - if (PHP_VERSION_ID >= 80100) { - $first = array_shift($handlers); - $this->assertInstanceOf(FiberHandler::class, $first); - - $next = array_shift($handlers); - $this->assertInstanceOf(AccessLogHandler::class, $next); - - array_unshift($handlers, $next, $first); - } - $first = array_shift($handlers); $this->assertInstanceOf(AccessLogHandler::class, $first); diff --git a/tests/AppTest.php b/tests/AppTest.php index 30506fa..d59955d 100644 --- a/tests/AppTest.php +++ b/tests/AppTest.php @@ -6,7 +6,6 @@ use FrameworkX\App; use FrameworkX\Container; use FrameworkX\ErrorHandler; -use FrameworkX\Io\FiberHandler; use FrameworkX\Io\MiddlewareHandler; use FrameworkX\Io\ReactiveHandler; use FrameworkX\Io\RouteHandler; @@ -52,11 +51,6 @@ public function testConstructWithMiddlewareAssignsGivenMiddleware(): void $handlers = $ref->getValue($handler); assert(is_array($handlers)); - if (PHP_VERSION_ID >= 80100) { - $first = array_shift($handlers); - $this->assertInstanceOf(FiberHandler::class, $first); - } - $this->assertCount(4, $handlers); $this->assertInstanceOf(AccessLogHandler::class, $handlers[0]); $this->assertInstanceOf(ErrorHandler::class, $handlers[1]); @@ -86,11 +80,6 @@ public function testConstructWithContainerAssignsDefaultHandlersAndContainerForR $handlers = $ref->getValue($handler); assert(is_array($handlers)); - if (PHP_VERSION_ID >= 80100) { - $first = array_shift($handlers); - $this->assertInstanceOf(FiberHandler::class, $first); - } - $this->assertCount(3, $handlers); $this->assertSame($accessLogHandler, $handlers[0]); $this->assertSame($errorHandler, $handlers[1]); @@ -122,11 +111,6 @@ public function testConstructWithContainerAndMiddlewareClassNameAssignsCallableF $handlers = $ref->getValue($handler); assert(is_array($handlers)); - if (PHP_VERSION_ID >= 80100) { - $first = array_shift($handlers); - $this->assertInstanceOf(FiberHandler::class, $first); - } - $this->assertCount(4, $handlers); $this->assertInstanceOf(AccessLogHandler::class, $handlers[0]); $this->assertInstanceOf(ErrorHandler::class, $handlers[1]); @@ -155,11 +139,6 @@ public function testConstructWithErrorHandlerOnlyAssignsErrorHandlerAfterDefault $handlers = $ref->getValue($handler); assert(is_array($handlers)); - if (PHP_VERSION_ID >= 80100) { - $first = array_shift($handlers); - $this->assertInstanceOf(FiberHandler::class, $first); - } - $this->assertCount(3, $handlers); $this->assertInstanceOf(AccessLogHandler::class, $handlers[0]); $this->assertSame($errorHandler, $handlers[1]); @@ -180,11 +159,6 @@ public function testConstructWithErrorHandlerClassOnlyAssignsErrorHandlerAfterDe $handlers = $ref->getValue($handler); assert(is_array($handlers)); - if (PHP_VERSION_ID >= 80100) { - $first = array_shift($handlers); - $this->assertInstanceOf(FiberHandler::class, $first); - } - $this->assertCount(3, $handlers); $this->assertInstanceOf(AccessLogHandler::class, $handlers[0]); $this->assertInstanceOf(ErrorHandler::class, $handlers[1]); @@ -207,11 +181,6 @@ public function testConstructWithContainerAndErrorHandlerAssignsErrorHandlerAfte $handlers = $ref->getValue($handler); assert(is_array($handlers)); - if (PHP_VERSION_ID >= 80100) { - $first = array_shift($handlers); - $this->assertInstanceOf(FiberHandler::class, $first); - } - $this->assertCount(3, $handlers); $this->assertInstanceOf(AccessLogHandler::class, $handlers[0]); $this->assertSame($errorHandler, $handlers[1]); @@ -238,11 +207,6 @@ public function testConstructWithContainerAndErrorHandlerClassAssignsErrorHandle $handlers = $ref->getValue($handler); assert(is_array($handlers)); - if (PHP_VERSION_ID >= 80100) { - $first = array_shift($handlers); - $this->assertInstanceOf(FiberHandler::class, $first); - } - $this->assertCount(3, $handlers); $this->assertInstanceOf(AccessLogHandler::class, $handlers[0]); $this->assertSame($errorHandler, $handlers[1]); @@ -273,11 +237,6 @@ public function testConstructWithMultipleContainersAndErrorHandlerClassAssignsEr $handlers = $ref->getValue($handler); assert(is_array($handlers)); - if (PHP_VERSION_ID >= 80100) { - $first = array_shift($handlers); - $this->assertInstanceOf(FiberHandler::class, $first); - } - $this->assertCount(3, $handlers); $this->assertInstanceOf(AccessLogHandler::class, $handlers[0]); $this->assertSame($errorHandler, $handlers[1]); @@ -309,11 +268,6 @@ public function testConstructWithMultipleContainersAndMiddlewareAssignsErrorHand $handlers = $ref->getValue($handler); assert(is_array($handlers)); - if (PHP_VERSION_ID >= 80100) { - $first = array_shift($handlers); - $this->assertInstanceOf(FiberHandler::class, $first); - } - $this->assertCount(4, $handlers); $this->assertInstanceOf(AccessLogHandler::class, $handlers[0]); $this->assertSame($errorHandler, $handlers[1]); @@ -338,11 +292,6 @@ public function testConstructWithMiddlewareAndErrorHandlerAssignsGivenErrorHandl $handlers = $ref->getValue($handler); assert(is_array($handlers)); - if (PHP_VERSION_ID >= 80100) { - $first = array_shift($handlers); - $this->assertInstanceOf(FiberHandler::class, $first); - } - $this->assertCount(5, $handlers); $this->assertInstanceOf(AccessLogHandler::class, $handlers[0]); $this->assertInstanceOf(ErrorHandler::class, $handlers[1]); @@ -382,11 +331,6 @@ public function testConstructWithMultipleContainersAndMiddlewareAndErrorHandlerC $handlers = $ref->getValue($handler); assert(is_array($handlers)); - if (PHP_VERSION_ID >= 80100) { - $first = array_shift($handlers); - $this->assertInstanceOf(FiberHandler::class, $first); - } - $this->assertCount(5, $handlers); $this->assertInstanceOf(AccessLogHandler::class, $handlers[0]); $this->assertSame($errorHandler1, $handlers[1]); @@ -412,11 +356,6 @@ public function testConstructWithAccessLogHandlerAndErrorHandlerAssignsHandlersA $handlers = $ref->getValue($handler); assert(is_array($handlers)); - if (PHP_VERSION_ID >= 80100) { - $first = array_shift($handlers); - $this->assertInstanceOf(FiberHandler::class, $first); - } - $this->assertCount(3, $handlers); $this->assertSame($accessLogHandler, $handlers[0]); $this->assertSame($errorHandler, $handlers[1]); @@ -437,11 +376,6 @@ public function testConstructWithAccessLogHandlerClassAndErrorHandlerClassAssign $handlers = $ref->getValue($handler); assert(is_array($handlers)); - if (PHP_VERSION_ID >= 80100) { - $first = array_shift($handlers); - $this->assertInstanceOf(FiberHandler::class, $first); - } - $this->assertCount(3, $handlers); $this->assertInstanceOf(AccessLogHandler::class, $handlers[0]); $this->assertInstanceOf(ErrorHandler::class, $handlers[1]); @@ -470,11 +404,6 @@ public function testConstructWithContainerAndAccessLogHandlerClassAndErrorHandle $handlers = $ref->getValue($handler); assert(is_array($handlers)); - if (PHP_VERSION_ID >= 80100) { - $first = array_shift($handlers); - $this->assertInstanceOf(FiberHandler::class, $first); - } - $this->assertCount(3, $handlers); $this->assertSame($accessLogHandler, $handlers[0]); $this->assertSame($errorHandler, $handlers[1]); @@ -499,11 +428,6 @@ public function testConstructWithMiddlewareBeforeAccessLogHandlerAndErrorHandler $handlers = $ref->getValue($handler); assert(is_array($handlers)); - if (PHP_VERSION_ID >= 80100) { - $first = array_shift($handlers); - $this->assertInstanceOf(FiberHandler::class, $first); - } - $this->assertCount(5, $handlers); $this->assertInstanceOf(ErrorHandler::class, $handlers[0]); $this->assertNotSame($errorHandler, $handlers[0]); @@ -539,11 +463,6 @@ public function testConstructWithMultipleContainersAndAccessLogHandlerClassAndEr $handlers = $ref->getValue($handler); assert(is_array($handlers)); - if (PHP_VERSION_ID >= 80100) { - $first = array_shift($handlers); - $this->assertInstanceOf(FiberHandler::class, $first); - } - $this->assertCount(3, $handlers); $this->assertSame($accessLogHandler, $handlers[0]); $this->assertSame($errorHandler, $handlers[1]); @@ -580,11 +499,6 @@ public function testConstructWithMultipleContainersAndMiddlewareAssignsDefaultHa $handlers = $ref->getValue($handler); assert(is_array($handlers)); - if (PHP_VERSION_ID >= 80100) { - $first = array_shift($handlers); - $this->assertInstanceOf(FiberHandler::class, $first); - } - $this->assertCount(4, $handlers); $this->assertSame($accessLogHandler, $handlers[0]); $this->assertSame($errorHandler, $handlers[1]); @@ -1749,16 +1663,6 @@ private function createAppWithoutLogger(callable ...$middleware): App $handlers = $ref->getValue($middleware); assert(is_array($handlers)); - if (PHP_VERSION_ID >= 80100) { - $first = array_shift($handlers); - $this->assertInstanceOf(FiberHandler::class, $first); - - $next = array_shift($handlers); - $this->assertInstanceOf(AccessLogHandler::class, $next); - - array_unshift($handlers, $next, $first); - } - $first = array_shift($handlers); $this->assertInstanceOf(AccessLogHandler::class, $first); diff --git a/tests/Io/ReactiveHandlerTest.php b/tests/Io/ReactiveHandlerTest.php index 70e63a5..18f13e0 100644 --- a/tests/Io/ReactiveHandlerTest.php +++ b/tests/Io/ReactiveHandlerTest.php @@ -7,8 +7,10 @@ use PHPUnit\Framework\TestCase; use React\EventLoop\Loop; use React\Http\Message\Response; +use React\Promise\Promise; use React\Socket\ConnectionInterface; use React\Socket\Connector; +use function React\Async\await; class ReactiveHandlerTest extends TestCase { @@ -187,6 +189,75 @@ public function testRunWillListenForHttpRequestAndSendBackHttpResponseOverSocket }); } + public function testRunWillOnlyRestartLoopAfterAwaitingWhenFibersAreNotAvailable(): void + { + $socket = stream_socket_server('127.0.0.1:0'); + assert(is_resource($socket)); + $addr = stream_socket_get_name($socket, false); + assert(is_string($addr)); + fclose($socket); + + $handler = new ReactiveHandler($addr); + + $logger = $this->createMock(LogStreamHandler::class); + + // $handler->logger = $logger; + $ref = new \ReflectionProperty($handler, 'logger'); + $ref->setAccessible(true); + $ref->setValue($handler, $logger); + + Loop::futureTick(function () use ($addr, $logger): void { + $connector = new Connector(); + $promise = $connector->connect($addr); + + // the loop will only need to be restarted if fibers are not available (PHP < 8.1) + if (PHP_VERSION_ID < 80100) { + $logger->expects($this->once())->method('log')->with('Warning: Loop restarted. Upgrade to react/async v4 recommended for production use.'); + } else { + $logger->expects($this->never())->method('log'); + } + + /** @var \React\Promise\PromiseInterface $promise */ + $promise->then(function (ConnectionInterface $connection): void { + $connection->on('data', function (string $data): void { + $this->assertEquals("HTTP/1.0 200 OK\r\nContent-Length: 3\r\n\r\nOK\n", $data); + }); + + // lovely: remove socket server on client connection close to terminate loop + $connection->on('close', function (): void { + Loop::futureTick(function (): void { + $resources = get_resources(); + $socket = end($resources); + assert(is_resource($socket)); + + Loop::removeReadStream($socket); + fclose($socket); + + Loop::stop(); + }); + }); + + $connection->write("GET /unknown HTTP/1.0\r\nHost: localhost\r\n\r\n"); + }); + }); + + $done = false; + $handler->run(function () use (&$done): Response { + $promise = new Promise(function (callable $resolve) use (&$done): void { + Loop::futureTick(function () use ($resolve, &$done): void { + $resolve(null); + $done = true; + }); + }); + await($promise); + + return new Response(200, ['Date' => '', 'Server' => ''], "OK\n"); + }); + + // check the loop kept running after awaiting the promise + $this->assertTrue($done); + } + public function testRunWillReportHttpErrorForInvalidClientRequest(): void { $socket = stream_socket_server('127.0.0.1:0');