From 304adf5759e0ff962cd2acb6d82840f19a82dbd2 Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Tue, 3 Feb 2026 12:03:28 +0100 Subject: [PATCH] perf(mounts): Replace many array_ by a simple loop And allow to abort once we found a node when getFirstNodeById is called. Signed-off-by: Carl Schwan --- lib/private/Files/Node/Folder.php | 2 +- lib/private/Files/Node/Root.php | 92 ++++++++++++++++------------- lib/public/Files/IRootFolder.php | 2 +- tests/lib/Files/Node/FolderTest.php | 4 +- 4 files changed, 54 insertions(+), 46 deletions(-) diff --git a/lib/private/Files/Node/Folder.php b/lib/private/Files/Node/Folder.php index f103a34c9e92d..7546d6820bd26 100644 --- a/lib/private/Files/Node/Folder.php +++ b/lib/private/Files/Node/Folder.php @@ -337,7 +337,7 @@ public function getAppDataDirectoryName(): string { * in. * * @param int $id - * @return array + * @return list */ protected function getByIdInRootMount(int $id): array { if (!method_exists($this->root, 'createNode')) { diff --git a/lib/private/Files/Node/Root.php b/lib/private/Files/Node/Root.php index 0c121dc8b0527..ae689db132a53 100644 --- a/lib/private/Files/Node/Root.php +++ b/lib/private/Files/Node/Root.php @@ -368,7 +368,7 @@ public function getFirstNodeByIdInPath(int $id, string $path): ?INode { } } } - $node = current($this->getByIdInPath($id, $path)); + $node = current($this->getByIdInPath($id, $path, true)); if (!$node) { return null; } @@ -380,9 +380,10 @@ public function getFirstNodeByIdInPath(int $id, string $path): ?INode { } /** - * @return INode[] + * @return list + * @note $onlyFirst is not part of the public API, only used by getFirstNodeByIdInPath */ - public function getByIdInPath(int $id, string $path): array { + public function getByIdInPath(int $id, string $path, bool $onlyFirst = false): array { $mountCache = $this->getUserMountCache(); $setupManager = $this->mountManager->getSetupManager(); if ($path !== '' && strpos($path, '/', 1) > 0) { @@ -405,22 +406,20 @@ public function getByIdInPath(int $id, string $path): array { // // so instead of using the cached entries directly, we instead filter the current mounts by the rootid of the cache entry - $mountRootIds = array_map(function ($mount) { - return $mount->getRootId(); - }, $mountInfosContainingFiles); - $mountRootPaths = array_map(function ($mount) { - return $mount->getRootInternalPath(); - }, $mountInfosContainingFiles); - $mountProviders = array_unique(array_map(function ($mount) { - return $mount->getMountProvider(); - }, $mountInfosContainingFiles)); $mountPoints = array_map(fn (ICachedMountInfo $mountInfo) => $mountInfo->getMountPoint(), $mountInfosContainingFiles); - $mountRoots = array_combine($mountRootIds, $mountRootPaths); + /** @var array $mountRoots */ + $mountRoots = []; + $mountProviders = []; + foreach ($mountInfosContainingFiles as $mountInfo) { + $mountRoots[$mountInfo->getRootId()] = $mountInfo->getRootInternalPath(); + $mountProviders[] = $mountInfo->getMountProvider(); + } + $mountProviders = array_unique($mountProviders); $mounts = $this->mountManager->getMountsByMountProvider($path, $mountProviders); $mountsContainingFile = array_filter($mounts, fn (IMountPoint $mount) => in_array($mount->getMountPoint(), $mountPoints)); - // if we haven't found a relevant mount that is setup, but we do have relevant mount infos + // if we haven't found a relevant mount that is set up, but we do have relevant mount infos // we try to load them from the mount info. if (count($mountsContainingFile) === 0 && count($mountInfosContainingFiles) > 0) { // in order to minimize the cost of this, we only use the mount infos from one user. @@ -439,41 +438,37 @@ public function getByIdInPath(int $id, string $path): array { $mountsContainingFile = array_filter(array_map($this->mountManager->getMountFromMountInfo(...), $mountInfosContainingFiles)); } - if (count($mountsContainingFile) === 0) { - if ($user === $this->getAppDataDirectoryName()) { - $folder = $this->get($path); - if ($folder instanceof Folder) { - return $folder->getByIdInRootMount($id); - } else { - throw new \Exception('getByIdInPath with non folder'); - } + $userManager = Server::get(IUserManager::class); + $foundMount = false; + $nodes = []; + usort($mountsContainingFile, static fn (IMountPoint $a, IMountPoint $b): int => $b->getMountPoint() <=> $a->getMountPoint()); + foreach ($mountsContainingFile as $mount) { + $foundMount = true; + + $storage = $mount->getStorage(); + if ($storage === null) { + continue; } - return []; - } - $nodes = array_map(function (IMountPoint $mount) use ($id, $mountRoots) { - $rootInternalPath = $mountRoots[$mount->getStorageRootId()]; - $cacheEntry = $mount->getStorage()->getCache()->get($id); - if (!$cacheEntry) { - return null; + $cacheEntry = $storage->getCache()->get($id); + if ($cacheEntry === false) { + continue; } + $rootInternalPath = $mountRoots[$mount->getStorageRootId()]; + // cache jails will hide the "true" internal path $internalPath = ltrim($rootInternalPath . '/' . $cacheEntry->getPath(), '/'); $pathRelativeToMount = substr($internalPath, strlen($rootInternalPath)); $pathRelativeToMount = ltrim($pathRelativeToMount, '/'); $absolutePath = rtrim($mount->getMountPoint() . $pathRelativeToMount, '/'); - $storage = $mount->getStorage(); - if ($storage === null) { - return null; - } $ownerId = $storage->getOwner($pathRelativeToMount); if ($ownerId !== false) { - $owner = Server::get(IUserManager::class)->get($ownerId); + $owner = $userManager->get($ownerId); } else { $owner = null; } - return $this->createNode($absolutePath, new FileInfo( + $node = $this->createNode($absolutePath, new FileInfo( $absolutePath, $storage, $cacheEntry->getPath(), @@ -481,15 +476,28 @@ public function getByIdInPath(int $id, string $path): array { $mount, $owner, )); - }, $mountsContainingFile); - $nodes = array_filter($nodes); + if (PathHelper::getRelativePath($path, $node->getPath()) !== null) { + $nodes[] = $node; + if ($onlyFirst) { + return $nodes; + } + } + } - $folders = array_filter($nodes, function (Node $node) use ($path) { - return PathHelper::getRelativePath($path, $node->getPath()) !== null; - }); - usort($folders, static fn (Node $a, Node $b): int => $b->getPath() <=> $a->getPath()); - return $folders; + if (!$foundMount) { + if ($user === $this->getAppDataDirectoryName()) { + $folder = $this->get($path); + if ($folder instanceof Folder) { + return $folder->getByIdInRootMount($id); + } else { + throw new \RuntimeException('getByIdInPath with non folder'); + } + } + return []; + } + + return $nodes; } public function getNodeFromCacheEntryAndMount(ICacheEntry $cacheEntry, IMountPoint $mountPoint): INode { diff --git a/lib/public/Files/IRootFolder.php b/lib/public/Files/IRootFolder.php index fb8532f8c8153..10672a2b3cdbd 100644 --- a/lib/public/Files/IRootFolder.php +++ b/lib/public/Files/IRootFolder.php @@ -36,7 +36,7 @@ public function getUserFolder($userId); * * @param int $id * @param string $path - * @return Node[] + * @return list * * @since 24.0.0 */ diff --git a/tests/lib/Files/Node/FolderTest.php b/tests/lib/Files/Node/FolderTest.php index 0fc3115e13f76..3eac05d4a2821 100644 --- a/tests/lib/Files/Node/FolderTest.php +++ b/tests/lib/Files/Node/FolderTest.php @@ -706,8 +706,8 @@ public function testGetByIdMultipleStorages(): void { $node = new Folder($root, $view, '/bar/foo'); $result = $node->getById(1); $this->assertEquals(2, count($result)); - $this->assertEquals('/bar/foo/qwerty', $result[0]->getPath()); - $this->assertEquals('/bar/foo/asd/foo/qwerty', $result[1]->getPath()); + $this->assertEquals('/bar/foo/asd/foo/qwerty', $result[0]->getPath()); + $this->assertEquals('/bar/foo/qwerty', $result[1]->getPath()); } public static function uniqueNameProvider(): array {