WordPress.org

Make WordPress Core

Opened 11 years ago

Last modified 3 months ago

#6814 assigned enhancement

Async media crunching

Reported by: matt Owned by: koopersmith
Milestone: Future Release Priority: normal
Severity: normal Version: 2.5
Component: Upload Keywords: blessed has-dev-note
Focuses: performance Cc:

Description

The upload part of the new multi-uploader is pretty nice now, but it blocks on the "crunching" phase, which can sometimes take 20-60 seconds, I assume to create medium thumbnails and such.

The crunching part of the upload should not block the next file beginning the upload process, it should happen asynchronously with the rest of the process.

Attachments (2)

capture.png (12.0 KB) - added by johnbillion 7 years ago.
wp_trac_thumbslater_6814.php (3.9 KB) - added by ramon fincken 3 months ago.
our_thumbnails-later_php_file.php

Download all attachments as: .zip

Change History (25)

#1 @tellyworth
11 years ago

I'm not sure how common it is, but in my testing a good part of the 'Crunching' time is actually the final part of the file upload. It seems that swfupload triggers the 100% progress event (which is what causes Crunching to be displayed) when it begins sending the final chunk of the upload, rather than when it finishes sending it. I don't know of a solution to this.

In other words, the time spent creating thumbnails is only a fraction of the Crunching time, and might be negligible. Some basic logging with microtime() in async-upload.php would confirm it.

If it turns out thumbnail creation does take a significant amount of time, then yes, it should be done asynchronously.

#2 follow-up: @endolith
11 years ago

Thumbnail and medium size image creation should not be done on upload at all. They should be created only when called for, so that they always reflect the size settings in Miscellaneous.

#3 in reply to: ↑ 2 @Otto42
11 years ago

Replying to endolith:

Thumbnail and medium size image creation should not be done on upload at all. They should be created only when called for, so that they always reflect the size settings in Miscellaneous.

-1. Highly disagree. This would add a ton of CPU time, as the image would need to be resized on every call to it, basically. Either that or it leaves tons of old files lying around every time you change the image sizes.

Furthermore, I don't think that changing a setting somewhere should instantly change the content/size of all the images in all your old posts.

#4 @Denis-de-Bernardy
10 years ago

  • Component changed from Administration to Media

#5 @Denis-de-Bernardy
10 years ago

  • Component changed from Media to Upload

#6 @ryan
10 years ago

  • Milestone changed from 2.9 to Future Release

#7 in reply to: ↑ description @johnbillion
7 years ago

  • Keywords close added

Replying to matt:

The crunching part of the upload should not block the next file beginning the upload process, it should happen asynchronously with the rest of the process.

This no longer happens since we switched to Plupload. Subsequent files begin uploading while the previous file is crunching.

@johnbillion
7 years ago

#8 @SergeyBiryukov
7 years ago

  • Milestone Future Release deleted
  • Resolution set to duplicate
  • Status changed from new to closed

#9 @johnbillion
7 years ago

  • Keywords close removed
  • Resolution duplicate deleted
  • Status changed from closed to reopened

azaozz has confirmed this uploads are still synchronous. There is JS/UI lag after a file finishes crunching, giving the impression of async uploads.

#10 @SergeyBiryukov
7 years ago

  • Milestone set to Awaiting Review

#11 @nacin
7 years ago

  • Milestone changed from Awaiting Review to 3.5

#12 @bpetty
7 years ago

  • Keywords punt added
  • Milestone changed from 3.5 to Future Release

Discussed punting this in IRC during scrub.

#13 @SergeyBiryukov
7 years ago

  • Keywords punt removed

#14 @matt
7 years ago

  • Owner changed from tellyworth to koop
  • Status changed from reopened to assigned

Let's put this one in 3.6 early, it's silly that four years later we still haven't addressed this.

#15 @matt
7 years ago

  • Owner changed from koop to koopersmith

#16 @bpetty
7 years ago

  • Cc bpetty added

#17 @bpetty
5 years ago

Now that Plupload 2.1 had just been merged, this ticket is back on the table for a review.

I'm sure we don't have the time to push it before beta 3.9, certainly not without a patch in the next day or two, but worth mentioning since the Plupload update (#25663).

#18 @SergeyBiryukov
4 years ago

#31807 was marked as a duplicate.

#19 @ramon fincken
3 months ago

  • Focuses performance added
  • Keywords has-dev-note added

Stepping in on this one .. we have some code that we ship at all our customers' installs, it will temporary remove the extra image sizes. Therefore no extra images will be created based on the original upload. It sets a meta flag on the uploaded item.

On each WP call, except a new upload the thumbs will be created.
A small snippet of this part:

<?php
$sql = $wpdb->prepare( 'SELECT post_id FROM '. $wpdb->postmeta . ' WHERE meta_key = %s LIMIT %d', $this->thumbslaterkey, $this->batch_limit ); 
foreach( $wpdb->get_results( $sql ) as $thumbs_later ) { 
  if( $this->init( $thumbs_later->post_id ) ) { 
     delete_post_meta( $thumbs_later->post_id, $this->thumbslaterkey ); 
  } 
}

It is not said that this postmeta is the best in terms of optimization. A non-autoloaded option could also be used for this. However we use a batch limit in the SELECT clause.

Are you interested in the whole code or the pseudocode ?

This ticket was mentioned in Slack in #core-media by ramonfincken. View the logs.


3 months ago

#21 @azaozz
3 months ago

Thanks @ramon-fincken, this sounds like something we can use/learn from on #40439? (See the patch there too, it saves the image meta after each sub-size is created instead of waiting for all to be done).

A pseudo code should be good for now I think :)

#22 @ramon fincken
3 months ago

Ok so here we go .. sorry about no comments in code. We ship this at every customer as we see loads of folders from their local machines are just dragged and dropped to the media upload.

It is used as as MU-plugin. Boot it by:

        include_once '/YOURDIR/thumbnails-later/managedwphosting_thumbnails_later.php';
        add_action('init' , 'ManagedWPHostingRegenerateThumbnails_Later');

The only thing I needed due to not be touching Core on this one is the flag MWP_THUMBSLATER_FLAG (defined). The define is just an extra pre-caution, I think it could be removed.

So: it will temporary remove the extra image sizes. (super low prio action flag 999) Therefore no extra images will be created based on the original upload. It sets a meta flag on the uploaded item.
On each WP call, except a new upload the thumbs will be created.

It is not said that this postmeta is the best in terms of optimization. A non-autoloaded option could also be used for this. However we use a batch limit in the SELECT clause.

Parts of the code are based on the Regenerate thumbnails plugin by @viper007bond
When checking the code right now I see that there might be a small hickup in the SQL query. What if the attachment is in trash? Not a big thing but stil.. waste of perfomance.

If you have any questions feel free to ask now or at WCEU Berlin next month. I will be present at the contributor day.

I will attach the PHP MU-plugin file in a sec.

@ramon fincken
3 months ago

our_thumbnails-later_php_file.php

#23 @ramon fincken
3 months ago

Update: postmeta is better than an option

Update2:

if( $this->init( $thumbs_later->post_id ) ) { 
     delete_post_meta( $thumbs_later->post_id, $this->thumbslaterkey ); 
  }


Update3:
this should be removed

                if ( $this->fullsizepath ) {
                                return $this->fullsizepath;
                        }

might need an else to delete the meta key (when errors) else you will keep looping over the very same IDs over and over again due to them beeing in error.

Last edited 3 months ago by ramon fincken (previous) (diff)
Note: See TracTickets for help on using tickets.