Optimize BigPipe::getPlaceholderOrder()

The Needs Review Queue Bot → tested this issue. It either no longer applies to Drupal core, or fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

✨ Interface previews/skeleton screens through optional "preview" or "placeholder" templates Fixed changed the BigPipe::getPlaceholderOrder() logic.

1. +++ b/core/modules/big_pipe/src/Render/BigPipe.php
   @@ -735,20 +715,44 @@ protected function getPlaceholderOrder($html, $placeholders) {
   +    $ordered_placeholders = [];
   +
   +    $needle = '<span data-big-pipe-placeholder-id="';
   +    $needle_len = strlen($needle);
   +    $end = 0;
   +    while (($start = strpos($html, $needle, $end)) !== FALSE) {
   +      $start += $needle_len;
   +      $end = strpos($html, '"', $start);
   +      if ($end === FALSE) {
   +        break;
   +      }
   

   This is irrelevant since ✨ Interface previews/skeleton screens through optional "preview" or "placeholder" templates Fixed .

2. +++ b/core/modules/big_pipe/src/Render/BigPipe.php
   @@ -735,20 +715,44 @@ protected function getPlaceholderOrder($html, $placeholders) {
   +      // Skip unknown placeholders.
   +      if (!isset($placeholders[$placeholder_id])) {
   +        continue;
   +      }
   +      // If the placeholder occurs multiple times, only keep the first.
   +      if (isset($ordered_placeholders[$placeholder_id]) || isset($message_placeholders[$placeholder_id])) {
   +        continue;
   +      }
   

   I think this is the remaining optimization potential.

I wanted to reroll #11.2 on HEAD, but:

1. +      // If the placeholder occurs multiple times, only keep the first.
   +      if (isset($ordered_placeholders[$placeholder_id]) || isset($message_placeholders[$placeholder_id])) {
   +        continue;
   +      }
   

   This is already handled by

   $placeholder_ids = array_unique($placeholder_ids);
   

   in HEAD.

2. +      // Skip unknown placeholders.
   +      if (!isset($placeholders[$placeholder_id])) {
   +        continue;
   +      }
   

   This is only possible in theory — if somebody literally hardcoded a BigPipe placeholder in their theme's or module's markup. That'd be a bug there. I'd rather have BigPipe fail in that case. Also, I've yet to see the very first bug report for this (I just finished triaging the BigPipe module's issue queue).

Attached patch against Drupal 10.1.x