Catalog image resizing performance problems

[hostep]

Preconditions

1. Magento develop branch (1856c28 - January 15 2017)
2. PHP 7.0.14

Steps to reproduce

1. Have a shop with a bunch of products with images assigned to them
2. Run bin/magento catalog:images:resize

Expected result

1. The command should only produced images for the themes which are currently active in the frontend
2. The command shouldn't produce multiple files which are binary exactly the same

Actual result

1. The command produces files for all the installed themes, even if they aren't being used in the frontend
2. The command produced files multiple times which are binary exactly the same

Discussion

While trying to figure out why it takes over 12 hours to run the command bin/magento catalog:image:resize on a very beefy server with a Magento CE 2.1.2 shop with about 6500 products and 8500 images, I found a couple of performance problems in the code:

1. The command generates resized images for all themes, so if you are not using Magento/blank or Magento/luma on the frontend, you still get images resized for those themes, and they take a lot of time to generate, so this is not good
2. After fixing nr 1, I looked at the generated files and compared them using a hashing function to see if there were duplicated files in the result. And this turned out to be the case.
   It was always caused by a difference in the image type (thumbnail, small_image, image, ...). All the other parameters (width, height, keep frame, transparency, quality, ...) to generate a unique file are correct I think. But I don't think the distinction on image type is important, but maybe I'm missing something?

Possible solution

1. I created a PR over here: Only resize images for themes which are being used instead of all installed themes. #8142
2. In Magento\Catalog\Model\Product\Image::getMiscParams, remove the line:
   'image_type' => $this->getDestinationSubdir(),

Result

I ran a very small benchmark using a test shop with 3 products and 5 images in total. Only Magento/blank and Magento/luma themes are installed and only Magento/luma is active in frontend:

1. Originally: 11 seconds, 165 files are generated.
   After applying PR: 9 seconds, 135 files are generated
2. Originally: 11 seconds, 165 files are generated.
   After removing line mentioned above: 9 seconds, 130 files are generated

Combined result of both optimizations:

• Originally: 11 seconds, 165 files are generated.
  After both optimizations: 7 seconds, 105 files are generated

I think this is a significant enough improvement for you guys to at least consider optimizing this :)

Thanks!

[choukalos]

Tracking internally in one ticket as MAGETWO-60316

[onepack]

What is the status of this? It is a BIG issue.
One shop is now short of space as we had a test and production site that were identical.
200GB was NOT enough as the image directory alone was using between 50 and 60 GB x2 (dev and prod) and then the rest of the space on the server was used by logs and cache etc.
First after the migration of magento 1 to two I did not check about the resources used by the images but when the shop almost went down I found out this was the source of the problems.
When 200GB is not enough to run a testing and production site with about 6000 products then magento is just not doing this right.

[hostep]

This is a small update with some new observations seen by testing Magento CE 2.1.6. Watch out, my initial post was about the develop branch, It looks like the develop branch is now a lot different then Magento 2.1.6 for this kind of functionality. Anyway, this is an update specifically for version 2.1.6.

In my initial post, for the second problem, I suggested to remove the image_type from Magento\Catalog\Model\Product\Image::getMiscParams, but it looks like while testing Magento CE 2.1.6 that this isn't the correct location to remove it. In Magento CE 2.1.6, it's better to remove it from Magento\Catalog\Model\Product\Image\ParamsBuilder::build

[hostep]

And another update: instead of removing image_type altogether, we should hardcode it to a certain value which is being used today, like thumbnail, this will avoid having to run bin/magento catalog:images:resize yet again. Otherwise it would change the hash being used in the directory structure again and all the product images would disappear from the product listing pages again, just like they did after the upgrade to Magento 2.1.6.

[hostep]

And yet another update. Manipulating image_type in Magento\Catalog\Model\Product\Image\ParamsBuilder::build which I mentioned above, isn't the right solution, as image_type is being used in Magento\Catalog\Model\View\Asset\Image::getPlaceHolder to figure out what placeholder to use.

My new proposition is to change this in Magento\Catalog\Model\View\Asset\Image::getMiscPath as follows:

diff --git a/app/code/Magento/Catalog/Model/View/Asset/Image.php b/app/code/Magento/Catalog/Model/View/Asset/Image.php
index 05f7044cbf1..0fd6690224d 100755
--- a/app/code/Magento/Catalog/Model/View/Asset/Image.php
+++ b/app/code/Magento/Catalog/Model/View/Asset/Image.php
@@ -194,7 +194,20 @@ class Image implements LocalInterface
      */
     private function getMiscPath()
     {
-        return $this->encryptor->hash(implode('_', $this->miscParams), Encryptor::HASH_VERSION_MD5);
+        $miscParams = $this->miscParams;
+
+        // since 'image_type' has no influence as to how an image is manipulated (the resulting files are binary the same if all the other params match),
+        // it makes no sense to include it in the hash caluclation
+        // the best solution would be to remove it, but to avoid introducing new backwards incompatible hashes being generated here,
+        // we decided to hardcode 'image_type' to one which was already in use before: 'thumbnail'
+        // if we simply removed it, we would have to re-run `bin/magento catalog:images:resize`, to again generate all new images, and that's just very annoying
+
+        if (isset($miscParams['image_type']))
+        {
+            $miscParams['image_type'] = 'thumbnail';
+        }
+
+        return $this->encryptor->hash(implode('_', $miscParams), Encryptor::HASH_VERSION_MD5);
     }

     /**

This is for Magento 2.1.6, I haven't checked out the latest develop branch code, to see if this also applies over there.