Skip to content

Fix/setup for path optimizations #58325

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

Draft
salmart-dev wants to merge 5 commits into
base: fix/mount-manager-improve-get-by-provider
Choose a base branch
from
+47 −13
Draft

Fix/setup for path optimizations #58325

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
4987175
fix: avoid recomputing list of mounts by provider
salmart-dev Feb 11, 2026
7a5379c
fix: check if path is set up earlier
salmart-dev Feb 13, 2026
eabf571
fix: get mount from memory if already setup
salmart-dev Feb 13, 2026
828cd54
fix: mark paths as set up even if no mounts inside
salmart-dev Feb 13, 2026
b3d95d6
feat: detect N+1 setupForPath calls and setup parent instead [skip ci]
salmart-dev Feb 13, 2026
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
23 changes: 16 additions & 7 deletions lib/private/Files/Mount/Manager.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
class Manager implements IMountManager {
/** @var array<string, IMountPoint> */
private array $mounts = [];
private array $mountsByProvider = [];
private bool $areMountsSorted = false;
/** @var list<string>|null $mountKeys */
private ?array $mountKeys = null;
Expand All @@ -36,7 +37,11 @@ public function __construct(SetupManagerFactory $setupManagerFactory) {
}

public function addMount(IMountPoint $mount): void {
$this->mounts[$mount->getMountPoint()] = $mount;
$mountPoint = $mount->getMountPoint();
$mountProvider = $mount->getMountProvider();
$this->mounts[$mountPoint] = $mount;
$this->mountsByProvider[$mountProvider] ??= [];
$this->mountsByProvider[$mountProvider][$mountPoint] = $mount;
$this->pathCache->clear();
$this->inPathCache->clear();
$this->areMountsSorted = false;
Expand Down Expand Up @@ -167,6 +172,7 @@ private function binarySearch(array $sortedArray, array $sortedKeys, string $pre

public function clear(): void {
$this->mounts = [];
$this->mountsByProvider = [];
$this->pathCache->clear();
$this->inPathCache->clear();
}
Expand Down Expand Up @@ -236,15 +242,18 @@ public function getSetupManager(): SetupManager {
* @param string[] $mountProviders
* @return array<string, IMountPoint>
*/
public function getMountsByMountProvider(string $path, array $mountProviders) {
public function getMountsByMountProvider(string $path, array $mountProviders): array {
$this->getSetupManager()->setupForProvider($path, $mountProviders);
if (in_array('', $mountProviders)) {
if (\in_array('', $mountProviders)) {
return $this->mounts;
} else {
return array_filter($this->mounts, function ($mount) use ($mountProviders) {
return in_array($mount->getMountProvider(), $mountProviders);
});
}

$mounts = [];
foreach ($mountProviders as $mountProvider) {
$mounts[] = $this->mountsByProvider[$mountProvider];
}

return array_merge(...$mounts);
}

/**
Expand Down
37 changes: 31 additions & 6 deletions lib/private/Files/SetupManager.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,10 +130,10 @@ public function isSetupComplete(IUser $user): bool {
* Checks if a path has been cached either directly or through a full setup
* of one of its parents.
*/
private function isPathSetup(string $path): bool {
private function isPathSetup(string $path, bool $withChildren = false): bool {
// if the exact path was already setup with or without children
if (array_key_exists($path, $this->setupMountProviderPaths)) {
return true;
return !$withChildren || $this->setupMountProviderPaths[$path] === self::SETUP_WITH_CHILDREN;
}

// or if any of the ancestors was fully setup
Expand Down Expand Up @@ -480,14 +480,30 @@ public function setupForPath(string $path, bool $includeChildren = false): void
return;
}

$mountPointLikePath = rtrim($path, '/') . '/';
// if we're lucky, $path is already a mount point, if so exit early
if ($this->isPathSetup($mountPointLikePath, $includeChildren)) {
return;
}

$parentPath = dirname($path) . '/';
// avoid N+1 setups for the same parent path
if ($this->shouldSetupParent($parentPath)) {
$this->setupForPath($parentPath, true);
return;
}

if (!isset($this->setupUserMountProviders[$user->getUID()])) {
$this->setupUserMountProviders[$user->getUID()] = [];
}
$setupProviders = &$this->setupUserMountProviders[$user->getUID()];
$currentProviders = [];

try {
$cachedMount = $this->userMountCache->getMountForPath($user, $path);
// if the mount is already set up, get it from memory
$cachedMount = $this->isPathSetup($mountPointLikePath) ?
$this->mountManager->find($mountPointLikePath) :
$this->userMountCache->getMountForPath($user, $path);
} catch (NotFoundException $e) {
$this->setupForUser($user);
return;
Expand All @@ -514,6 +530,13 @@ public function setupForPath(string $path, bool $includeChildren = false): void
return;
}

// mark the path as cached (without children for now...)
$this->setupMountProviderPaths[$mountPoint] = self::SETUP_WITHOUT_CHILDREN;

// track how many times we are setting up files for the same parent
$this->setupPaths[$parentPath] ??= 0;
$this->setupPaths[$parentPath]++;

if (is_a($mountProvider, IPartialMountProvider::class, true)) {
$rootId = $cachedMount->getRootId();
$rootMetadata = $this->fileAccess->getByFileId($rootId);
Expand All @@ -522,8 +545,6 @@ public function setupForPath(string $path, bool $includeChildren = false): void
return;
}
$providerArgs = new MountProviderArgs($cachedMount, $rootMetadata);
// mark the path as cached (without children for now...)
$this->setupMountProviderPaths[$mountPoint] = self::SETUP_WITHOUT_CHILDREN;
$authoritativeMounts[] = array_values(
$this->mountProviderCollection->getUserMountsFromProviderByPath(
$mountProvider,
Expand Down Expand Up @@ -585,6 +606,7 @@ public function setupForPath(string $path, bool $includeChildren = false): void
);
}

$this->setupMountProviderPaths[$mountPoint] = self::SETUP_WITH_CHILDREN;
if (!empty($authoritativeCachedMounts)) {
$rootIds = array_map(
fn (ICachedMountInfo $mount) => $mount->getRootId(),
Expand All @@ -597,7 +619,6 @@ public function setupForPath(string $path, bool $includeChildren = false): void
$rootsMetadata[$id] = $fileMetadata;
}
}
$this->setupMountProviderPaths[$mountPoint] = self::SETUP_WITH_CHILDREN;
foreach ($authoritativeCachedMounts as $providerClass => $cachedMounts) {
$providerArgs = array_values(array_filter(array_map(
static function (ICachedMountInfo $info) use ($rootsMetadata) {
Expand Down Expand Up @@ -639,6 +660,10 @@ static function (ICachedMountInfo $info) use ($rootsMetadata) {
$this->eventLogger->end('fs:setup:user:path');
}

private function shouldSetupParent(string $parentPath): bool {
return ($this->setupPaths[$parentPath] ?? 0) >= 10;
}

private function fullSetupRequired(IUser $user): bool {
if ($this->forceFullSetup) {
return true;
Expand Down