From 79712f66965e6ac7c2f1a024e8f40b2fe36564fb Mon Sep 17 00:00:00 2001 From: Benjamin Walker Date: Fri, 2 May 2025 18:37:45 +1000 Subject: [PATCH] MDL-82511 cache: Improve performance of cleanup in cache_cron_task --- public/cache/classes/helper.php | 52 ++++++++++++++-- public/cache/tests/cache_helper_test.php | 77 ++++++++++++++++++++++++ 2 files changed, 124 insertions(+), 5 deletions(-) diff --git a/public/cache/classes/helper.php b/public/cache/classes/helper.php index 5a07028765d..9a66d347025 100644 --- a/public/cache/classes/helper.php +++ b/public/cache/classes/helper.php @@ -756,10 +756,16 @@ class helper { debugging('Cache stores used for session definitions should ideally be searchable.', DEBUG_DEVELOPER); continue; } + + // Load all of the keys into memory so we can compare against multiple prefixes. + $keys = $store->find_all(); + sort($keys); + // Get all of the last access keys. - $keys = $store->find_by_prefix(session_cache::LASTACCESS); + $lastaccess = array_filter($keys, fn($key) => str_starts_with($key, session_cache::LASTACCESS)); $todelete = []; - foreach ($store->get_many($keys) as $key => $value) { + $prefixtodelete = []; + foreach ($store->get_many($lastaccess) as $key => $value) { $expiresvalue = 0; if ($value instanceof ttl_wrapper) { $expiresvalue = $value->data; @@ -771,11 +777,16 @@ class helper { $expires = (int) $expiresvalue; if ($expires > 0 && $expires < $purgetime) { - $prefix = substr($key, strlen(session_cache::LASTACCESS)); - $foundbyprefix = $store->find_by_prefix($prefix); - $todelete = array_merge($todelete, [$key], $foundbyprefix); + $todelete[] = $key; + $prefixtodelete[] = substr($key, strlen(session_cache::LASTACCESS)); } } + + // Match all of the prefixes to delete to keys to delete. + if ($prefixtodelete) { + sort($prefixtodelete); + $todelete = array_merge($todelete, self::filter_sorted_keys_by_prefixes($keys, $prefixtodelete)); + } if ($todelete) { $outcome = (int) $store->delete_many($todelete); if ($output) { @@ -788,6 +799,37 @@ class helper { } } + /** + * Filters a sorted list of keys by a sorted list of prefixes. + * This relies on the sorting to reduce the number of comparisons. + * + * @param array $keys a sorted array of keys + * @param array $prefixes a sorted array of prefixes + * @return array of keys containing any of the prefixes + */ + public static function filter_sorted_keys_by_prefixes(array $keys, array $prefixes): array { + // Reverse the prefixes to allow for simple processing. + $prefixes = array_reverse($prefixes); + + $matches = []; + $prefix = array_pop($prefixes); + foreach ($keys as $key) { + // The keys and prefixes are sorted so we only need to compare against one prefix at a time. + // This is done inside a loop to check the next prefix at break points. + while ($prefix) { + if (str_starts_with($key, $prefix)) { + $matches[] = $key; + } else if ($prefix < $key) { + // The key has moved past the current prefix alphabetically, so check the next. + $prefix = array_pop($prefixes); + continue; + } + break; + } + } + return $matches; + } + /** * Returns an array of stores that would meet the requirements for every definition. * diff --git a/public/cache/tests/cache_helper_test.php b/public/cache/tests/cache_helper_test.php index 135e3bf9494..5d36e3416d7 100644 --- a/public/cache/tests/cache_helper_test.php +++ b/public/cache/tests/cache_helper_test.php @@ -60,4 +60,81 @@ final class cache_helper_test extends \advanced_testcase { [new \stdClass(), true], ]; } + + /** + * Test the filter_sorted_keys_by_prefixes method. + * + * @param array $keys + * @param array $prefixes + * @param array $expected + * @dataProvider filter_sorted_keys_by_prefixes_provider + */ + public function test_filter_sorted_keys_by_prefixes(array $keys, array $prefixes, array $expected): void { + $this->assertEquals($expected, helper::filter_sorted_keys_by_prefixes($keys, $prefixes)); + } + + /** + * Data provider for filter_sorted_keys_by_prefixes tests. + * + * @return array + */ + public static function filter_sorted_keys_by_prefixes_provider(): array { + return [ + 'simple match' => [ + 'keys' => ['aa', 'ab', 'ba', 'bb'], + 'prefixes' => ['a'], + 'expected' => ['aa', 'ab'], + ], + 'multiple prefixes match' => [ + 'keys' => ['aa', 'ab', 'ba', 'bb', 'ca', 'cb'], + 'prefixes' => ['a', 'c'], + 'expected' => ['aa', 'ab', 'ca', 'cb'], + ], + 'consecutive prefixes match' => [ + 'keys' => ['aa', 'ab', 'ba', 'bb', 'ca', 'cb'], + 'prefixes' => ['a', 'b'], + 'expected' => ['aa', 'ab', 'ba', 'bb'], + ], + 'overlapping prefixes' => [ + 'keys' => ['a', 'ab', 'abc', 'abcd'], + 'prefixes' => ['ab', 'abc'], + 'expected' => ['ab', 'abc', 'abcd'], + ], + 'exact match' => [ + 'keys' => ['a', 'b', 'c'], + 'prefixes' => ['a', 'c'], + 'expected' => ['a', 'c'], + ], + 'duplicate keys' => [ + 'keys' => ['a', 'a', 'b', 'c'], + 'prefixes' => ['a'], + 'expected' => ['a', 'a'], + ], + 'duplicate prefixes' => [ + 'keys' => ['a', 'b', 'c'], + 'prefixes' => ['a', 'a', 'b'], + 'expected' => ['a', 'b'], + ], + 'unsorted keys boundry' => [ + 'keys' => ['c', 'b', 'a'], + 'prefixes' => ['a'], + 'expected' => [], + ], + 'unsorted prefixes boundry' => [ + 'keys' => ['a', 'b', 'c'], + 'prefixes' => ['d', 'a'], + 'expected' => [], + ], + 'empty keys' => [ + 'keys' => [], + 'prefixes' => ['a'], + 'expected' => [], + ], + 'empty prefixes' => [ + 'keys' => ['a', 'b', 'c'], + 'prefixes' => [], + 'expected' => [], + ], + ]; + } }