Make WordPress Core

Opened 7 years ago

Last modified 7 years ago

#40441 new enhancement

High server resource usage and timeouts during image uploads

Reported by: enshrined's profile enshrined Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version:
Component: Media Keywords:
Focuses: Cc:

Description

When uploading an image, server resource usage can run high and the resize process can easily timeout causing the upload to fail. This is often down the creation of intermediate image sizes and is especially noticeable when plugins/themes have custom image sizes defined.

This issue has been mentioned before in both #37840 and #36534.

#40439 proposes saving the progress of intermediate image creation so resizing can be resumed on retry.

This ticket is mainly for discussion around ways to reduce potential server implications when uploading an image and creating intermediate image sizes.

Change History (5)

#1 @blobfolio
7 years ago

A common bottleneck comes from source files that are inappropriately large, which subsequently require extra processing power and memory for every downsize pass.

One workaround would be to generate a more reasonable pseudo-source if the original is excessive (e.g. 2000px cap if the source is > 5000px) and use that for any thumbnails of equal or lesser size. This will have some quality loss due to recompression so would need to be user-controlled, probably defaulting to DON'T. But it would definitely help with timeout issues on budget shared hosting.

There is also optimization potential within the class-wp-image* files themselves: they could maintain a static copy of the last source image resource to have been generated rather than destroying all traces of it at the end of the run. protected static $_last = array( $filepath => $resource ); Since this is so often called in a loop using the same file over and over again, it would allow the class to refer to the cached source rather than starting from scratch at each iteration. In cases where an application initiates the image editor against multiple sources (like a bulk thumbnail regeneration plugin), the usual cleanup can be run at the beginning of each new cycle to free up memory held for the last one (rather than the end of every cycle).

While this does mean there will be an uncleared resource at the end of the process, it should still translate to a decent performance win, particularly in cases where the source files are gratuitously large.

#2 @enshrined
7 years ago

@blobfolio I'm not sure I'd agree with using a pseudo-source as this would either have to be user definable, which goes against "Decisions, not Options" or large enough to accommodate all uses. For example, I worked on a site recently that have needed to background images to look sharp on smart TVs etc and as such images up to 6000px were loaded onto the site by the client. A edge case, I understand but still, there will be people who want those larger images and if all resizing is done from a 2000px image this stops that.

I may be wrong, but I'm also fairly certain that the WP_Image_Editor_* classes only load the original image resource once which is then stored within protected $image. If intermediate image size creation is called correctly, using the multi_resize method then it should just resize using this pre-loaded image resource which then gets destroyed in the __destruct method.

Personally, I think a good way to go would be to decouple intermediate image size creation from the upload process completely. As long as it's tied to the upload there will always be issues when themes and plugins add significant numbers of image sizes, as there's only so much optimisation that can be done here.

I'd love to see a background queueing system used to enable us to accomplish this, a good start is the one at https://github.com/A5hleyRich/wp-background-processing. Using this method original images could be uploaded and stored and the intermediate sizes queued to be created, as each completes it can then update the attachment meta with the appropriate data.

I created a plugin to play around with this concept before which is available at https://github.com/darylldoyle/WP-Background-Image-Processing if anyone is interested in taking a look.

#3 @blobfolio
7 years ago

@enshrined, I apologize, I thought the concept of some sort of recoverable/queue-based thumbnail generation was already a foregone conclusion. That is by far the best overall solution. I thought this thread was more about identifying where specifically (aside from "doing too many things in one go") the image processes are too heavy for the types of servers people are actually using.

Outside of the chain of image processing itself, the single biggest issue I see is not WordPress, but what users are doing to it and where they're trying to do it from. To that end, I see the decision (from the WP side) as moot. We can either offer patches that could help extend the portability of the software to less capable environments (that people are already using anyway), or not. That would be an "option", definitely, and one that I wouldn't want to even see as a default. But having it in place would provide a solution for people in that situation.

You're right, the ->multi_resize() function does preserve the source for the duration of its operation. But after that it is gone. Since so much image handling is achieved by passing filenames and attachment IDs through filters, most hookable alterations would have no choice but to reinstantiate the editor object anew. Static cache would preserve the resource between instances (though to keep memory from blowing up we probably would want to keep it to just the "last" file). All in all this would probably be a bigger deal on Windows than Linux since file system operations are more expensive to perform there (under Windows, that is).

#4 follow-up: @enshrined
7 years ago

Apologies @blobfolio, I didn't know if the topic had been discussed in enough depth to make the assumption that a queue based system would be used. That said, I'm glad we both agree on that methodology.

I see what you mean about the multi_resize function but I've personally not seen many themes or plugins re-instantiating the WP_Image_Editor_* classes, do you have any examples of why they may want to do this? As far as I can see, the only things this class can really do is resize, crop, rotate and flip. Resizing would surely be best handled through either defining an image size or using the intermediate_image_sizes_advanced filter? I suppose I'm just curious as to why a developer would want direct access to these specific classes.

#5 in reply to: ↑ 4 @blobfolio
7 years ago

Replying to enshrined:

I see what you mean about the multi_resize function but I've personally not seen many themes or plugins re-instantiating the WP_Image_Editor_* classes, do you have any examples of why they may want to do this?

In terms of user space, I'm more concerned with filter hooks and wrapper functions than direct calls to the image classes. Their documentation is much easier to locate and understand than the image editor classes, so developers will use them more often. Here's an example:

Themes often define their own thumbnail sizes. But when a user changes themes, old media isn't going to be resized en masse to match the new definitions. On the frontend, src calls might return the original (huge) source media and srcset calls might return nothing at all, both leading to imperfect presentation. A theme could work around this by hooking into filters like image_downsize, wp_calculate_image_srcset, etc., to run checks to see if the sized file exists, and if not, create it with image_make_intermediate_size(). Works like a charm, except that image_make_intermediate_size() will create a new editor object at each pass.

Note: See TracTickets for help on using tickets.