Skip to content

Commit f7e25d3

Browse files
CarlSchwanAndyScherzinger
authored andcommitted
perf: Avoid sorting an already sorted array
Improve performance a bit as this is in the hotpath and take ~2s in prod. Signed-off-by: Carl Schwan <carlschwan@kde.org>
1 parent 2104207 commit f7e25d3

8 files changed

Lines changed: 33 additions & 66 deletions

File tree

lib/private/Files/Mount/Manager.php

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ class Manager implements IMountManager {
2626
private ?array $mountKeys = null;
2727
/** @var CappedMemoryCache<IMountPoint> */
2828
private CappedMemoryCache $pathCache;
29-
/** @var CappedMemoryCache<IMountPoint[]> */
29+
/** @var CappedMemoryCache<list<IMountPoint>> */
3030
private CappedMemoryCache $inPathCache;
3131
private SetupManager $setupManager;
3232

@@ -110,11 +110,6 @@ public function find(string $path): IMountPoint {
110110
throw new NotFoundException('No mount for path ' . $path . ' existing mounts (' . count($this->mounts) . '): ' . implode(',', array_keys($this->mounts)));
111111
}
112112

113-
/**
114-
* Find all mounts in $path
115-
*
116-
* @return IMountPoint[]
117-
*/
118113
#[\Override]
119114
public function findIn(string $path): array {
120115
$this->setupManager->setupForPath($path, true);

lib/private/Files/Node/LazyFolder.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ public function getMount(string $mountPoint): IMountPoint {
109109
}
110110

111111
/**
112-
* @return IMountPoint[]
112+
* @return list<IMountPoint>
113113
*/
114114
public function getMountsIn(string $mountPoint): array {
115115
return $this->__call(__FUNCTION__, func_get_args());

lib/private/Files/Node/Root.php

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -136,10 +136,6 @@ public function getMount(string $mountPoint): IMountPoint {
136136
return $this->mountManager->find($mountPoint);
137137
}
138138

139-
/**
140-
* @param string $mountPoint
141-
* @return IMountPoint[]
142-
*/
143139
#[\Override]
144140
public function getMountsIn(string $mountPoint): array {
145141
return $this->mountManager->findIn($mountPoint);

lib/private/Files/Utils/Scanner.php

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -54,12 +54,11 @@ public function __construct(
5454
}
5555

5656
/**
57-
* get all storages for $dir
57+
* Get all storages for a specific directory.
5858
*
59-
* @param string $dir
6059
* @return array<string, IMountPoint>
6160
*/
62-
protected function getMounts($dir) {
61+
protected function getMounts(string $dir): array {
6362
//TODO: move to the node based fileapi once that's done
6463
$this->setupManager->tearDown();
6564

@@ -79,7 +78,7 @@ protected function getMounts($dir) {
7978
/**
8079
* attach listeners to the scanner
8180
*/
82-
protected function attachListener(MountPoint $mount) {
81+
protected function attachListener(IMountPoint $mount): void {
8382
/** @var \OC\Files\Cache\Scanner $scanner */
8483
$scanner = $mount->getStorage()->getScanner();
8584
$scanner->listen('\OC\Files\Cache\Scanner', 'scanFile', function ($path) use ($mount): void {
@@ -103,7 +102,7 @@ protected function attachListener(MountPoint $mount) {
103102
});
104103
}
105104

106-
public function backgroundScan(string $dir) {
105+
public function backgroundScan(string $dir): void {
107106
$mounts = $this->getMounts($dir);
108107
foreach ($mounts as $mount) {
109108
try {
@@ -244,7 +243,7 @@ public function scan(string $dir = '', $recursive = \OC\Files\Cache\Scanner::SCA
244243
}
245244
}
246245

247-
private function triggerPropagator(IStorage $storage, $internalPath) {
246+
private function triggerPropagator(IStorage $storage, $internalPath): void {
248247
$storage->getPropagator()->propagateChange($internalPath, time());
249248
}
250249
}

lib/private/Files/View.php

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1570,10 +1570,6 @@ public function getDirectoryContent(string $directory, ?string $mimeTypeFilter =
15701570
//add a folder for any mountpoint in this directory and add the sizes of other mountpoints to the folders
15711571
$mounts = Filesystem::getMountManager()->findIn($path);
15721572

1573-
// make sure nested mounts are sorted after their parent mounts
1574-
// otherwise doesn't propagate the etag across storage boundaries correctly
1575-
usort($mounts, static fn (IMountPoint $a, IMountPoint $b): int => $a->getMountPoint() <=> $b->getMountPoint());
1576-
15771573
$dirLength = strlen($path);
15781574
foreach ($mounts as $mount) {
15791575
$mountPoint = $mount->getMountPoint();

lib/public/Files/IRootFolder.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ public function getByIdInPath(int $id, string $path);
6161
public function getFirstNodeByIdInPath(int $id, string $path): ?Node;
6262

6363
/**
64-
* @return IMountPoint[]
64+
* @return list<IMountPoint>
6565
*
6666
* @since 28.0.0
6767
*/

lib/public/Files/Mount/IMountManager.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ public function find(string $path): ?IMountPoint;
5555
* Find all mounts in $path
5656
*
5757
* @param string $path
58-
* @return IMountPoint[]
58+
* @return list<IMountPoint> Returns a sorted list of mount point
5959
* @since 8.2.0
6060
*/
6161
public function findIn(string $path): array;

tests/lib/Files/Utils/ScannerTest.php

Lines changed: 24 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
<?php
22

3+
declare(strict_types=1);
4+
35
/**
46
* SPDX-FileCopyrightText: 2016-2024 Nextcloud GmbH and Nextcloud contributors
57
* SPDX-FileCopyrightText: 2016 ownCloud, Inc.
@@ -21,45 +23,35 @@
2123
use OCP\IUser;
2224
use OCP\IUserManager;
2325
use OCP\Server;
26+
use PHPUnit\Framework\Attributes\DataProvider;
27+
use PHPUnit\Framework\Attributes\Group;
2428
use Psr\Log\LoggerInterface;
29+
use Test\TestCase;
30+
use Test\Util\User\Dummy;
2531

2632
class TestScanner extends Scanner {
27-
/**
28-
* @var MountPoint[] $mounts
29-
*/
30-
private $mounts = [];
31-
32-
/**
33-
* @param MountPoint $mount
34-
*/
35-
public function addMount($mount) {
36-
$this->mounts[] = $mount;
33+
/** @var array<string, MountPoint> $mounts */
34+
private array $mounts = [];
35+
36+
public function addMount(MountPoint $mount): void {
37+
$this->mounts[$mount->getMountPoint()] = $mount;
3738
}
3839

3940
#[\Override]
40-
protected function getMounts($dir) {
41+
protected function getMounts(string $dir): array {
4142
return $this->mounts;
4243
}
4344
}
4445

45-
/**
46-
* Class ScannerTest
47-
*
48-
*
49-
* @package Test\Files\Utils
50-
*/
51-
#[\PHPUnit\Framework\Attributes\Group('DB')]
52-
class ScannerTest extends \Test\TestCase {
53-
/**
54-
* @var \Test\Util\User\Dummy
55-
*/
56-
private $userBackend;
46+
#[Group('DB')]
47+
class ScannerTest extends TestCase {
48+
private Dummy $userBackend;
5749

5850
#[\Override]
5951
protected function setUp(): void {
6052
parent::setUp();
6153

62-
$this->userBackend = new \Test\Util\User\Dummy();
54+
$this->userBackend = new Dummy();
6355
Server::get(IUserManager::class)->registerBackend($this->userBackend);
6456
$this->loginAsUser();
6557
}
@@ -166,25 +158,14 @@ public function testScanSubMount(): void {
166158
$this->assertTrue($cache->inCache('folder/bar.txt'));
167159
}
168160

169-
public static function invalidPathProvider(): array {
170-
return [
171-
[
172-
'../',
173-
],
174-
[
175-
'..\\',
176-
],
177-
[
178-
'../..\\../',
179-
],
180-
];
161+
public static function invalidPathProvider(): \Generator {
162+
yield [ '../' ];
163+
yield [ '..\\' ];
164+
yield [ '../..\\../' ];
181165
}
182166

183-
/**
184-
* @param string $invalidPath
185-
*/
186-
#[\PHPUnit\Framework\Attributes\DataProvider('invalidPathProvider')]
187-
public function testInvalidPathScanning($invalidPath): void {
167+
#[DataProvider(methodName: 'invalidPathProvider')]
168+
public function testInvalidPathScanning(string $invalidPath): void {
188169
$this->expectException(\InvalidArgumentException::class);
189170
$this->expectExceptionMessage('Invalid path to scan');
190171

@@ -249,14 +230,14 @@ public function testShallow(): void {
249230
);
250231
$scanner->addMount($mount);
251232

252-
$scanner->scan('', $recusive = false);
233+
$scanner->scan('', false);
253234
$this->assertTrue($cache->inCache('folder'));
254235
$this->assertFalse($cache->inCache('folder/subfolder'));
255236
$this->assertTrue($cache->inCache('foo.txt'));
256237
$this->assertFalse($cache->inCache('folder/bar.txt'));
257238
$this->assertFalse($cache->inCache('folder/subfolder/foobar.txt'));
258239

259-
$scanner->scan('folder', $recusive = false);
240+
$scanner->scan('folder', false);
260241
$this->assertTrue($cache->inCache('folder'));
261242
$this->assertTrue($cache->inCache('folder/subfolder'));
262243
$this->assertTrue($cache->inCache('foo.txt'));

0 commit comments

Comments
 (0)