Comments: Always lazily load comment meta.

In [34270] introduced lazy loading of comment meta. However, this was only in the context of `WP_Query`. Other parts of the codebase, like `WP_Comment_Query` did not lazily load comment meta. In this change, calls to `update_meta_cache` are now replaced with `wp_lazyload_comment_meta`, that instead of priming comment meta caches, just adds them to the queue to be primed it ever called. This results in far less database queries, as there a number of places where comment meta is being primed unnecessarily and never used. Adding everything to the comment meta queue, also means that if comment meta is used, that is all loaded in a single database / cache call.

Follow on from [55671], [55747].

Props spacedmonkey, peterwilsoncc, flixos90, mukesh27.
Fixes #57801.

git-svn-id: https://develop.svn.wordpress.org/trunk@55749 602fd350-edb4-49c9-b593-d223f7449a82
This commit is contained in:
Jonny Harris 2023-05-11 12:25:51 +00:00
parent 1534d12067
commit 3114eda235
6 changed files with 124 additions and 31 deletions

View File

@ -165,8 +165,6 @@ class WP_Comments_List_Table extends WP_List_Table {
$_comments = get_comments( $args );
if ( is_array( $_comments ) ) {
update_comment_cache( $_comments );
$this->items = array_slice( $_comments, 0, $comments_per_page );
$this->extra_items = array_slice( $_comments, $comments_per_page );

View File

@ -481,12 +481,16 @@ class WP_Comment_Query {
$comment_ids = array_map( 'intval', $comment_ids );
if ( $this->query_vars['update_comment_meta_cache'] ) {
wp_lazyload_comment_meta( $comment_ids );
}
if ( 'ids' === $this->query_vars['fields'] ) {
$this->comments = $comment_ids;
return $this->comments;
}
_prime_comment_caches( $comment_ids, $this->query_vars['update_comment_meta_cache'] );
_prime_comment_caches( $comment_ids, false );
// Fetch full comment objects from the primed cache.
$_comments = array();

View File

@ -2813,7 +2813,7 @@ class WP_Query {
$comment_ids = $wpdb->get_col( $comments_request );
wp_cache_add( $cache_key, $comment_ids, 'comment-queries' );
}
_prime_comment_caches( $comment_ids, false );
_prime_comment_caches( $comment_ids );
// Convert to WP_Comment.
/** @var WP_Comment[] */
@ -3372,7 +3372,7 @@ class WP_Query {
$comment_ids = $wpdb->get_col( $comments_request );
wp_cache_add( $comment_cache_key, $comment_ids, 'comment-queries' );
}
_prime_comment_caches( $comment_ids, false );
_prime_comment_caches( $comment_ids );
// Convert to WP_Comment.
/** @var WP_Comment[] */
@ -3487,11 +3487,6 @@ class WP_Query {
}
}
// If comments have been fetched as part of the query, make sure comment meta lazy-loading is set up.
if ( ! empty( $this->comments ) ) {
wp_queue_comments_for_comment_meta_lazyload( $this->comments );
}
if ( ! $q['suppress_filters'] ) {
/**
* Filters the array of retrieved posts after they've been fetched and

View File

@ -1440,12 +1440,11 @@ function comments_template( $file = '/comments.php', $separate_comments = false
$comment_author_url = esc_url( $commenter['comment_author_url'] );
$comment_args = array(
'orderby' => 'comment_date_gmt',
'order' => 'ASC',
'status' => 'approve',
'post_id' => $post->ID,
'no_found_rows' => false,
'update_comment_meta_cache' => false, // We lazy-load comment meta for performance.
'orderby' => 'comment_date_gmt',
'order' => 'ASC',
'status' => 'approve',
'post_id' => $post->ID,
'no_found_rows' => false,
);
if ( get_option( 'thread_comments' ) ) {
@ -2380,8 +2379,6 @@ function wp_list_comments( $args = array(), $comments = null ) {
$parsed_args['reverse_top_level'] = ( 'desc' === get_option( 'comment_order' ) );
}
wp_queue_comments_for_comment_meta_lazyload( $_comments );
if ( empty( $parsed_args['walker'] ) ) {
$walker = new Walker_Comment();
} else {

View File

@ -484,6 +484,21 @@ function get_comment_meta( $comment_id, $key = '', $single = false ) {
return get_metadata( 'comment', $comment_id, $key, $single );
}
/**
* Queue comment meta for lazy-loading.
*
* @since 6.3.0
*
* @param array $comment_ids List of comment IDs.
*/
function wp_lazyload_comment_meta( array $comment_ids ) {
if ( empty( $comment_ids ) ) {
return;
}
$lazyloader = wp_metadata_lazyloader();
$lazyloader->queue_objects( 'comment', $comment_ids );
}
/**
* Updates comment meta field based on comment ID.
*
@ -514,6 +529,9 @@ function update_comment_meta( $comment_id, $meta_key, $meta_value, $prev_value =
* Queues comments for metadata lazy-loading.
*
* @since 4.5.0
* @since 6.3.0 Use wp_lazyload_comment_meta() for lazy-loading of comment meta.
*
* @see wp_lazyload_comment_meta()
*
* @param WP_Comment[] $comments Array of comment objects.
*/
@ -528,10 +546,7 @@ function wp_queue_comments_for_comment_meta_lazyload( $comments ) {
}
}
if ( $comment_ids ) {
$lazyloader = wp_metadata_lazyloader();
$lazyloader->queue_objects( 'comment', $comment_ids );
}
wp_lazyload_comment_meta( $comment_ids );
}
/**
@ -3331,6 +3346,7 @@ function update_comment_cache( $comments, $update_meta_cache = true ) {
*
* @since 4.4.0
* @since 6.1.0 This function is no longer marked as "private".
* @since 6.3.0 Use wp_lazyload_comment_meta() for lazy-loading of comment meta.
*
* @see update_comment_cache()
* @global wpdb $wpdb WordPress database abstraction object.
@ -3345,7 +3361,11 @@ function _prime_comment_caches( $comment_ids, $update_meta_cache = true ) {
if ( ! empty( $non_cached_ids ) ) {
$fresh_comments = $wpdb->get_results( sprintf( "SELECT $wpdb->comments.* FROM $wpdb->comments WHERE comment_ID IN (%s)", implode( ',', array_map( 'intval', $non_cached_ids ) ) ) );
update_comment_cache( $fresh_comments, $update_meta_cache );
update_comment_cache( $fresh_comments, false );
}
if ( $update_meta_cache ) {
wp_lazyload_comment_meta( $comment_ids );
}
}

View File

@ -11,7 +11,7 @@ class Tests_Comment_MetaCache extends WP_UnitTestCase {
*
* @covers ::update_comment_meta
*/
public function test_update_comment_meta_cache_should_default_to_true() {
public function test_update_comment_meta_cache_should_default_to_lazy_loading() {
$p = self::factory()->post->create( array( 'post_status' => 'publish' ) );
$comment_ids = self::factory()->comment->create_post_comments( $p, 3 );
@ -22,18 +22,55 @@ class Tests_Comment_MetaCache extends WP_UnitTestCase {
// Clear comment cache, just in case.
clean_comment_cache( $comment_ids );
$q = new WP_Comment_Query(
$num_queries = get_num_queries();
$q = new WP_Comment_Query(
array(
'post_ID' => $p,
)
);
$this->assertSame( 2, get_num_queries() - $num_queries, 'Querying comments is expected to make two queries' );
$num_queries = get_num_queries();
foreach ( $comment_ids as $cid ) {
get_comment_meta( $cid, 'foo', 'bar' );
}
$this->assertSame( $num_queries, get_num_queries() );
$this->assertSame( 1, get_num_queries() - $num_queries, 'Querying comments is expected to make two queries' );
}
/**
* @ticket 57801
*
* @covers ::wp_lazyload_comment_meta
*/
public function test_update_comment_meta_cache_should_default_to_lazy_loading_fields_id() {
$p = self::factory()->post->create( array( 'post_status' => 'publish' ) );
$comment_ids = self::factory()->comment->create_post_comments( $p, 3 );
foreach ( $comment_ids as $cid ) {
update_comment_meta( $cid, 'foo', 'bar' );
}
// Clear comment cache, just in case.
clean_comment_cache( $comment_ids );
$num_queries = get_num_queries();
$q = new WP_Comment_Query(
array(
'post_ID' => $p,
'fields' => 'ids',
)
);
$this->assertSame( 1, get_num_queries() - $num_queries, 'Querying comments is expected to make two queries' );
$num_queries = get_num_queries();
foreach ( $comment_ids as $cid ) {
get_comment_meta( $cid, 'foo', 'bar' );
}
$this->assertSame( 1, get_num_queries() - $num_queries, 'Comment meta is expected to be lazy loaded' );
}
/**
@ -52,19 +89,59 @@ class Tests_Comment_MetaCache extends WP_UnitTestCase {
// Clear comment cache, just in case.
clean_comment_cache( $comment_ids );
$q = new WP_Comment_Query(
$num_queries = get_num_queries();
$q = new WP_Comment_Query(
array(
'post_ID' => $p,
'update_comment_meta_cache' => true,
)
);
$this->assertSame( 2, get_num_queries() - $num_queries, 'Comments should be queries and primed in two database queries' );
$num_queries = get_num_queries();
foreach ( $comment_ids as $cid ) {
get_comment_meta( $cid, 'foo', 'bar' );
}
$this->assertSame( $num_queries, get_num_queries() );
$this->assertSame( 1, get_num_queries() - $num_queries, 'Comment meta should be loaded in one database query' );
}
/**
* @ticket 57801
*
* @covers ::update_comment_meta
*/
public function test_update_comment_meta_cache_true_multiple() {
$posts = self::factory()->post->create_many( 3 );
$all_comment_ids = array();
foreach ( $posts as $p ) {
$comment_ids = self::factory()->comment->create_post_comments( $p, 3 );
foreach ( $comment_ids as $cid ) {
update_comment_meta( $cid, 'foo', 'bar' );
$all_comment_ids[] = $cid;
}
$num_queries = get_num_queries();
$q = new WP_Comment_Query(
array(
'post_ID' => $p,
'update_comment_meta_cache' => true,
)
);
$this->assertSame( 1, get_num_queries() - $num_queries, 'Comment query should only add one query' );
}
$filter = new MockAction();
add_filter( 'update_comment_metadata_cache', array( $filter, 'filter' ), 10, 2 );
$num_queries = get_num_queries();
get_comment_meta( $comment_ids[0], 'foo', 'bar' );
$this->assertSame( 1, get_num_queries() - $num_queries, 'Comment meta should be loaded in one database query' );
$args = $filter->get_args();
$first = reset( $args );
$prime_comment_ids = end( $first );
$this->assertSameSets( $prime_comment_ids, $all_comment_ids, 'All comment meta should be loaded all at once' );
}
/**
@ -92,7 +169,7 @@ class Tests_Comment_MetaCache extends WP_UnitTestCase {
get_comment_meta( $cid, 'foo', 'bar' );
}
$this->assertSame( $num_queries + 3, get_num_queries() );
$this->assertSame( 3, get_num_queries() - $num_queries );
}
/**
@ -120,12 +197,12 @@ class Tests_Comment_MetaCache extends WP_UnitTestCase {
// First request will hit the database.
$num_queries = get_num_queries();
get_comment_meta( $comment_ids[0], 'sauce' );
$this->assertSame( $num_queries + 1, get_num_queries() );
$this->assertSame( 1, get_num_queries() - $num_queries );
// Second and third requests should be in cache.
get_comment_meta( $comment_ids[1], 'sauce' );
get_comment_meta( $comment_ids[2], 'sauce' );
$this->assertSame( $num_queries + 1, get_num_queries() );
$this->assertSame( 1, get_num_queries() - $num_queries );
}
}
}
@ -134,6 +211,7 @@ class Tests_Comment_MetaCache extends WP_UnitTestCase {
* @ticket 34047
*
* @covers ::get_comment_meta
* @covers ::wp_lazyload_comment_meta
*/
public function test_comment_meta_should_be_lazy_loaded_in_comment_feed_queries() {
$posts = self::factory()->post->create_many( 2, array( 'post_status' => 'publish' ) );
@ -182,6 +260,7 @@ class Tests_Comment_MetaCache extends WP_UnitTestCase {
* @ticket 34047
*
* @covers ::get_comment_meta
* @covers ::wp_lazyload_comment_meta
*/
public function test_comment_meta_should_be_lazy_loaded_in_single_post_comment_feed_queries() {
$posts = self::factory()->post->create_many( 2, array( 'post_status' => 'publish' ) );