Problem/Motivation

When profiling a site, I noticed that schema_metatag's breadcrumb manager causes breadcrumbs to be fetched a second time - running all the breadcrumb services' build() methods twice. Because breadcrumbs are generally per-page, this is a lot of additional work across a site.

It's not really possible to cache this in schema_metatag, or not unless it decorated core's breadcrumb service. Since other metatag-ish modules might do the same thing, I think we should add a static cache to core so that breadcrumb building can only ever happen once.

https://git.drupalcode.org/project/schema_metatag/-/blob/3.1.x/src/Plugi...

Should probably use the cache.memory bin.

Steps to reproduce

  1. Place the System Breadcrumb block in a region that is rendered on taxonomy term pages.
  2. Add code to call \Drupal::service('breadcrumb')->build() for the current route during page rendering, following the approach used by schema_metatag's BreadcrumbList implementation.
  3. Implement hook_system_breadcrumb_alter() to log each invocation for debugging purposes.
  4. Load the taxonomy term page once as an anonymous user and compare the logs. Two breadcrumb builds are recorded on the main branch, while only one is recorded with the merge request applied.

Proposed resolution

BreadcrumbManager::build() stores the generated breadcrumb in the cache.memory bin. The cache ID consists of the route name and a hash of the raw route parameters. Each cache entry holds a WeakReference to the request for which the breadcrumb was built, ensuring that cached breadcrumbs are reused only within the same request. Cache entries use the breadcrumb's cache tags for invalidation.

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Issue fork drupal-3621448

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

catch created an issue. See original summary.

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

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

chaitanyadessai’s picture

Status: Active » Needs review

Please review MR.

smustgrave’s picture

Status: Needs review » Needs work
chaitanyadessai’s picture

Status: Needs work » Needs review
  • The static cache uses the cache.memory bin.
  • The cache ID combines the route name with a hash of the raw route parameters.
  • Each entry stores a WeakReference to the request it was built for, ensuring that cached breadcrumbs are reused only within the same request. This prevents failures such as TermTranslationTest, where kernel tests reuse the container across requests.
  • Cache entries include the breadcrumb's cache tags, allowing them to be removed when those tags are invalidated.
  • Change record: https://www.drupal.org/node/3630675
smustgrave’s picture

Was AI used here..

smustgrave’s picture

Status: Needs review » Needs work

Also realized for good practice summary should be filled in.

Please note https://www.drupal.org/docs/develop/issues/issue-procedures-and-etiquett... summary needs to be in own words, only mention as AI appears to have entered this ticket. Which is fine but we need to follow the policy.

chaitanyadessai’s picture

Issue summary: View changes
chaitanyadessai’s picture

Status: Needs work » Needs review
chaitanyadessai’s picture

Thanks @smustgrave for the review. AI was used which I verified locally.