Use a single per-theme cache tag for block config entities

Created on 11 February 2023, over 1 year ago

Problem/Motivation

Drupal sites typically have a lot of blocks and block cache tags (umami frontpage has about 20). They usually rarely change and if you change the block overview, we often change

At the same time, if you have multiple themes, changing blocks from one theme wont' change the others, so the global list cache tag doesn't need to be invalidated.

Strangely, there's also a a Block::postSave() that invalidates the block configs cache tag on saving a new one, explaining that it contains the theme, but as far as I see, that's not actually the case, leftover?

The performance impact of this isn't huge. In the umami example, this results in about 20 fewer cache tags in the http header/page cache, but we only save very few queries as block render caches often come with additional unique cache tags (views, block_content:id, menus, ..). But each tag in an IN () condition also costs a tiny bit I suppose.

I also removed the block_view cache tag, I can't think of a scenario where that is needed. I think that was pretty much the first cache tag we ever added, before we had list and per entity cache tags. We could also look into removing those entirely, or alternatively, use them for view displays, because I see another 10 cache tags for view displays (block_content, media and node), we could drop those in favor of the _view cache tag.

Last, the global list cache tag for blocks is still there but not actually invalidated, we should either remove it or add it back.

This will fail a lot of cache tests that expect those tags but hopefully not much else.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

πŸ“Œ Task
Status

Needs review

Version

10.1 ✨

Component
BlockΒ  β†’

Last updated 3 days ago

Created by

πŸ‡¨πŸ‡­Switzerland Berdir Switzerland

Live updates comments and jobs are added and updated live.
  • Performance

    It affects performance. It is often combined with the Needs profiling tag.

Sign in to follow issues

Comments & Activities

  • Issue created by @Berdir
  • Status changed to Needs review over 1 year ago
  • πŸ‡¨πŸ‡­Switzerland Berdir Switzerland
  • πŸ‡¨πŸ‡­Switzerland Berdir Switzerland

    ok, test showed an edge case which is when there are no blocks yet on the page. For now restoring the generic list cache tag, which removes some of the benefit of this, possibly we can add the theme cache tag to the page instead. The list cache tag might also be needed for other lists (REST, JSON:API), if we don't add it to the page it won't hurt to invalidate it.

    BlockSystemBrandingTest shows off another change, if we no longer tie the cache tag to the underlying config, we don't see changes that are done directly to config, but I think we don't support that on other entity types that customize this as well.

Production build 0.71.5 2024