Skip to content

perf(files): keep the mimetype table in the local cache #64091

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
solracsf wants to merge 1 commit into
base: master
Choose a base branch
from
+125 −15
Open

perf(files): keep the mimetype table in the local cache #64091

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 67 additions & 12 deletions lib/private/Files/Type/Loader.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand All @@ -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<int, string> */
protected array $mimetypes;
protected array $mimetypes = [];

/** @psalm-var array<string, int> */
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');
}

/**
Expand All @@ -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;
}

/**
Expand All @@ -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];
}
Expand All @@ -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]);
}

Expand All @@ -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);
}

/**
Expand Down Expand Up @@ -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<int, string> $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;
}
}

Expand Down
61 changes: 58 additions & 3 deletions tests/lib/Files/Type/LoaderTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -17,14 +20,16 @@
#[Group('DB')]
class LoaderTest extends TestCase {
protected IDBConnection $db;
protected ICache $cache;
protected Loader $loader;

#[\Override]
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]
Expand All @@ -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');
Expand Down Expand Up @@ -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'));
}
}
Loading