From 883f7510253210bcaaaed6d92144dc05f9bfc7f5 Mon Sep 17 00:00:00 2001
From: Kristiaan Van den Eynde <kristiaan@factorial.io>
Date: Thu, 13 Mar 2025 14:27:17 +0100
Subject: [PATCH 01/25] Update variation cache so chain lookups are stored in
memory until the next set, invalidate or delete.
---
core/lib/Drupal/Core/Cache/VariationCache.php | 30 +++++++++++++++++--
1 file changed, 28 insertions(+), 2 deletions(-)
diff --git a/core/lib/Drupal/Core/Cache/VariationCache.php b/core/lib/Drupal/Core/Cache/VariationCache.php
index 3e3080b32301..2a210bc7245d 100644
--- a/core/lib/Drupal/Core/Cache/VariationCache.php
+++ b/core/lib/Drupal/Core/Cache/VariationCache.php
@@ -12,6 +12,13 @@
*/
class VariationCache implements VariationCacheInterface {
+ /**
+ * Stores redirect chain lookups until the next SET.
+ *
+ * @var array
+ */
+ protected array $redirectChainCache = [];
+
/**
* Constructs a new VariationCache object.
*
@@ -224,6 +231,7 @@ public function set(array $keys, $data, CacheableDependencyInterface $cacheabili
$this->cacheBackend->set($chain_cid, new CacheRedirect($cacheability));
}
+ unset($this->redirectChainCache[$this->createCacheIdFast($keys, $initial_cacheability)]);
$this->cacheBackend->set($cid, $data, $this->maxAgeToExpire($cacheability->getCacheMaxAge()), $optimized_cacheability->getCacheTags());
}
@@ -232,6 +240,13 @@ public function set(array $keys, $data, CacheableDependencyInterface $cacheabili
*/
public function delete(array $keys, CacheableDependencyInterface $initial_cacheability): void {
$chain = $this->getRedirectChain($keys, $initial_cacheability);
+
+ // Don't need to delete what could not be found.
+ if (end($chain) === FALSE) {
+ return;
+ }
+
+ unset($this->redirectChainCache[$this->createCacheIdFast($keys, $initial_cacheability)]);
$this->cacheBackend->delete(array_key_last($chain));
}
@@ -240,6 +255,13 @@ public function delete(array $keys, CacheableDependencyInterface $initial_cachea
*/
public function invalidate(array $keys, CacheableDependencyInterface $initial_cacheability): void {
$chain = $this->getRedirectChain($keys, $initial_cacheability);
+
+ // Don't need to invalidate what could not be found.
+ if (end($chain) === FALSE) {
+ return;
+ }
+
+ unset($this->redirectChainCache[$this->createCacheIdFast($keys, $initial_cacheability)]);
$this->cacheBackend->invalidate(array_key_last($chain));
}
@@ -261,7 +283,11 @@ public function invalidate(array $keys, CacheableDependencyInterface $initial_ca
* to query the cache for that result.
*/
protected function getRedirectChain(array $keys, CacheableDependencyInterface $initial_cacheability): array {
- $cid = $this->createCacheIdFast($keys, $initial_cacheability);
+ $cid = $initial_cid = $this->createCacheIdFast($keys, $initial_cacheability);
+ if (isset($this->redirectChainCache[$cid])) {
+ return $this->redirectChainCache[$cid];
+ }
+
$chain[$cid] = $result = $this->cacheBackend->get($cid);
while ($result && $result->data instanceof CacheRedirect) {
@@ -269,7 +295,7 @@ protected function getRedirectChain(array $keys, CacheableDependencyInterface $i
$chain[$cid] = $result = $this->cacheBackend->get($cid);
}
- return $chain;
+ return $this->redirectChainCache[$initial_cid] = $chain;
}
/**
--
GitLab
From 1c9f750dac4e04d0c5266ca4d6f0f1515ceb7269 Mon Sep 17 00:00:00 2001
From: Kristiaan Van den Eynde <kristiaan@factorial.io>
Date: Thu, 13 Mar 2025 15:25:19 +0100
Subject: [PATCH 02/25] Build in some safety for when cache context values
change during a request.
---
core/lib/Drupal/Core/Cache/VariationCache.php | 36 +++++++++++++++++--
1 file changed, 33 insertions(+), 3 deletions(-)
diff --git a/core/lib/Drupal/Core/Cache/VariationCache.php b/core/lib/Drupal/Core/Cache/VariationCache.php
index 2a210bc7245d..a8fda8cd8209 100644
--- a/core/lib/Drupal/Core/Cache/VariationCache.php
+++ b/core/lib/Drupal/Core/Cache/VariationCache.php
@@ -13,7 +13,7 @@
class VariationCache implements VariationCacheInterface {
/**
- * Stores redirect chain lookups until the next SET.
+ * Stores redirect chain lookups until the next set, invalidate or delete.
*
* @var array
*/
@@ -40,7 +40,7 @@ public function __construct(
*/
public function get(array $keys, CacheableDependencyInterface $initial_cacheability) {
$chain = $this->getRedirectChain($keys, $initial_cacheability);
- return array_pop($chain);
+ return end($chain);
}
/**
@@ -284,8 +284,15 @@ public function invalidate(array $keys, CacheableDependencyInterface $initial_ca
*/
protected function getRedirectChain(array $keys, CacheableDependencyInterface $initial_cacheability): array {
$cid = $initial_cid = $this->createCacheIdFast($keys, $initial_cacheability);
+
+ // See if we previously stored the redirect chain in memory. We do need to
+ // run a validity check because cache context values might have changed
+ // since the last time we got the chain. In theory that should never happen
+ // during a single request, but better safe than sorry.
if (isset($this->redirectChainCache[$cid])) {
- return $this->redirectChainCache[$cid];
+ if ($this->redirectChainIsValid($keys, $this->redirectChainCache[$cid])) {
+ return $this->redirectChainCache[$cid];
+ }
}
$chain[$cid] = $result = $this->cacheBackend->get($cid);
@@ -298,6 +305,29 @@ protected function getRedirectChain(array $keys, CacheableDependencyInterface $i
return $this->redirectChainCache[$initial_cid] = $chain;
}
+ /**
+ * Validates a redirect chain for the current cache context values.
+ *
+ * @param string[] $keys
+ * The cache keys used to build the chain.
+ * @param array $chain
+ * The redirect chain to validate.
+ *
+ * @return bool
+ * Whether the redirect chain is valid.
+ */
+ protected function redirectChainIsValid(array $keys, array $chain): bool {
+ foreach ($chain as $result) {
+ if ($result->data instanceof CacheRedirect) {
+ $cid = $this->createCacheIdFast($keys, $result->data);
+ if (!isset($chain[$cid])) {
+ return FALSE;
+ }
+ }
+ }
+ return TRUE;
+ }
+
/**
* Maps a max-age value to an "expire" value for the Cache API.
*
--
GitLab
From f80c6ea86e04032e9950194d9103cde9e0b8e285 Mon Sep 17 00:00:00 2001
From: Kristiaan Van den Eynde <kristiaan@factorial.io>
Date: Thu, 13 Mar 2025 15:34:25 +0100
Subject: [PATCH 03/25] Fix notices
---
core/lib/Drupal/Core/Cache/VariationCache.php | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/core/lib/Drupal/Core/Cache/VariationCache.php b/core/lib/Drupal/Core/Cache/VariationCache.php
index a8fda8cd8209..81e4353f18db 100644
--- a/core/lib/Drupal/Core/Cache/VariationCache.php
+++ b/core/lib/Drupal/Core/Cache/VariationCache.php
@@ -318,7 +318,7 @@ protected function getRedirectChain(array $keys, CacheableDependencyInterface $i
*/
protected function redirectChainIsValid(array $keys, array $chain): bool {
foreach ($chain as $result) {
- if ($result->data instanceof CacheRedirect) {
+ if ($result && $result->data instanceof CacheRedirect) {
$cid = $this->createCacheIdFast($keys, $result->data);
if (!isset($chain[$cid])) {
return FALSE;
--
GitLab
From 773bc39af04799c2232cc63d327930a5f37eaf90 Mon Sep 17 00:00:00 2001
From: Kristiaan Van den Eynde <kristiaan@factorial.io>
Date: Thu, 13 Mar 2025 15:52:49 +0100
Subject: [PATCH 04/25] Update performance tests
---
.../tests/src/FunctionalJavascript/PerformanceTest.php | 4 ++--
.../FunctionalJavascript/StandardPerformanceTest.php | 10 +++++-----
2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/core/modules/navigation/tests/src/FunctionalJavascript/PerformanceTest.php b/core/modules/navigation/tests/src/FunctionalJavascript/PerformanceTest.php
index 1acfd62fb1da..b7e1a693df7c 100644
--- a/core/modules/navigation/tests/src/FunctionalJavascript/PerformanceTest.php
+++ b/core/modules/navigation/tests/src/FunctionalJavascript/PerformanceTest.php
@@ -73,13 +73,13 @@ public function testLogin(): void {
$expected = [
'QueryCount' => 4,
- 'CacheGetCount' => 59,
+ 'CacheGetCount' => 58,
'CacheGetCountByBin' => [
'config' => 11,
'data' => 6,
'discovery' => 10,
'bootstrap' => 6,
- 'dynamic_page_cache' => 2,
+ 'dynamic_page_cache' => 1,
'render' => 23,
'menu' => 1,
],
diff --git a/core/profiles/standard/tests/src/FunctionalJavascript/StandardPerformanceTest.php b/core/profiles/standard/tests/src/FunctionalJavascript/StandardPerformanceTest.php
index 0c3d7300ea33..2cc271326a16 100644
--- a/core/profiles/standard/tests/src/FunctionalJavascript/StandardPerformanceTest.php
+++ b/core/profiles/standard/tests/src/FunctionalJavascript/StandardPerformanceTest.php
@@ -127,15 +127,15 @@ protected function testAnonymous(): void {
$this->assertSame($expected_queries, $recorded_queries);
$expected = [
'QueryCount' => 36,
- 'CacheGetCount' => 122,
+ 'CacheGetCount' => 109,
'CacheGetCountByBin' => [
'page' => 1,
'config' => 21,
'data' => 8,
'discovery' => 38,
'bootstrap' => 8,
- 'dynamic_page_cache' => 2,
- 'render' => 35,
+ 'dynamic_page_cache' => 1,
+ 'render' => 23,
'default' => 5,
'entity' => 2,
'menu' => 2,
@@ -202,7 +202,7 @@ protected function testAnonymous(): void {
$this->assertSame($expected_queries, $recorded_queries);
$expected = [
'QueryCount' => 10,
- 'CacheGetCount' => 92,
+ 'CacheGetCount' => 81,
'CacheSetCount' => 16,
'CacheDeleteCount' => 0,
'CacheTagInvalidationCount' => 0,
@@ -259,7 +259,7 @@ protected function testAnonymous(): void {
$this->assertSame($expected_queries, $recorded_queries);
$expected = [
'QueryCount' => 14,
- 'CacheGetCount' => 80,
+ 'CacheGetCount' => 69,
'CacheSetCount' => 17,
'CacheDeleteCount' => 0,
'CacheTagInvalidationCount' => 0,
--
GitLab
From 82a27b945ae87d4173559b9bdb30027ce4b6d571 Mon Sep 17 00:00:00 2001
From: Sascha Grossenbacher <saschagros@gmail.com>
Date: Thu, 13 Mar 2025 20:07:49 +0100
Subject: [PATCH 05/25] Attempt to support getMultiple()
---
core/lib/Drupal/Core/Cache/VariationCache.php | 23 ++++++++++++++++++-
1 file changed, 22 insertions(+), 1 deletion(-)
diff --git a/core/lib/Drupal/Core/Cache/VariationCache.php b/core/lib/Drupal/Core/Cache/VariationCache.php
index 81e4353f18db..b829c04e2434 100644
--- a/core/lib/Drupal/Core/Cache/VariationCache.php
+++ b/core/lib/Drupal/Core/Cache/VariationCache.php
@@ -53,19 +53,32 @@ public function getMultiple(array $items): array {
// cache backend.
//
// Create a map of CIDs with their associated $items index and cache keys.
+ $results = [];
$cid_map = [];
foreach ($items as $index => [$keys, $cacheability]) {
$cid = $this->createCacheIdFast($keys, $cacheability);
+
+ // See if we previously stored the redirect chain in memory. We do need to
+ // run a validity check because cache context values might have changed
+ // since the last time we got the chain. In theory that should never happen
+ // during a single request, but better safe than sorry.
+ if (isset($this->redirectChainCache[$cid])) {
+ if ($this->redirectChainIsValid($keys, $this->redirectChainCache[$cid])) {
+ $results[$index] = end($this->redirectChainCache[$cid]);
+ continue;
+ }
+ }
+
$cid_map[$cid] = [
'index' => $index,
'keys' => $keys,
+ 'initial' => $cid,
];
}
// Go over all CIDs and update the map according to found redirects. If the
// map is empty, it means we've followed all CIDs to their final result or
// lack thereof.
- $results = [];
while (!empty($cid_map)) {
$new_cid_map = [];
@@ -73,6 +86,8 @@ public function getMultiple(array $items): array {
foreach ($this->cacheBackend->getMultiple($fetch_cids) as $cid => $result) {
$info = $cid_map[$cid];
+ $this->redirectChainCache[$info['initial']][$cid] = $result;
+
// Add redirects to the next CID map, so the next iteration can look
// them all up in one ::getMultiple() call to the cache backend.
if ($result->data instanceof CacheRedirect) {
@@ -84,6 +99,12 @@ public function getMultiple(array $items): array {
$results[$info['index']] = $result;
}
+ // Any cache misses are still in $fetch_cids, ensure they are set.
+ foreach ($fetch_cids as $fetch_cid) {
+ $info = $cid_map[$cid];
+ $this->redirectChainCache[$info['initial']][$fetch_cid] = FALSE;
+ }
+
$cid_map = $new_cid_map;
}
--
GitLab
From 31d44e52e7e4297ee399b4f23745b23b95068ed9 Mon Sep 17 00:00:00 2001
From: Sascha Grossenbacher <saschagros@gmail.com>
Date: Thu, 13 Mar 2025 20:15:32 +0100
Subject: [PATCH 06/25] fix and update some tests
---
core/lib/Drupal/Core/Cache/VariationCache.php | 2 +-
.../tests/src/FunctionalJavascript/StandardPerformanceTest.php | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/core/lib/Drupal/Core/Cache/VariationCache.php b/core/lib/Drupal/Core/Cache/VariationCache.php
index b829c04e2434..bf626a98791d 100644
--- a/core/lib/Drupal/Core/Cache/VariationCache.php
+++ b/core/lib/Drupal/Core/Cache/VariationCache.php
@@ -101,7 +101,7 @@ public function getMultiple(array $items): array {
// Any cache misses are still in $fetch_cids, ensure they are set.
foreach ($fetch_cids as $fetch_cid) {
- $info = $cid_map[$cid];
+ $info = $cid_map[$fetch_cid];
$this->redirectChainCache[$info['initial']][$fetch_cid] = FALSE;
}
diff --git a/core/profiles/standard/tests/src/FunctionalJavascript/StandardPerformanceTest.php b/core/profiles/standard/tests/src/FunctionalJavascript/StandardPerformanceTest.php
index 2cc271326a16..5cf9b0675950 100644
--- a/core/profiles/standard/tests/src/FunctionalJavascript/StandardPerformanceTest.php
+++ b/core/profiles/standard/tests/src/FunctionalJavascript/StandardPerformanceTest.php
@@ -314,7 +314,7 @@ protected function testLogin(): void {
$this->assertSame($expected_queries, $recorded_queries);
$expected = [
'QueryCount' => 17,
- 'CacheGetCount' => 84,
+ 'CacheGetCount' => 81,
'CacheSetCount' => 1,
'CacheDeleteCount' => 1,
'CacheTagInvalidationCount' => 0,
--
GitLab
From 976dd227a8d72cde00948befce394a08d96a0e01 Mon Sep 17 00:00:00 2001
From: Sascha Grossenbacher <saschagros@gmail.com>
Date: Thu, 13 Mar 2025 22:53:59 +0100
Subject: [PATCH 07/25] Only cache redirects and misses
---
core/lib/Drupal/Core/Cache/VariationCache.php | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/core/lib/Drupal/Core/Cache/VariationCache.php b/core/lib/Drupal/Core/Cache/VariationCache.php
index bf626a98791d..2b411e1e7ce9 100644
--- a/core/lib/Drupal/Core/Cache/VariationCache.php
+++ b/core/lib/Drupal/Core/Cache/VariationCache.php
@@ -86,13 +86,13 @@ public function getMultiple(array $items): array {
foreach ($this->cacheBackend->getMultiple($fetch_cids) as $cid => $result) {
$info = $cid_map[$cid];
- $this->redirectChainCache[$info['initial']][$cid] = $result;
// Add redirects to the next CID map, so the next iteration can look
// them all up in one ::getMultiple() call to the cache backend.
if ($result->data instanceof CacheRedirect) {
$redirect_cid = $this->createCacheIdFast($info['keys'], $result->data);
$new_cid_map[$redirect_cid] = $info;
+ $this->redirectChainCache[$info['initial']][$cid] = $result;
continue;
}
@@ -323,7 +323,13 @@ protected function getRedirectChain(array $keys, CacheableDependencyInterface $i
$chain[$cid] = $result = $this->cacheBackend->get($cid);
}
- return $this->redirectChainCache[$initial_cid] = $chain;
+ $chain[$cid] = $result;
+ if ($result === FALSE) {
+ $this->redirectChainCache[$initial_cid] = $chain;
+ }
+
+ return $chain;
+
}
/**
--
GitLab
From abfe2e608321e12c0229616c067336cda5736122 Mon Sep 17 00:00:00 2001
From: Sascha Grossenbacher <saschagros@gmail.com>
Date: Fri, 14 Mar 2025 09:42:32 +0100
Subject: [PATCH 08/25] phpcs
---
core/lib/Drupal/Core/Cache/VariationCache.php | 1 -
1 file changed, 1 deletion(-)
diff --git a/core/lib/Drupal/Core/Cache/VariationCache.php b/core/lib/Drupal/Core/Cache/VariationCache.php
index 2b411e1e7ce9..b6e2e4006c41 100644
--- a/core/lib/Drupal/Core/Cache/VariationCache.php
+++ b/core/lib/Drupal/Core/Cache/VariationCache.php
@@ -86,7 +86,6 @@ public function getMultiple(array $items): array {
foreach ($this->cacheBackend->getMultiple($fetch_cids) as $cid => $result) {
$info = $cid_map[$cid];
-
// Add redirects to the next CID map, so the next iteration can look
// them all up in one ::getMultiple() call to the cache backend.
if ($result->data instanceof CacheRedirect) {
--
GitLab
From fd3a5cd8a0fa9bcdaf59154f7e53ae5c543fd758 Mon Sep 17 00:00:00 2001
From: Kristiaan Van den Eynde <kristiaan@factorial.io>
Date: Fri, 14 Mar 2025 10:15:20 +0100
Subject: [PATCH 09/25] Update logic to store entire chain except for the final
part if it's a cache hit.
---
core/lib/Drupal/Core/Cache/VariationCache.php | 24 ++++++++++++++-----
.../StandardPerformanceTest.php | 2 +-
2 files changed, 19 insertions(+), 7 deletions(-)
diff --git a/core/lib/Drupal/Core/Cache/VariationCache.php b/core/lib/Drupal/Core/Cache/VariationCache.php
index b6e2e4006c41..57cce3a61503 100644
--- a/core/lib/Drupal/Core/Cache/VariationCache.php
+++ b/core/lib/Drupal/Core/Cache/VariationCache.php
@@ -311,24 +311,36 @@ protected function getRedirectChain(array $keys, CacheableDependencyInterface $i
// during a single request, but better safe than sorry.
if (isset($this->redirectChainCache[$cid])) {
if ($this->redirectChainIsValid($keys, $this->redirectChainCache[$cid])) {
- return $this->redirectChainCache[$cid];
+ $chain = $this->redirectChainCache[$cid];
}
}
- $chain[$cid] = $result = $this->cacheBackend->get($cid);
+ // Initiate the chain if we couldn't retrieve (a partial) one from memory.
+ // If we did find one, we continue our search from the last redirect in the
+ // chain in case we had a cache hit before, or we take the FALSE at the end
+ // of the chain from a previous cache miss, bypassing the while loop below.
+ if (empty($chain)) {
+ $chain[$cid] = $result = $this->cacheBackend->get($cid);
+ }
+ else {
+ $result = end($chain);
+ }
while ($result && $result->data instanceof CacheRedirect) {
$cid = $this->createCacheIdFast($keys, $result->data);
$chain[$cid] = $result = $this->cacheBackend->get($cid);
}
- $chain[$cid] = $result;
- if ($result === FALSE) {
- $this->redirectChainCache[$initial_cid] = $chain;
+ // When storing the redirect chain in memory we must take care to not store
+ // a cache hit as they can be invalidated, unlike CacheRedirect objects. We
+ // do store the rest of the chain because redirects can be reused safely.
+ $chain_to_cache = $chain;
+ if ($result !== FALSE) {
+ array_pop($chain_to_cache);
}
+ $this->redirectChainCache[$initial_cid] = $chain_to_cache;
return $chain;
-
}
/**
diff --git a/core/profiles/standard/tests/src/FunctionalJavascript/StandardPerformanceTest.php b/core/profiles/standard/tests/src/FunctionalJavascript/StandardPerformanceTest.php
index 5cf9b0675950..2cc271326a16 100644
--- a/core/profiles/standard/tests/src/FunctionalJavascript/StandardPerformanceTest.php
+++ b/core/profiles/standard/tests/src/FunctionalJavascript/StandardPerformanceTest.php
@@ -314,7 +314,7 @@ protected function testLogin(): void {
$this->assertSame($expected_queries, $recorded_queries);
$expected = [
'QueryCount' => 17,
- 'CacheGetCount' => 81,
+ 'CacheGetCount' => 84,
'CacheSetCount' => 1,
'CacheDeleteCount' => 1,
'CacheTagInvalidationCount' => 0,
--
GitLab
From cb5f495f9b8f45f08a7b8270acb1270bd62c7217 Mon Sep 17 00:00:00 2001
From: Kristiaan Van den Eynde <kristiaan@factorial.io>
Date: Fri, 14 Mar 2025 10:15:50 +0100
Subject: [PATCH 10/25] Temporarily revert getMultiple changes so we can see
impact of recent change on test fails
---
core/lib/Drupal/Core/Cache/VariationCache.php | 21 +------------------
1 file changed, 1 insertion(+), 20 deletions(-)
diff --git a/core/lib/Drupal/Core/Cache/VariationCache.php b/core/lib/Drupal/Core/Cache/VariationCache.php
index 57cce3a61503..f7e9d36b53fe 100644
--- a/core/lib/Drupal/Core/Cache/VariationCache.php
+++ b/core/lib/Drupal/Core/Cache/VariationCache.php
@@ -53,32 +53,20 @@ public function getMultiple(array $items): array {
// cache backend.
//
// Create a map of CIDs with their associated $items index and cache keys.
- $results = [];
$cid_map = [];
foreach ($items as $index => [$keys, $cacheability]) {
$cid = $this->createCacheIdFast($keys, $cacheability);
- // See if we previously stored the redirect chain in memory. We do need to
- // run a validity check because cache context values might have changed
- // since the last time we got the chain. In theory that should never happen
- // during a single request, but better safe than sorry.
- if (isset($this->redirectChainCache[$cid])) {
- if ($this->redirectChainIsValid($keys, $this->redirectChainCache[$cid])) {
- $results[$index] = end($this->redirectChainCache[$cid]);
- continue;
- }
- }
-
$cid_map[$cid] = [
'index' => $index,
'keys' => $keys,
- 'initial' => $cid,
];
}
// Go over all CIDs and update the map according to found redirects. If the
// map is empty, it means we've followed all CIDs to their final result or
// lack thereof.
+ $results = [];
while (!empty($cid_map)) {
$new_cid_map = [];
@@ -91,19 +79,12 @@ public function getMultiple(array $items): array {
if ($result->data instanceof CacheRedirect) {
$redirect_cid = $this->createCacheIdFast($info['keys'], $result->data);
$new_cid_map[$redirect_cid] = $info;
- $this->redirectChainCache[$info['initial']][$cid] = $result;
continue;
}
$results[$info['index']] = $result;
}
- // Any cache misses are still in $fetch_cids, ensure they are set.
- foreach ($fetch_cids as $fetch_cid) {
- $info = $cid_map[$fetch_cid];
- $this->redirectChainCache[$info['initial']][$fetch_cid] = FALSE;
- }
-
$cid_map = $new_cid_map;
}
--
GitLab
From e58e01139ab5c70d4e6be67e50f477984d47918c Mon Sep 17 00:00:00 2001
From: Kristiaan Van den Eynde <kristiaan@factorial.io>
Date: Fri, 14 Mar 2025 14:55:50 +0100
Subject: [PATCH 11/25] Fix some tests already
---
core/lib/Drupal/Core/Cache/VariationCache.php | 15 +++++++++++----
.../jsonapi/tests/src/Functional/NodeTest.php | 8 +++++++-
.../Views/StyleSerializerEntityTest.php | 9 +++++++++
3 files changed, 27 insertions(+), 5 deletions(-)
diff --git a/core/lib/Drupal/Core/Cache/VariationCache.php b/core/lib/Drupal/Core/Cache/VariationCache.php
index f7e9d36b53fe..981af4b309cb 100644
--- a/core/lib/Drupal/Core/Cache/VariationCache.php
+++ b/core/lib/Drupal/Core/Cache/VariationCache.php
@@ -290,10 +290,8 @@ protected function getRedirectChain(array $keys, CacheableDependencyInterface $i
// run a validity check because cache context values might have changed
// since the last time we got the chain. In theory that should never happen
// during a single request, but better safe than sorry.
- if (isset($this->redirectChainCache[$cid])) {
- if ($this->redirectChainIsValid($keys, $this->redirectChainCache[$cid])) {
- $chain = $this->redirectChainCache[$cid];
- }
+ if (isset($this->redirectChainCache[$cid]) && $this->redirectChainIsValid($keys, $this->redirectChainCache[$cid])) {
+ $chain = $this->redirectChainCache[$cid];
}
// Initiate the chain if we couldn't retrieve (a partial) one from memory.
@@ -411,4 +409,13 @@ protected function createCacheIdFast(array $keys, CacheableDependencyInterface $
return implode(':', $keys);
}
+ /**
+ * Reset statically cached variables.
+ *
+ * This is only used by tests.
+ */
+ public function reset() {
+ $this->redirectChainCache = [];
+ }
+
}
diff --git a/core/modules/jsonapi/tests/src/Functional/NodeTest.php b/core/modules/jsonapi/tests/src/Functional/NodeTest.php
index aa40c2446c5c..33e96003d6ad 100644
--- a/core/modules/jsonapi/tests/src/Functional/NodeTest.php
+++ b/core/modules/jsonapi/tests/src/Functional/NodeTest.php
@@ -399,7 +399,13 @@ protected function assertCacheableNormalizations(): void {
* @internal
*/
protected function assertNormalizedFieldsAreCached(array $field_names): void {
- $cache = \Drupal::service('variation_cache.jsonapi_normalizations')->get(['node--camelids', $this->entity->uuid(), $this->entity->language()->getId()], new CacheableMetadata());
+ $variation_cache = \Drupal::service('variation_cache.jsonapi_normalizations');
+
+ // Because we warm caches in different requests, we do not properly populate
+ // the internal properties of our variation cache. Reset it.
+ $variation_cache->reset();
+
+ $cache = $variation_cache->get(['node--camelids', $this->entity->uuid(), $this->entity->language()->getId()], new CacheableMetadata());
$cached_fields = $cache->data['fields'];
$this->assertSameSize($field_names, $cached_fields);
array_walk($field_names, function ($field_name) use ($cached_fields) {
diff --git a/core/modules/rest/tests/src/Functional/Views/StyleSerializerEntityTest.php b/core/modules/rest/tests/src/Functional/Views/StyleSerializerEntityTest.php
index 9175d6f5da49..af91d1634272 100644
--- a/core/modules/rest/tests/src/Functional/Views/StyleSerializerEntityTest.php
+++ b/core/modules/rest/tests/src/Functional/Views/StyleSerializerEntityTest.php
@@ -220,6 +220,11 @@ protected function addRequestWithFormat($format): void {
*/
public function testRestRenderCaching(): void {
$this->drupalLogin($this->adminUser);
+
+ /** @var \Drupal\Core\Cache\VariationCacheFactoryInterface $vc_factory */
+ $variation_cache_factory = \Drupal::service('variation_cache_factory');
+ $variation_cache = $variation_cache_factory->get('render');
+
/** @var \Drupal\Core\Render\RenderCacheInterface $render_cache */
$render_cache = \Drupal::service('render_cache');
@@ -268,6 +273,10 @@ public function testRestRenderCaching(): void {
$this->assertSession()->responseHeaderEquals('content-type', 'application/json');
$this->assertCacheContexts($cache_contexts);
$this->assertCacheTags($cache_tags);
+
+ // Because we warm caches in different requests, we do not properly populate
+ // the internal properties of our variation cache. Reset it.
+ $variation_cache->reset();
$this->assertNotEmpty($render_cache->get($original));
$result_xml = $this->drupalGet('test/serialize/entity', ['query' => ['_format' => 'xml']]);
--
GitLab
From f311425fa02b1fff1f46e522db86af6392429e76 Mon Sep 17 00:00:00 2001
From: Kristiaan Van den Eynde <kristiaan@factorial.io>
Date: Fri, 14 Mar 2025 15:38:48 +0100
Subject: [PATCH 12/25] phpstan
---
core/lib/Drupal/Core/Cache/VariationCache.php | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/core/lib/Drupal/Core/Cache/VariationCache.php b/core/lib/Drupal/Core/Cache/VariationCache.php
index 981af4b309cb..2d3b67b58be5 100644
--- a/core/lib/Drupal/Core/Cache/VariationCache.php
+++ b/core/lib/Drupal/Core/Cache/VariationCache.php
@@ -414,7 +414,7 @@ protected function createCacheIdFast(array $keys, CacheableDependencyInterface $
*
* This is only used by tests.
*/
- public function reset() {
+ public function reset(): void {
$this->redirectChainCache = [];
}
--
GitLab
From 3fbc1664d7eef2ca6b4e3fc8c927c7a09f2e50d4 Mon Sep 17 00:00:00 2001
From: Kristiaan Van den Eynde <kristiaan@factorial.io>
Date: Fri, 14 Mar 2025 16:15:35 +0100
Subject: [PATCH 13/25] Fix final test, other JS test seemed like a random
failure as it went green locally.
---
.../views/tests/src/Functional/Plugin/CacheWebTest.php | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/core/modules/views/tests/src/Functional/Plugin/CacheWebTest.php b/core/modules/views/tests/src/Functional/Plugin/CacheWebTest.php
index 29902c6aee4d..e1b403eb53ff 100644
--- a/core/modules/views/tests/src/Functional/Plugin/CacheWebTest.php
+++ b/core/modules/views/tests/src/Functional/Plugin/CacheWebTest.php
@@ -62,6 +62,10 @@ public function testCacheOutputOnPage(): void {
$view->save();
$this->container->get('router.builder')->rebuildIfNeeded();
+ /** @var \Drupal\Core\Cache\VariationCacheFactoryInterface $vc_factory */
+ $variation_cache_factory = \Drupal::service('variation_cache_factory');
+ $variation_cache = $variation_cache_factory->get('render');
+
/** @var \Drupal\Core\Render\RenderCacheInterface $render_cache */
$render_cache = \Drupal::service('render_cache');
$cache_element = DisplayPluginBase::buildBasicRenderable('test_display', 'page_1');
@@ -70,6 +74,11 @@ public function testCacheOutputOnPage(): void {
$this->drupalGet('test-display');
$this->assertSession()->statusCodeEquals(200);
+
+ // Because we warm caches in different requests, we do not properly populate
+ // the internal properties of our variation cache. Reset it.
+ $variation_cache->reset();
+
$this->assertNotEmpty($render_cache->get($cache_element));
$cache_tags = [
'config:user.role.anonymous',
--
GitLab
From 7ed117a8b8e87754de685d1c44769adb990f3272 Mon Sep 17 00:00:00 2001
From: Sascha Grossenbacher <saschagros@gmail.com>
Date: Sun, 16 Mar 2025 11:30:09 +0100
Subject: [PATCH 14/25] Update umami performance test
---
.../OpenTelemetryNodePagePerformanceTest.php | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/core/profiles/demo_umami/tests/src/FunctionalJavascript/OpenTelemetryNodePagePerformanceTest.php b/core/profiles/demo_umami/tests/src/FunctionalJavascript/OpenTelemetryNodePagePerformanceTest.php
index 7204388bbb34..4e3bb56b62de 100644
--- a/core/profiles/demo_umami/tests/src/FunctionalJavascript/OpenTelemetryNodePagePerformanceTest.php
+++ b/core/profiles/demo_umami/tests/src/FunctionalJavascript/OpenTelemetryNodePagePerformanceTest.php
@@ -288,7 +288,7 @@ protected function testNodePageWarmCache(): void {
$expected = [
'QueryCount' => 172,
- 'CacheGetCount' => 247,
+ 'CacheGetCount' => 231,
'CacheGetCountByBin' => [
'page' => 1,
'config' => 66,
@@ -296,8 +296,8 @@ protected function testNodePageWarmCache(): void {
'data' => 13,
'entity' => 18,
'bootstrap' => 8,
- 'dynamic_page_cache' => 2,
- 'render' => 67,
+ 'dynamic_page_cache' => 1,
+ 'render' => 52,
'default' => 3,
'menu' => 2,
],
--
GitLab
From 48ab7c633e205168092d451cc8c7060a4eca1d7b Mon Sep 17 00:00:00 2001
From: Kristiaan Van den Eynde <kristiaan@factorial.io>
Date: Wed, 19 Mar 2025 14:44:58 +0100
Subject: [PATCH 15/25] Support getMultiple() again
---
core/lib/Drupal/Core/Cache/VariationCache.php | 48 +++++++++++++++++--
1 file changed, 45 insertions(+), 3 deletions(-)
diff --git a/core/lib/Drupal/Core/Cache/VariationCache.php b/core/lib/Drupal/Core/Cache/VariationCache.php
index 2d3b67b58be5..fcda1c05067e 100644
--- a/core/lib/Drupal/Core/Cache/VariationCache.php
+++ b/core/lib/Drupal/Core/Cache/VariationCache.php
@@ -15,6 +15,18 @@ class VariationCache implements VariationCacheInterface {
/**
* Stores redirect chain lookups until the next set, invalidate or delete.
*
+ * Array keys are the cache IDs constructed from the cache keys and initial
+ * cacheability and values are arrays where each step of a redirect chain is
+ * recorded.
+ *
+ * These arrays are indexed by the cache IDs being followed during the chain
+ * and the CacheRedirect objects that construe the chain. At the end there
+ * should always be a value of FALSE for a cache miss, or a CacheRedirect for
+ * a cache hit because we cannot store the cache hit itself into a property
+ * that does not support invalidation based on cache metadata. By storing the
+ * last CacheRedirect that led to the hit, we can at least avoid having to
+ * retrieve the entire chain again to get to the actual cached data.
+ *
* @var array
*/
protected array $redirectChainCache = [];
@@ -52,21 +64,43 @@ public function getMultiple(array $items): array {
// following of redirect chains by calling ::getMultiple() on the underlying
// cache backend.
//
+ // However, ::getRedirectChain() has an internal cache that we could both
+ // benefit from and contribute to whenever we call this function. So any use
+ // or manipulation of $this->redirectChainCache below is for optimization
+ // purposes. You can read up on how the internal cache is structured on the
+ // property documentation of $this->redirectChainCache.
+ //
// Create a map of CIDs with their associated $items index and cache keys.
- $cid_map = [];
+ $cid_map = $results = [];
foreach ($items as $index => [$keys, $cacheability]) {
- $cid = $this->createCacheIdFast($keys, $cacheability);
+ $cid = $initial_cid = $this->createCacheIdFast($keys, $cacheability);
+
+ // Immediately set cache misses on the results or fast-forward the CID map
+ // to look from the last known redirect onwards.
+ if (isset($this->redirectChainCache[$cid]) && $this->redirectChainIsValid($keys, $this->redirectChainCache[$cid])) {
+ $last_item = end($this->redirectChainCache[$cid]);
+
+ // Immediately set cache misses on the results.
+ if ($last_item === FALSE) {
+ $results[$index] = $last_item;
+ continue;
+ }
+ // Prime the CID map with the last known redirect for the initial CID.
+ if ($last_item->data instanceof CacheRedirect) {
+ $cid = $this->createCacheIdFast($keys, $last_item->data);
+ }
+ }
$cid_map[$cid] = [
'index' => $index,
'keys' => $keys,
+ 'initial' => $initial_cid,
];
}
// Go over all CIDs and update the map according to found redirects. If the
// map is empty, it means we've followed all CIDs to their final result or
// lack thereof.
- $results = [];
while (!empty($cid_map)) {
$new_cid_map = [];
@@ -79,12 +113,20 @@ public function getMultiple(array $items): array {
if ($result->data instanceof CacheRedirect) {
$redirect_cid = $this->createCacheIdFast($info['keys'], $result->data);
$new_cid_map[$redirect_cid] = $info;
+ $this->redirectChainCache[$info['initial']][$cid] = $result;
continue;
}
$results[$info['index']] = $result;
}
+ // Any CID that did not get a cache hit is still in $fetch_cids. Add them
+ // to the internal redirect chain cache as a miss.
+ foreach ($fetch_cids as $fetch_cid) {
+ $info = $cid_map[$fetch_cid];
+ $this->redirectChainCache[$info['initial']][$fetch_cid] = FALSE;
+ }
+
$cid_map = $new_cid_map;
}
--
GitLab
From 0621807df3f7b68761026d61cd67e58957704f4e Mon Sep 17 00:00:00 2001
From: Kristiaan Van den Eynde <kristiaan@factorial.io>
Date: Wed, 19 Mar 2025 14:45:12 +0100
Subject: [PATCH 16/25] Mark the reset method as internal
---
core/lib/Drupal/Core/Cache/VariationCache.php | 2 ++
1 file changed, 2 insertions(+)
diff --git a/core/lib/Drupal/Core/Cache/VariationCache.php b/core/lib/Drupal/Core/Cache/VariationCache.php
index fcda1c05067e..0e40fbe0fe34 100644
--- a/core/lib/Drupal/Core/Cache/VariationCache.php
+++ b/core/lib/Drupal/Core/Cache/VariationCache.php
@@ -455,6 +455,8 @@ protected function createCacheIdFast(array $keys, CacheableDependencyInterface $
* Reset statically cached variables.
*
* This is only used by tests.
+ *
+ * @internal
*/
public function reset(): void {
$this->redirectChainCache = [];
--
GitLab
From b463c23952c4a107ce53c630c265a73134f86bd1 Mon Sep 17 00:00:00 2001
From: Kristiaan Van den Eynde <kristiaan@factorial.io>
Date: Wed, 19 Mar 2025 14:53:55 +0100
Subject: [PATCH 17/25] construe -> construct
---
core/lib/Drupal/Core/Cache/VariationCache.php | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/core/lib/Drupal/Core/Cache/VariationCache.php b/core/lib/Drupal/Core/Cache/VariationCache.php
index 0e40fbe0fe34..5a95be6b0dd0 100644
--- a/core/lib/Drupal/Core/Cache/VariationCache.php
+++ b/core/lib/Drupal/Core/Cache/VariationCache.php
@@ -20,7 +20,7 @@ class VariationCache implements VariationCacheInterface {
* recorded.
*
* These arrays are indexed by the cache IDs being followed during the chain
- * and the CacheRedirect objects that construe the chain. At the end there
+ * and the CacheRedirect objects that construct the chain. At the end there
* should always be a value of FALSE for a cache miss, or a CacheRedirect for
* a cache hit because we cannot store the cache hit itself into a property
* that does not support invalidation based on cache metadata. By storing the
--
GitLab
From c15d3c3ca97fed8d78edaea2bd2b6ada149bf6a4 Mon Sep 17 00:00:00 2001
From: Kristiaan Van den Eynde <kristiaan@factorial.io>
Date: Wed, 19 Mar 2025 14:58:17 +0100
Subject: [PATCH 18/25] Simplify comment
---
core/lib/Drupal/Core/Cache/VariationCache.php | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/core/lib/Drupal/Core/Cache/VariationCache.php b/core/lib/Drupal/Core/Cache/VariationCache.php
index 5a95be6b0dd0..b3790d0fff85 100644
--- a/core/lib/Drupal/Core/Cache/VariationCache.php
+++ b/core/lib/Drupal/Core/Cache/VariationCache.php
@@ -75,8 +75,7 @@ public function getMultiple(array $items): array {
foreach ($items as $index => [$keys, $cacheability]) {
$cid = $initial_cid = $this->createCacheIdFast($keys, $cacheability);
- // Immediately set cache misses on the results or fast-forward the CID map
- // to look from the last known redirect onwards.
+ // Try to optimize based on the redirect chain cache.
if (isset($this->redirectChainCache[$cid]) && $this->redirectChainIsValid($keys, $this->redirectChainCache[$cid])) {
$last_item = end($this->redirectChainCache[$cid]);
--
GitLab
From 7902e2d4b41e9a059f7c5b9c5eb1c3587cfa17b4 Mon Sep 17 00:00:00 2001
From: Kristiaan Van den Eynde <kristiaan@factorial.io>
Date: Wed, 19 Mar 2025 15:17:38 +0100
Subject: [PATCH 19/25] Oh wait, on getMultiple() we're not supposed to return
FALSE for misses.
---
core/lib/Drupal/Core/Cache/VariationCache.php | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/core/lib/Drupal/Core/Cache/VariationCache.php b/core/lib/Drupal/Core/Cache/VariationCache.php
index b3790d0fff85..aad67acb6bf5 100644
--- a/core/lib/Drupal/Core/Cache/VariationCache.php
+++ b/core/lib/Drupal/Core/Cache/VariationCache.php
@@ -71,7 +71,7 @@ public function getMultiple(array $items): array {
// property documentation of $this->redirectChainCache.
//
// Create a map of CIDs with their associated $items index and cache keys.
- $cid_map = $results = [];
+ $cid_map = [];
foreach ($items as $index => [$keys, $cacheability]) {
$cid = $initial_cid = $this->createCacheIdFast($keys, $cacheability);
@@ -79,9 +79,8 @@ public function getMultiple(array $items): array {
if (isset($this->redirectChainCache[$cid]) && $this->redirectChainIsValid($keys, $this->redirectChainCache[$cid])) {
$last_item = end($this->redirectChainCache[$cid]);
- // Immediately set cache misses on the results.
+ // Immediately skip processing the CID for cache misses.
if ($last_item === FALSE) {
- $results[$index] = $last_item;
continue;
}
// Prime the CID map with the last known redirect for the initial CID.
@@ -100,6 +99,7 @@ public function getMultiple(array $items): array {
// Go over all CIDs and update the map according to found redirects. If the
// map is empty, it means we've followed all CIDs to their final result or
// lack thereof.
+ $results = [];
while (!empty($cid_map)) {
$new_cid_map = [];
--
GitLab
From 7dcf497ed16bf9c3ba6d344d4e35108b2b474ec5 Mon Sep 17 00:00:00 2001
From: Kristiaan Van den Eynde <kristiaan@factorial.io>
Date: Wed, 19 Mar 2025 15:51:06 +0100
Subject: [PATCH 20/25] Cache gets -2 across the board after getMultiple
support
---
.../OpenTelemetryNodePagePerformanceTest.php | 4 ++--
.../src/FunctionalJavascript/StandardPerformanceTest.php | 8 ++++----
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/core/profiles/demo_umami/tests/src/FunctionalJavascript/OpenTelemetryNodePagePerformanceTest.php b/core/profiles/demo_umami/tests/src/FunctionalJavascript/OpenTelemetryNodePagePerformanceTest.php
index f237e79830a9..03a4b2cd9328 100644
--- a/core/profiles/demo_umami/tests/src/FunctionalJavascript/OpenTelemetryNodePagePerformanceTest.php
+++ b/core/profiles/demo_umami/tests/src/FunctionalJavascript/OpenTelemetryNodePagePerformanceTest.php
@@ -288,7 +288,7 @@ protected function testNodePageWarmCache(): void {
$expected = [
'QueryCount' => 172,
- 'CacheGetCount' => 231,
+ 'CacheGetCount' => 229,
'CacheGetCountByBin' => [
'page' => 1,
'config' => 66,
@@ -297,7 +297,7 @@ protected function testNodePageWarmCache(): void {
'entity' => 18,
'bootstrap' => 8,
'dynamic_page_cache' => 1,
- 'render' => 52,
+ 'render' => 50,
'default' => 3,
'menu' => 2,
],
diff --git a/core/profiles/standard/tests/src/FunctionalJavascript/StandardPerformanceTest.php b/core/profiles/standard/tests/src/FunctionalJavascript/StandardPerformanceTest.php
index 68d6523426e6..2cb6c35e80f3 100644
--- a/core/profiles/standard/tests/src/FunctionalJavascript/StandardPerformanceTest.php
+++ b/core/profiles/standard/tests/src/FunctionalJavascript/StandardPerformanceTest.php
@@ -127,7 +127,7 @@ protected function testAnonymous(): void {
$this->assertSame($expected_queries, $recorded_queries);
$expected = [
'QueryCount' => 36,
- 'CacheGetCount' => 111,
+ 'CacheGetCount' => 109,
'CacheGetCountByBin' => [
'page' => 1,
'config' => 21,
@@ -135,7 +135,7 @@ protected function testAnonymous(): void {
'discovery' => 38,
'bootstrap' => 8,
'dynamic_page_cache' => 1,
- 'render' => 25,
+ 'render' => 23,
'default' => 5,
'entity' => 2,
'menu' => 2,
@@ -225,7 +225,7 @@ protected function testAnonymous(): void {
$this->assertSame($expected_queries, $recorded_queries);
$expected = [
'QueryCount' => 10,
- 'CacheGetCount' => 83,
+ 'CacheGetCount' => 81,
'CacheSetCount' => 16,
'CacheDeleteCount' => 0,
'CacheTagInvalidationCount' => 0,
@@ -306,7 +306,7 @@ protected function testAnonymous(): void {
$this->assertSame($expected_queries, $recorded_queries);
$expected = [
'QueryCount' => 14,
- 'CacheGetCount' => 68,
+ 'CacheGetCount' => 66,
'CacheSetCount' => 17,
'CacheDeleteCount' => 0,
'CacheTagInvalidationCount' => 0,
--
GitLab
From 02c843b5693873f913c16b5eb926ae89e519f8c2 Mon Sep 17 00:00:00 2001
From: Kristiaan Van den Eynde <kristiaan@factorial.io>
Date: Mon, 31 Mar 2025 18:53:50 +0200
Subject: [PATCH 21/25] Didn't know we had JsonApiPerformanceTest now...
---
.../FunctionalJavascript/JsonApiPerformanceTest.php | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/core/modules/jsonapi/tests/src/FunctionalJavascript/JsonApiPerformanceTest.php b/core/modules/jsonapi/tests/src/FunctionalJavascript/JsonApiPerformanceTest.php
index 055b009a0c5e..b1b0934b5da3 100644
--- a/core/modules/jsonapi/tests/src/FunctionalJavascript/JsonApiPerformanceTest.php
+++ b/core/modules/jsonapi/tests/src/FunctionalJavascript/JsonApiPerformanceTest.php
@@ -87,7 +87,7 @@ public function testGetIndividual(): void {
$expected = [
'QueryCount' => 26,
- 'CacheGetCount' => 44,
+ 'CacheGetCount' => 42,
'CacheGetCountByBin' => [
'config' => 8,
'data' => 8,
@@ -95,8 +95,8 @@ public function testGetIndividual(): void {
'discovery' => 13,
'entity' => 2,
'default' => 4,
- 'dynamic_page_cache' => 2,
- 'jsonapi_normalizations' => 2,
+ 'dynamic_page_cache' => 1,
+ 'jsonapi_normalizations' => 1,
],
'CacheSetCount' => 16,
'CacheSetCountByBin' => [
@@ -206,7 +206,7 @@ public function testGetIndividual(): void {
$expected = [
'QueryCount' => 15,
- 'CacheGetCount' => 47,
+ 'CacheGetCount' => 43,
'CacheGetCountByBin' => [
'config' => 8,
'data' => 8,
@@ -214,8 +214,8 @@ public function testGetIndividual(): void {
'entity' => 2,
'default' => 4,
'bootstrap' => 4,
- 'dynamic_page_cache' => 4,
- 'jsonapi_normalizations' => 4,
+ 'dynamic_page_cache' => 2,
+ 'jsonapi_normalizations' => 2,
],
'CacheSetCount' => 3,
'CacheDeleteCount' => 0,
--
GitLab
From b0cc8ab3324ead7d9f821a072324f53b4005a9e9 Mon Sep 17 00:00:00 2001
From: Kristiaan Van den Eynde <kristiaan@factorial.io>
Date: Tue, 1 Apr 2025 11:01:12 +0200
Subject: [PATCH 22/25] Trying something wild, swapping out validation logic
with something new
---
core/lib/Drupal/Core/Cache/VariationCache.php | 80 ++++++++++++-------
1 file changed, 49 insertions(+), 31 deletions(-)
diff --git a/core/lib/Drupal/Core/Cache/VariationCache.php b/core/lib/Drupal/Core/Cache/VariationCache.php
index aad67acb6bf5..3068dc56143c 100644
--- a/core/lib/Drupal/Core/Cache/VariationCache.php
+++ b/core/lib/Drupal/Core/Cache/VariationCache.php
@@ -73,20 +73,24 @@ public function getMultiple(array $items): array {
// Create a map of CIDs with their associated $items index and cache keys.
$cid_map = [];
foreach ($items as $index => [$keys, $cacheability]) {
- $cid = $initial_cid = $this->createCacheIdFast($keys, $cacheability);
-
- // Try to optimize based on the redirect chain cache.
- if (isset($this->redirectChainCache[$cid]) && $this->redirectChainIsValid($keys, $this->redirectChainCache[$cid])) {
- $last_item = end($this->redirectChainCache[$cid]);
+ // Try to optimize based on the cached redirect chain.
+ if ($chain = $this->getValidatedCachedRedirectChain($keys, $cacheability)) {
+ $last_item = end($chain);
// Immediately skip processing the CID for cache misses.
if ($last_item === FALSE) {
continue;
}
+
+ // We do not need to calculate the initial CID as its part of the chain.
+ $initial_cid = array_key_first($chain);
+
// Prime the CID map with the last known redirect for the initial CID.
- if ($last_item->data instanceof CacheRedirect) {
- $cid = $this->createCacheIdFast($keys, $last_item->data);
- }
+ assert($last_item->data instanceof CacheRedirect);
+ $cid = $this->createCacheIdFast($keys, $last_item->data);
+ }
+ else {
+ $cid = $initial_cid = $this->createCacheIdFast($keys, $cacheability);
}
$cid_map[$cid] = [
@@ -325,24 +329,18 @@ public function invalidate(array $keys, CacheableDependencyInterface $initial_ca
* to query the cache for that result.
*/
protected function getRedirectChain(array $keys, CacheableDependencyInterface $initial_cacheability): array {
- $cid = $initial_cid = $this->createCacheIdFast($keys, $initial_cacheability);
-
- // See if we previously stored the redirect chain in memory. We do need to
- // run a validity check because cache context values might have changed
- // since the last time we got the chain. In theory that should never happen
- // during a single request, but better safe than sorry.
- if (isset($this->redirectChainCache[$cid]) && $this->redirectChainIsValid($keys, $this->redirectChainCache[$cid])) {
- $chain = $this->redirectChainCache[$cid];
- }
+ $chain = $this->getValidatedCachedRedirectChain($keys, $initial_cacheability);
// Initiate the chain if we couldn't retrieve (a partial) one from memory.
- // If we did find one, we continue our search from the last redirect in the
- // chain in case we had a cache hit before, or we take the FALSE at the end
- // of the chain from a previous cache miss, bypassing the while loop below.
if (empty($chain)) {
+ $cid = $initial_cid = $this->createCacheIdFast($keys, $initial_cacheability);
$chain[$cid] = $result = $this->cacheBackend->get($cid);
}
+ // If we did find one, we continue our search from the last valid redirect
+ // in the chain or bypass the while loop below in case the chain ends in
+ // FALSE, indicating a previous cache miss.
else {
+ $initial_cid = array_key_first($chain);
$result = end($chain);
}
@@ -364,26 +362,46 @@ protected function getRedirectChain(array $keys, CacheableDependencyInterface $i
}
/**
- * Validates a redirect chain for the current cache context values.
+ * Retrieved the redirect chain from cache, validating each part.
*
* @param string[] $keys
- * The cache keys used to build the chain.
- * @param array $chain
- * The redirect chain to validate.
+ * The cache keys to retrieve the redirect chain for.
+ * @param \Drupal\Core\Cache\CacheableDependencyInterface $initial_cacheability
+ * The initial cacheability for the redirect chain.
*
- * @return bool
- * Whether the redirect chain is valid.
+ * @return array
+ * The part of the cached redirect chain, if any, that is still valid.
*/
- protected function redirectChainIsValid(array $keys, array $chain): bool {
- foreach ($chain as $result) {
+ protected function getValidatedCachedRedirectChain(array $keys, CacheableDependencyInterface $initial_cacheability): array {
+ $cid = $this->createCacheIdFast($keys, $initial_cacheability);
+ if (!isset($this->redirectChainCache[$cid])) {
+ return [];
+ }
+
+ // Only use that part of the redirect chain that is still valid. Even though
+ // we do not store cache hits in the internal redirect chain cache, we can
+ // still reuse the whole chain up until what would have been a cache hit.
+ //
+ // If part of a redirect chain no longer matches because cache contexts
+ // changed values, we could perhaps still reuse part of the chain until we
+ // encounter a redirect for the changed cache context value.
+ //
+ // There is one special case: If the very last item of the chain is a cache
+ // redirect, and we cannot find anything for it, we still add the redirect
+ // to the validated chain because the only way a cached chain ends in a
+ // redirect is if it led to a cache hit in ::getRedirectChain().
+ $valid_parts = [];
+ $last_key = array_key_last($this->redirectChainCache[$cid]);
+ foreach ($this->redirectChainCache[$cid] as $key => $result) {
if ($result && $result->data instanceof CacheRedirect) {
$cid = $this->createCacheIdFast($keys, $result->data);
- if (!isset($chain[$cid])) {
- return FALSE;
+ if (!isset($chain[$cid]) && $last_key !== $key) {
+ break;
}
}
+ $valid_parts[$key] = $result;
}
- return TRUE;
+ return $valid_parts;
}
/**
--
GitLab
From c09ac607a863e06dda36280e856ebd6b9dea70cf Mon Sep 17 00:00:00 2001
From: Kristiaan Van den Eynde <kristiaan@factorial.io>
Date: Tue, 1 Apr 2025 11:14:11 +0200
Subject: [PATCH 23/25] phpstan
---
core/lib/Drupal/Core/Cache/VariationCache.php | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/core/lib/Drupal/Core/Cache/VariationCache.php b/core/lib/Drupal/Core/Cache/VariationCache.php
index 3068dc56143c..fe0408b0f392 100644
--- a/core/lib/Drupal/Core/Cache/VariationCache.php
+++ b/core/lib/Drupal/Core/Cache/VariationCache.php
@@ -377,6 +377,7 @@ protected function getValidatedCachedRedirectChain(array $keys, CacheableDepende
if (!isset($this->redirectChainCache[$cid])) {
return [];
}
+ $chain = $this->redirectChainCache[$cid];
// Only use that part of the redirect chain that is still valid. Even though
// we do not store cache hits in the internal redirect chain cache, we can
@@ -391,8 +392,8 @@ protected function getValidatedCachedRedirectChain(array $keys, CacheableDepende
// to the validated chain because the only way a cached chain ends in a
// redirect is if it led to a cache hit in ::getRedirectChain().
$valid_parts = [];
- $last_key = array_key_last($this->redirectChainCache[$cid]);
- foreach ($this->redirectChainCache[$cid] as $key => $result) {
+ $last_key = array_key_last($chain);
+ foreach ($chain as $key => $result) {
if ($result && $result->data instanceof CacheRedirect) {
$cid = $this->createCacheIdFast($keys, $result->data);
if (!isset($chain[$cid]) && $last_key !== $key) {
--
GitLab
From dc8151a56863ed975ebca45879fafc1020138c37 Mon Sep 17 00:00:00 2001
From: Kristiaan Van den Eynde <kristiaan@factorial.io>
Date: Tue, 1 Apr 2025 11:35:24 +0200
Subject: [PATCH 24/25] Even fewer cache gets now
---
.../OpenTelemetryNodePagePerformanceTest.php | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/core/profiles/demo_umami/tests/src/FunctionalJavascript/OpenTelemetryNodePagePerformanceTest.php b/core/profiles/demo_umami/tests/src/FunctionalJavascript/OpenTelemetryNodePagePerformanceTest.php
index 314838064591..ee3a60bd012c 100644
--- a/core/profiles/demo_umami/tests/src/FunctionalJavascript/OpenTelemetryNodePagePerformanceTest.php
+++ b/core/profiles/demo_umami/tests/src/FunctionalJavascript/OpenTelemetryNodePagePerformanceTest.php
@@ -287,7 +287,7 @@ protected function testNodePageWarmCache(): void {
$expected = [
'QueryCount' => 171,
- 'CacheGetCount' => 205,
+ 'CacheGetCount' => 202,
'CacheGetCountByBin' => [
'page' => 1,
'config' => 66,
@@ -296,7 +296,7 @@ protected function testNodePageWarmCache(): void {
'data' => 13,
'entity' => 18,
'dynamic_page_cache' => 1,
- 'render' => 24,
+ 'render' => 21,
'default' => 3,
'menu' => 2,
],
--
GitLab
From 295caf19543276cd7c74e6f09555ae5a8ffb22df Mon Sep 17 00:00:00 2001
From: catch <6915-catch@users.noreply.drupalcode.org>
Date: Tue, 1 Apr 2025 11:51:27 +0000
Subject: [PATCH 25/25] Apply 1 suggestion(s) to 1 file(s)
---
core/lib/Drupal/Core/Cache/VariationCache.php | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/core/lib/Drupal/Core/Cache/VariationCache.php b/core/lib/Drupal/Core/Cache/VariationCache.php
index fe0408b0f392..1d26eea8c7ee 100644
--- a/core/lib/Drupal/Core/Cache/VariationCache.php
+++ b/core/lib/Drupal/Core/Cache/VariationCache.php
@@ -67,7 +67,7 @@ public function getMultiple(array $items): array {
// However, ::getRedirectChain() has an internal cache that we could both
// benefit from and contribute to whenever we call this function. So any use
// or manipulation of $this->redirectChainCache below is for optimization
- // purposes. You can read up on how the internal cache is structured on the
+ // purposes. A description of the internal cache structure is on the
// property documentation of $this->redirectChainCache.
//
// Create a map of CIDs with their associated $items index and cache keys.
--
GitLab