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

use thumbnail, if possible #1

Open
jcupitt opened this issue Sep 13, 2018 · 15 comments
Open

use thumbnail, if possible #1

jcupitt opened this issue Sep 13, 2018 · 15 comments

Comments

@jcupitt
Copy link

jcupitt commented Sep 13, 2018

Hello! I'm the libvips maintainer. This is very cool!

I had a couple of suggestions after looking through the code that could improve performance and drop memory use.

Use thumbnail, if you can

I would use thumbnail, if possible. It's much, much quicker than newFromBuffer and resize, especially for large reductions on JPG images, and will give higher quality results too, since it will automatically premultiply PNG alpha for you.

For example, I think this is roughly what you are doing at the moment:

#!/usr/bin/env php
<?php

require __DIR__ . '/vendor/autoload.php';

use Jcupitt\Vips;

# disable vips caching
Vips\Config::cacheSetMax(0);

for ($i = 0; $i < 100; $i++) {
  $contents = file_get_contents($argv[1]); 
  $image = Vips\Image::newFromBuffer($contents);
  $target_width = 256;
  $image = $image->resize($target_width / $image->width);
  $out = $image->writeToBuffer(".jpg");
}

If I run this on my laptop with a 6k x 4k JPG image, I see:

$ /usr/bin/time -f %M:%e ./try275.php ~/pics/theo.jpg
135168:18.91

ie. 135MB of memory, 19s of CPU. If I change it to be:

#!/usr/bin/env php
<?php

require __DIR__ . '/vendor/autoload.php';

use Jcupitt\Vips;

# disable libvips caching
Vips\Config::cacheSetMax(0);

for ($i = 0; $i < 100; $i++) {
  $image = Vips\Image::thumbnail($argv[1], 256);
  $out = $image->writeToBuffer(".jpg");
} 

I see:

$ /usr/bin/time -f %M:%e ./try276.php ~/pics/theo.jpg
51704:4.69

So 50MB of memory, under 5s of CPU.

You are cropping before resize, which you can't do with thumbnail. I would use thumbnail anyway, and crop afterwards using scaled down coordinates.

Disable caching

By default, libvips will cache the last 1,000 operations or so. This is usually great for performance, but you are not going to be repeatedly operating on the same image, so it is likely to bloat memory for no gain.

I would add:

Vips\Config::cacheSetMax(0);

To your startup code.

If I remove that line from try275.php (the non-thumbnail version above) I see:

$ /usr/bin/time -f %M:%e ./try275.php ~/pics/theo.jpg
213140:18.65

An extra 90MB of memory used for no speedup.

@jcupitt
Copy link
Author

jcupitt commented Sep 13, 2018

I thought of a couple more things.

thumbnail is quick because it's able to exploit JPEG shrink-on-load (probably obvious, but I forgot to say).

newFromBuffer itself is very quick (it just reads the header -- image decode is delayed until the first pixel value is needed), so you can still use that to get the image size before calling thumbnail with whatever parameters you need.

I guess there's a potential race between image processing starting and thumbnail being triggered. Is that why you use get_file_contents? You could still use get_file_contents and just switch to calling thumbnail_buffer for the resize.

There's a chapter in the docs about how file open works:

http://jcupitt.github.io/libvips/API/current/How-it-opens-files.md.html

Which might also help explain thumbnail.

Anyway, just suggestions, nice job on the plugin.

@joppuyo
Copy link
Member

joppuyo commented Sep 22, 2018

Hey, great to hear from you!

Thanks for the suggestions, I'm actually already in progress of adding thumbnail support to the plugin. There's some issues with compatibility as most linux systems come with binaries earlier than 8.5 but this can of course fixed by simple version check so those with a later version get the performance boost 🙂

I agree that some logic can be a bit strange because the plugin must adhere to WordPress image editor rules, but it's a good thing if using thumbnail_buffer instead of thumbnail doesn't have a lot of overhead!

I'll check out the effect of disabling the cache! If it cuts down memory use I'll be sure to include it in the next version.

@andrefelipe
Copy link

Thumbs up for VIPS support in WordPress, have been looking for a while.

It is not uncommon for clients to upload a 10.000px image for the website to process. I usually handle the job in the hardware manner (more memory, more swap to prevent the server to hang) but we know VIPS could have been solved it right there.

Thanks @joppuyo and @jcupitt

@joppuyo
Copy link
Member

joppuyo commented May 26, 2020

This took a bit more time than I expected but the latest version now uses thumbnail instead of resize if it's supported by the libvips version.

Instead of thumbnail I opted to use thumbnail_image since it retains compatibility with rotating and cropping operations which WP performs prior to the resizing and obviously it doesn't work if the file is read directly from the disk. Also, the performance was identical.

I ran a few quick benchmarks and it seems like using thumbnail speeds up the resizing by 11% .

Disabling the cache reduced the peak memory use by 22% without any slowdown in the image processing.

