Problem/Motivation

Looking at cache gets/sets/gets in Umami performance tests I noticed these three blocks don't get placeholdered, whereas all the other blocks do, this means their cache items are looked up one by one instead of via multiple get and set.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#12 Screenshot from 2026-05-01 09-42-11.png42.45 KBcatch
#12 Screenshot from 2026-05-01 09-42-41.png50.04 KBcatch
#8 3587806-nr-bot_mqmxxvjn.txt91 bytesneeds-review-queue-bot

Issue fork drupal-3587806

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:

About issue forks

Comments

catch created an issue. See original summary.

catch’s picture

catch
he/him
Primary language English
commented
Status: Active » Needs review

catch opened merge request !15621

catch’s picture

catch
he/him
Primary language English
commented

Relatively small improvement but it's across almost all scenarios.

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

Very nice! I don't see anything wrong with this and have no additional feedback..

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

This makes we wonder when a block should not be placeholdered? Here's the docs:

   * When blocks of this type are rendered, indicate whether they should be
   * rendered in a placeholder or not. In general, blocks that attach libraries
   * and/or render entities should be placeholdered to optimize various aspects
   * of rendering performance.

Maybe we should change the default implementation to TRUE and make FALSE the unusual case?

smustgrave’s picture

smustgrave commented

I’d be +1 to changing to default to True

needs-review-queue-bot’s picture

needs-review-queue-bot commented
Status: Reviewed & tested by the community » Needs work
StatusFileSize
new 3587806-nr-bot_mqmxxvjn.txt91 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.

catch’s picture

catch
he/him
Primary language English
commented

I think changing the default to true would be a good follow-up we might want to only do that in 12.x or similar.

catch’s picture

catch
he/him
Primary language English
commented
Status: Needs work » Reviewed & tested by the community

Rebased and opened #3588012: Default ::createPlaceholder() to true for blog plugins.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

Shouldn't we do #3588012: Default ::createPlaceholder() to true for block plugins in main and not do this given the disruption we've already caused re #3533588: Core blocks now use placeholders, making it impossible for the theme to determine if the region is empty?

Or should we fix #3533588: Core blocks now use placeholders, making it impossible for the theme to determine if the region is empty - like we could create the ability for themes to disable placeholdering on the region level. Default it to FALSE - which means trust the block. Then main on main we can do #3588012: Default ::createPlaceholder() to true for block plugins and themes would have a way to opt regions out already.

catch’s picture

catch
he/him
Primary language English
commented
StatusFileSize
new Screenshot from 2026-05-01 09-42-41.png50.04 KB
new Screenshot from 2026-05-01 09-42-11.png42.45 KB

Well the site branding block at least is always going to be rendered, there's no conditional rendering there.

The messages block is always rendered with an HTML placeholder for JavaScript messages, even if that's not visible until a message is added, so from the point of view of the region, that's always rendered too.

The help block is conditionally rendered, but e.g. in Umami, the help and messages blocks are both in the 'highlighted' region, and because the messages block is always rendered, the highlighted region is never empty.

In Claro, which has separate help and messages blocks, this does result in an extra bit of empty HTML in the help region, but claro already has an empty help div in HEAD wrapping the region and afaict it doesn't result in any visual change.

We already made this change for views blocks several releases ago, which are far more likely to be conditionally rendered, and more often in sidebars etc. where the emptiness or not of a region is more likely to have an effect.

Attaching screenshots of the Claro change.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

Thanks @catch for #12 that's convinced me we should go ahead.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

Actually doesn't

Well the site branding block at least is always going to be rendered, there's no conditional rendering there.

mean this is more likely to cause issues. I.e. if a theme has a region that has the empty logic - this will break it.

catch’s picture

catch
he/him
Primary language English
commented

The site branding block will always have content in it, so the region it's in will never be empty and even if the theme is trying to remove empty regions that would never kick in. With or without this MR.

catch’s picture

catch
he/him
Primary language English
commented

Rebased.

  • alexpott committed e461cedb on main 
    perf: #3587806 Add placeholdering to help, messages, branding blocks

By...
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Version: main » 11.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)

Committed e461ced and pushed to main. Thanks!

catch opened merge request !15634

catch’s picture

catch
he/him
Primary language English
commented
Status: Patch (to be ported) » Reviewed & tested by the community

11.x backport MR is up - it's almost exactly the same just one or two numbers are different due to 12.x deprecations and removals.