From 7a91abb4396553836c4eaca369a6216c10e4a5d1 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Tue, 15 Aug 2023 18:41:53 +0200 Subject: [PATCH 1/3] improve di performance for cache Signed-off-by: Robin Appelman --- lib/composer/composer/autoload_classmap.php | 1 + lib/composer/composer/autoload_static.php | 1 + lib/private/Files/Cache/Cache.php | 80 +++++++++---------- lib/private/Files/Cache/CacheDependencies.php | 51 ++++++++++++ lib/private/Files/Cache/Storage.php | 4 +- lib/private/Files/Storage/Common.php | 19 +++-- lib/private/Files/Storage/Home.php | 2 +- 7 files changed, 103 insertions(+), 55 deletions(-) create mode 100644 lib/private/Files/Cache/CacheDependencies.php diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index 04511af269e07..291d3d22dc2c9 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -1338,6 +1338,7 @@ 'OC\\Files\\AppData\\AppData' => $baseDir . '/lib/private/Files/AppData/AppData.php', 'OC\\Files\\AppData\\Factory' => $baseDir . '/lib/private/Files/AppData/Factory.php', 'OC\\Files\\Cache\\Cache' => $baseDir . '/lib/private/Files/Cache/Cache.php', + 'OC\\Files\\Cache\\CacheDependencies' => $baseDir . '/lib/private/Files/Cache/CacheDependencies.php', 'OC\\Files\\Cache\\CacheEntry' => $baseDir . '/lib/private/Files/Cache/CacheEntry.php', 'OC\\Files\\Cache\\CacheQueryBuilder' => $baseDir . '/lib/private/Files/Cache/CacheQueryBuilder.php', 'OC\\Files\\Cache\\FailedCache' => $baseDir . '/lib/private/Files/Cache/FailedCache.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index 4754e82106deb..72c586d4a9276 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -1371,6 +1371,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OC\\Files\\AppData\\AppData' => __DIR__ . '/../../..' . '/lib/private/Files/AppData/AppData.php', 'OC\\Files\\AppData\\Factory' => __DIR__ . '/../../..' . '/lib/private/Files/AppData/Factory.php', 'OC\\Files\\Cache\\Cache' => __DIR__ . '/../../..' . '/lib/private/Files/Cache/Cache.php', + 'OC\\Files\\Cache\\CacheDependencies' => __DIR__ . '/../../..' . '/lib/private/Files/Cache/CacheDependencies.php', 'OC\\Files\\Cache\\CacheEntry' => __DIR__ . '/../../..' . '/lib/private/Files/Cache/CacheEntry.php', 'OC\\Files\\Cache\\CacheQueryBuilder' => __DIR__ . '/../../..' . '/lib/private/Files/Cache/CacheQueryBuilder.php', 'OC\\Files\\Cache\\FailedCache' => __DIR__ . '/../../..' . '/lib/private/Files/Cache/FailedCache.php', diff --git a/lib/private/Files/Cache/Cache.php b/lib/private/Files/Cache/Cache.php index 052b3c75ce806..a512bf76ace33 100644 --- a/lib/private/Files/Cache/Cache.php +++ b/lib/private/Files/Cache/Cache.php @@ -44,6 +44,7 @@ use OC\Files\Search\SearchComparison; use OC\Files\Search\SearchQuery; use OC\Files\Storage\Wrapper\Encryption; +use OC\SystemConfig; use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\EventDispatcher\IEventDispatcher; use OCP\Files\Cache\CacheEntryInsertedEvent; @@ -82,62 +83,53 @@ class Cache implements ICache { /** * @var array partial data for the cache */ - protected $partial = []; - - /** - * @var string - */ - protected $storageId; - - private $storage; - - /** - * @var Storage $storageCache - */ - protected $storageCache; - - /** @var IMimeTypeLoader */ - protected $mimetypeLoader; - - /** - * @var IDBConnection - */ - protected $connection; - - /** - * @var IEventDispatcher - */ - protected $eventDispatcher; - - /** @var QuerySearchHelper */ - protected $querySearchHelper; - - /** - * @param IStorage $storage - */ - public function __construct(IStorage $storage) { + protected array $partial = []; + protected string $storageId; + protected Storage $storageCache; + protected IMimeTypeLoader$mimetypeLoader; + protected IDBConnection $connection; + protected SystemConfig $systemConfig; + protected LoggerInterface $logger; + protected QuerySearchHelper $querySearchHelper; + protected IEventDispatcher $eventDispatcher; + protected IFilesMetadataManager $metadataManager; + + public function __construct( + private IStorage $storage, + // this constructor is used in to many pleases to easily do proper di + // so instead we group it all together + CacheDependencies $dependencies = null, + ) { $this->storageId = $storage->getId(); - $this->storage = $storage; if (strlen($this->storageId) > 64) { $this->storageId = md5($this->storageId); } - - $this->storageCache = new Storage($storage); - $this->mimetypeLoader = \OC::$server->getMimeTypeLoader(); - $this->connection = \OC::$server->getDatabaseConnection(); - $this->eventDispatcher = \OC::$server->get(IEventDispatcher::class); - $this->querySearchHelper = \OCP\Server::get(QuerySearchHelper::class); + if (!$dependencies) { + $dependencies = \OC::$server->get(CacheDependencies::class); + } + $this->storageCache = new Storage($this->storage, true, $dependencies->getConnection()); + $this->mimetypeLoader = $dependencies->getMimeTypeLoader(); + $this->connection = $dependencies->getConnection(); + $this->systemConfig = $dependencies->getSystemConfig(); + $this->logger = $dependencies->getLogger(); + $this->querySearchHelper = $dependencies->getQuerySearchHelper(); + $this->eventDispatcher = $dependencies->getEventDispatcher(); + $this->metadataManager = $dependencies->getMetadataManager(); } protected function getQueryBuilder() { return new CacheQueryBuilder( $this->connection, - \OC::$server->getSystemConfig(), - \OC::$server->get(LoggerInterface::class), - \OC::$server->get(IFilesMetadataManager::class), + $this->systemConfig, + $this->logger, + $this->metadataManager, ); } + public function getStorageCache(): Storage { + return $this->storageCache; + } + /** * Get the numeric storage id for this cache's storage * diff --git a/lib/private/Files/Cache/CacheDependencies.php b/lib/private/Files/Cache/CacheDependencies.php new file mode 100644 index 0000000000000..bd4d96a6cab99 --- /dev/null +++ b/lib/private/Files/Cache/CacheDependencies.php @@ -0,0 +1,51 @@ +mimeTypeLoader; + } + + public function getConnection(): IDBConnection { + return $this->connection; + } + + public function getEventDispatcher(): IEventDispatcher { + return $this->eventDispatcher; + } + + public function getQuerySearchHelper(): QuerySearchHelper { + return $this->querySearchHelper; + } + + public function getSystemConfig(): SystemConfig { + return $this->systemConfig; + } + + public function getLogger(): LoggerInterface { + return $this->logger; + } + + public function getMetadataManager(): IFilesMetadataManager { + return $this->metadataManager; + } +} diff --git a/lib/private/Files/Cache/Storage.php b/lib/private/Files/Cache/Storage.php index 01fc638cef85e..ba0f98f42f492 100644 --- a/lib/private/Files/Cache/Storage.php +++ b/lib/private/Files/Cache/Storage.php @@ -31,6 +31,7 @@ use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\Files\Storage\IStorage; +use OCP\IDBConnection; use Psr\Log\LoggerInterface; /** @@ -65,7 +66,7 @@ public static function getGlobalCache() { * @param bool $isAvailable * @throws \RuntimeException */ - public function __construct($storage, $isAvailable = true) { + public function __construct($storage, $isAvailable, IDBConnection $connection) { if ($storage instanceof IStorage) { $this->storageId = $storage->getId(); } else { @@ -76,7 +77,6 @@ public function __construct($storage, $isAvailable = true) { if ($row = self::getStorageById($this->storageId)) { $this->numericId = (int)$row['numeric_id']; } else { - $connection = \OC::$server->getDatabaseConnection(); $available = $isAvailable ? 1 : 0; if ($connection->insertIfNotExist('*PREFIX*storages', ['id' => $this->storageId, 'available' => $available])) { $this->numericId = $connection->lastInsertId('*PREFIX*storages'); diff --git a/lib/private/Files/Storage/Common.php b/lib/private/Files/Storage/Common.php index 3d5a2f098b2ac..6802ef6601755 100644 --- a/lib/private/Files/Storage/Common.php +++ b/lib/private/Files/Storage/Common.php @@ -43,6 +43,7 @@ namespace OC\Files\Storage; use OC\Files\Cache\Cache; +use OC\Files\Cache\CacheDependencies; use OC\Files\Cache\Propagator; use OC\Files\Cache\Scanner; use OC\Files\Cache\Updater; @@ -338,12 +339,20 @@ public function hasUpdated($path, $time) { return $this->filemtime($path) > $time; } + protected function getCacheDependencies(): CacheDependencies { + static $dependencies = null; + if (!$dependencies) { + $dependencies = \OC::$server->get(CacheDependencies::class); + } + return $dependencies; + } + public function getCache($path = '', $storage = null) { if (!$storage) { $storage = $this; } if (!isset($storage->cache)) { - $storage->cache = new Cache($storage); + $storage->cache = new Cache($storage, $this->getCacheDependencies()); } return $storage->cache; } @@ -398,13 +407,7 @@ public function getUpdater($storage = null) { } public function getStorageCache($storage = null) { - if (!$storage) { - $storage = $this; - } - if (!isset($this->storageCache)) { - $this->storageCache = new \OC\Files\Cache\Storage($storage); - } - return $this->storageCache; + return $this->getCache($storage)->getStorageCache(); } /** diff --git a/lib/private/Files/Storage/Home.php b/lib/private/Files/Storage/Home.php index 5100b15215b9a..051652b75622a 100644 --- a/lib/private/Files/Storage/Home.php +++ b/lib/private/Files/Storage/Home.php @@ -68,7 +68,7 @@ public function getCache($path = '', $storage = null) { $storage = $this; } if (!isset($this->cache)) { - $this->cache = new \OC\Files\Cache\HomeCache($storage); + $this->cache = new \OC\Files\Cache\HomeCache($storage, $this->getCacheDependencies()); } return $this->cache; } From e50c176428440e94a4eacb8d36e28b4f5564f170 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Wed, 16 Aug 2023 12:34:18 +0200 Subject: [PATCH 2/3] also improe cache ci for shared cache Signed-off-by: Robin Appelman --- .../tests/Service/StoragesServiceTest.php | 3 ++- apps/files_sharing/lib/Cache.php | 11 ++++++---- apps/files_sharing/lib/SharedStorage.php | 6 ++--- lib/private/Files/Cache/CacheDependencies.php | 6 +++++ lib/private/Files/Cache/Wrapper/CacheJail.php | 14 +++++++----- .../Files/Cache/Wrapper/CacheWrapper.php | 22 +++++++++---------- 6 files changed, 36 insertions(+), 26 deletions(-) diff --git a/apps/files_external/tests/Service/StoragesServiceTest.php b/apps/files_external/tests/Service/StoragesServiceTest.php index 4eaf70a8e84a0..8c38954dee6e8 100644 --- a/apps/files_external/tests/Service/StoragesServiceTest.php +++ b/apps/files_external/tests/Service/StoragesServiceTest.php @@ -44,6 +44,7 @@ use OCP\Files\Config\IUserMountCache; use OCP\Files\Mount\IMountPoint; use OCP\Files\Storage\IStorage; +use OCP\IDBConnection; use OCP\IUser; class CleaningDBConfig extends DBConfigService { @@ -315,7 +316,7 @@ public function testDeleteStorage($backendOptions, $rustyStorageId) { // manually trigger storage entry because normally it happens on first // access, which isn't possible within this test - $storageCache = new \OC\Files\Cache\Storage($rustyStorageId); + $storageCache = new \OC\Files\Cache\Storage($rustyStorageId, true, \OC::$server->get(IDBConnection::class)); /** @var IUserMountCache $mountCache */ $mountCache = \OC::$server->get(IUserMountCache::class); diff --git a/apps/files_sharing/lib/Cache.php b/apps/files_sharing/lib/Cache.php index 3011bc6466975..5160b7e821a4f 100644 --- a/apps/files_sharing/lib/Cache.php +++ b/apps/files_sharing/lib/Cache.php @@ -28,12 +28,14 @@ */ namespace OCA\Files_Sharing; +use OC\Files\Cache\CacheDependencies; use OC\Files\Cache\FailedCache; use OC\Files\Cache\Wrapper\CacheJail; use OC\Files\Search\SearchBinaryOperator; use OC\Files\Search\SearchComparison; use OC\Files\Storage\Wrapper\Jail; use OC\User\DisplayNameCache; +use OCP\Files\Cache\ICache; use OCP\Files\Cache\ICacheEntry; use OCP\Files\Search\ISearchBinaryOperator; use OCP\Files\Search\ISearchComparison; @@ -62,18 +64,19 @@ class Cache extends CacheJail { public function __construct( $storage, ICacheEntry $sourceRootInfo, - DisplayNameCache $displayNameCache, + CacheDependencies $dependencies, IShare $share ) { $this->storage = $storage; $this->sourceRootInfo = $sourceRootInfo; $this->numericId = $sourceRootInfo->getStorageId(); - $this->displayNameCache = $displayNameCache; + $this->displayNameCache = $dependencies->getDisplayNameCache(); $this->share = $share; parent::__construct( null, - '' + '', + $dependencies, ); } @@ -98,7 +101,7 @@ protected function getGetUnjailedRoot() { return $this->sourceRootInfo->getPath(); } - public function getCache() { + public function getCache(): ICache { if (is_null($this->cache)) { $sourceStorage = $this->storage->getSourceStorage(); if ($sourceStorage) { diff --git a/apps/files_sharing/lib/SharedStorage.php b/apps/files_sharing/lib/SharedStorage.php index de49e3c429474..53e9f60947e0b 100644 --- a/apps/files_sharing/lib/SharedStorage.php +++ b/apps/files_sharing/lib/SharedStorage.php @@ -32,6 +32,7 @@ */ namespace OCA\Files_Sharing; +use OC\Files\Cache\CacheDependencies; use OC\Files\Cache\FailedCache; use OC\Files\Cache\NullWatcher; use OC\Files\Cache\Watcher; @@ -40,7 +41,6 @@ use OC\Files\Storage\FailedStorage; use OC\Files\Storage\Home; use OC\Files\Storage\Wrapper\PermissionsMask; -use OC\User\DisplayNameCache; use OC\User\NoUserException; use OCA\Files_External\Config\ExternalMountPoint; use OCP\Constants; @@ -410,10 +410,10 @@ public function getCache($path = '', $storage = null) { return new FailedCache(); } - $this->cache = new \OCA\Files_Sharing\Cache( + $this->cache = new Cache( $storage, $sourceRoot, - \OC::$server->get(DisplayNameCache::class), + \OC::$server->get(CacheDependencies::class), $this->getShare() ); return $this->cache; diff --git a/lib/private/Files/Cache/CacheDependencies.php b/lib/private/Files/Cache/CacheDependencies.php index bd4d96a6cab99..7c51f3ff88421 100644 --- a/lib/private/Files/Cache/CacheDependencies.php +++ b/lib/private/Files/Cache/CacheDependencies.php @@ -3,6 +3,7 @@ namespace OC\Files\Cache; use OC\SystemConfig; +use OC\User\DisplayNameCache; use OCP\EventDispatcher\IEventDispatcher; use OCP\Files\IMimeTypeLoader; use OCP\FilesMetadata\IFilesMetadataManager; @@ -18,6 +19,7 @@ public function __construct( private SystemConfig $systemConfig, private LoggerInterface $logger, private IFilesMetadataManager $metadataManager, + private DisplayNameCache $displayNameCache, ) { } @@ -45,6 +47,10 @@ public function getLogger(): LoggerInterface { return $this->logger; } + public function getDisplayNameCache(): DisplayNameCache { + return $this->displayNameCache; + } + public function getMetadataManager(): IFilesMetadataManager { return $this->metadataManager; } diff --git a/lib/private/Files/Cache/Wrapper/CacheJail.php b/lib/private/Files/Cache/Wrapper/CacheJail.php index 73c9a0170197d..f9754e433df27 100644 --- a/lib/private/Files/Cache/Wrapper/CacheJail.php +++ b/lib/private/Files/Cache/Wrapper/CacheJail.php @@ -28,8 +28,10 @@ namespace OC\Files\Cache\Wrapper; use OC\Files\Cache\Cache; +use OC\Files\Cache\CacheDependencies; use OC\Files\Search\SearchBinaryOperator; use OC\Files\Search\SearchComparison; +use OCP\Files\Cache\ICache; use OCP\Files\Cache\ICacheEntry; use OCP\Files\Search\ISearchBinaryOperator; use OCP\Files\Search\ISearchComparison; @@ -45,12 +47,12 @@ class CacheJail extends CacheWrapper { protected $root; protected $unjailedRoot; - /** - * @param ?\OCP\Files\Cache\ICache $cache - * @param string $root - */ - public function __construct($cache, $root) { - parent::__construct($cache); + public function __construct( + ?ICache $cache, + string $root, + CacheDependencies $dependencies = null, + ) { + parent::__construct($cache, $dependencies); $this->root = $root; if ($cache instanceof CacheJail) { diff --git a/lib/private/Files/Cache/Wrapper/CacheWrapper.php b/lib/private/Files/Cache/Wrapper/CacheWrapper.php index 59da272787e82..a8b1d34c95646 100644 --- a/lib/private/Files/Cache/Wrapper/CacheWrapper.php +++ b/lib/private/Files/Cache/Wrapper/CacheWrapper.php @@ -30,33 +30,31 @@ namespace OC\Files\Cache\Wrapper; use OC\Files\Cache\Cache; -use OC\Files\Cache\QuerySearchHelper; +use OC\Files\Cache\CacheDependencies; use OCP\Files\Cache\ICache; use OCP\Files\Cache\ICacheEntry; -use OCP\Files\IMimeTypeLoader; use OCP\Files\Search\ISearchOperator; use OCP\Files\Search\ISearchQuery; -use OCP\IDBConnection; class CacheWrapper extends Cache { /** - * @var \OCP\Files\Cache\ICache + * @var ?ICache */ protected $cache; - /** - * @param \OCP\Files\Cache\ICache $cache - */ - public function __construct($cache) { + public function __construct(?ICache $cache, CacheDependencies $dependencies = null) { $this->cache = $cache; - if ($cache instanceof Cache) { + if (!$dependencies && $cache instanceof Cache) { $this->mimetypeLoader = $cache->mimetypeLoader; $this->connection = $cache->connection; $this->querySearchHelper = $cache->querySearchHelper; } else { - $this->mimetypeLoader = \OC::$server->get(IMimeTypeLoader::class); - $this->connection = \OC::$server->get(IDBConnection::class); - $this->querySearchHelper = \OC::$server->get(QuerySearchHelper::class); + if (!$dependencies) { + $dependencies = \OC::$server->get(CacheDependencies::class); + } + $this->mimetypeLoader = $dependencies->getMimeTypeLoader(); + $this->connection = $dependencies->getConnection(); + $this->querySearchHelper = $dependencies->getQuerySearchHelper(); } } From e9d97a568fd00251dcc3d68042140948fa0dc6da Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Mon, 12 Feb 2024 16:38:08 +0100 Subject: [PATCH 3/3] use OCP\Server Signed-off-by: Robin Appelman --- apps/files_external/tests/Service/StoragesServiceTest.php | 4 ++-- lib/private/Files/Cache/Wrapper/CacheWrapper.php | 3 ++- lib/private/Files/Storage/Common.php | 3 ++- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/apps/files_external/tests/Service/StoragesServiceTest.php b/apps/files_external/tests/Service/StoragesServiceTest.php index 8c38954dee6e8..1ea2176ac90f5 100644 --- a/apps/files_external/tests/Service/StoragesServiceTest.php +++ b/apps/files_external/tests/Service/StoragesServiceTest.php @@ -46,6 +46,7 @@ use OCP\Files\Storage\IStorage; use OCP\IDBConnection; use OCP\IUser; +use OCP\Server; class CleaningDBConfig extends DBConfigService { private $mountIds = []; @@ -67,7 +68,6 @@ public function clean() { * @group DB */ abstract class StoragesServiceTest extends \Test\TestCase { - /** * @var StoragesService */ @@ -316,7 +316,7 @@ public function testDeleteStorage($backendOptions, $rustyStorageId) { // manually trigger storage entry because normally it happens on first // access, which isn't possible within this test - $storageCache = new \OC\Files\Cache\Storage($rustyStorageId, true, \OC::$server->get(IDBConnection::class)); + $storageCache = new \OC\Files\Cache\Storage($rustyStorageId, true, Server::get(IDBConnection::class)); /** @var IUserMountCache $mountCache */ $mountCache = \OC::$server->get(IUserMountCache::class); diff --git a/lib/private/Files/Cache/Wrapper/CacheWrapper.php b/lib/private/Files/Cache/Wrapper/CacheWrapper.php index a8b1d34c95646..31410eea798e5 100644 --- a/lib/private/Files/Cache/Wrapper/CacheWrapper.php +++ b/lib/private/Files/Cache/Wrapper/CacheWrapper.php @@ -35,6 +35,7 @@ use OCP\Files\Cache\ICacheEntry; use OCP\Files\Search\ISearchOperator; use OCP\Files\Search\ISearchQuery; +use OCP\Server; class CacheWrapper extends Cache { /** @@ -50,7 +51,7 @@ public function __construct(?ICache $cache, CacheDependencies $dependencies = nu $this->querySearchHelper = $cache->querySearchHelper; } else { if (!$dependencies) { - $dependencies = \OC::$server->get(CacheDependencies::class); + $dependencies = Server::get(CacheDependencies::class); } $this->mimetypeLoader = $dependencies->getMimeTypeLoader(); $this->connection = $dependencies->getConnection(); diff --git a/lib/private/Files/Storage/Common.php b/lib/private/Files/Storage/Common.php index 6802ef6601755..65c2580e61c4c 100644 --- a/lib/private/Files/Storage/Common.php +++ b/lib/private/Files/Storage/Common.php @@ -64,6 +64,7 @@ use OCP\Files\Storage\IWriteStreamStorage; use OCP\Lock\ILockingProvider; use OCP\Lock\LockedException; +use OCP\Server; use Psr\Log\LoggerInterface; /** @@ -342,7 +343,7 @@ public function hasUpdated($path, $time) { protected function getCacheDependencies(): CacheDependencies { static $dependencies = null; if (!$dependencies) { - $dependencies = \OC::$server->get(CacheDependencies::class); + $dependencies = Server::get(CacheDependencies::class); } return $dependencies; }