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.

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

option to use imagick in preview generation with extended opts #1

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

nachoparker
Copy link
Owner

@nachoparker nachoparker commented Jun 28, 2019

This PR is a PoC that introduces the option of using Imagick for JPEG images in the previews engine.

Motivation

See this blog post.

There are two main areas where we could try to work to optimize tis

Description

This is an attempt at improving this situation through a combination of

  • Nextcloud server changes (this PR)
  • Preview Generator changes (this other PR -> rework to support multiple concurrent instances previewgenerator#1)
  • Proper configuration of the preview generator to generate the sizes that Files and Gallery really use.
  • New switches to allow fine grained control of quality, image size, filters used and so on.

This PR would be part number one of said attempt.

The main attractive of imagick in native multithreaded execution if compiled with openMP (the default at least in Debian).

Since we are not interested in implementing everything from scratch but only try to improve the most performance sensitive areas, I have extended OC_Image with a new class that over-rides some if its methods.

Since there are concerns about using imagick at all and in order to allow flexibility this new mechanism is optional. It can be enabled and tweaked through config options.

  'preview_use_imagick' => true,
  'preview_interpolate' => false,
  'preview_thread_limit' => 8,

preview_use_imagick = false or the lack of php-imagick revert to the old behavior using GD. Also we allow to disable interpolation to achieve faster previews, which in my opinion is mostly OK since it doesn't matter that much for small thumbnails. An alternative option is to only enable interpolation for the max preview (the one that shows big if you click on it in the gallery).

Goal

Generate the previews as fast as possible without sacrificing much quality.

Testing procedure

I tested these changes with a dataset comprised of 1.5GiB of high quality photos (~100 x 15MiB photos) on a 16 core x86 machine with 64MiB of RAM.

For the reasons stated above, I ran the tests with the following configuration

occ config:app:set previewgenerator squareSizes --value="32 256"                                                                                                                                                                                                                                                               
occ config:app:set previewgenerator widthSizes  --value="256 384"
occ config:app:set previewgenerator heightSizes --value="256"
occ config:system:set preview_max_x --value 2048
occ config:system:set preview_max_y --value 2048

All the files are already uploaded. The preview cache is cleaned every run with

rm -r data/appdata_*/preview/*
occ files:scan-app-data

In Imagick we are using a CATROM filter.

Results

Method Processes Threads Interpolate JPEG quality Time % of initial (proposed config) % of initial (default config)
GD (*) 1 1 yes 90 7m16 - -
GD 1 1 yes 90 1m50 - 25%
GD 1 1 yes 60 1m50 - 25%
GD 1 1 no (**) 90 48s 43% 11%
GD 1 1 no (**) 60 48s 43% 11%
Imagick 1 1 yes 90 1m53 102% 25%
Imagick 1 1 yes 60 1m51 100% 25%
Imagick 1 1 no 90 1m26 78% 19%
Imagick 1 1 no 60 1m23 75% 19%
Imagick 1 16 yes 90 1m18 71% 15%
Imagick 1 16 yes 60 1m22 74% 18%
----------------- ----------- --------- ------------- -------------- ------ ---------- ----------
GD 4 1 yes 60 28s 25% 6%
Imagick 4 1 no 90 23s 20% 5%
Imagick 4 1 no 60 22s 20% 5%
GD 16 1 yes 60 12s 11% 3%
Imagick 16 1 no 90 10s 9% 2%
Imagick 16 1 no 60 10s 9% 2%

(*) Default settings: 4096x4096 Max preview, all sizes enabled
(**) Awful, unacceptable quality

The multi-process tests (bottom half) are run using the modified Preview Generator -> nachoparker/previewgenerator#1

Conclusions

Imagick can outperform GD while retaining decent quality mainly due to the fact that is multi-threaded. Still the big bottleneck is in the PHP non native part. In order to improve that something like nextcloud#14953 would need to be implemented, or maybe try to optimize the current Previews code flow.

Since this is the case, better results are achieved by using the "multi-process" approach (nachoparker/previewgenerator#1). Even without server modifications, using the traditional GD method we can achieve a 4x speed boost, which would be 5x with Imagick (with the proposed settings).

Just configuring the Preview Generator properly will imply a 4x speed boost and lots of space saved without any other changes.

JPEG quality barely affects performance, but it mostly only affects storage space.

With these changes the user has more options to tweak performance according to their preferences and how powerful their machine is.

Future work

  • Study generating the images in the upload hook with multiple threads so we don't need the preview generator cron job.
  • Modify Preview Generator to spawn multiple threads using pthreads or parallel once there are ZTS PHP packages available (link link) instead of relying on an external script that launches several processes in parallel.

Signed-off-by: nachoparker nacho@ownyourbits.com

Signed-off-by: nachoparker <nacho@ownyourbits.com>
Signed-off-by: nachoparker <nacho@ownyourbits.com>
Signed-off-by: nachoparker <nacho@ownyourbits.com>
@stalker314314
Copy link

I really like this. Your configuration for preview generator and preview sizes are doing wonders on my setup! Thanks! Getting imagick in Nextcloud would be nice, but I understand it may be tough. I would still like to see it as first-class citizen (of course, under preview_use_imagick flag, with default to false). I looked at your changes, seems nice, I like factory pattern for getImage. Not sure if all those methods from JPEG class, should just end up in OC_Image_JPEG? Do you have any plans to push this to official Nextcloud repo?

@nachoparker
Copy link
Owner Author

nachoparker commented Jul 3, 2019

Thanks

Do you have any plans to push this to official Nextcloud repo?

Yes, I sent it for review. You can show your support there ;)

nextcloud#16158

@nachoparker
Copy link
Owner Author

I only did basic changes in order to test the improvement in performance, but in order for it to be merged it will probably be tidied up a bit, at least reorganized into separate files and so on.

Let's keep the conversation in the NC PR. Will block comments here

Repository owner locked and limited conversation to collaborators Jul 3, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants