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.

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

Automatically start new fiber for each request on PHP 8.1+ #117

Merged
merged 2 commits into from
Feb 17, 2022

Conversation

clue
Copy link
Owner

@clue clue commented Feb 17, 2022

This changeset ensures X automatically starts a new fiber for each request on PHP 8.1+. This means you can now simply use await() in your controller code without having to wrap anything in any explicit async() calls anymore. This makes building and consuming async APIs easier than ever before.

$app->get('/book/{isbn}', function (Psr\Http\Message\ServerRequestInterface $request) use ($db) {
    $isbn = $request->getAttribute('isbn');
    $result = await($db->query(
        'SELECT title FROM book WHERE isbn = ?',
        [$isbn]
    ));

    assert($result instanceof React\MySQL\QueryResult);
    $data = $result->resultRows[0]['title'];

    return React\Http\Message\Response::plaintext(
        $data
    );
});

The first commit introduces a simple fiber implementation that works very similar to the async() function, but without introducing any external dependencies. The second commit optimizes this approach to only return a promise on demand internally.

Using fibers still shows a noticeable performance impact (~20% in my synthetic tests, but I invite more people to run their own benchmarks!), but there are still a number of outstanding optimizations that will improve this in the future. The optimization in the second commit already shows a ~35% performance improvement over using the simple async() implementation and there's definitely room for similar improvements in other areas. I've already filed reactphp/async#30 upstream and will file more follow-up PRs once this is merged.

The test suite also employs a couple of workarounds that will no longer be needed once reactphp/async#27 is addressed upstream. I've already prepared the required changes (talking 8+ hours of work here!) and will again file the PR once the above PR is merged.

Enjoy! 🚀

Builds on top of #116 and #62

@clue clue added the new feature New feature or request label Feb 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants