Return early in CacheTagsChecksumTrait::isValid() (and possibly elsewhere) if there are no tags to check

Hi!
I hope is fine, i have added the snippet of @Wim Leers in https://www.drupal.org/project/drupal/issues/3421881#comment-15456992 in here :)

The code looks fine (😇), but the comment doesn't quite make sense to me: the comment is more complicated than the logic here 😄 It's much simpler: if there's no cache tags, then there is no cache tag to invalidate, hence it's always valid.

Agreed on the comment, it is always the hardest part :D :D , but i liked your last sentence so i have used it ;)

@catch, you mean in core/modules/system/tests/modules/performance_test/src/Cache/CacheTagsChecksumDecorator.php::isValid(), correct?

if we do as well there the

    if (empty($tags)) {
      return TRUE;
    }

should we also do a $this->logCacheTagOperation([], 0, 0, CacheTagOperation::isValid); (or smth like that before the return? Or there is no need to log the operation?

Just a nit — this looks great!

Also, am I missing something, or is this actually saving DB queries?! Nonsensical queries like

SELECT [tag], [invalidations] FROM {cachetags} WHERE [tag] IN ()'

🙈

I noticed we're only changing isValid(), what about the other two methods we're tracking in our performance tests: getCurrentChecksum and invalidateTags?

Re #11, not really:

    $query_tags = array_diff($tags, array_keys($this->tagCache));
    if ($query_tags) {
      $tag_invalidations = $this->getTagInvalidationCounts($query_tags);
      $this->tagCache += $tag_invalidations;
      // Fill static cache with empty objects for tags not found in the storage.
      $this->tagCache += array_fill_keys(array_diff($query_tags, array_keys($tag_invalidations)), 0);
    }

If $tags is empty, then so will $query_tags so we weren't running a query for that. It was showing up in OpenTelemtry, though, because we were tracking all calls in the decorator. This patch fixes that so you'll see a significantly reduced amount of them being tracked.

Committed and pushed 02c07301fc to 11.x and 86594c44d6 to 10.3.x. Thanks!

Sooooo... a follow-up for getCurrentChecksum and invalidateTags then? :D

More specifically, I was thinking about:

• Only the top of CacheTagsChecksumDecorator::invalidateTags() because CacheTagsChecksumTrait already short-circuits if the list of tags is empty.
• The top of CacheTagsChecksumDecorator::getCurrentChecksum()
• The top of CacheTagsChecksumTrait::getCurrentChecksum() because it runs a bunch of code that will end up doing nothing if the list of tags is empty