Cache BP_Messages_Thread::get_recipients() to reduce inbox queries by 20

[wpdennis]

The inbox (and sentbox) calls ​BP_Messages_Thread::get_recipients() twice per conversation in messages-loop.php. Once via bp_has_message_threads() and once via bp_message_thread_total_and_unread_count().

So, in an inbox/sentbox with 10 conversations, that are 20 queries on each page load. I added some caching to get_recipients(), to remove these 20 queries:

public function get_recipients() {
	$recipients = wp_cache_get( 'recipients-' . $this->thread_id, 'bp_messages' );

	if ( false === $recipients ) {
		global $wpdb, $bp;

		$results = $wpdb->get_results( $wpdb->prepare( "SELECT * FROM {$bp->messages->table_name_recipients} WHERE thread_id = %d", $this->thread_id ) );

		foreach ( (array) $results as $recipient ) {
			$recipients[$recipient->user_id] = $recipient;
		}

		wp_cache_set( 'recipients-' . $this->thread_id, $recipients, 'bp_messages' );
	}

	/**
	 * Filters the recipients of a message thread.
	 *
	 * @since BuddyPress (2.2.0)
	 *
	 * @param array $recipients Array of recipient objects.
	 * @param int   $thread_id  ID of the current thread.
	 */
	return apply_filters( 'bp_messages_thread_get_recipients', $recipients, $this->thread_id );
}

Is there a way to change the recipient in a conversion where we would need to add some cache invalidation? I haven't found one, except maybe the deletion of an user?

[wpdennis]

While digging deeper, I opened a second ticket: #6221

I think we have two issues here:

a) messages-loop.php calls things in a way, where redundant code (with db queries) is executed twice. This could be worth a look on its own and if so, #6221 would be the better ticket to adress this issue.

b) get_recipients() should be trivial to cache, since the data is really persistant. So we could save at minimum 10 queries in any scenario and in the current implementation even 20 queries. So we could keep this ticket just for the caching of get_recipient(), despite possible changes to messages-loop.php.

[boonebgorges]

Very good catch on get_recipients(). We should definitely invalidate cache on thread deletion. I'm not sure whether it's possible in the BP interface to add a user to a thread when sending a subsequent message, but it's possible in the data model, so to be safe let's also invalidate a thread's recipient when a message is added to the thread.

[boonebgorges]

In 9482:

Cache the results of BP_Messages_Thread::get_recipients().

This can save a large number of queries on certain inbox views.

Props wpdennis.
See #6221. Fixes #6220.

[r-a-y]

Need to bust the recipient cache whenever a thread is read / unread.

See 6220.read.patch.

[r-a-y]

In 9714:

Messages: Bust recipient cache when a message thread is marked as read or unread.

See r9482, which introduces message recipient caching.

Fixes #6220.