WordPress.org

Make WordPress Core

Opened 3 years ago

Last modified 3 months ago

#16894 new defect (bug)

Bulk load Comment Meta upon access

Reported by: dd32 Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 3.1
Component: Comments Keywords: needs-patch 2nd-opinion
Focuses: performance Cc:

Description

When we query posts, we bulk load all of the posts meta and taxonomies at the time of request. Since adding comment meta, we load the comment meta on-the-fly since nothing really currently uses it in core.

I've added a filter to get_comment_text which modifies the comment content based on the value of a meta key.

The problem arises when multiple comments are displayed on a page, The metadata for every comment is loaded individually:

SELECT comment_id, meta_key, meta_value FROM wp_commentmeta WHERE comment_id IN (4)
SELECT comment_id, meta_key, meta_value FROM wp_commentmeta WHERE comment_id IN (5)
SELECT comment_id, meta_key, meta_value FROM wp_commentmeta WHERE comment_id IN (6)
SELECT comment_id, meta_key, meta_value FROM wp_commentmeta WHERE comment_id IN (7)
SELECT comment_id, meta_key, meta_value FROM wp_commentmeta WHERE comment_id IN (34)
SELECT comment_id, meta_key, meta_value FROM wp_commentmeta WHERE comment_id IN (35)
SELECT comment_id, meta_key, meta_value FROM wp_commentmeta WHERE comment_id IN (37)

in all cases, the function stack is:

comments_template, wp_list_comments, Walker->paged_walk, Walker_Comment->display_element, Walker->display_element, call_user_func_array, Walker_Comment->start_el, call_user_func, twentyten_comment, comment_text, get_comment_text, apply_filters, call_user_func_array, MY_FUNCTION_HOOKED_TO_GET_COMMENT_TEXT, get_comment_meta, get_metadata, update_meta_cache #12 (0.7ms)

It isn't ideal that every comment causes an extra query, instead, we should bulk load comment meta for the comments being displayed.

However, Since comment meta is not always used, in order to keep the load down, I'd suggest that we only bulk load the comment meta on the first request for comment meta, this should allow current users to have no detrimental affect on query count/performance, whilst allowing those who use comment meta to manage each comment better performance.

Change History (4)

comment:1 dd323 years ago

Adding this to the code in question bulk loads the comment meta for all requests for comment meta (which is suitable for the site in question I'm working on):

add_filter('the_comments', 'cache_all_comment_meta', 100); // using get_comments()
add_filter('comments_array', 'cache_all_comment_meta', 100); // using Direct SQL in comments_template() 
function cache_all_comment_meta($comments) {
	$comment_ids = wp_list_pluck($comments, 'comment_ID');
	update_meta_cache('comment', $comment_ids);
	return $comments;
}
comments_template, apply_filters, call_user_func_array, cache_all_comment_meta, update_meta_cache #12 (0.7ms)
SELECT comment_id, meta_key, meta_value FROM wp_commentmeta WHERE comment_id IN (4,5,6,7,34,35,36,37)
Last edited 3 years ago by dd32 (previous) (diff)

comment:2 scribu3 years ago

A similar approach is being considered for post thumbnails: #15447

comment:3 dd323 years ago

An implementation of lazy loading of the comment meta, which requires a bit of juggling due to the fact that this is being implemented as an external dropin.

class Lazy_Load_Comment_Meta {
	var $last_comment_query_ids = array();
	function __construct() {
		add_filter('get_comment_metadata', array(&$this, 'lazy_load_comment_metadata'), 100, 2);
		add_filter('the_comments', array(&$this, 'lazy_load_comment_metadata_fillids'), 100);
		add_filter('comments_array', array(&$this, 'lazy_load_comment_metadata_fillids'), 100);
	}

	function lazy_load_comment_metadata($data, $object_id) {
		if ( !wp_cache_get($object_id, 'comment_meta') ) {
			if ( !empty($this->last_comment_query_ids) )
				update_meta_cache('comment', $this->last_comment_query_ids );
		}
		return null;
	}
	function lazy_load_comment_metadata_fillids($comments) {
		foreach ( $comments as $c )
			$this->last_comment_query_ids[] = $c->comment_ID;
		return $comments;
	}
}

Stack:

Front: 
comments_template, wp_list_comments, Walker->paged_walk, Walker_Comment->display_element, Walker->display_element, call_user_func_array, Walker_Comment->start_el, call_user_func, twentyten_comment, comment_text, get_comment_text, apply_filters, call_user_func_array, secret_hooked_function, get_comment_meta, get_metadata, apply_filters, call_user_func_array, Lazy_Load_Comment_Meta->lazy_load_comment_metadata, update_meta_cache

Back:
WP_Comments_List_Table->display, WP_List_Table->display_rows_or_placeholder, WP_List_Table->display_rows, WP_Comments_List_Table->single_row, WP_List_Table->single_row_columns, call_user_func, WP_Comments_List_Table->column_comment, comment_text, get_comment_text, apply_filters, call_user_func_array, secret_hooked_function, get_comment_meta, get_metadata, apply_filters, call_user_func_array, Lazy_Load_Comment_Meta->lazy_load_comment_metadata, update_meta_cache

comment:4 nacin3 months ago

  • Component changed from Performance to Comments
  • Focuses performance added

I would love this.

Note: See TracTickets for help on using tickets.