Contrib.social

Negative performance impact on (admin_)toolbar

Open on Drupal.org →
Created on 23 January 2025, 3 months ago

On a large drupal installation with toolbar enabled (also the admin toolbar) we had performance issues.

After debugging and profiling for hours, I found out, that this module is one of the reasons for our performance issues.

The toolbar is rendering the performance preview as part of the toolbar. Unfortunately, the module adds the cache-context "url" in the src/ResponsivePreview.php file:

    if (!$this->currentUser->hasPermission('access responsive preview')) {
      return $items;
    }
    $device_definition = $this->entityTypeManager->getDefinition('responsive_preview_device');

    $items['responsive_preview']['#cache']['tags'] = Cache::mergeTags(
      $device_definition->getListCacheTags(),
      ['config:node_type_list'],
    );
    $items['responsive_preview']['#cache']['contexts'] = Cache::mergeContexts(
      $items['responsive_preview']['#cache']['contexts'],
      ['route.is_admin', 'url'],
    );

This means, that the full url path is cached with the whole admin toolbar. The admin toolbar is ususally a large render array with many links. If we add the url as a cache-context, the toolbar has to be re-rendered for every single admin page.

This is very costly and will slow down your server if you have a lot of editors.

I propose to remove the cache tag or change it to url.site. It will make your system a lot faster.

Find the attached patch:

🐛 Bug report
Status

Active

Version

2.2

Component

Code

Created by

🇨🇭Switzerland ayalon

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

Merge Requests

Comments & Activities

  • Issue created by @ayalon
  • First commit to issue fork.
  • Merge request !24Update cache tag from url to site url. → (Open) created by rajeshreeputra
  • Comment 3 months ago →
    🇮🇳India rajeshreeputra Pune

    agree with you!

  • Status changed to Needs work 4 days ago
  • Comment 4 days ago →
    🇬🇧United Kingdom joachim

    The caching per-url is definitely going to hurt performance.

    But the reason appears to be this that's getting added into the toolbar:

            '#attached' => [
          'library' => ['responsive_preview/drupal.responsive-preview'],
          'drupalSettings' => [
            'responsive_preview' => [
              'url' => ltrim($url, '/'),
            ],
          ],
        ],

    with the $url coming from

        $url = $this->getPreviewUrl();

    where the value is the current URL, e.g.

    Object { url: "node/6506" }

    So changing the cache context to 'url.site' means that the first hit to build the toolbar will cache that specific URL, and all subsequent page loads will show the cached data with that URL. That will mean you'll get the wrong page opened in the preview!

    The right way to fix this is to either:

    - fix the JS so it doesn't need this URL in the settings -- I don't understand why we need to store a value for the current URL, when we can just get it from **the current URL**!
    - use a lazy builder for the responsive_preview toolbar item

contrib.social Blog FAQ Discussions
Production build 0.71.5 2024