#15447 closed enhancement (fixed)
Cache Post Thumbnails
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 3.2 | Priority: | normal |
Severity: | normal | Version: | |
Component: | Performance | Keywords: | has-patch |
Focuses: | Cc: |
Description
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?
Attachments (6)
Change History (41)
#1
@
13 years ago
- Keywords needs-patch added
- Milestone changed from Awaiting Review to Future Release
- Type changed from feature request to enhancement
#2
@
13 years ago
- Component changed from General to Post Thumbnails
- Keywords reporter-feedback added
#3
@
13 years ago
Of course it's querying for attachment data, since it needs the thumbnail url, at least.
#4
@
13 years ago
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?
#5
@
13 years ago
- Keywords 3.2-early added; reporter-feedback removed
We could add a cache_post_thumbnails() method to WP_Query, which theme authors could call before displaying a Loop with thumbs.
#8
@
13 years ago
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.
#9
follow-up:
↓ 10
@
13 years ago
Or cache if in_the_loop on first call to a post thumbnail template tag?
I like that idea.
#10
in reply to:
↑ 9
@
13 years ago
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.
#11
@
13 years ago
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.
#12
follow-up:
↓ 14
@
13 years ago
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)?
#14
in reply to:
↑ 12
;
follow-up:
↓ 15
@
12 years ago
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?
#15
in reply to:
↑ 14
@
12 years ago
- Keywords has-patch added; needs-patch removed
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.
#16
follow-ups:
↓ 19
↓ 20
@
12 years ago
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.
#17
@
12 years ago
15447.3.diff removes the extra caching key.
Also, it removes unnecessary array_filter( array_unique( $thumb_ids ) )
#18
@
12 years ago
All of the update_*_cache()
functions would more aptly be named populate_*_cache()
, but I guess that's for another ticket.
#19
in reply to:
↑ 16
@
12 years ago
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.
#20
in reply to:
↑ 16
@
12 years ago
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).
#21
@
12 years ago
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.
#22
@
12 years ago
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.
#23
follow-up:
↓ 25
@
12 years ago
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.
#25
in reply to:
↑ 23
@
12 years ago
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 :)
#26
@
12 years ago
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.
#28
@
12 years ago
- Component changed from Post Thumbnails to Performance
- Keywords 3.2-early removed
- Milestone changed from Future Release to 3.2
#29
@
12 years ago
- Keywords needs-refresh added
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
?
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.