Verified Commit 02c07301 authored by Alex Pott's avatar Alex Pott
Browse files

Issue #3422981 by catch, gorkagr, Wim Leers: Return early in... 

Issue #3422981 by catch, gorkagr, Wim Leers: Return early in CacheTagsChecksumTrait::isValid() (and possibly elsewhere) if there are no tags to check
parent 213906e2

core/lib/Drupal/Core/Cache/CacheTagsChecksumTrait.php

+5 −0
Original line number Diff line number Diff line
@@ -110,6 +110,11 @@ public function getCurrentChecksum(array $tags) { 
   * Implements \Drupal\Core\Cache\CacheTagsChecksumInterface::isValid()

   */

  public function isValid($checksum, array $tags) {

    // If there are no cache tags, then there is no cache tag to validate,

    // hence it's always valid.

    if (empty($tags)) {

      return TRUE;

    }

    // Any cache reads in this request involving cache tags whose invalidation

    // has been delayed due to an in-progress transaction are not allowed to use

    // data stored in cache; it must be assumed to be stale. This forces those

core/modules/system/tests/modules/performance_test/src/Cache/CacheTagsChecksumDecorator.php

+5 −0
Original line number Diff line number Diff line
@@ -30,6 +30,11 @@ public function getCurrentChecksum(array $tags) { 
   * {@inheritdoc}

   */

  public function isValid($checksum, array $tags) {

    // If there are no cache tags, the cache item is always valid, and the child

    // method will be a no-op, so don't log anything.

    if (empty($tags)) {

      return $this->checksumInvalidator->isValid($checksum, $tags);

    }

    $start = microtime(TRUE);

    $return = $this->checksumInvalidator->isValid($checksum, $tags);

    $stop = microtime(TRUE);

core/profiles/demo_umami/tests/src/FunctionalJavascript/OpenTelemetryAuthenticatedPerformanceTest.php

+1 −1
Original line number Diff line number Diff line
@@ -40,7 +40,7 @@ public function testFrontPageAuthenticatedWarmCache(): void { 
    $this->assertSame(0, $performance_data->getCacheSetCount());

    $this->assertSame(0, $performance_data->getCacheDeleteCount());

    $this->assertSame(0, $performance_data->getCacheTagChecksumCount());

    $this->assertSame(54, $performance_data->getCacheTagIsValidCount());

    $this->assertSame(13, $performance_data->getCacheTagIsValidCount());

    $this->assertSame(0, $performance_data->getCacheTagInvalidationCount());

  }

core/profiles/standard/tests/src/FunctionalJavascript/StandardPerformanceTest.php

+5 −5
Original line number Diff line number Diff line
@@ -60,7 +60,7 @@ public function testAnonymous() { 
    $this->assertSame(47, $performance_data->getCacheSetCount());

    $this->assertSame(0, $performance_data->getCacheDeleteCount());

    $this->assertCountBetween(143, 146, $performance_data->getCacheTagChecksumCount());

    $this->assertCountBetween(177, 180, $performance_data->getCacheTagIsValidCount());

    $this->assertCountBetween(47, 50, $performance_data->getCacheTagIsValidCount());

    $this->assertSame(0, $performance_data->getCacheTagInvalidationCount());



    // Test node page.
@@ -74,7 +74,7 @@ public function testAnonymous() { 
    $this->assertSame(16, $performance_data->getCacheSetCount());

    $this->assertSame(0, $performance_data->getCacheDeleteCount());

    $this->assertCountBetween(79, 80, $performance_data->getCacheTagChecksumCount());

    $this->assertCountBetween(149, 150, $performance_data->getCacheTagIsValidCount());

    $this->assertCountBetween(41, 42, $performance_data->getCacheTagIsValidCount());

    $this->assertSame(0, $performance_data->getCacheTagInvalidationCount());



    // Test user profile page.
@@ -87,7 +87,7 @@ public function testAnonymous() { 
    $this->assertSame(81, $performance_data->getCacheGetCount());

    $this->assertSame(16, $performance_data->getCacheSetCount());

    $this->assertSame(0, $performance_data->getCacheDeleteCount());

    $this->assertCountBetween(129, 130, $performance_data->getCacheTagIsValidCount());

    $this->assertCountBetween(36, 37, $performance_data->getCacheTagIsValidCount());

    $this->assertSame(0, $performance_data->getCacheTagInvalidationCount());

  }
@@ -118,7 +118,7 @@ public function testLogin(): void { 
    $this->assertSame(1, $performance_data->getCacheSetCount());

    $this->assertSame(1, $performance_data->getCacheDeleteCount());

    $this->assertSame(1, $performance_data->getCacheTagChecksumCount());

    $this->assertSame(69, $performance_data->getCacheTagIsValidCount());

    $this->assertSame(28, $performance_data->getCacheTagIsValidCount());

    $this->assertSame(0, $performance_data->getCacheTagInvalidationCount());

  }
@@ -151,7 +151,7 @@ public function testLoginBlock(): void { 
    $this->assertSame(1, $performance_data->getCacheSetCount());

    $this->assertSame(1, $performance_data->getCacheDeleteCount());

    $this->assertSame(1, $performance_data->getCacheTagChecksumCount());

    $this->assertSame(107, $performance_data->getCacheTagIsValidCount());

    $this->assertSame(31, $performance_data->getCacheTagIsValidCount());

    $this->assertSame(0, $performance_data->getCacheTagInvalidationCount());

  }

core/tests/Drupal/Tests/PerformanceTestTrait.php

+17 −6
Original line number Diff line number Diff line
@@ -4,6 +4,7 @@ 


namespace Drupal\Tests;



use Drupal\Core\Database\Event\DatabaseEvent;

use Drupal\performance_test\Cache\CacheTagOperation;

use OpenTelemetry\API\Trace\SpanKind;

use OpenTelemetry\Contrib\Otlp\OtlpHttpTransportFactory;
@@ -130,11 +131,7 @@ public function collectPerformanceData(callable $callable, ?string $service_name 
      foreach ($performance_test_data['database_events'] as $event) {

        // Don't log queries from the database cache backend because they're

        // logged separately as cache operations.

        $database_cache = FALSE;

        if (isset($event->caller['class'])) {

          $database_cache = is_a(str_replace('\\\\', '\\', $event->caller['class']), '\Drupal\Core\Cache\DatabaseBackend', TRUE) || is_a(str_replace('\\\\', '\\', $event->caller['class']), 'Drupal\Core\Cache\DatabaseCacheTagsChecksum', TRUE);

        }

        if (!$database_cache) {

        if (!static::isDatabaseCache($event)) {

          $query_count++;

        }

      }
@@ -361,7 +358,7 @@ private function openTelemetryTracing(array $messages, string $service_name): vo 
      $performance_test_data = $collection->get('performance_test_data');

      $query_events = $performance_test_data['database_events'] ?? [];

      foreach ($query_events as $key => $event) {

        if (isset($event->caller['class']) && is_a(str_replace('\\\\', '\\', $event->caller['class']), '\Drupal\Core\Cache\DatabaseBackend', TRUE)) {

        if (static::isDatabaseCache($event)) {

          continue;

        }

        // Use the first part of the database query for the span name.
@@ -460,4 +457,18 @@ protected function assertCountBetween(int $min, int $max, int $actual) { 
    );

  }



  /**

   * Checks whether a database event is from the database cache implementation.

   *

   * @param Drupal\Core\Database\Event\DatabaseEvent $event

   *   The database event.

   *

   * @return bool

   *   Whether the event was triggered by the database cache implementation.

   */

  protected static function isDatabaseCache(DatabaseEvent $event): bool {

    $class = str_replace('\\\\', '\\', $event->caller['class']);

    return is_a($class, '\Drupal\Core\Cache\DatabaseBackend', TRUE) || is_a($class, '\Drupal\Core\Cache\DatabaseCacheTagsChecksum', TRUE);

  }



}