Project

General

Profile

Actions

Feature #101446

open

Prevent re processing images with "same" processing instructions

Added by Daniel Siepmann 9 months ago. Updated 7 months ago.

Status:
Accepted
Priority:
Should have
Assignee:
-
Category:
Image Generation / GIFBUILDER
Target version:
-
Start date:
2023-07-26
Due date:
% Done:

0%

Estimated time:
PHP Version:
8.1
Tags:
Complexity:
Sprint Focus:

Description

Foremost I'm not sure if TYPO3 should change that. Still I try to explain our scenario and a possible solution. I'm fine if TYPO3 decides to reject the issue.

Environment:
TYPO3 11.5.23
PHP 8.1

Our scenario:
We had a small code that called applyProcessingInstructions() of TYPO3\CMS\Extbase\Service\ImageService
The code looked like this:

$cropParam = ($crop)?'c':'';
$processingInstructions = [
    'width' => $imageWidth . $cropParam,
    'crop' => $this->getCropArea($imageObj, $cropVariant),
];
if ($forceRatio > 0) {
    $processingInstructions['height'] = ($crop) ? $imageWidth / $forceRatio . $cropParam : null;
}
$processedImage = $this->imageService->applyProcessingInstructions($imageObj, $processingInstructions);

We than needed to make changes to the code base and ended up with the following code:

$cropParam = ($crop) ? 'c' : '';
$processingInstructions = [
    'width' => $imageWidth . $cropParam,
    'height' => null,
    'crop' => $this->getCropArea($image, $cropVariant),
];
if ($forceRatio > 0 && $crop) {
    $processingInstructions['height'] = ($imageWidth / $forceRatio) . $cropParam;
}

return $this->imageService->applyProcessingInstructions(
    $image,
    $processingInstructions
);

We didn't expect anything bad from that change. Once deployed our system went down due to heavy system load, caused by many gm (=graphics magick) processed.
We reverted the change and thanks to Hannes with his experience we found the root cause. We modified the $processingInstructions that is used to create a hash in order to find entries from database table sys_file_processedfile. Our modifications lead to a different array (height before, crop and height now always was set). That lead to a different hash leading to not finding processed images, leading to re-processing all images.

My proposal:
Add some streamlining to the incoming processing instructions. E.g. alphabetical sorting, array filter to filter out empty values like `null`. Something in that regard should prevent such issues for systems.
It forces an update wizard in order to update all existing entries with the sanitized and updated hash version in order to prevent re processing all images after system update.

Actions #1

Updated by Daniel Siepmann 9 months ago

  • Description updated (diff)
Actions #2

Updated by Benni Mack 7 months ago

  • Status changed from New to Accepted

I agree and I know the problem:

I've put some work in a "pre-patch": https://review.typo3.org/c/Packages/TYPO3.CMS/+/81153

There is a comment that we should actually do a ksort() at some point, which I think is necessary, but then we'd need to re-process all processed files.

Actions #3

Updated by Daniel Siepmann 7 months ago

Does the order actually change the processed files? Otherwise an update wizard could be created that iterates over all existing processed db entries. Each entry still has the config, this can be deserialized, normalized, written back, together with an updated configurationsha1.

That way the existing data can be migrated.

But maybe I'm missing something here and take things too easy.

Actions

Also available in: Atom PDF