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:

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 files
  • Framework manager sign-off @larowlan in #12
  • Release manager sign-off @catch in #4
  • Write 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&nbsp;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&nbsp;policies</th>
			<td><a href="https://github.com/revoltphp/event-loop/security">Revolt Event Loop Security Policy</a></td>
		</tr>
		<tr>
			<th>Security&nbsp;issue&nbsp;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 EventLoop provided 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.

Issue fork drupal-3425114

Command icon 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:

Comments

Kingdutch created an issue. See original summary.

kingdutch’s picture

Title: Add revoltphp/event-loop dependency to core » Add revolt/event-loop dependency to core
Issue summary: View changes
Status: Active » Needs review
Issue tags: +Needs framework manager review, +Needs release manager review

catch’s picture

+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.

catch’s picture

Version: 10.3.x-dev » 11.x-dev
needs-review-queue-bot’s picture

Status: Needs review » Needs work
StatusFileSize
new90 bytes

The 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.

mxr576 made their first commit to this issue’s fork.

mxr576’s picture

Status: Needs work » Needs review

Rebased

larowlan’s picture

Do we have a list or meta somewhere of places we want to use async for?

catch’s picture

larowlan’s picture

Thanks, removing tag

smustgrave’s picture

Issue summary: View changes
Status: Needs review » Reviewed & tested by the community

Last 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.

needs-review-queue-bot’s picture

Status: Reviewed & tested by the community » Needs work
StatusFileSize
new90 bytes

The 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.

godotislate made their first commit to this issue’s fork.

godotislate’s picture

Status: Needs work » Needs review

Rebased.

smustgrave’s picture

Status: Needs review » Needs work

Appears to need another rebase, will keep an eye out to get remarked quickly

godotislate’s picture

Status: Needs work » Needs review

Rebased again.

smustgrave’s picture

Status: Needs review » Reviewed & tested by the community

Rebase appears green!

catch’s picture

Issue summary: View changes
Status: Reviewed & tested by the community » Fixed

I'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!

  • catch committed fcece3bb on 11.x
    Issue #3425114 by kingdutch, godotislate, mxr576, catch, larowlan: Add...
catch’s picture

alexpott’s picture

Why is this in the the root composer.json - it doesn't belong there. It should only be in core/composer.json.

  • alexpott committed 80d5032c on 11.x
    Issue #3425114 follow-up by alexpott: Add revolt/event-loop dependency...
alexpott’s picture

Fixed #24 in a quick follow-up.

quietone’s picture

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.