Skip to content
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.

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

Improve performance when dealing with large numbers of shares #10724

Merged
merged 11 commits into from Aug 24, 2018
25 changes: 21 additions & 4 deletions apps/files_sharing/lib/MountProvider.php
Expand Up @@ -27,6 +27,8 @@

namespace OCA\Files_Sharing;

use OC\Cache\CappedMemoryCache;
use OC\Files\View;
use OCP\Files\Config\IMountProvider;
use OCP\Files\Storage\IStorageFactory;
use OCP\IConfig;
Expand Down Expand Up @@ -84,28 +86,43 @@ public function getMountsForUser(IUser $user, IStorageFactory $storageFactory) {
$superShares = $this->buildSuperShares($shares, $user);

$mounts = [];
$view = new View('/' . $user->getUID() . '/files');
$ownerViews = [];
$sharingDisabledForUser = $this->shareManager->sharingDisabledForUser($user->getUID());
$foldersExistCache = new CappedMemoryCache();
foreach ($superShares as $share) {
try {
$mounts[] = new SharedMount(
/** @var \OCP\Share\IShare $parentShare */
$parentShare = $share[0];
$owner = $parentShare->getShareOwner();
if (!isset($ownerViews[$owner])) {
$ownerViews[$owner] = new View('/' . $parentShare->getShareOwner() . '/files');
}
$mount = new SharedMount(
'\OCA\Files_Sharing\SharedStorage',
$mounts,
[
'user' => $user->getUID(),
// parent share
'superShare' => $share[0],
'superShare' => $parentShare,
// children/component of the superShare
'groupedShares' => $share[1],
'ownerView' => $ownerViews[$owner],
'sharingDisabledForUser' => $sharingDisabledForUser
],
$storageFactory
$storageFactory,
$view,
$foldersExistCache
);
$mounts[$mount->getMountPoint()] = $mount;
} catch (\Exception $e) {
$this->logger->logException($e);
$this->logger->error('Error while trying to create shared mount');
}
}

// array_filter removes the null values from the array
return array_filter($mounts);
return array_values(array_filter($mounts));
}

/**
Expand Down
38 changes: 19 additions & 19 deletions apps/files_sharing/lib/SharedMount.php
Expand Up @@ -28,10 +28,12 @@

namespace OCA\Files_Sharing;

use OC\Cache\CappedMemoryCache;
use OC\Files\Filesystem;
use OC\Files\Mount\MountPoint;
use OC\Files\Mount\MoveableMount;
use OC\Files\View;
use OCP\Files\Storage\IStorageFactory;

/**
* Shared mount points can be moved by the user
Expand Down Expand Up @@ -61,19 +63,19 @@ class SharedMount extends MountPoint implements MoveableMount {
/**
* @param string $storage
* @param SharedMount[] $mountpoints
* @param array|null $arguments
* @param \OCP\Files\Storage\IStorageFactory $loader
* @param array $arguments
* @param IStorageFactory $loader
* @param View $recipientView
*/
public function __construct($storage, array $mountpoints, $arguments = null, $loader = null) {
public function __construct($storage, array $mountpoints, $arguments, IStorageFactory $loader, View $recipientView, CappedMemoryCache $folderExistCache) {
$this->user = $arguments['user'];
$this->recipientView = new View('/' . $this->user . '/files');
$this->recipientView = $recipientView;

$this->superShare = $arguments['superShare'];
$this->groupedShares = $arguments['groupedShares'];

$newMountPoint = $this->verifyMountPoint($this->superShare, $mountpoints);
$newMountPoint = $this->verifyMountPoint($this->superShare, $mountpoints, $folderExistCache);
$absMountPoint = '/' . $this->user . '/files' . $newMountPoint;
$arguments['ownerView'] = new View('/' . $this->superShare->getShareOwner() . '/files');
parent::__construct($storage, $absMountPoint, $arguments, $loader);
}

Expand All @@ -84,12 +86,18 @@ public function __construct($storage, array $mountpoints, $arguments = null, $lo
* @param SharedMount[] $mountpoints
* @return string
*/
private function verifyMountPoint(\OCP\Share\IShare $share, array $mountpoints) {
private function verifyMountPoint(\OCP\Share\IShare $share, array $mountpoints, CappedMemoryCache $folderExistCache) {

$mountPoint = basename($share->getTarget());
$parent = dirname($share->getTarget());

if (!$this->recipientView->is_dir($parent)) {
if ($folderExistCache->hasKey($parent)) {
$parentExists = $folderExistCache->get($parent);
} else {
$parentExists = $this->recipientView->is_dir($parent);
$folderExistCache->set($parent, $parentExists);
}
if (!$parentExists) {
$parent = Helper::getShareFolder($this->recipientView);
}

Expand Down Expand Up @@ -135,19 +143,11 @@ private function generateUniqueTarget($path, $view, array $mountpoints) {
$name = $pathinfo['filename'];
$dir = $pathinfo['dirname'];

// Helper function to find existing mount points
$mountpointExists = function ($path) use ($mountpoints) {
foreach ($mountpoints as $mountpoint) {
if ($mountpoint->getShare()->getTarget() === $path) {
return true;
}
}
return false;
};

$i = 2;
while ($view->file_exists($path) || $mountpointExists($path)) {
$absolutePath = $this->recipientView->getAbsolutePath($path) . '/';
while ($view->file_exists($path) || isset($mountpoints[$absolutePath])) {
$path = Filesystem::normalizePath($dir . '/' . $name . ' (' . $i . ')' . $ext);
$absolutePath = $this->recipientView->getAbsolutePath($path) . '/';
$i++;
}

Expand Down
10 changes: 9 additions & 1 deletion apps/files_sharing/lib/SharedStorage.php
Expand Up @@ -79,6 +79,9 @@ class SharedStorage extends \OC\Files\Storage\Wrapper\Jail implements ISharedSto

private $options;

/** @var boolean */
private $sharingDisabledForUser;

public function __construct($arguments) {
$this->ownerView = $arguments['ownerView'];
$this->logger = \OC::$server->getLogger();
Expand All @@ -87,6 +90,11 @@ public function __construct($arguments) {
$this->groupedShares = $arguments['groupedShares'];

$this->user = $arguments['user'];
if (isset($arguments['sharingDisabledForUser'])) {
$this->sharingDisabledForUser = $arguments['sharingDisabledForUser'];
} else {
$this->sharingDisabledForUser = false;
}

parent::__construct([
'storage' => null,
Expand Down Expand Up @@ -193,7 +201,7 @@ public function getPermissions($target = '') {
$permissions |= \OCP\Constants::PERMISSION_DELETE;
}

if (\OCP\Util::isSharingDisabledForUser()) {
if ($this->sharingDisabledForUser) {
$permissions &= ~\OCP\Constants::PERMISSION_SHARE;
}

Expand Down
51 changes: 34 additions & 17 deletions lib/private/Files/Config/UserMountCache.php
Expand Up @@ -101,17 +101,31 @@ public function registerMounts(IUser $user, array $mounts) {
}
}, $mounts);
$newMounts = array_values(array_filter($newMounts));
$newMountRootIds = array_map(function (ICachedMountInfo $mount) {
return $mount->getRootId();
}, $newMounts);
$newMounts = array_combine($newMountRootIds, $newMounts);

$cachedMounts = $this->getMountsForUser($user);
$mountDiff = function (ICachedMountInfo $mount1, ICachedMountInfo $mount2) {
// since we are only looking for mounts for a specific user comparing on root id is enough
return $mount1->getRootId() - $mount2->getRootId();
};
$cachedMountRootIds = array_map(function (ICachedMountInfo $mount) {
return $mount->getRootId();
}, $cachedMounts);
$cachedMounts = array_combine($cachedMountRootIds, $cachedMounts);

/** @var ICachedMountInfo[] $addedMounts */
$addedMounts = array_udiff($newMounts, $cachedMounts, $mountDiff);
/** @var ICachedMountInfo[] $removedMounts */
$removedMounts = array_udiff($cachedMounts, $newMounts, $mountDiff);
$addedMounts = [];
$removedMounts = [];

foreach ($newMounts as $rootId => $newMount) {
if (!isset($cachedMounts[$rootId])) {
$addedMounts[] = $newMount;
}
}

foreach ($cachedMounts as $rootId => $cachedMount) {
if (!isset($newMounts[$rootId])) {
$removedMounts[] = $cachedMount;
}
}

$changedMounts = $this->findChangedMounts($newMounts, $cachedMounts);

Expand All @@ -135,16 +149,19 @@ public function registerMounts(IUser $user, array $mounts) {
* @return ICachedMountInfo[]
*/
private function findChangedMounts(array $newMounts, array $cachedMounts) {
$new = [];
foreach ($newMounts as $mount) {
$new[$mount->getRootId()] = $mount;
}
$changed = [];
foreach ($newMounts as $newMount) {
foreach ($cachedMounts as $cachedMount) {
foreach ($cachedMounts as $cachedMount) {
$rootId = $cachedMount->getRootId();
if (isset($new[$rootId])) {
$newMount = $new[$rootId];
if (
$newMount->getRootId() === $cachedMount->getRootId() &&
(
$newMount->getMountPoint() !== $cachedMount->getMountPoint() ||
$newMount->getStorageId() !== $cachedMount->getStorageId() ||
$newMount->getMountId() !== $cachedMount->getMountId()
)
$newMount->getMountPoint() !== $cachedMount->getMountPoint() ||
$newMount->getStorageId() !== $cachedMount->getStorageId() ||
$newMount->getMountId() !== $cachedMount->getMountId()
) {
$changed[] = $newMount;
}
Expand Down Expand Up @@ -197,7 +214,7 @@ private function dbRowToMountInfo(array $row) {
}
$mount_id = $row['mount_id'];
if (!is_null($mount_id)) {
$mount_id = (int) $mount_id;
$mount_id = (int)$mount_id;
}
return new CachedMountInfo($user, (int)$row['storage_id'], (int)$row['root_id'], $row['mount_point'], $mount_id, isset($row['path']) ? $row['path'] : '');
}
Expand Down
50 changes: 31 additions & 19 deletions lib/private/Files/Mount/Manager.php
Expand Up @@ -38,8 +38,12 @@ class Manager implements IMountManager {
/** @var CappedMemoryCache */
private $pathCache;

/** @var CappedMemoryCache */
private $inPathCache;

public function __construct() {
$this->pathCache = new CappedMemoryCache();
$this->inPathCache = new CappedMemoryCache();
}

/**
Expand All @@ -48,6 +52,7 @@ public function __construct() {
public function addMount(IMountPoint $mount) {
$this->mounts[$mount->getMountPoint()] = $mount;
$this->pathCache->clear();
$this->inPathCache->clear();
}

/**
Expand All @@ -60,16 +65,18 @@ public function removeMount(string $mountPoint) {
}
unset($this->mounts[$mountPoint]);
$this->pathCache->clear();
$this->inPathCache->clear();
}

/**
* @param string $mountPoint
* @param string $target
*/
public function moveMount(string $mountPoint, string $target){
public function moveMount(string $mountPoint, string $target) {
$this->mounts[$target] = $this->mounts[$mountPoint];
unset($this->mounts[$mountPoint]);
$this->pathCache->clear();
$this->inPathCache->clear();
}

/**
Expand All @@ -80,32 +87,29 @@ public function moveMount(string $mountPoint, string $target){
*/
public function find(string $path) {
\OC_Util::setupFS();
$path = $this->formatPath($path);
if (isset($this->mounts[$path])) {
return $this->mounts[$path];
}
$path = Filesystem::normalizePath($path);

if (isset($this->pathCache[$path])) {
return $this->pathCache[$path];
}

\OC_Hook::emit('OC_Filesystem', 'get_mountpoint', ['path' => $path]);
$foundMountPoint = '';
$mountPoints = array_keys($this->mounts);
$foundMountPointLength = 0;
foreach ($mountPoints as $mountpoint) {
if (\strlen($mountpoint) > $foundMountPointLength && strpos($path, $mountpoint) === 0) {
$foundMountPoint = $mountpoint;
$foundMountPointLength = \strlen($foundMountPoint);
$current = $path;
while (true) {
$mountPoint = $current . '/';
if (isset($this->mounts[$mountPoint])) {
$this->pathCache[$path] = $this->mounts[$mountPoint];
return $this->mounts[$mountPoint];
}
}

if (isset($this->mounts[$foundMountPoint])) {
$this->pathCache[$path] = $this->mounts[$foundMountPoint];
return $this->mounts[$foundMountPoint];
}
if ($current === '') {
return null;
}

return null;
$current = dirname($current);
if ($current === '.' || $current === '/') {
$current = '';
}
}
}

/**
Expand All @@ -117,6 +121,11 @@ public function find(string $path) {
public function findIn(string $path): array {
\OC_Util::setupFS();
$path = $this->formatPath($path);

if (isset($this->inPathCache[$path])) {
return $this->inPathCache[$path];
}

$result = [];
$pathLength = \strlen($path);
$mountPoints = array_keys($this->mounts);
Expand All @@ -125,12 +134,15 @@ public function findIn(string $path): array {
$result[] = $this->mounts[$mountPoint];
}
}

$this->inPathCache[$path] = $result;
return $result;
}

public function clear() {
$this->mounts = [];
$this->pathCache->clear();
$this->inPathCache->clear();
}

/**
Expand Down