Do not generate unnecessary max preview file

[phpbg]

See #16963

Hi, this is a big one for me, please provide feedback and test.
The preview code probably need some refactoring, but I just focused on fixing the issue.

There are a few open TODO items, but I wanted some feedback before proceeding...

[phpbg]

This pull request provides a 9x speed improvement on my setup when uploading photos from web browser

[kesselb]

@rullzer @skjnldsv would you mind to have a first look? I don't know where these -max previews are used.

[rullzer]

I am not a fan of this. The max preview is there for a reason. First of all it is often requested as well at some point (when opening in the viewer for example). But mostly because otherwise we have to load in the huge original image file every time we need to generate a different preview size.

Some other things I noticed

1. How do you measure the speedup? The preview should not be triggered during upload at all. So it is the time until the thumbnail is loaded?
2. I tried this on my test and production instance and I get nowhere near the 9x speedup. Are you perhaps on USB storage?

[phpbg]

As stated in #16963, I upload from the webpage.
As soon as files are uploaded the browser requires previews.

In my setup, files are on a cifs share with decent performances (but indeed, there is no cache on network storage).

I measured the time to upload 20 JPEG images (8MB each).
Without the patch: 3min, with the patch: 20s.

[phpbg]

Hi,
anyone has more feedback on this?

[phpbg]

ariselseng/camerarawpreviews#31

#15075

[kesselb]

Thanks @phpbg for investigating 👍

There are some reports about the slow image preview generation around so its good to look at different approaches or possible optimizations.

Like @rullzer I don't see a huge performance improvement with my development setup.

I used https://www.pexels.com/photo/photography-of-person-walking-on-road-1236701/ for my benchmarks. Copied the image 50x times and uploaded it. Repeated the test three times.

Start: Press open in the file picker dialog
Stop: Last thumbnail loaded

Nginx with PHP-FPM (pm = dynamic, pm.max_children = 5)
Patch: 1:10, 0:58, 0:59
Master: 1:23, 1:23, 1:24

Feel free to repeat the test and share your results 🥇

[phpbg]

I had:
Patch: 0:35 0:32 0:34
Master: 1:39 1:45 1:40

I wonder how is your server configured ? FPM ? static vs dynamic ? My server gets completely overloaded without patch, and i think it makes much longer response times...
I have:

/etc/php/7.2/fpm/pool.d/www.conf
pm = dynamic
pm.max_children = 20

We should also measure getting previews of a different size after the first previews are generated, to check @rullzer remark about re-reading original image vs reading -max preview

[kesselb]

I run the above tests with Nginx and PHP-FPM (pm = dynamic, pm.max_children = 5).

Nginx with PHP-FPM (pm = dynamic, pm.max_children = 20)
Patch: 1:03, 1:03, 1:05
Master: 1:28, 1:30, 1:26

Here the results for your configuration. I assigned two cores and 4096mb memory to my development box. Guess the box is not able to handle so much requests at the same time.

[ariselseng]

With my camerarawpreviews app it would certainly take longer time without the max-preview image. To support external storage, the RAW file is copied to a temporary file and then read. Depending on the speed of the external storage and general IO speeds will determine how much extra time this will take per image.

Maybe generating the max-preview in the background in cron would be a better idea? Or perhaps on upload?

[phpbg]

Are the thumbnails encrypted when encryption is on? Because in such case you probably don't want an unencrypted -max preview file...

I guess in the end we should:
1. let the user decide in an option if he wants the -max preview file (it's a cache for the cache, the user choose itself a trade off between IO or CPU)
2. try to generate preview from a bigger preview if it already exists, as discussed here: #16158 (comment)

What do you think?

[kesselb]

Or perhaps on upload?

I would try that. Guess there are good reason why it's not done by default. There are hooks/events in place for upload a file. I think we could do that as app.

What do you think?

I think we should improve the situation in general. There are many setups around with different stacks. Some have better cpu others faster io. Not sure if there is a perfect way. Just opened a folder with around 700 images on my production instance and yes it's bad 😟

[MichaIng]

I know it's an old PR, but the topic is still completely unresolved. I think this PR perfectly makes sense, so only the actually required preview is generated. I don't get the argument to generate the max size preview to speed up further preview generations:

• Not sure how large the average image in a gallery is, but I bet that most is made by mobile phones and probably fullHD resolution. The current default max preview size of 4096x4096 leads to a preview, which is a duplicate of the original in very most cases (my guess at least) and hence does not speed up subsequent preview generations at all.
• When a single image is opened in the photo viewer, it is totally okay if that single preview is then generated, on first access, but IMO it doesn't make sense to do this directly on upload, where it is not clear if the web UI is used at all to watch the images, or finally this is done on clients only.

So I clearly vote for a preview-on-demand only implementation, which does not trigger any preview, as long as it is not requested by an app or viewer which uses that exact preview size. For anyone who uses the web interface to view image galleries, instead of doing this on a client, the preview generator app is mostly mandatory anyway, to get a smooth experience, which is then fine as it runs in background.

Another aspect of this is huge oc_filecache tables and disk usage, when each image, due to multiple previews, leads to more than doubled disk usage compared to having the original stored only (this is true in my case) and multiple times more oc_filecache entries. This database table is generally an issue already, on smaller machines like SBCs, as it can become very large, and especially large image galleries which imply multiple additional entries for each preview of each image, can blow it up significantly. This is okay when those preview are used through the web interface, but bad if those are never used, aside of the small sized one triggered by the files app.

[kesselb]

It's valid that we need to improve the preview generation. I'm closing this pull request because this approach does not work.

@MichaIng would you mind to add your analysis to #13709?

[MichaIng]

Not sure, it's not exactly the same part of this topic, respectively mixes many topics and approaches together, many correct things have been mentioned without any concrete result.

This PR ships a solution for a specific part of the issue, which looks good to me, and generally the IMO right approach to generate only what is specifically required in each situation, when using the web UI.

The whole topic around previews is generally scattered across a large number of PRs, issues and forum threads and to me it looks like PRs that address a specific part of it are the most realistic way to enhance the situation for now. Sad that this one does not work, although I didn't understand yet why that is.

[kesselb]

I tested this pr a while ago and could not reproduce the performance improvements. Yep it's a bit faster without the max preview but the next time you need another preview it's necessary to open the original file again.

[MichaIng]

That will simply depend on network speed vs hardware performance:

• When the network is slow, the server-side preview generation does not play such a big relative role, compared to request and transfer times.
• Same when the server is fast. On SBCs the difference will be more notable.

With a max preview, the next preview will only be faster generated, when:

• The max preview is smaller than the original image, and I'm not sure how often this is the case with the default 4096x4096 max preview size.
• The next preview is larger than the first preview. When we talk about the files app, this is likely the case, but not when the image is uploaded e.g. through a client, then viewed via Photos app and then the files app is browsed or so. If I understand it correctly, always the next larger preview is used (when present) to generate a new preview.
• There is a next preview generated at all: Probably the image was uploaded through the files app but never viewed via web interface at all (aside of the tiny thumb in the files app itself).

While the downsides are assured to be there (in significance of course depending on hardware), the benefits are only there when those three assumptions are met. And finally, when I open the image viewer via web UI, it is okay when a single large preview is then generated on demand, or the original loaded when it is smaller. When (up)loading many images it is where the significant delay and hanging server may happen, at least on weaker machines.