Make WordPress Core

Opened 12 years ago

Closed 7 days ago

#19949 closed enhancement (maybelater)

Make update_post_thumbnail_cache() work for any instance

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

Description

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 12 years ago.

Download all attachments as: .zip

Change History (21)

@scribu
12 years ago

#1 @Ploobers
12 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
12 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();

  the_post_thumbnail();

endwhile;

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() )
  update_post_thumbnail_cache();
Last edited 12 years ago by scribu (previous) (diff)

#3 @scribu
12 years ago

  • Keywords commit removed

#4 follow-up: @scribu
12 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
12 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
12 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
12 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
12 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 12 years ago by scribu (previous) (diff)

#9 @nacin
12 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
12 years ago

  • Milestone changed from 3.4 to Future Release

#11 @SergeyBiryukov
11 years ago

  • Keywords punt removed

#12 follow-up: @Ploobers
11 years ago

Is this going to make it into 3.5?

#13 in reply to: ↑ 12 @nacin
11 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
11 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
11 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
10 years ago

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

#17 @chriscct7
8 years ago

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

#18 @kirasong
8 years 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
8 years ago

#18067 was marked as a duplicate.

#20 @pbearne
7 days ago

  • Resolution set to maybelater
  • Status changed from new to closed

A part patch was added but this would need a WP query rewrite to fix so moving to maybe later

Note: See TracTickets for help on using tickets.