Big thanks to @jcupitt for the tips!

Here are the benchmarks if anyone is interested:

Screenshot 2020-05-26 at 19 12 23

Screenshot 2020-05-26 at 19 13 04

Screenshot 2020-05-26 at 19 33 29

@jcupitt
Copy link
Author

jcupitt commented May 26, 2020

Hey, nice!

But thumbnail_image is really slow, it's just for emergencies. It can't exploit shrink-on-load :(

Try this:

john@kiwi:~/pics$ /usr/bin/time -f %M:%e vips thumbnail_image wtc.jpg x.jpg 128
59412:2.00
john@kiwi:~/pics$ /usr/bin/time -f %M:%e vips thumbnail wtc.jpg x.jpg 128
37888:0.33

thumbnail is more than 6x faster and needs 40% less memory. That's for a large (10k x 10k pixel) image, but you'll see useful savings on medium-sized images too.

It will also give better quality for vector formats like SVG or PDF since it can directly render to the correct size, rather than rendering at the wrong size and then resizing.

@joppuyo
Copy link
Member

joppuyo commented May 26, 2020

I'm using 20 images from Unsplash which are about 3000-5000 pixels on the larger side.

You can see from the benchmarks that comparing vips thumbnail file which uses thumbnail to vips no cache which uses thumbnail_image has negligible performance difference.

It might also be that I'm using the library in some sort of wrong way? Or maybe thumbnail works somehow differently if it's used with the vips php library instead of command line?

I'm going to run a few more tests to see if I can see a performance boost with thumbnail.

It would be possible to modify the logic so that thumbnail is used when doing only resizing and thumbnail_image when there is rotation or cropping involved but I don't want to make the code more complicated if there is no performance benefit.

@jcupitt
Copy link
Author

jcupitt commented May 26, 2020

It should be really dramatic. Here's a 6k x 4k image:

john@kiwi:~/pics$ time vips thumbnail nina.jpg x.jpg 128
real	0m0.114s
user	0m0.098s
sys	0m0.017s
john@kiwi:~/pics$ time vips thumbnail_image nina.jpg x.jpg 128
real	0m0.483s
user	0m0.482s
sys	0m0.063s

A 4x speedup.

I would do rotate and crop after thumbnail, so you are rotating and cropping a much smaller image.

@chatelp
Copy link

chatelp commented Dec 21, 2020

Hi @jcupitt, I'm using the latest version of this plugin that has switched to thumbnail instead of resize as per your suggestion. Except that now I'm getting errors on some resizes (extract_area: bad extract area) that are not happening with resize. Do you have any idea why?

Thanks 👍

@jcupitt
Copy link
Author

jcupitt commented Dec 21, 2020

Hi @chatelp, post an example that fails and I'll have a look.

@chatelp
Copy link

chatelp commented Dec 21, 2020

Thanks @jcupitt ! It happens when trying to get a 650x433px thumbnail of the attached image. This is a custom thumbnail size registered in my WP install: add_image_size('custome-thumb', 650, 9999);

The _resize function of this plugin is called for each thumbnail size but it fails only on this particular one. More precisely, it fails with the extract_area: bad extract area error at the crop stage that happens in this function after the call to thumbnail_image has been made first. See attached debug session capture bellow.

If I replace the call to thumbnail_image to a call to resize, then crop doesn't fail.

Capture d’écran 2020-12-21 à 18 27 57

_DSC0509

@jcupitt
Copy link
Author

jcupitt commented Dec 21, 2020

I think your PHP is not quite right -- you are calculating scale in two different ways, one for thumbnail and one for crop. In this case, they are giving two different numbers, and that's causing the crop to drift out of bounds.

I would change it to always calculate scale in the same manner.

@chatelp
Copy link

chatelp commented Dec 21, 2020

I see, thanks so much for looking into it, but I'm afraid it's not my PHP code, and that's the problem. I'm just using the vips plugin as-is... for now. So, you are suggesting that I change the code pertaining to scale computation in the _resize method, right?

Also, could you explain why is there the need to do a crop following a resize (or thumbnail_image), even when the crop parameter of the _resize function is set to false. I'm sure I'm missing something since I'm no vips expert... nor php expert for that matter :)

@jcupitt
Copy link
Author

jcupitt commented Dec 21, 2020

It looks like a minor bug in vips-image-editor to me -- someone would need to patch it and open a PR.

Sure, you could probably skip the crop in that case, though I think there wouldn't be a performance win.

@chatelp
Copy link

chatelp commented Dec 21, 2020

I just checked for the other thumbnails that are working, and the scale value is never the same, even for the images where the crop is working... doesn't that exclude the scale value discrepancy as the source of the extract_area: bad extract area error ? @joppuyo are you still working on this?

@jcupitt
Copy link
Author

jcupitt commented Dec 22, 2020

Yes, the rounding will only fail for some images.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants