From 9b4d5146d5e0fc8b2c25d2e0e0976c2489cb9659 Mon Sep 17 00:00:00 2001 From: Glandos Date: Tue, 1 Nov 2022 16:15:30 +0100 Subject: List preview directory only once getCachedPreview used to call `getFile`, and this calls `getDirectoryListing` (or underlying function that list directory) to find the file. This was done for every preview spec. Now, this is done only once at the beginning of the loop, and the array is just iterated when needed to find the correct entry. Signed-off-by: Glandos --- lib/private/Preview/Generator.php | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) (limited to 'lib') diff --git a/lib/private/Preview/Generator.php b/lib/private/Preview/Generator.php index 47f2952c6e6..a005d46adb4 100644 --- a/lib/private/Preview/Generator.php +++ b/lib/private/Preview/Generator.php @@ -171,6 +171,8 @@ class Generator { [$maxWidth, $maxHeight] = $this->getPreviewSize($maxPreview, $previewVersion); $preview = null; + // List every existing preview first instead of trying to find them one by one + $previewFiles = $previewFolder->getDirectoryListing(); foreach ($specifications as $specification) { $width = $specification['width'] ?? -1; @@ -197,7 +199,7 @@ class Generator { // 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(); @@ -208,6 +210,8 @@ class Generator { } $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); @@ -649,7 +653,7 @@ class Generator { } /** - * @param ISimpleFolder $previewFolder + * @param array $files Array of FileInfo, as the result of getDirectoryListing() * @param int $width * @param int $height * @param bool $crop @@ -659,10 +663,14 @@ class Generator { * * @throws NotFoundException */ - private function getCachedPreview(ISimpleFolder $previewFolder, $width, $height, $crop, $mimeType, $prefix) { + private function getCachedPreview($files, $width, $height, $crop, $mimeType, $prefix) { $path = $this->generatePath($width, $height, $crop, $mimeType, $prefix); - - return $previewFolder->getFile($path); + foreach($files as $file) { + if ($file->getName() === $path) { + return $file; + } + } + throw new NotFoundException(); } /** -- cgit v1.2.3 From b6c65fee2acf14bd50daada653d3a40c663d16eb Mon Sep 17 00:00:00 2001 From: Glandos Date: Tue, 1 Nov 2022 22:09:50 +0100 Subject: php-cs-fix Signed-off-by: Glandos --- lib/private/Preview/Generator.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/private/Preview/Generator.php b/lib/private/Preview/Generator.php index a005d46adb4..fb57d5a78be 100644 --- a/lib/private/Preview/Generator.php +++ b/lib/private/Preview/Generator.php @@ -665,7 +665,7 @@ class Generator { */ private function getCachedPreview($files, $width, $height, $crop, $mimeType, $prefix) { $path = $this->generatePath($width, $height, $crop, $mimeType, $prefix); - foreach($files as $file) { + foreach ($files as $file) { if ($file->getName() === $path) { return $file; } -- cgit v1.2.3 From 4954bead1dc197217c068e25a8fc27e01c1a874e Mon Sep 17 00:00:00 2001 From: Glandos Date: Tue, 1 Nov 2022 22:16:56 +0100 Subject: Ensure max preview image is not null Signed-off-by: Glandos --- lib/private/Preview/Generator.php | 3 +++ 1 file changed, 3 insertions(+) (limited to 'lib') diff --git a/lib/private/Preview/Generator.php b/lib/private/Preview/Generator.php index fb57d5a78be..3ca84a9b6f6 100644 --- a/lib/private/Preview/Generator.php +++ b/lib/private/Preview/Generator.php @@ -208,6 +208,9 @@ class Generator { if ($maxPreviewImage === null) { $maxPreviewImage = $this->helper->getImage($maxPreview); } + if ($maxPreviewImage === null) { + throw new NotFoundException(); + } $preview = $this->generatePreview($previewFolder, $maxPreviewImage, $width, $height, $crop, $maxWidth, $maxHeight, $previewVersion); // New file, augment our array -- cgit v1.2.3 From 06a7e903830979556e6b582df0c5be648f465da1 Mon Sep 17 00:00:00 2001 From: Glandos Date: Wed, 2 Nov 2022 21:35:16 +0100 Subject: improve parameter doc Signed-off-by: Glandos --- lib/private/Preview/Generator.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/private/Preview/Generator.php b/lib/private/Preview/Generator.php index 3ca84a9b6f6..f5fb46f2c49 100644 --- a/lib/private/Preview/Generator.php +++ b/lib/private/Preview/Generator.php @@ -656,7 +656,7 @@ class Generator { } /** - * @param array $files Array of FileInfo, as the result of getDirectoryListing() + * @param ISimpleFile[] $files Array of FileInfo, as the result of getDirectoryListing() * @param int $width * @param int $height * @param bool $crop -- cgit v1.2.3 From 24c121347a402e26bafb6366ff7854f488d1b8b5 Mon Sep 17 00:00:00 2001 From: Glandos Date: Wed, 2 Nov 2022 21:35:54 +0100 Subject: Revert 0e49b40 Signed-off-by: Glandos --- lib/private/Preview/Generator.php | 3 --- 1 file changed, 3 deletions(-) (limited to 'lib') diff --git a/lib/private/Preview/Generator.php b/lib/private/Preview/Generator.php index f5fb46f2c49..a0b955ae2b7 100644 --- a/lib/private/Preview/Generator.php +++ b/lib/private/Preview/Generator.php @@ -208,9 +208,6 @@ class Generator { if ($maxPreviewImage === null) { $maxPreviewImage = $this->helper->getImage($maxPreview); } - if ($maxPreviewImage === null) { - throw new NotFoundException(); - } $preview = $this->generatePreview($previewFolder, $maxPreviewImage, $width, $height, $crop, $maxWidth, $maxHeight, $previewVersion); // New file, augment our array -- cgit v1.2.3 From 3440387f0f800cb4ee7380d59f65ccd64a265148 Mon Sep 17 00:00:00 2001 From: Glandos Date: Sat, 5 Nov 2022 23:03:58 +0100 Subject: gather code from small and max preview use directory listing in both functions to gain 25% speed on run where every preview already exist. Signed-off-by: Glandos --- lib/private/Preview/Generator.php | 126 +++++++++++--------------------------- 1 file changed, 36 insertions(+), 90 deletions(-) (limited to 'lib') diff --git a/lib/private/Preview/Generator.php b/lib/private/Preview/Generator.php index a0b955ae2b7..10a02d15fa1 100644 --- a/lib/private/Preview/Generator.php +++ b/lib/private/Preview/Generator.php @@ -137,6 +137,8 @@ class Generator { } $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) { @@ -150,7 +152,7 @@ class Generator { && 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(); @@ -161,7 +163,7 @@ class Generator { } // 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(); @@ -171,8 +173,6 @@ class Generator { [$maxWidth, $maxHeight] = $this->getPreviewSize($maxPreview, $previewVersion); $preview = null; - // List every existing preview first instead of trying to find them one by one - $previewFiles = $previewFolder->getDirectoryListing(); foreach ($specifications as $specification) { $width = $specification['width'] ?? -1; @@ -236,76 +236,20 @@ class Generator { /** * 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; - } + private function getSmallImagePreview(ISimpleFolder $previewFolder, array $previewFiles, File $file, string $mimeType, string $prefix, bool $crop): ISimpleFile { + $width = 256; + $height = 256; - // 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(); - } - - return $file; - } + try { + return $this->getCachedPreview($previewFiles, $width, $height, $crop, $mimeType, $prefix); + } catch (NotFoundException) { + return $this->generateProviderPreview($previewFolder, $file, $width, $height, $crop, false, $mimeType, $prefix); } - - throw new NotFoundException('No provider successfully handled the preview generation'); } /** @@ -402,22 +346,30 @@ class Generator { /** * @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. + 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 @@ -435,13 +387,10 @@ class Generator { 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); } @@ -450,15 +399,7 @@ class Generator { 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) { @@ -474,7 +415,7 @@ class Generator { } } - throw new NotFoundException(); + throw new NotFoundException('No provider successfully handled the preview generation'); } /** @@ -491,15 +432,19 @@ class Generator { * @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; @@ -641,7 +586,8 @@ class Generator { 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()); @@ -664,7 +610,7 @@ class Generator { * @throws NotFoundException */ private function getCachedPreview($files, $width, $height, $crop, $mimeType, $prefix) { - $path = $this->generatePath($width, $height, $crop, $mimeType, $prefix); + $path = $this->generatePath($width, $height, $crop, false, $mimeType, $prefix); foreach ($files as $file) { if ($file->getName() === $path) { return $file; -- cgit v1.2.3 From e542e60dbcb688d28f1335d20709555c0528e8cd Mon Sep 17 00:00:00 2001 From: Glandos Date: Sun, 13 Nov 2022 22:20:16 +0100 Subject: try to make linters happy Signed-off-by: Glandos --- lib/private/Preview/Generator.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'lib') diff --git a/lib/private/Preview/Generator.php b/lib/private/Preview/Generator.php index 10a02d15fa1..ed9474fafb2 100644 --- a/lib/private/Preview/Generator.php +++ b/lib/private/Preview/Generator.php @@ -236,7 +236,7 @@ class Generator { /** * Generate a small image straight away without generating a max preview first * Preview generated is 256x256 - * + * * @param ISimpleFile[] $previewFiles * * @throws NotFoundException @@ -247,7 +247,7 @@ class Generator { try { return $this->getCachedPreview($previewFiles, $width, $height, $crop, $mimeType, $prefix); - } catch (NotFoundException) { + } catch (NotFoundException $e) { return $this->generateProviderPreview($previewFolder, $file, $width, $height, $crop, false, $mimeType, $prefix); } } -- cgit v1.2.3