Make WordPress Core

Opened 5 years ago

Last modified 14 months ago

#19949 new enhancement

Make update_post_thumbnail_cache() work for any instance

Reported by: scribu Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version:
Component: Post Thumbnails Keywords: dev-feedback reporter-feedback needs-patch
Focuses: performance Cc:


It would be useful if update_post_thumbnail_cache() [17883] would accept an optional $wp_query parameter, so that users aren't forced to use query_posts() in order to benefit from it.

Attachments (1)

19949.diff (707 bytes) - added by scribu 5 years ago.

Download all attachments as: .zip

Change History (20)

5 years ago

#1 @Ploobers
5 years ago

Awesome. Easy fix that can have major ramifications on site performance. This will reduce my database calls by over 100 per page without having to do my own queries or leave the WP API.

#2 @scribu
5 years ago

Example usage:

$my_query = new WP_Query( array( 'tag' => 'foo' ) );

update_post_thumbnail_cache( $my_query );

while ( $my_query->have_posts() ) : $my_query->the_post();



Well, actually, that would also cause an update_post_thumbnail_cache() call for the main query, which might be unnecessary.

LE: Because the_post_thumbnail() has this code:

if ( in_the_loop() )
Last edited 5 years ago by scribu (previous) (diff)

#3 @scribu
5 years ago

  • Keywords commit removed

#4 follow-up: @scribu
5 years ago

It bugs me how we seem to be pushing users towards query_posts() instead of away from it.

#5 in reply to: ↑ 4 @nacin
5 years ago

Replying to scribu:

It bugs me how we seem to be pushing users towards query_posts() instead of away from it.

I agree.

#6 @nacin
5 years ago

Just a thought:

class Nacin_Cache_Thumbnails_for_Queries {
   public $loops = array();
   function __construct() {
      add_action( 'loop_start', array( $this, 'loop_start' ) );
      add_action( 'loop_end', array( $this, 'loop_end' ) );
      add_action( 'begin_fetch_post_thumbnail_html', array( $this, 'fetch_thumbnail' ) );
   function loop_start( $query ) {
      array_push( $this->loops, $query );
   function loop_end() {
      array_pop( $this->loops );
   function fetch_thumbnail() {
       $query = end( $this->loops );
       update_post_thumbnail_cache( $query );
new Nacin_Cache_Thumbnails_for_Queries;

#7 @nacin
5 years ago

  • Keywords 3.5-early punt added

Per conversation in IRC, let's come up with the appropriate balance of API and voodoo, and revisit in 3.5.

#8 @scribu
5 years ago

On the one hand, it would be nice if developers would never have to call update_post_thumbnail_cache() manually. Nacin's solution works pretty well in that respect.

On the other hand, it wouldn't work at all with get_posts().

Ideally, we would have both:

  1. a standalone function that, given an array of posts, caches the thumbnail attachments
  2. some magic that calls that function automatically for any Loop
Last edited 5 years ago by scribu (previous) (diff)

#9 @nacin
5 years ago

In [20646]:

Add $wp_query parameter to update_post_thumbnail_cache(). props scribu. see #19949, fixes that ticket for 3.4.

#10 @nacin
5 years ago

  • Milestone changed from 3.4 to Future Release

#11 @SergeyBiryukov
5 years ago

  • Keywords punt removed

#12 follow-up: @Ploobers
5 years ago

Is this going to make it into 3.5?

#13 in reply to: ↑ 12 @nacin
5 years ago

Replying to Ploobers:

Is this going to make it into 3.5?

Negative. This ticket needs a burst of creativity.

In many respects it is a rethink for how the loop works. Imagine if a post could be tied as coming from a loop (a collection of posts), and when terms, meta, or featured images gets called for that post, we attempt to query all other posts in that collection at once.

#14 @Ploobers
5 years ago

It's become very frustrating for me to try and maintain two separate query styles, as I prefer to use the built-in get_posts, but as I'm using thumbnails all over my site in custom loops, not having the images cached is causing 50-150 extra queries per page. In the meantime, I've just been using my own direct query.

SELECT p.ID, p.post_title, p.post_name, p.post_parent, p.post_type, a.ID AS image_id,
		MAX( IF ( apm.meta_key='_wp_attachment_metadata', apm.meta_value,' ') ) as 'image_data',
		MAX( IF ( apm.meta_key='_wp_attached_file', apm.meta_value,' ') ) as 'filepath'
		FROM sr_posts p
		INNER JOIN sr_postmeta pm ON p.ID = pm.post_id
		INNER JOIN sr_posts a ON pm.meta_value = a.ID
		INNER JOIN sr_postmeta apm ON a.ID = apm.post_id
		WHERE p.post_parent = '$connect_id'
		AND pm.meta_key = '_thumbnail_id'
		AND ( apm.meta_key = '_wp_attachment_metadata'
		OR apm.meta_key = '_wp_attached_file')
		$where GROUP BY p.ID

#15 @Ploobers
5 years ago

Even if running the meta cache is an expensive call, it doesn't even have to be auto-tied into the loop. It could be something for advanced calls through new WP Query objects. As scribu said, it'd be great if it magically worked everywhere, but even a manual call would be a huge improvement.

#16 @nacin
3 years ago

  • Component changed from Performance to Post Thumbnails
  • Focuses performance added

#17 @chriscct7
21 months ago

  • Keywords needs-refresh dev-feedback added; 3.5-early removed

#18 @mikeschroder
21 months ago

  • Keywords reporter-feedback needs-patch added; has-patch needs-refresh removed

The patch on this ticket has already been committed. It seems like the focus has shifted.

What's left on this ticket? It might need an updated title, or a new bug to handle the focus.

#19 @flixos90
14 months ago

#18067 was marked as a duplicate.

Note: See TracTickets for help on using tickets.