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.

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

Jump to bottom

[Performance] Reuse preview directory listing #35129

Merged
merged 8 commits into from
Apr 17, 2023
Merged

[Performance] Reuse preview directory listing #35129

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
9b4d514
List preview directory only once
Glandos Nov 1, 2022
b6c65fe
php-cs-fix
Glandos Nov 1, 2022
4954bea
Ensure max preview image is not null
Glandos Nov 1, 2022
06a7e90
improve parameter doc
Glandos Nov 2, 2022
24c1213
Revert 0e49b40
Glandos Nov 2, 2022
384a1d3
adapt test to new workflow
Glandos Nov 3, 2022
3440387
gather code from small and max preview
Glandos Nov 5, 2022
e542e60
try to make linters happy
Glandos Nov 13, 2022
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
140 changes: 47 additions & 93 deletions lib/private/Preview/Generator.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,8 @@ public function generatePreviews(File $file, array $specifications, $mimeType =
}

$previewFolder = $this->getPreviewFolder($file);
// List every existing preview first instead of trying to find them one by one
$previewFiles = $previewFolder->getDirectoryListing();

$previewVersion = '';
if ($file instanceof IVersionedPreviewFile) {
Expand All @@ -150,7 +152,7 @@ public function generatePreviews(File $file, array $specifications, $mimeType =
&& preg_match(Imaginary::supportedMimeTypes(), $mimeType)
&& $this->config->getSystemValueString('preview_imaginary_url', 'invalid') !== 'invalid') {
$crop = $specifications[0]['crop'] ?? false;
$preview = $this->getSmallImagePreview($previewFolder, $file, $mimeType, $previewVersion, $crop);
$preview = $this->getSmallImagePreview($previewFolder, $previewFiles, $file, $mimeType, $previewVersion, $crop);

if ($preview->getSize() === 0) {
$preview->delete();
Expand All @@ -161,7 +163,7 @@ public function generatePreviews(File $file, array $specifications, $mimeType =
}

// Get the max preview and infer the max preview sizes from that
$maxPreview = $this->getMaxPreview($previewFolder, $file, $mimeType, $previewVersion);
$maxPreview = $this->getMaxPreview($previewFolder, $previewFiles, $file, $mimeType, $previewVersion);
$maxPreviewImage = null; // only load the image when we need it
if ($maxPreview->getSize() === 0) {
$maxPreview->delete();
Expand Down Expand Up @@ -197,7 +199,7 @@ public function generatePreviews(File $file, array $specifications, $mimeType =
// Try to get a cached preview. Else generate (and store) one
try {
try {
$preview = $this->getCachedPreview($previewFolder, $width, $height, $crop, $maxPreview->getMimeType(), $previewVersion);
$preview = $this->getCachedPreview($previewFiles, $width, $height, $crop, $maxPreview->getMimeType(), $previewVersion);
} catch (NotFoundException $e) {
if (!$this->previewManager->isMimeSupported($mimeType)) {
throw new NotFoundException();
Expand All @@ -208,6 +210,8 @@ public function generatePreviews(File $file, array $specifications, $mimeType =
}

$preview = $this->generatePreview($previewFolder, $maxPreviewImage, $width, $height, $crop, $maxWidth, $maxHeight, $previewVersion);
// New file, augment our array
$previewFiles[] = $preview;
}
} catch (\InvalidArgumentException $e) {
throw new NotFoundException("", 0, $e);
Expand All @@ -233,75 +237,19 @@ public function generatePreviews(File $file, array $specifications, $mimeType =
* Generate a small image straight away without generating a max preview first
* Preview generated is 256x256
*
* @param ISimpleFile[] $previewFiles
*
* @throws NotFoundException
*/
private function getSmallImagePreview(ISimpleFolder $previewFolder, File $file, string $mimeType, string $prefix, bool $crop): ISimpleFile {
$nodes = $previewFolder->getDirectoryListing();

foreach ($nodes as $node) {
$name = $node->getName();
if (($prefix === '' || str_starts_with($name, $prefix))) {
// Prefix match
if (str_starts_with($name, $prefix . '256-256-crop') && $crop) {
// Cropped image
return $node;
}

if (str_starts_with($name, $prefix . '256-256.') && !$crop) {
// Uncropped image
return $node;
}
}
}

$previewProviders = $this->previewManager->getProviders();
foreach ($previewProviders as $supportedMimeType => $providers) {
// Filter out providers that does not support this mime
if (!preg_match($supportedMimeType, $mimeType)) {
continue;
}

foreach ($providers as $providerClosure) {
$provider = $this->helper->getProvider($providerClosure);
if (!($provider instanceof IProviderV2)) {
continue;
}

if (!$provider->isAvailable($file)) {
continue;
}

$preview = $this->helper->getThumbnail($provider, $file, 256, 256, $crop);

if (!($preview instanceof IImage)) {
continue;
}

// Try to get the extension.
try {
$ext = $this->getExtention($preview->dataMimeType());
} catch (\InvalidArgumentException $e) {
// Just continue to the next iteration if this preview doesn't have a valid mimetype
continue;
}

$path = $this->generatePath(256, 256, $crop, $preview->dataMimeType(), $prefix);
try {
$file = $previewFolder->newFile($path);
if ($preview instanceof IStreamImage) {
$file->putContent($preview->resource());
} else {
$file->putContent($preview->data());
}
} catch (NotPermittedException $e) {
throw new NotFoundException();
}
private function getSmallImagePreview(ISimpleFolder $previewFolder, array $previewFiles, File $file, string $mimeType, string $prefix, bool $crop): ISimpleFile {
$width = 256;
$height = 256;

return $file;
}
try {
return $this->getCachedPreview($previewFiles, $width, $height, $crop, $mimeType, $prefix);
} catch (NotFoundException $e) {
return $this->generateProviderPreview($previewFolder, $file, $width, $height, $crop, false, $mimeType, $prefix);
}

throw new NotFoundException('No provider successfully handled the preview generation');
}

/**
Expand Down Expand Up @@ -398,22 +346,30 @@ public function getNumConcurrentPreviews(string $type): int {

/**
* @param ISimpleFolder $previewFolder
* @param ISimpleFile[] $previewFiles
* @param File $file
* @param string $mimeType
* @param string $prefix
* @return ISimpleFile
* @throws NotFoundException
*/
private function getMaxPreview(ISimpleFolder $previewFolder, File $file, $mimeType, $prefix) {
$nodes = $previewFolder->getDirectoryListing();

foreach ($nodes as $node) {
private function getMaxPreview(ISimpleFolder $previewFolder, array $previewFiles, File $file, $mimeType, $prefix) {
// We don't know the max preview size, so we can't use getCachedPreview.
// It might have been generated with a higher resolution than the current value.
kesselb marked this conversation as resolved.
Show resolved Hide resolved
foreach ($previewFiles as $node) {
$name = $node->getName();
if (($prefix === '' || strpos($name, $prefix) === 0) && strpos($name, 'max')) {
return $node;
}
}

$maxWidth = $this->config->getSystemValueInt('preview_max_x', 4096);
$maxHeight = $this->config->getSystemValueInt('preview_max_y', 4096);

return $this->generateProviderPreview($previewFolder, $file, $maxWidth, $maxHeight, false, true, $mimeType, $prefix);
}

private function generateProviderPreview(ISimpleFolder $previewFolder, File $file, int $width, int $height, bool $crop, bool $max, string $mimeType, string $prefix) {
$previewProviders = $this->previewManager->getProviders();
foreach ($previewProviders as $supportedMimeType => $providers) {
// Filter out providers that does not support this mime
Expand All @@ -431,13 +387,10 @@ private function getMaxPreview(ISimpleFolder $previewFolder, File $file, $mimeTy
continue;
}

$maxWidth = $this->config->getSystemValueInt('preview_max_x', 4096);
$maxHeight = $this->config->getSystemValueInt('preview_max_y', 4096);

$previewConcurrency = $this->getNumConcurrentPreviews('preview_concurrency_new');
$sem = self::guardWithSemaphore(self::SEMAPHORE_ID_NEW, $previewConcurrency);
try {
$preview = $this->helper->getThumbnail($provider, $file, $maxWidth, $maxHeight);
$preview = $this->helper->getThumbnail($provider, $file, $width, $height);
} finally {
self::unguardWithSemaphore($sem);
}
Expand All @@ -446,15 +399,7 @@ private function getMaxPreview(ISimpleFolder $previewFolder, File $file, $mimeTy
continue;
}

// Try to get the extention.
try {
$ext = $this->getExtention($preview->dataMimeType());
} catch (\InvalidArgumentException $e) {
// Just continue to the next iteration if this preview doesn't have a valid mimetype
continue;
}

$path = $prefix . (string)$preview->width() . '-' . (string)$preview->height() . '-max.' . $ext;
$path = $this->generatePath($preview->width(), $preview->height(), $crop, $max, $preview->dataMimeType(), $prefix);
try {
$file = $previewFolder->newFile($path);
if ($preview instanceof IStreamImage) {
Expand All @@ -470,7 +415,7 @@ private function getMaxPreview(ISimpleFolder $previewFolder, File $file, $mimeTy
}
}

throw new NotFoundException();
throw new NotFoundException('No provider successfully handled the preview generation');
}

/**
Expand All @@ -487,15 +432,19 @@ private function getPreviewSize(ISimpleFile $file, string $prefix = '') {
* @param int $width
* @param int $height
* @param bool $crop
* @param bool $max
* @param string $mimeType
* @param string $prefix
* @return string
*/
private function generatePath($width, $height, $crop, $mimeType, $prefix) {
private function generatePath($width, $height, $crop, $max, $mimeType, $prefix) {
$path = $prefix . (string)$width . '-' . (string)$height;
if ($crop) {
$path .= '-crop';
}
if ($max) {
$path .= '-max';
}

$ext = $this->getExtention($mimeType);
$path .= '.' . $ext;
Expand Down Expand Up @@ -637,7 +586,8 @@ private function generatePreview(ISimpleFolder $previewFolder, IImage $maxPrevie
self::unguardWithSemaphore($sem);
}

$path = $this->generatePath($width, $height, $crop, $preview->dataMimeType(), $prefix);

$path = $this->generatePath($width, $height, $crop, false, $preview->dataMimeType(), $prefix);
try {
$file = $previewFolder->newFile($path);
$file->putContent($preview->data());
Expand All @@ -649,7 +599,7 @@ private function generatePreview(ISimpleFolder $previewFolder, IImage $maxPrevie
}

/**
* @param ISimpleFolder $previewFolder
* @param ISimpleFile[] $files Array of FileInfo, as the result of getDirectoryListing()
* @param int $width
* @param int $height
* @param bool $crop
Expand All @@ -659,10 +609,14 @@ private function generatePreview(ISimpleFolder $previewFolder, IImage $maxPrevie
*
* @throws NotFoundException
*/
private function getCachedPreview(ISimpleFolder $previewFolder, $width, $height, $crop, $mimeType, $prefix) {
$path = $this->generatePath($width, $height, $crop, $mimeType, $prefix);

return $previewFolder->getFile($path);
private function getCachedPreview($files, $width, $height, $crop, $mimeType, $prefix) {
$path = $this->generatePath($width, $height, $crop, false, $mimeType, $prefix);
foreach ($files as $file) {
if ($file->getName() === $path) {
return $file;
}
}
throw new NotFoundException();
}

/**
Expand Down
19 changes: 7 additions & 12 deletions tests/lib/Preview/GeneratorTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,15 +105,12 @@ public function testGetCachedPreview() {
$maxPreview->method('getMimeType')
->willReturn('image/png');

$previewFolder->method('getDirectoryListing')
->willReturn([$maxPreview]);

$previewFile = $this->createMock(ISimpleFile::class);
$previewFile->method('getSize')->willReturn(1000);
$previewFile->method('getName')->willReturn('256-256.png');

$previewFolder->method('getFile')
->with($this->equalTo('256-256.png'))
->willReturn($previewFile);
$previewFolder->method('getDirectoryListing')
->willReturn([$maxPreview, $previewFile]);

$this->legacyEventDispatcher->expects($this->once())
->method('dispatch')
Expand Down Expand Up @@ -344,14 +341,12 @@ public function testReturnCachedPreviewsWithoutCheckingSupportedMimetype() {
$maxPreview->method('getMimeType')
->willReturn('image/png');

$previewFolder->method('getDirectoryListing')
->willReturn([$maxPreview]);

$preview = $this->createMock(ISimpleFile::class);
$preview->method('getSize')->willReturn(1000);
$previewFolder->method('getFile')
->with($this->equalTo('1024-512-crop.png'))
->willReturn($preview);
$preview->method('getName')->willReturn('1024-512-crop.png');

$previewFolder->method('getDirectoryListing')
->willReturn([$maxPreview, $preview]);

$this->previewManager->expects($this->never())
->method('isMimeSupported');
Expand Down