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.

Sign up for GitHub

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 performance by only using FiberHandler for ReactiveHandler #237

Merged
merged 1 commit into from
Sep 1, 2023
Merged

Improve performance by only using FiberHandler for ReactiveHandler #237

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 0 additions & 6 deletions src/App.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

namespace FrameworkX;

use FrameworkX\Io\FiberHandler;
use FrameworkX\Io\MiddlewareHandler;
use FrameworkX\Io\ReactiveHandler;
use FrameworkX\Io\RedirectHandler;
Expand Down Expand Up @@ -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);
Expand Down
3 changes: 2 additions & 1 deletion src/Io/ReactiveHandler.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
11 changes: 0 additions & 11 deletions tests/AppMiddlewareTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);

Expand Down
96 changes: 0 additions & 96 deletions tests/AppTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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]);
Expand Down Expand Up @@ -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]);
Expand Down Expand Up @@ -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]);
Expand Down Expand Up @@ -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]);
Expand All @@ -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]);
Expand All @@ -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]);
Expand All @@ -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]);
Expand Down Expand Up @@ -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]);
Expand Down Expand Up @@ -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]);
Expand All @@ -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]);
Expand Down Expand Up @@ -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]);
Expand All @@ -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]);
Expand All @@ -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]);
Expand Down Expand Up @@ -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]);
Expand All @@ -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]);
Expand Down Expand Up @@ -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]);
Expand Down Expand Up @@ -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]);
Expand Down Expand Up @@ -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);

Expand Down
71 changes: 71 additions & 0 deletions tests/Io/ReactiveHandlerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down Expand Up @@ -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<ConnectionInterface> $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');
Expand Down