Skip to content

Commit

Permalink
Merge pull request #237 from clue-labs/reactive-fibers
Browse files Browse the repository at this point in the history 
Improve performance by only using `FiberHandler` for `ReactiveHandler`
  • Loading branch information
@SimonFrings
SimonFrings authored Sep 1, 2023
2 parents 4da7203 + a5c3967 commit 7f64e51
Show file tree
Hide file tree
Showing 5 changed files with 73 additions and 114 deletions.
6 changes: 0 additions & 6 deletions src/App.php
View file
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
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
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
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
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

0 comments on commit 7f64e51

Please sign in to comment.