Avoid updating post meta and term cache#7171
Avoid updating post meta and term cache#7171waviaei wants to merge 3 commits intoWordPress:trunkfrom
Conversation
|
Hi @waviaei! 👋 Thank you for your contribution to WordPress! 💖 It looks like this is your first pull request to No one monitors this repository for new pull requests. Pull requests must be attached to a Trac ticket to be considered for inclusion in WordPress Core. To attach a pull request to a Trac ticket, please include the ticket's full URL in your pull request description. Pull requests are never merged on GitHub. The WordPress codebase continues to be managed through the SVN repository that this GitHub repository mirrors. Please feel free to open pull requests to work on any contribution you are making. More information about how GitHub pull requests can be used to contribute to WordPress can be found in the Core Handbook. Please include automated tests. Including tests in your pull request is one way to help your patch be considered faster. To learn about WordPress' test suites, visit the Automated Testing page in the handbook. If you have not had a chance, please review the Contribute with Code page in the WordPress Core Handbook. The Developer Hub also documents the various coding standards that are followed:
Thank you, |
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
Specify fields ids, and pass to wp_get_post_revisions() to improve performance.
| } | ||
|
|
||
| $revisions = wp_get_post_revisions( $post->ID ); | ||
| $revisions = wp_get_post_revisions( $post->ID, array( 'fields' => 'ids' ) ); |
There was a problem hiding this comment.
Due to the call to wp_is_post_autosave() later in this function, limiting the query to IDs only will cause an increase in the number of DB queries. As a POC test:
/**
* @ticket 61849
*/
public function test_wp_list_post_revisions_db_queries() {
$post = get_default_post_to_edit( 'post', true );
$post_id = $post->ID;
// Create a few revisions.
foreach ( range( 1, 5 ) as $i ) {
wp_update_post(
array(
'post_status' => 'publish',
'post_content' => 'Content ' . $i,
'ID' => $post_id,
)
);
}
// Flush the cache.
wp_cache_flush();
// Get number of queries before calling wp_list_post_revisions.
$queries = get_num_queries();
get_echo( 'wp_list_post_revisions', array( $post_id ) );
$number_of_queries = get_num_queries() - $queries;
// Expect four queries for the revisions.
$this->assertSame( 4, $number_of_queries );
}On trunk the test passes, on this branch it fails:
$ ./vendor/bin/phpunit --group 61849
Installing...
Running as single site... To run multisite, use -c tests/phpunit/multisite.xml
Not running ajax tests. To execute these, use --group ajax.
Not running ms-files tests. To execute these, use --group ms-files.
Not running external-http tests. To execute these, use --group external-http.
PHPUnit 9.6.19 by Sebastian Bergmann and contributors.
Warning: Your XML configuration validates against a deprecated schema.
Suggestion: Migrate your XML configuration using "--migrate-configuration"!
F 1 / 1 (100%)
Time: 00:00.160, Memory: 199.00 MB
There was 1 failure:
1) Tests_Post_Revisions::test_wp_list_post_revisions_db_queries
Failed asserting that 8 is identical to 4.
/vagrant/wordpress-develop/tests/phpunit/tests/post/revisions.php:48
There was a problem hiding this comment.
@peterwilsoncc Thank you for the POC test. I run the same test and yes, more queries here. I will look to revert the commit.
| 'update_post_meta_cache' => false, | ||
| 'update_post_term_cache' => false, |
There was a problem hiding this comment.
For posts with footnotes enabled the footnotes meta data is included in the revisions (see src/wp-includes/blocks/footnotes.php) so I think the meta cache will need to remain primed.
You may be able to check if the post type supports footnotes.
Not priming the term cache makes sense though.
There was a problem hiding this comment.
@peterwilsoncc Footnotes - thats a good point, thanks. But still, wp_list_post_revisions() doesn't need to use any data from postmeta. I am thinking, as long as post information is primed, that is all it needs here.
There was a problem hiding this comment.
@waviaei Footnotes are included on the revision screen. Plugin developers can also register meta data with revisions and may choose to display that on the revisions screen too.
There was a problem hiding this comment.
Pull request overview
This PR aims to optimize performance when retrieving post revisions by preventing unnecessary database queries for post meta and term caches. The changes add default parameters to skip cache updates for revision queries, which is appropriate since revisions typically don't use this metadata.
Changes:
- Added
update_post_meta_cacheandupdate_post_term_cachedefaults set tofalseinwp_get_post_revisions() - Modified
wp_list_post_revisions()to fetch only revision IDs instead of full post objects
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/wp-includes/revision.php | Adds performance optimization flags to prevent unnecessary cache updates when fetching revisions |
| src/wp-includes/post-template.php | Attempts to optimize by fetching only revision IDs, but implementation is incomplete |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
|
|
||
| $revisions = wp_get_post_revisions( $post->ID ); | ||
| $revisions = wp_get_post_revisions( $post->ID, array( 'fields' => 'ids' ) ); |
There was a problem hiding this comment.
Changing to fetch only IDs with 'fields' => 'ids' is problematic because the code after this point (lines 2044-2054) expects post objects. Specifically, wp_is_post_autosave() on line 2049 and wp_post_revision_title_expanded() on line 2054 will receive IDs instead of objects. While these functions can accept IDs, they will internally call wp_get_post_revision() or get_post() to fetch the full post objects anyway, which defeats the performance optimization intended by this change. The optimization is incomplete.
Set
update_post_meta_cacheandupdate_post_term_cacheto false, for to use as default$argsto be passed toget_children().Trac ticket: https://core.trac.wordpress.org/ticket/61849
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.