From f4ad2f9bd2eaa2bc4865f60b3587e92069190cc8 Mon Sep 17 00:00:00 2001 From: Git'Fellow <12234510+solracsf@users.noreply.github.com> Date: Sun, 6 Sep 2026 13:56:35 +0200 Subject: [PATCH] perf(files): keep the mimetype table in the local cache OC\Files\Type\Loader loaded the whole oc_mimetypes table from the database in every PHP process that touched the filesystem: in a captured workload the `SELECT id, mimetype FROM oc_mimetypes` statement ran 154 times across 169 connections. The table is append-only and tiny, so keep it in the local memcache (createLocal('mimetypes'), one hour TTL) and serve lookups from there. A cached copy can miss rows another process or host added after it was taken, so an unknown id or mimetype reloads the table from the database once per loader instance before it is reported as missing; store() and reset() drop the cached copy. Signed-off-by: Git'Fellow <12234510+solracsf@users.noreply.github.com> --- lib/private/Files/Type/Loader.php | 79 ++++++++++++++++++++++++----- tests/lib/Files/Type/LoaderTest.php | 61 ++++++++++++++++++++-- 2 files changed, 125 insertions(+), 15 deletions(-) diff --git a/lib/private/Files/Type/Loader.php b/lib/private/Files/Type/Loader.php index 49cd44d1772db..ef5a9acf81363 100644 --- a/lib/private/Files/Type/Loader.php +++ b/lib/private/Files/Type/Loader.php @@ -12,6 +12,8 @@ use OCP\AppFramework\Db\TTransactional; use OCP\DB\Exception as DBException; use OCP\Files\IMimeTypeLoader; +use OCP\ICache; +use OCP\ICacheFactory; use OCP\IDBConnection; /** @@ -22,20 +24,32 @@ class Loader implements IMimeTypeLoader { use TTransactional; + private const string CACHE_KEY = 'mimetypes'; + private const int CACHE_TTL = 3600; + /** @psalm-var array */ - protected array $mimetypes; + protected array $mimetypes = []; /** @psalm-var array */ - protected array $mimetypeIds; + protected array $mimetypeIds = []; + + private ICache $cache; + + /** + * The cached copy of the table may lack rows added by another process, + * so a miss reloads from the database once per instance. + */ + private bool $reloaded = false; /** * @param IDBConnection $dbConnection + * @param ICacheFactory $cacheFactory */ public function __construct( private IDBConnection $dbConnection, + ICacheFactory $cacheFactory, ) { - $this->mimetypes = []; - $this->mimetypeIds = []; + $this->cache = $cacheFactory->createLocal('mimetypes'); } /** @@ -46,10 +60,10 @@ public function getMimetypeById(int $id): ?string { if (!$this->mimetypes) { $this->loadMimetypes(); } - if (isset($this->mimetypes[$id])) { - return $this->mimetypes[$id]; + if (!isset($this->mimetypes[$id])) { + $this->reloadOnMiss(); } - return null; + return $this->mimetypes[$id] ?? null; } /** @@ -60,6 +74,9 @@ public function getId(string $mimetype): int { if (!$this->mimetypeIds) { $this->loadMimetypes(); } + if (!isset($this->mimetypeIds[$mimetype])) { + $this->reloadOnMiss(); + } if (isset($this->mimetypeIds[$mimetype])) { return $this->mimetypeIds[$mimetype]; } @@ -74,6 +91,9 @@ public function exists(string $mimetype): bool { if (!$this->mimetypeIds) { $this->loadMimetypes(); } + if (!isset($this->mimetypeIds[$mimetype])) { + $this->reloadOnMiss(); + } return isset($this->mimetypeIds[$mimetype]); } @@ -84,6 +104,8 @@ public function exists(string $mimetype): bool { public function reset(): void { $this->mimetypes = []; $this->mimetypeIds = []; + $this->reloaded = false; + $this->cache->remove(self::CACHE_KEY); } /** @@ -118,30 +140,63 @@ protected function store(string $mimetype): int { if ($id === false) { throw new \Exception("Database threw an unique constraint on inserting a new mimetype, but couldn't return the ID for this very mimetype"); } - $mimetypeId = (int)$id; } $this->mimetypes[$mimetypeId] = $mimetype; $this->mimetypeIds[$mimetype] = $mimetypeId; + $this->cache->remove(self::CACHE_KEY); + return $mimetypeId; } /** - * Load all mimetypes from DB + * Load all mimetypes from the cache, falling back to the DB */ private function loadMimetypes(): void { + $cached = $this->cache->get(self::CACHE_KEY); + if (is_array($cached) && $cached !== []) { + $this->setMimetypes($cached); + return; + } + $this->loadMimetypesFromDatabase(); + } + + private function reloadOnMiss(): void { + if ($this->reloaded) { + return; + } + $this->reloaded = true; + $this->loadMimetypesFromDatabase(); + } + + private function loadMimetypesFromDatabase(): void { $qb = $this->dbConnection->getQueryBuilder(); $qb->select('id', 'mimetype') ->from('mimetypes'); - $result = $qb->executeQuery(); $results = $result->fetchAllAssociative(); $result->closeCursor(); + $mimetypes = []; foreach ($results as $row) { - $this->mimetypes[(int)$row['id']] = $row['mimetype']; - $this->mimetypeIds[$row['mimetype']] = (int)$row['id']; + $mimetypes[(int)$row['id']] = (string)$row['mimetype']; + } + $this->setMimetypes($mimetypes); + if ($mimetypes !== []) { + $this->cache->set(self::CACHE_KEY, $mimetypes, self::CACHE_TTL); + } + } + + /** + * @param array $mimetypes + */ + private function setMimetypes(array $mimetypes): void { + $this->mimetypes = []; + $this->mimetypeIds = []; + foreach ($mimetypes as $id => $mimetype) { + $this->mimetypes[(int)$id] = $mimetype; + $this->mimetypeIds[$mimetype] = (int)$id; } } diff --git a/tests/lib/Files/Type/LoaderTest.php b/tests/lib/Files/Type/LoaderTest.php index 35c549f321e94..12982ccd83ce2 100644 --- a/tests/lib/Files/Type/LoaderTest.php +++ b/tests/lib/Files/Type/LoaderTest.php @@ -9,6 +9,9 @@ namespace Test\Files\Type; use OC\Files\Type\Loader; +use OC\Memcache\ArrayCache; +use OCP\ICache; +use OCP\ICacheFactory; use OCP\IDBConnection; use OCP\Server; use PHPUnit\Framework\Attributes\Group; @@ -17,6 +20,7 @@ #[Group('DB')] class LoaderTest extends TestCase { protected IDBConnection $db; + protected ICache $cache; protected Loader $loader; #[\Override] @@ -24,7 +28,8 @@ protected function setUp(): void { parent::setUp(); $this->db = Server::get(IDBConnection::class); - $this->loader = new Loader($this->db); + $this->cache = new ArrayCache(); + $this->loader = $this->createLoader($this->db); } #[\Override] @@ -38,13 +43,24 @@ protected function tearDown(): void { parent::tearDown(); } - public function testGetMimetype(): void { + private function createLoader(IDBConnection $db): Loader { + $cacheFactory = $this->createMock(ICacheFactory::class); + $cacheFactory->method('createLocal')->willReturn($this->cache); + return new Loader($db, $cacheFactory); + } + + private function insertMimetype(string $mimetype): int { $qb = $this->db->getQueryBuilder(); $qb->insert('mimetypes') ->values([ - 'mimetype' => $qb->createPositionalParameter('testing/mymimetype') + 'mimetype' => $qb->createPositionalParameter($mimetype) ]); $qb->executeStatement(); + return $qb->getLastInsertId(); + } + + public function testGetMimetype(): void { + $this->insertMimetype('testing/mymimetype'); $this->assertTrue($this->loader->exists('testing/mymimetype')); $mimetypeId = $this->loader->getId('testing/mymimetype'); @@ -84,4 +100,43 @@ public function testStoreExists(): void { $this->assertEquals($mimetypeId, $mimetypeId2); } + + public function testMimetypesAreServedFromTheLocalCache(): void { + $mimetypeId = $this->insertMimetype('testing/cached'); + $this->assertEquals('testing/cached', $this->loader->getMimetypeById($mimetypeId)); + + $db = $this->createMock(IDBConnection::class); + $db->expects($this->never())->method('getQueryBuilder'); + $loader = $this->createLoader($db); + + $this->assertEquals('testing/cached', $loader->getMimetypeById($mimetypeId)); + $this->assertEquals($mimetypeId, $loader->getId('testing/cached')); + $this->assertTrue($loader->exists('testing/cached')); + } + + public function testUnknownIdIsReloadedFromTheDatabase(): void { + $this->assertTrue($this->loader->exists('httpd/unix-directory')); + + // added by another process after this instance loaded the table + $mimetypeId = $this->insertMimetype('testing/late'); + + $this->assertEquals('testing/late', $this->loader->getMimetypeById($mimetypeId)); + $this->assertEquals($mimetypeId, $this->createLoader($this->db)->getId('testing/late')); + // only one reload per instance + $this->assertNull($this->loader->getMimetypeById(12345)); + } + + public function testStoreAndResetInvalidateTheCache(): void { + $this->assertTrue($this->loader->exists('httpd/unix-directory')); + $this->assertTrue($this->cache->hasKey('mimetypes')); + + $this->loader->getId('testing/new'); + $this->assertFalse($this->cache->hasKey('mimetypes')); + + $this->assertTrue($this->createLoader($this->db)->exists('testing/new')); + $this->assertTrue($this->cache->hasKey('mimetypes')); + + $this->loader->reset(); + $this->assertFalse($this->cache->hasKey('mimetypes')); + } }