Cache Post Thumbnails

[TheDeadMedic]

Database queries can rapidly increase when using the post thumbnails feature, especially whilst in the loop (on average, 2 additional queries per post).

Suggest caching all thumbnails for the current query - will usually drop to just two additional DB queries in total, regardless of number of posts.

// only run if feature supported by theme

$thumbnail_IDs = array();
foreach ($posts as $post)
	$thumbnail_IDs[] = get_post_thumbnail_id($post->ID);

if ($thumbnail_IDs) {
	$thumbnail_IDs = array_filter( array_unique($thumbnail_IDs) );
	get_posts(array(
		'update_post_term_cache' => false,
		'include' => $thumbnail_IDs,
		'post_type' => 'attachment',
		'post_status' => 'inherit',
		'nopaging' => true
	));
}

Obviously the code above is pure example - when, and where, this is best placed I'm not sure. Perhaps hooked to the 'wp' action, so it only runs for the main query?

[nacin]

I don't see how thumbnails results in more queries. We do a full slurp of metadata for posts in the loop, which would include those IDs. Can you post the queries? Is it querying for attachment data? That's not unheard of for, say, galleries.

[scribu]

Of course it's querying for attachment data, since it needs the thumbnail url, at least.

[johnbillion]

I've written a plugin which does exactly this. On a large site I'm working on with 14 (yeah, fourteen) loops on the home page and thumbnails *everywhere* the number of queries was through the roof.

Here's the relevant code which preloads attachments for all posts in the main loop (my custom loops also call the 'loop_start' action - usually this is not the case).

function cache_thumbnail_posts( $q ) {
	$types = $thumb_ids = array();
	foreach ( $q->posts as $p )
		$types[$p->post_type] = $p->post_type;
	unset( $types['attachment'] );
	if ( empty( $types ) )
		return $q;
	foreach ( $q->posts as $p )
		$thumb_ids[] = get_post_thumbnail_id( $p->ID );
	$thumb_ids = array_filter( $thumb_ids );
	if ( empty( $thumb_ids ) )
		return $q;
	$thumb_ids = implode( ',', $thumb_ids );
	$preload = get_posts( 'post_type=attachment&include=' . $thumb_ids );
	return $q;
}
add_filter( 'loop_start', array( $this, 'cache_thumbnail_posts' ) );

Looks a bit complex but it means it only preloads the attachments when it needs to.

One thing to consider is that this will add unnecessary queries if the current theme isn't displaying the post thumbnails in the loop.

Thoughts?

[scribu]

We could add a cache_post_thumbnails() method to WP_Query, which theme authors could call before displaying a Loop with thumbs.

[johnbillion]

Or how about add_theme_support('preload-thumbnails')?

[scribu]

That's no good, because this has to be set per loop, not per theme.

[nacin]

It's one query, so why not add it when the theme supports thumbails? Or cache if in_the_loop on first call to a post thumbnail template tag?

Keep it simple. Theme authors shouldn't be given extra tags just for caching.

[scribu]

Or cache if in_the_loop on first call to a post thumbnail template tag?

I like that idea.

[johnbillion]

Replying to scribu:

Or cache if in_the_loop on first call to a post thumbnail template tag?

I like that idea.

Same here. That's the way to go. An extra bit of logic so it doesn't fire when is_single etc and that'd be spot on.

[nacin]

Well, we'd need to run the query anyway, so in that case it'd only be run for one post, versus all of them. We'd just do it for all posts in $wp_query->posts.

[TheDeadMedic]

Perhaps caching should also depend on the query arg 'cache_results', and possibly a new query arg 'update_post_thumbnail_cache' (to match the pattern of the existing 'update_post_term_cache' and 'update_post_meta_cache' args)?

So by default (if thumbnail feature supported), thumbnails are cached for the main query (i.e. the request), but a theme author has the option to cache/not cache for their own loop(s)?

[garyc40]

cache post thumbnails on first call of get_the_post_thumbnail() in a loop

[garyc40]

Replying to TheDeadMedic:

Perhaps caching should also depend on the query arg 'cache_results', and possibly a new query arg 'update_post_thumbnail_cache' (to match the pattern of the existing 'update_post_term_cache' and 'update_post_meta_cache' args)?

