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

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

@@ -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

@@ -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

@@ -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

@@ -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

@@ -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);
+  }
+
 }