Problem/Motivation
As part of #3394423: Adopt the Revolt event loop for async task orchestration the proposal to standardize async implementations on the revoltphp/event-loop package was met with enthusiasm. This issue includes the steps needed to add the dependency to the core composer files so that development using the library can begin.
Steps to reproduce
Proposed resolution
For a full reasoning of why the Revolt Event Loop over other options (such as a manual fiber implementation) see #3394423: Adopt the Revolt event loop for async task orchestration. The module evaluation has been copied below.
Dependency Evaluation
Maintainership of the package
The package is maintained by the Revolt organization which publicly consists of:
- Aaron Piotrowski who was responsible for the Fiber RFC
- Niklas Keller who was co-author on the Fiber RFC and co-maintains Amp with Aaron
- Saif Eddin Gmati Engineer at Bumble and previous Symfony memebr
Security policies of the package
The package lists that it supports the stable 1.x version and uses GitHub's private vulnerability reporting system.
https://github.com/revoltphp/event-loop/security
Expected release and support cycles
Current stable release (1.x) has existed since November 3rd 2022. There is no published release cycle but Revolt uses Semantic Versioning.
Code quality
Written by experienced developers. Covered by Psalm and PHPUnit.
Other dependencies it would add, if any (the full tree, not just direct dependencies), and evaluations for those dependencies as well
Revolt's only requirement is PHP 8.1 or newer.
It's intentionally built to be adopted by libraries to ensure that things that require an event loop work together.
Remaining tasks
MR to update composer.json filesFramework manager sign-off@larowlan in #12Release manager sign-off@catch in #4Write release-notes snippet
Issues after this one:
#3483239: Use revolt to prewarm caches during lock waits
#3425210: Ensure asynchronous tasks that run beyond the length of a request have the chance to complete before process exit
#3425212: [PP-1] Migrate BigPipe and Renderer fiber implementation to use Revolt Event Loop
#3257726: [meta] Use Fibers for concurrency in core
#3394423: Adopt the Revolt event loop for async task orchestration
Documentation Change
The following snippet can be added to Current PHP dependencies:
<tr>
<th>
<h3 id="revolt-event-loop">Revolt Event Loop</h3>
</th>
</tr>
<tr>
<th>Repository</th>
<td>https://github.com/revoltphp/event-loop</td>
</tr>
<tr>
<th>Release cycle</th>
<td>No fixed schedule. Follows semantic versioning. (<a href="https://github.com/revoltphp/event-loop?tab=readme-ov-file#versioning">Revolt Event Loop Versioning</a>)</td>
</tr>
<tr>
<th>Security policies</th>
<td><a href="https://github.com/revoltphp/event-loop/security">Revolt Event Loop Security Policy</a></td>
</tr>
<tr>
<th>Security issue reporting</th>
<td><a href="https://github.com/revoltphp/event-loop/security/advisories/new">Private vulnerability reporting form on GitHub</a></td>
</tr>
<tr>
<th>Contact(s)</th>
<td></td>
</tr>
User interface changes
-
API changes
- Drupal now allows using primitives from the
EventLoopprovided by Revolt to schedule async tasks.
Data model changes
-
Release notes snippet
Drupal has adopted the Revolt Event Loop which is already used by asynchronous libraries such as Amphp. By adopting the event loop, Drupal lets go of managing its own Fiber invocations which simplifies implementations and improves interoperability with other asynchronous tasks. This allows Drupal contrib to make use of any async libraries that are compatible with the Revolt event loop.
| Comment | File | Size | Author |
|---|---|---|---|
| #14 | 3425114-nr-bot.txt | 90 bytes | needs-review-queue-bot |
Issue fork drupal-3425114
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
- 3425114-add-revoltphpevent-loop-dependency
changes, plain diff MR !6865
Comments
Comment #2
kingdutchComment #4
catch+1 to this. Having looked at (but not actually tried to use) the react/amp projects I'm not sure how useful those will be for us, but revolt is a layer below both of those, and would save us copying the fiber foreach loops around as we slowly introduce async support into more places.
Taking the RM review tag off, but leaving the FM review tag on so this gets a +1 from another core committer who's not me.
Comment #5
catchComment #6
needs-review-queue-bot commentedThe Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #8
mxr576Rebased
Comment #9
larowlanDo we have a list or meta somewhere of places we want to use async for?
Comment #10
andypostYes, there's 2 issues blocked on it, listed in "Referenced by"
#3425210: Ensure asynchronous tasks that run beyond the length of a request have the chance to complete before process exit
#3425212: [PP-1] Migrate BigPipe and Renderer fiber implementation to use Revolt Event Loop
Comment #11
catch#3257726: [meta] Use Fibers for concurrency in core is the meta.
Comment #12
larowlanThanks, removing tag
Comment #13
smustgrave commentedLast 2 remaining tasks have been resolved (updated IS with comments they were)
Dependency evaluation appears complete and has gotten multiple +1s.
Going out on a limb and marking.
Comment #14
needs-review-queue-bot commentedThe Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #16
godotislateRebased.
Comment #17
smustgrave commentedAppears to need another rebase, will keep an eye out to get remarked quickly
Comment #18
godotislateRebased again.
Comment #19
smustgrave commentedRebase appears green!
Comment #20
catchI've split out #3483239: Use revolt to prewarm caches during lock waits from #3257725: Add a cache prewarm API to simplify the dependencies for that issue.
Added the various issues waiting on this one to the issue summary.
I worked on the initial Fibers implementation for core, however I was not familiar with Revolt at that time (to be fair it was only a few months old then - spun out of two older async projects as part of those projects also adopting Fibers). And while some of the concepts are new for Drupal, having them codified by a library that's also used by both amphp and react PHP should hopefully make them easier for people to pick up than manually re-implementing some of them with raw Fibers ourselves.
Committed/pushed to 11.x, thanks!
Comment #23
catchComment #24
alexpottWhy is this in the the root composer.json - it doesn't belong there. It should only be in core/composer.json.
Comment #26
alexpottFixed #24 in a quick follow-up.
Comment #27
quietone commentedTagging due to changes in the Core dependency release cycles and evaluation criteria policy.