So by default (if thumbnail feature supported), thumbnails are cached for the main query (i.e. the request), but a theme author has the option to cache/not cache for their own loop(s)?

I don't think that's a good idea. There are loops that don't output thumbnails at all, so caching thumbnails whenever a loop start seems wasteful.

Ignore attachment:15447.diff​ for the time being, I just realized there's a bug with it (using permanent cache and modifying the thumbnail of a post does not properly invalidate the cache).

Any suggestion on how to tell if get_post_thumbnail() is being called for the first time in a loop?

[garyc40]

Ignore attachment:15447.diff​ for the time being, I just realized there's a bug with it (using permanent cache and modifying the thumbnail of a post does not properly invalidate the cache).

Any suggestion on how to tell if get_post_thumbnail() is being called for the first time in a loop?

Never mind this. attachment:15447.diff​ works.

Still wondering if there's a better way to tell if get_post_thumbnail() is being called for the first time in a loop.

[scribu]

Check $wp_query->current_post before attempting to cache

[scribu]

You can check $wp_query->current_post. See 15447.2.diff​

With that in place, I doubt the usefulness of permanently caching wether the current loops thumbnails were cached or not.

[scribu]

Remove extra caching key

[scribu]

15447.3.diff​ removes the extra caching key.

Also, it removes unnecessary array_filter( array_unique( $thumb_ids ) )

[scribu]

All of the update_*_cache() functions would more aptly be named populate_*_cache(), but I guess that's for another ticket.

[garyc40]

Replying to scribu:

You can check $wp_query->current_post. See 15447.2.diff​

With that in place, I doubt the usefulness of permanently caching wether the current loops thumbnails were cached or not.

This assumes that get_the_post_thumbnail() is always called at the loop start. Unless there are themes that treat the first post differently (not outputting the thumbnail, for example), it's a safe assumption.

[garyc40]

Replying to scribu:

With that in place, I doubt the usefulness of permanently caching wether the current loops thumbnails were cached or not.

It might be useful when you're using a permanent cache plugin. The same loop might be requested multiple times on a busy site, without another cache key in place, there will be unnecessary db queries for attachments in update_thumbnail_cache() (the get_posts() call).

[scribu]

By that logic, we should also cache WP_Query itself, since it involves even more work, but we don't.

I think reducing N queries to 1 is good enough.

[nacin]

15447.3.diff​ is what I was picturing when I commented a few months ago.

Why can it only fire on the first post? I imagine it should be able to fire at any point, once per loop. Keep in mind subsequent calls to it would simply hit the object cache anyway, but it shouldn't be difficult to ensure it gets fired only once per loop.

[scribu]

Use $wp_query->_thumbnails_cached flag

[scribu]

15447.4.diff​ is similar to 15447.diff, except it uses a private property on the $wp_query object instead of wp_cache_*().

This ensures a single call per loop, while not requiring get_the_post_thumbnail() to be called for the first post.

[nacin]

15447.4.diff​

:-)

[garyc40]

prevent notice for $wp_query->_thumbnails_cached

[garyc40]

Replying to scribu:

15447.4.diff​ is similar to 15447.diff, except it uses a private property on the $wp_query object instead of wp_cache_*().

Attached another patch that prevent undefined property notice for 15447.4.diff​ :)

[scribu]

Cool. What would be nice would be to allow update_thumbnail_cache() to be used for custom loops too.

I'm thinking of adding an optional $_wp_query parameter. Then, knowledgeable developers could call it before rolling their custom loop.

[scribu]

Similar: #16894

[greuben]

15447.5.diff​ is not working( It should be ! empty instead of empty ). However, isn't that variable supposed to be defined in WP_Query class?. If that property is private aren't we supposed to use functions like set and get?

[nacin]

If someone can refresh this patch in the next day or so, it'll go into 3.2. This is a good enhancement.

[greuben]

Patch refreshed

[nacin]

In [17883]:

Cache post thumbnails in the loop. props garyc40, scribu, greuben. fixes #15447.

[scribu]

Follow-up: #19949