Skip to content

Commit a008816

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 d35773b commit a008816

File tree

4 files changed

+52
-47
lines changed

4 files changed

+52
-47
lines changed

build/psalm-baseline.xml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3640,7 +3640,6 @@
36403640
</file>
36413641
<file src="lib/private/Files/Node/Root.php">
36423642
<LessSpecificReturnStatement>
3643-
<code><![CDATA[$folders]]></code>
36443643
<code><![CDATA[$this->mountManager->findByNumericId($numericId)]]></code>
36453644
<code><![CDATA[$this->mountManager->findByStorageId($storageId)]]></code>
36463645
<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
@@ -333,7 +333,7 @@ public function getAppDataDirectoryName(): string {
333333
* in.
334334
*
335335
* @param int $id
336-
* @return array
336+
* @return list<Node>
337337
*/
338338
protected function getByIdInRootMount(int $id): array {
339339
if (!method_exists($this->root, 'createNode')) {

lib/private/Files/Node/Root.php

Lines changed: 50 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -389,7 +389,7 @@ public function getFirstNodeByIdInPath(int $id, string $path): ?INode {
389389
}
390390
}
391391
}
392-
$node = current($this->getByIdInPath($id, $path));
392+
$node = current($this->getByIdInPath($id, $path, true));
393393
if (!$node) {
394394
return null;
395395
}
@@ -401,10 +401,10 @@ public function getFirstNodeByIdInPath(int $id, string $path): ?INode {
401401
}
402402

403403
/**
404-
* @param int $id
405-
* @return Node[]
404+
* @return list<INode>
405+
* @note $onlyFirst is not part of the public API, only used by getFirstNodeByIdInPath
406406
*/
407-
public function getByIdInPath(int $id, string $path): array {
407+
public function getByIdInPath(int $id, string $path, bool $onlyFirst = false): array {
408408
$mountCache = $this->getUserMountCache();
409409
$setupManager = $this->mountManager->getSetupManager();
410410
if ($path !== '' && strpos($path, '/', 1) > 0) {
@@ -427,17 +427,12 @@ public function getByIdInPath(int $id, string $path): array {
427427
//
428428
// so instead of using the cached entries directly, we instead filter the current mounts by the rootid of the cache entry
429429

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

442437
$mounts = $this->mountManager->getMountsByMountProvider($path, $mountProviders);
443438
$mountsContainingFile = array_filter($mounts, fn (IMountPoint $mount) => in_array($mount->getMountPoint(), $mountPoints));
@@ -461,59 +456,70 @@ public function getByIdInPath(int $id, string $path): array {
461456
$mountsContainingFile = array_filter(array_map($this->mountManager->getMountFromMountInfo(...), $mountInfosContainingFiles));
462457
}
463458

464-
if (count($mountsContainingFile) === 0) {
465-
if ($user === $this->getAppDataDirectoryName()) {
466-
$folder = $this->get($path);
467-
if ($folder instanceof Folder) {
468-
return $folder->getByIdInRootMount($id);
469-
} else {
470-
throw new \Exception('getByIdInPath with non folder');
471-
}
459+
$userManager = Server::get(IUserManager::class);
460+
$foundMount = false;
461+
$nodes = [];
462+
foreach ($mountsContainingFile as $mountInfo) {
463+
$mount = $this->mountManager->getMountFromMountInfo($mountInfo);
464+
if ($mount === null) {
465+
continue;
472466
}
473-
return [];
474-
}
467+
$foundMount = true;
475468

476-
$nodes = array_map(function (IMountPoint $mount) use ($id, $mountRoots) {
477-
$rootInternalPath = $mountRoots[$mount->getStorageRootId()];
478-
$cacheEntry = $mount->getStorage()->getCache()->get($id);
479-
if (!$cacheEntry) {
480-
return null;
469+
$storage = $mount->getStorage();
470+
if ($storage === null) {
471+
continue;
481472
}
482473

474+
$cacheEntry = $storage->getCache()->get($id);
475+
if ($cacheEntry === false) {
476+
continue;
477+
}
478+
479+
$rootInternalPath = $mountRoots[$mount->getStorageRootId()];
480+
483481
// cache jails will hide the "true" internal path
484482
$internalPath = ltrim($rootInternalPath . '/' . $cacheEntry->getPath(), '/');
485483
$pathRelativeToMount = substr($internalPath, strlen($rootInternalPath));
486484
$pathRelativeToMount = ltrim($pathRelativeToMount, '/');
487485
$absolutePath = rtrim($mount->getMountPoint() . $pathRelativeToMount, '/');
488-
$storage = $mount->getStorage();
489-
if ($storage === null) {
490-
return null;
491-
}
492486
$ownerId = $storage->getOwner($pathRelativeToMount);
493487
if ($ownerId !== false) {
494-
$owner = Server::get(IUserManager::class)->get($ownerId);
488+
$owner = $userManager->get($ownerId);
495489
} else {
496490
$owner = null;
497491
}
498-
return $this->createNode($absolutePath, new FileInfo(
492+
$node = $this->createNode($absolutePath, new FileInfo(
499493
$absolutePath,
500494
$storage,
501495
$cacheEntry->getPath(),
502496
$cacheEntry,
503497
$mount,
504498
$owner,
505499
));
506-
}, $mountsContainingFile);
507500

508-
$nodes = array_filter($nodes);
501+
if (PathHelper::getRelativePath($path, $node->getPath()) !== null) {
502+
$nodes[] = $node;
503+
if ($onlyFirst) {
504+
return $nodes;
505+
}
506+
}
507+
}
509508

510-
$folders = array_filter($nodes, function (Node $node) use ($path) {
511-
return PathHelper::getRelativePath($path, $node->getPath()) !== null;
512-
});
513-
usort($folders, function ($a, $b) {
514-
return $b->getPath() <=> $a->getPath();
515-
});
516-
return $folders;
509+
if (!$foundMount) {
510+
if ($user === $this->getAppDataDirectoryName()) {
511+
$folder = $this->get($path);
512+
if ($folder instanceof Folder) {
513+
return $folder->getByIdInRootMount($id);
514+
} else {
515+
throw new \Exception('getByIdInPath with non folder');
516+
}
517+
}
518+
return [];
519+
}
520+
521+
usort($nodes, static fn (Node $a, Node $b): int => $b->getPath() <=> $a->getPath());
522+
return $nodes;
517523
}
518524

519525
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)