Opened 2 years ago

Last modified 2 years ago

#16894 new defect (bug)

Bulk load Comment Meta upon access

Reported by: dd32 Owned by:
Priority: normal Milestone: Future Release
Component: Performance Version: 3.1
Severity: normal Keywords: needs-patch 2nd-opinion
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 (3)

comment:1   dd322 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 2 years ago by dd32 (previous) (diff)

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

comment:3   dd322 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
Note: See TracTickets for help on using tickets.