From 13c2d7a91b1b12bc8a7e283c3cbfb22b1d5cce74 Mon Sep 17 00:00:00 2001 From: Stefan Weil Date: Fri, 16 Oct 2026 11:26:00 +0200 Subject: [PATCH] [TASK] Add worker-local document pool to skip XML re-parsing Reading a document from the file-based tx_dlf_doc cache requires deserializing the entry and re-parsing the entire METS XML in MetsDocument::__wakeup(), and every cache hit then re-runs the lazy data loaders (physical structure, structure links, metadata array, file groups). For documents that are requested repeatedly by the same PHP-FPM worker - typical for crawlers, and for pages that fetch the same document several times, e.g. periodika issue lists - this is the dominant CPU cost. Keep a short-lived, per-worker in-memory pool of recently used documents in DocumentCacheManager. A pool hit skips the file cache, the deserialization, the XML re-parsing and the lazy reloading entirely and reuses the already computed data of the document. The pool is a static array (one per FPM worker process) with a sliding TTL of 300 s and an entry cap of 5 documents, evicting the least recently used entry first. Because a PHP-FPM worker handles only one request at a time, sharing instances between sequential requests of the same worker is safe. set(), remove() and flush() keep the pool in sync with the file cache, including the LOAD_FAILED sentinel. Unit tests cover pool hits, TTL expiration, LRU eviction, pool filling from the file cache and pool synchronization by setFail(), remove() and flush(). Since the pool is per-worker state, it would survive individual functional tests and leak a document loaded by one test (possibly with different settings) into the next one. A resetPool() method clears the pool without touching the file cache; the functional test base class calls it for each test case, mirroring the existing Helper::resetIndexNameCache() and Indexer::resetProcessedDocs() test isolation pattern. Assisted-by: OpenCode / qwen3.8-27b-thinking (Alibaba Cloud) Signed-off-by: Stefan Weil --- Classes/Common/DocumentCacheManager.php | 108 +++++++- Tests/Functional/FunctionalTestCase.php | 6 + .../Unit/Common/DocumentCacheManagerTest.php | 243 ++++++++++++++++++ 3 files changed, 352 insertions(+), 5 deletions(-) create mode 100644 Tests/Unit/Common/DocumentCacheManagerTest.php diff --git a/Classes/Common/DocumentCacheManager.php b/Classes/Common/DocumentCacheManager.php index e31a1a132e..c0e3da18be 100644 --- a/Classes/Common/DocumentCacheManager.php +++ b/Classes/Common/DocumentCacheManager.php @@ -35,6 +35,39 @@ class DocumentCacheManager implements SingletonInterface */ public const LOAD_FAILED = '__DLF_DOC_LOAD_FAILED__'; + /** + * Lifetime of entries in the in-memory pool in seconds. An entry is + * refreshed (sliding TTL) as long as it is requested regularly. + */ + private const POOL_TTL = 300; + + /** + * Maximum number of documents kept in the in-memory pool per + * PHP-FPM worker. The oldest (least recently used) entry is + * evicted first. + */ + private const POOL_MAX_ENTRIES = 5; + + /** + * In-memory, per-PHP-FPM-worker pool of recently used documents. + * + * Reading a document from the file-based TYPO3 cache requires + * deserializing the entry and re-parsing the complete METS XML in + * AbstractDocument::__wakeup(). Documents that are requested + * repeatedly (e.g. by crawlers) are therefore additionally kept + * in this short-lived pool: a pool hit skips the file cache, the + * deserialization and the XML re-parsing entirely, and the already + * computed lazy data (physical structure, metadata array, file + * groups) is reused as well. + * + * Note: PHP static state lives per FPM worker process and is only + * accessed by one request at a time, so sharing the instances + * between sequential requests of the same worker is safe. + * + * @var array + */ + protected static array $pool = []; + /** * @var FrontendInterface */ @@ -51,6 +84,11 @@ public function __construct() /** * Get document instance from cache. * + * The per-worker in-memory pool is checked first; on a hit the + * fully loaded document is returned without touching the file + * cache. On a miss the file cache is read and the result is added + * to the pool (subject to its TTL and entry cap). + * * @access public * * @param string $location @@ -61,13 +99,30 @@ public function __construct() */ public function get(string $location) { - return $this->cache->get($this->getIdentifier($location)); + $identifier = $this->getIdentifier($location); + $now = time(); + if (isset(self::$pool[$identifier])) { + [$document, $cachedAt] = self::$pool[$identifier]; + // Re-insert the entry to keep least-recently-used ordering. + unset(self::$pool[$identifier]); + if ($now - $cachedAt < self::POOL_TTL) { + self::$pool[$identifier] = [$document, $now]; + return $document; + } + } + $instance = $this->cache->get($identifier); + if ($instance instanceof AbstractDocument) { + self::$pool[$identifier] = [$instance, $now]; + self::evictOldestPoolEntries(); + } + return $instance; } /** * Set cache for document instance. * - * The entry uses the cache's default lifetime (one day). + * The entry uses the cache's default lifetime (one day) in the + * file cache and POOL_TTL in the per-worker in-memory pool. * * @access public * @@ -78,7 +133,11 @@ public function get(string $location) */ public function set(string $location, AbstractDocument $currentDocument): void { - $this->cache->set($this->getIdentifier($location), $currentDocument); + $identifier = $this->getIdentifier($location); + $this->cache->set($identifier, $currentDocument); + unset(self::$pool[$identifier]); + self::$pool[$identifier] = [$currentDocument, time()]; + self::evictOldestPoolEntries(); } /** @@ -97,6 +156,7 @@ public function set(string $location, AbstractDocument $currentDocument): void */ public function setFail(string $location): void { + unset(self::$pool[$this->getIdentifier($location)]); $this->cache->set($this->getIdentifier($location), self::LOAD_FAILED, [], $this->getFailLifetime()); } @@ -115,7 +175,7 @@ private function getFailLifetime(): int } /** - * Remove all documents from cache. + * Remove all documents from cache and from the in-memory pool. * * @access public * @@ -123,11 +183,32 @@ private function getFailLifetime(): int */ public function flush(): void { + self::$pool = []; $this->cache->flush(); } /** - * Remove single document from cache. + * Reset the in-memory pool without touching the file cache. + * + * Unlike flush(), which is part of the public cache API, this is + * intended for testing only: the pool is per-worker state that + * survives individual functional tests, and a pooled document + * loaded by a previous test (possibly with different settings) + * would leak into the next test. + * + * @access public + * + * @static + * + * @return void + */ + public static function resetPool(): void + { + self::$pool = []; + } + + /** + * Remove single document from cache and from the in-memory pool. * * @access public * @@ -137,6 +218,7 @@ public function flush(): void */ public function remove(string $location): void { + unset(self::$pool[$this->getIdentifier($location)]); $this->cache->remove($this->getIdentifier($location)); } @@ -153,4 +235,20 @@ private function getIdentifier(string $location): string { return hash('md5', $location); } + + /** + * Evict least-recently-used entries from the in-memory pool until + * it contains at most POOL_MAX_ENTRIES documents. + * + * @access private + * + * @return void + */ + private static function evictOldestPoolEntries(): void + { + $excess = count(self::$pool) - self::POOL_MAX_ENTRIES; + for ($i = 0; $i < $excess; ++$i) { + unset(self::$pool[array_key_first(self::$pool)]); + } + } } diff --git a/Tests/Functional/FunctionalTestCase.php b/Tests/Functional/FunctionalTestCase.php index 53d369b3d1..4923c4ed82 100644 --- a/Tests/Functional/FunctionalTestCase.php +++ b/Tests/Functional/FunctionalTestCase.php @@ -14,6 +14,7 @@ use Dotenv\Dotenv; use GuzzleHttp\Client as HttpClient; +use Kitodo\Dlf\Common\DocumentCacheManager; use Kitodo\Dlf\Common\Helper; use Kitodo\Dlf\Common\Indexer; use Kitodo\Dlf\Common\Solr\Solr; @@ -117,6 +118,11 @@ public function setUp(): void parent::setUp(); + // The in-memory document pool is per-worker state that survives + // individual tests; reset it so that no document loaded by a + // previous test (possibly with different settings) leaks in. + DocumentCacheManager::resetPool(); + $this->persistenceManager = GeneralUtility::makeInstance(PersistenceManager::class); $this->baseUrl = 'http://web:8000/public/typo3temp/var/tests/functional-' . $this->identifier . '/'; diff --git a/Tests/Unit/Common/DocumentCacheManagerTest.php b/Tests/Unit/Common/DocumentCacheManagerTest.php new file mode 100644 index 0000000000..538a9c591f --- /dev/null +++ b/Tests/Unit/Common/DocumentCacheManagerTest.php @@ -0,0 +1,243 @@ + + * + * This file is part of the Kitodo and TYPO3 projects. + * + * @license GNU General Public License version 3 or later. + * For the full copyright and license information, please read the + * LICENSE.txt file that was distributed with this source code. + */ + +namespace Kitodo\Dlf\Tests\Unit\Common; + +use Kitodo\Dlf\Common\AbstractDocument; +use Kitodo\Dlf\Common\DocumentCacheManager; +use PHPUnit\Framework\Attributes\Test; +use PHPUnit\Framework\MockObject\MockObject; +use ReflectionClass; +use ReflectionProperty; +use TYPO3\CMS\Core\Cache\Frontend\FrontendInterface; +use TYPO3\CMS\Core\Cache\CacheManager; +use TYPO3\CMS\Core\Utility\GeneralUtility; +use TYPO3\TestingFramework\Core\Unit\UnitTestCase; + +class DocumentCacheManagerTest extends UnitTestCase +{ + /** + * @var string + */ + protected string $location = 'https://example.org/sample1.xml'; + + protected function tearDown(): void + { + $this->flushPool(); + GeneralUtility::purgeInstances(); + parent::tearDown(); + } + + /** + * Get the DocumentCacheManager with a mocked cache frontend. + * + * The manager's constructor builds its frontend through the + * (statically cached) CacheManager singleton, so the cache manager + * is replaced by a mock before the manager is instantiated. + * + * @access private + * + * @return DocumentCacheManager + */ + private function getCacheManager(): DocumentCacheManager + { + $cacheManagerMock = $this->createMock(CacheManager::class); + $cacheManagerMock->method('getCache')->willReturn( + $this->createMock(FrontendInterface::class) + ); + GeneralUtility::setSingletonInstance(CacheManager::class, $cacheManagerMock); + + // ExtensionConfiguration is read from TYPO3_CONF_VARS. + $GLOBALS['TYPO3_CONF_VARS']['EXTENSIONS']['dlf'] = ['general' => []]; + return GeneralUtility::makeInstance(DocumentCacheManager::class); + } + + /** + * Reset the static in-memory pool. + * + * @access private + * + * @return void + */ + private function flushPool(): void + { + $reflection = new ReflectionClass(DocumentCacheManager::class); + $reflection->setStaticPropertyValue('pool', []); + } + + /** + * A pool hit returns the very same document instance without reading + * the file cache again, which is what skips the XML re-parsing. + */ + #[Test] + public function getFromPoolHitsReturnsSameInstanceWithoutReadingFileCache(): void + { + $cacheManager = $this->getCacheManager(); + $cache = $this->getCacheProperty($cacheManager); + $document = $this->getDocument(); + $cache->expects(self::once())->method('set'); + $cacheManager->set($this->location, $document); + + $cache->expects(self::never())->method('get'); + self::assertSame($document, $cacheManager->get($this->location)); + } + + /** + * Pool entries expire after the pool TTL; a stale entry is dropped + * and the file cache is read again instead of being returned. + */ + #[Test] + public function expiredPoolEntriesAreReadFromTheFileCacheAgain(): void + { + $cacheManager = $this->getCacheManager(); + $cache = $this->getCacheProperty($cacheManager); + $document = $this->getDocument(); + $cache->method('set')->willReturnSelf(); + $cacheManager->set($this->location, $document); + + $reflection = new ReflectionClass(DocumentCacheManager::class); + $pool = $reflection->getStaticPropertyValue('pool'); + $pool[array_key_first($pool)][1] -= 301; + $reflection->setStaticPropertyValue('pool', $pool); + + $cache->expects(self::once())->method('get')->willReturn(false); + self::assertFalse($cacheManager->get($this->location)); + } + + /** + * More than five documents in the pool evict the least recently used + * one first. + */ + #[Test] + public function poolEvictsLeastRecentlyUsedDocument(): void + { + $cacheManager = $this->getCacheManager(); + $cache = $this->getCacheProperty($cacheManager); + $cache->method('set')->willReturnSelf(); + $documents = []; + foreach (range(1, 6) as $i) { + $documents[$i] = $this->getDocument(); + $cacheManager->set("https://example.org/sample$i.xml", $documents[$i]); + } + + // sample1 has been evicted, sample6 is still pooled. + $cache->expects(self::once())->method('get')->willReturn(false); + self::assertFalse($cacheManager->get('https://example.org/sample1.xml')); + $cache->expects(self::never())->method('get'); + self::assertSame($documents[6], $cacheManager->get('https://example.org/sample6.xml')); + } + + /** + * Loading a new document from the file cache adds it to the pool, so + * a subsequent read for the same location does not touch the file + * cache again. + */ + #[Test] + public function documentReadFromFileCacheIsAddedToPool(): void + { + $cacheManager = $this->getCacheManager(); + $cache = $this->getCacheProperty($cacheManager); + $document = $this->getDocument(); + $cache->expects(self::once())->method('get')->willReturn($document); + self::assertSame($document, $cacheManager->get($this->location)); + + $cache->expects(self::never())->method('get'); + self::assertSame($document, $cacheManager->get($this->location)); + } + + /** + * remove() also removes the document from the in-memory pool. + */ + #[Test] + public function removeAlsoRemovesFromPool(): void + { + $cacheManager = $this->getCacheManager(); + $cache = $this->getCacheProperty($cacheManager); + $document = $this->getDocument(); + $cache->method('set')->willReturnSelf(); + $cacheManager->set($this->location, $document); + + $cache->expects(self::once())->method('remove'); + $cacheManager->remove($this->location); + + $cache->expects(self::once())->method('get')->willReturn(false); + self::assertFalse($cacheManager->get($this->location)); + } + + /** + * flush() also clears the in-memory pool. + */ + #[Test] + public function flushAlsoClearsPool(): void + { + $cacheManager = $this->getCacheManager(); + $cache = $this->getCacheProperty($cacheManager); + $document = $this->getDocument(); + $cache->method('set')->willReturnSelf(); + $cacheManager->set($this->location, $document); + + $cache->expects(self::once())->method('flush'); + $cacheManager->flush(); + + $cache->expects(self::once())->method('get')->willReturn(false); + self::assertFalse($cacheManager->get($this->location)); + } + + /** + * setFail() drops the document from the pool, so a subsequent read + * goes to the file cache and can return the LOAD_FAILED sentinel. + */ + #[Test] + public function setFailRemovesFromPool(): void + { + $cacheManager = $this->getCacheManager(); + $cache = $this->getCacheProperty($cacheManager); + $document = $this->getDocument(); + $cache->method('set')->willReturnSelf(); + $cacheManager->set($this->location, $document); + + $cacheManager->setFail($this->location); + + $cache->expects(self::once()) + ->method('get') + ->willReturn(DocumentCacheManager::LOAD_FAILED); + self::assertSame(DocumentCacheManager::LOAD_FAILED, $cacheManager->get($this->location)); + } + + /** + * The mocked cache frontend of the manager under test. + * + * @access private + * + * @param DocumentCacheManager $cacheManager + * + * @return FrontendInterface&MockObject + */ + private function getCacheProperty(DocumentCacheManager $cacheManager): FrontendInterface&MockObject + { + $reflection = new ReflectionProperty($cacheManager, 'cache'); + return $reflection->getValue($cacheManager); + } + + /** + * Create a document instance without calling the (protected) + * AbstractDocument constructor, which would require XML. + * + * @access private + * + * @return AbstractDocument&MockObject + */ + private function getDocument(): AbstractDocument&MockObject + { + return $this->createMock(AbstractDocument::class); + } +}