Make WordPress Core

Opened 8 years ago

Closed 8 years ago

#36792 closed enhancement (duplicate)

Cache Post ids in WP_Query

Reported by: spacedmonkey's profile spacedmonkey Owned by:
Milestone: Priority: normal
Severity: normal Version: 3.4
Component: Query Keywords: needs-unit-tests needs-patch
Focuses: performance Cc:

Description

Current the WP_Query performs an SQL query every time it is run. The result of which is not cached. When the sql query runs, it selects on the columns in the post table and sets up the post objects. The newer query classes however, only select the id column and then loop around to get each object using the get_* function. When the comment query for example queries for ids, it then queries the result. This cached result is invalided using the "last_changed" value in the comments group. This caching should be added to the WP_Query class as well.

Attachments (1)

36792.patch (1022 bytes) - added by spacedmonkey 8 years ago.

Download all attachments as: .zip

Change History (6)

@spacedmonkey
8 years ago

#1 @ocean90
8 years ago

  • Keywords has-patch needs-unit-tests added

#2 @boonebgorges
8 years ago

  • Component changed from Posts, Post Types to Query
  • Keywords needs-patch added; has-patch removed
  • Milestone changed from Awaiting Review to Future Release

Thanks for opening the ticket. Caching this query would be nice.

We need tests demonstrating invalidation for all post CRUD operations.

The post will break found_posts and other parts of WP_Query that depend on the SELECT FOUND_ROWS() query. It may be wise for the cache to be structured as an array:

wp_cache_set( $cache_key, array(
    'results' => $results,
    'found_rows' => $found_rows,
), 'posts' );

Caching full query results has implications for 'cache_results'. 'cache_results' is set to false if you're using an object cache drop-in, because of the unnecessary overhead of update_post_caches() in a context where most references to posts are served via the $post global rather than the cache. See #12611 for the backstory. This overhead could be largely mitigated by implementing multiGet() and multiSet(), but this is complicated because it's not fully supported by all object cache backends. See https://core.trac.wordpress.org/ticket/31245#comment:18 and ensuing discussion.

In any case, the reasoning behind 'cache_results' when the only "results" being cached were post objects does not apply when we're caching the post IDs. I think the simple answer, for now, is that we should skip the 'cache_results' check.

We should also reassess the split_the_query check in light of the proposed patch. Specifically, don't see a reason why we couldn't cache the results of non-split queries, though we'll have to be careful about the way the cache key is generated. (Split queries and non-split queries ought to be able to share cache keys if the only difference is the $wpdb->posts.* fields clause, since we'd only be storing post IDs in the cache.) See #18536.

#3 @dd32
8 years ago

Just throwing a mention for a plugin which adds this cache functionality, and I've seen no ill side effects from: https://github.com/Automattic/advanced-post-cache/blob/master/advanced-post-cache.php

This ticket was mentioned in Slack in #core-multisite by spacedmonkey. View the logs.


8 years ago

#5 @ocean90
8 years ago

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

Duplicate of #22176.

Note: See TracTickets for help on using tickets.