CurrentPathContext inadvertently bubbles up 'route' cache context

Created on 23 July 2020, about 5 years ago
Updated 24 March 2023, over 2 years ago

When working on improving render caching of menu blocks, I noticed each entry had the canonical path for the current path as part of its cid resulting in many different variants.

After some digging I discovered this is due to the following snippet in "\Drupal\rules\ContextProvider\CurrentPathContext::getRuntimeContexts":

  Url::fromRoute('<current>', [], ['absolute' => TRUE])->toString();

This gets called early on in the rendering process causing the "\Drupal\Core\RouteProcessor\RouteProcessorCurrent::processOutbound" (https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21RouteProc...)

if ($bubbleable_metadata) {
   $bubbleable_metadata->addCacheContexts(['route']);
}

to bubble up the "route" cache tag. When subsequent blocks get rendered they inadvertently create cache redirects (i.e. variants) in the cache_render table based on the route.

A quick fix would be to handle the bubbleable metadata in the CurrentPathContext class. This can be accomplished by adding the `TRUE` flag to the toString function of Url. In doing so the bubbleable metadata will be returned directly instead of pushed up the chain. See https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Url.php/f...

$values = [
      'path' => $this->currentPathStack->getPath(),
      'url' => Url::fromRoute('<current>', [], ['absolute' => TRUE])->toString(TRUE)->getGeneratedUrl(),
    ];

    $context_definition = new ContextDefinition('current_path', $this->t('Current path'));
    $context = new Context($context_definition, $values);
    $cacheability = new CacheableMetadata();
    $cacheability->setCacheContexts(['url.path']);
    $context->addCacheableDependency($cacheability);

    $result = [
      'current_path' => $context,
    ];

This prevents the 'route' cache tag from bubbling up further. However, I am not sure why we even have the absolute url available in the context? Is this required? Or is the path enough?

πŸ› Bug report
Status

RTBC

Version

3.0

Component

Rules Core

Created by

πŸ‡ΊπŸ‡ΈUnited States Etroid

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • πŸ‡³πŸ‡±Netherlands tbsiqueira Rotterdam

    We also tested patch #6 and it resolved the caching issue, for users that had access to menu the pageload dropped from 6 seconds to 1 second.

  • πŸ‡ΊπŸ‡ΈUnited States tr Cascadia
  • πŸ‡ΊπŸ‡ΈUnited States chrisolof

    This bug is a bad one for performance. On our site it was behind a very problematic "route" cache context bubbling onto many of our site's render arrays, causing lots of unnecessary re-rendering per route and a pile-up of identical render cache entries (again, per route). This has the effect of largely disabling the benefits of render caching, while at the same time ballooning render cache write operations and render cache storage needs.

    I can confirm that rules-3161036-6-v2.patch from #6 solves the issue, and seems to do so in a way that preserves cacheability metadata from URL generation where needed without letting it bubble onto unrelated render arrays. It doesn't seem like it would be wise to just totally toss this metadata out, as the v1 patch does.

Production build 0.71.5 2024