Skip to content

Commit ff4bfba

Browse files
committed
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 <carlschwan@kde.org>
1 parent 580d81e commit ff4bfba

File tree

4 files changed

+52
-48
lines changed

4 files changed

+52
-48
lines changed

build/psalm-baseline.xml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3777,7 +3777,6 @@
37773777
</file>
37783778
<file src="lib/private/Files/Node/Root.php">
37793779
<LessSpecificReturnStatement>
3780-
<code><![CDATA[$folders]]></code>
37813780
<code><![CDATA[$this->mountManager->findByNumericId($numericId)]]></code>
37823781
<code><![CDATA[$this->mountManager->findByStorageId($storageId)]]></code>
37833782
<code><![CDATA[$this->mountManager->findIn($mountPoint)]]></code>

lib/private/Files/Node/Folder.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -338,7 +338,7 @@ public function getAppDataDirectoryName(): string {
338338
* in.
339339
*
340340
* @param int $id
341-
* @return array
341+
* @return list<Node>
342342
*/
343343
protected function getByIdInRootMount(int $id): array {
344344
if (!method_exists($this->root, 'createNode')) {

lib/private/Files/Node/Root.php

Lines changed: 50 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
use OCP\Cache\CappedMemoryCache;
2020
use OCP\EventDispatcher\IEventDispatcher;
2121
use OCP\Files\Cache\ICacheEntry;
22+
use OCP\Files\Config\ICachedMountInfo;
2223
use OCP\Files\Config\IUserMountCache;
2324
use OCP\Files\Events\Node\FilesystemTornDownEvent;
2425
use OCP\Files\IRootFolder;
@@ -388,7 +389,7 @@ public function getFirstNodeByIdInPath(int $id, string $path): ?INode {
388389
}
389390
}
390391
}
391-
$node = current($this->getByIdInPath($id, $path));
392+
$node = current($this->getByIdInPath($id, $path, true));
392393
if (!$node) {
393394
return null;
394395
}
@@ -400,10 +401,10 @@ public function getFirstNodeByIdInPath(int $id, string $path): ?INode {
400401
}
401402

402403
/**
403-
* @param int $id
404-
* @return Node[]
404+
* @return list<INode>
405+
* @note $onlyFirst is not part of the public API, only used by getFirstNodeByIdInPath
405406
*/
406-
public function getByIdInPath(int $id, string $path): array {
407+
public function getByIdInPath(int $id, string $path, bool $onlyFirst = false): array {
407408
$mountCache = $this->getUserMountCache();
408409
$setupManager = $this->mountManager->getSetupManager();
409410
if ($path !== '' && strpos($path, '/', 1) > 0) {
@@ -426,72 +427,76 @@ public function getByIdInPath(int $id, string $path): array {
426427
//
427428
// so instead of using the cached entries directly, we instead filter the current mounts by the rootid of the cache entry
428429

429-
$mountRootIds = array_map(function ($mount) {
430-
return $mount->getRootId();
431-
}, $mountsContainingFile);
432-
$mountRootPaths = array_map(function ($mount) {
433-
return $mount->getRootInternalPath();
434-
}, $mountsContainingFile);
435-
$mountProviders = array_unique(array_map(function ($mount) {
436-
return $mount->getMountProvider();
437-
}, $mountsContainingFile));
438-
$mountRoots = array_combine($mountRootIds, $mountRootPaths);
430+
/** @var array<int, string> $mountRoots */
431+
$mountRoots = [];
432+
foreach ($mountsContainingFile as $mountInfo) {
433+
$mountRoots[$mountInfo->getRootId()] = $mountInfo->getRootInternalPath();
434+
}
439435

440-
$mountsContainingFile = array_filter(array_map($this->mountManager->getMountFromMountInfo(...), $mountsContainingFile));
436+
$userManager = Server::get(IUserManager::class);
437+
$foundMount = false;
438+
$nodes = [];
439+
foreach ($mountsContainingFile as $mountInfo) {
440+
$mount = $this->mountManager->getMountFromMountInfo($mountInfo);
441+
if ($mount === null) {
442+
continue;
443+
}
444+
$foundMount = true;
441445

442-
if (count($mountsContainingFile) === 0) {
443-
if ($user === $this->getAppDataDirectoryName()) {
444-
$folder = $this->get($path);
445-
if ($folder instanceof Folder) {
446-
return $folder->getByIdInRootMount($id);
447-
} else {
448-
throw new \Exception('getByIdInPath with non folder');
449-
}
446+
$storage = $mount->getStorage();
447+
if ($storage === null) {
448+
continue;
450449
}
451-
return [];
452-
}
453450

454-
$nodes = array_map(function (IMountPoint $mount) use ($id, $mountRoots) {
455-
$rootInternalPath = $mountRoots[$mount->getStorageRootId()];
456-
$cacheEntry = $mount->getStorage()->getCache()->get($id);
457-
if (!$cacheEntry) {
458-
return null;
451+
$cacheEntry = $storage->getCache()->get($id);
452+
if ($cacheEntry === false) {
453+
continue;
459454
}
460455

456+
$rootInternalPath = $mountRoots[$mount->getStorageRootId()];
457+
461458
// cache jails will hide the "true" internal path
462459
$internalPath = ltrim($rootInternalPath . '/' . $cacheEntry->getPath(), '/');
463460
$pathRelativeToMount = substr($internalPath, strlen($rootInternalPath));
464461
$pathRelativeToMount = ltrim($pathRelativeToMount, '/');
465462
$absolutePath = rtrim($mount->getMountPoint() . $pathRelativeToMount, '/');
466-
$storage = $mount->getStorage();
467-
if ($storage === null) {
468-
return null;
469-
}
470463
$ownerId = $storage->getOwner($pathRelativeToMount);
471464
if ($ownerId !== false) {
472-
$owner = Server::get(IUserManager::class)->get($ownerId);
465+
$owner = $userManager->get($ownerId);
473466
} else {
474467
$owner = null;
475468
}
476-
return $this->createNode($absolutePath, new FileInfo(
469+
$node = $this->createNode($absolutePath, new FileInfo(
477470
$absolutePath,
478471
$storage,
479472
$cacheEntry->getPath(),
480473
$cacheEntry,
481474
$mount,
482475
$owner,
483476
));
484-
}, $mountsContainingFile);
485477

486-
$nodes = array_filter($nodes);
478+
if (PathHelper::getRelativePath($path, $node->getPath()) !== null) {
479+
$nodes[] = $node;
480+
if ($onlyFirst) {
481+
return $nodes;
482+
}
483+
}
484+
}
487485

488-
$folders = array_filter($nodes, function (Node $node) use ($path) {
489-
return PathHelper::getRelativePath($path, $node->getPath()) !== null;
490-
});
491-
usort($folders, function ($a, $b) {
492-
return $b->getPath() <=> $a->getPath();
493-
});
494-
return $folders;
486+
if (!$foundMount) {
487+
if ($user === $this->getAppDataDirectoryName()) {
488+
$folder = $this->get($path);
489+
if ($folder instanceof Folder) {
490+
return $folder->getByIdInRootMount($id);
491+
} else {
492+
throw new \Exception('getByIdInPath with non folder');
493+
}
494+
}
495+
return [];
496+
}
497+
498+
usort($nodes, static fn (Node $a, Node $b): int => $b->getPath() <=> $a->getPath());
499+
return $nodes;
495500
}
496501

497502
public function getNodeFromCacheEntryAndMount(ICacheEntry $cacheEntry, IMountPoint $mountPoint): INode {

lib/public/Files/IRootFolder.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ public function getUserFolder($userId);
3636
*
3737
* @param int $id
3838
* @param string $path
39-
* @return Node[]
39+
* @return list<Node>
4040
*
4141
* @since 24.0.0
4242
*/

0 commit comments

Comments
 (0)