Add Fibers support to Drupal\Core\Render\Renderer

This looks like pattern so "fiber's loop" could be extracted to some trait or static method somehow

Wow! It feels like a whole new world of concurrency.

+++ b/core/lib/Drupal/Core/Render/Renderer.php
@@ -161,6 +161,21 @@ public function renderPlain(&$elements) {
+  public function doRenderPlaceholder(&$placeholder_element) {

Can we add typehints?

The Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch 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.

Nice! 🤩

\Drupal\Core\Render\Renderer::renderPlaceholder() does 3 things:

1. Prepare: tweak the original render array to avoid re-auto-placeholdering
2. Render: the actual work
3. Store & bubble: store the rendered output in the render array and bubble up to the render context

I'm kinda worried about the new public doRenderPlaceholder() which does parts of the first and the second. The 3 different aspects are sprinkled throughout various parts: some in fiber creation, some in the new public method, some in the fiber processing. I'm hoping we can tighten that up a bit? (It took me a good while to grok what's going on because of this 🙈)

I like #12 much better! 🤩 Much easier to understand, because the original logic flow can still easily be seen:

1. public renderPlaceholder() now just calls 2 other things (doRender() + doReplace()) → public API surface unchanged 👍
2. protected replacePlaceholders() calls those 2 same things directly instead of calling renderPlaceholder(), to give it more control 👍

The most important caller of public renderPlaceholder() is BigPipe. This is fine, because BigPipe has its own parallellization.

Why both?

An obvious question is: do we need both?

This is my analysis/understanding, could you confirm its correctness, @catch? 🤓🙏

1. This issue makes \Drupal\Core\Render\Placeholder\SingleFlushStrategy faster, by rendering the different placeholders in parallel (in the sense that it's blocking I/O can happen in parallel), but still remains a single flush. Placeholders can be rendered in arbitrary order, but they must ALL be rendered for Renderer::renderPlaceholders() to finish (and of course, messages last).
2. #3377570: Add PHP Fibers support to BigPipe makes \Drupal\big_pipe\Render\Placeholder\BigPipeStrategy faster, by rendering in parallel (same limitations as above) and streaming the fastest to render placeholder (least I/O-blocked) to the client first (but still guaranteeing that the messages placeholder will be rendered & streamed last).

The two issues are complementary: this issue makes all placeholders renderable in fibers and provides performance benefits (both back end and front end) even when BigPipe is disabled, whereas the other issue makes it possible to not have to tweak the placeholder render order: whichever one is least blocked on I/O will be served fastest (also both back end and front end, but only more front end performance benefits: just the same back end performance gain).

without trying to pass the same array to different fibers by reference, which I do not even want to find out if it works because it would be impossible to grasp even if it somehow does.

😱🤣 Yeah let's not!

#13 is correct, I also opened #3383521 […] looks like the most work/least return (because we would render placeholders out of order, but still need to flush them in order

Yeah I personally would lean to not doing that issue — or rather, change the scope documenting why it's not worth it 😇

This is the amazing thing with fibers for me, the complete separation […] means we can add support in completely different places without conflicts or introducing interdependencies.

💯💯💯💯 🥳

Then it fits perfectly into Drupal's render API where at the very highest rendering level we end up with a set of placeholders which could have been nested deep inside entity/field/block/layout rendering pipelines, but end up getting processed all at once.

All that hard work you, I, @FabianX and a few others did at the end of the Drupal 8 cycle is finally reaching its full potential 😊🤝

Wasn't entirely sure how to test.

Did the same thing as #3377570: Add PHP Fibers support to BigPipe and applied the patch
Verified forms and messages still render just fine.

Since #3377570: Add PHP Fibers support to BigPipe is RTBC going to RTBC this too!

Committed d41f1a0 and pushed to 11.x. Thanks!

Found that this has the same issue as described here https://www.drupal.org/project/drupal/issues/3377570#comment-15271179

The fiber loop itself is never suspended due to $iterations being reset at every iteration of the while loop. Changed that in MR!5003

@ReINFaTe can you please open a follow up.

I had to change line 184 to

// Render the placeholder into markup.
$markup = $this->renderPlain($placeholder_element);
// return $markup;
return $markup ? $markup : '';
}

or it throws a "null given" error