Skip to:
Content

BuddyPress.org

Opened 11 years ago

Closed 10 years ago

#967 closed defect (bug) (wontfix)

[patch] unnecessary extra query caused by bp_dtheme_show_home_blog

Reported by: junsuijin Owned by: junsuijin
Milestone: 1.5 Priority: major
Severity: Version:
Component: Core Keywords: has-patch, dev-feedback
Cc:

Description

I just tested this in trunk rev 1786 and the provided patch will still reduce the total query count by 1. The query that is eliminated is the one that gets the posts to populate the $wp global. This query is extraneous as bp_dtheme_show_home_blog makes its own query to get the posts.

Attachments (1)

reduce-queries-bp_dtheme_show_home_blog.patch (679 bytes) - added by junsuijin 11 years ago.

Download all attachments as: .zip

Change History (10)

#1 @junsuijin
11 years ago

  • Priority changed from minor to major

As asked in #buddypress-dev, this function can indeed be extended as follows to remove extra queries from all bp pages, including a quick test of 1 query removed on the members dir, 2 gone on a logged in users profile, 4 gone on the forums dir:

function bp_filter_wp_posts_request( $query ) {
	global $bp;

	if ( $bp->current_component != BP_HOME_BLOG_SLUG && bp_is_blog_page() )
		return $query;

	// nullify the initial WordPress posts_request
	remove_filter( 'posts_request', 'bp_filter_wp_posts_request', 0 );
	return;
}
add_filter( 'posts_request', 'bp_filter_wp_posts_request', 0 );

The thing about this, is that the $bp->current_component != BP_HOME_BLOG_SLUG bit is only going to necessarily apply to the bp-sn-parent's child themes (well really only bp-default and any others that use the /blog format), so we should probably make a hook at that point in the function, and use the bp-sn-parent functions.php to put that bit in there, thus allowing other themes to alter this check as necessary.

#2 @apeatling
11 years ago

  • Milestone changed from 1.1 to 1.2

This is important to fix up, but it's too risky for 1.1. Will look at this for 1.2.

#3 @junsuijin
11 years ago

I've not encountered any problems using this patch; however, I've just posted the following ticket to http://core.trac.wordpress.org/ticket/10886
The method in this ticket adds an earlier short-circuit point, further reducing queries and loading times on every BuddyPress page, save the 'recent posts' for members page, which it actually increases queries on (but does not break).

Once this patch is committed to WPMU, we can use something like the following code:

function bp_define_no_query() {
	if ( ! bp_is_blog_page() && ! bp_is_user_recent_posts() && ! defined( 'NO_QUERY' ) )
		define( 'NO_QUERY', true );
}
add_action( 'init', 'bp_define_no_query', 0 );

#4 @junsuijin
11 years ago

Just updated the WP ticket, the new method saves more time, but as mentioned in the WP ticket, adds extra queries (likely due to a lack of caching), when site-wide posts are queried. This is easy enough to prevent in case end users want to include those kinds of widgets on BuddyPress pages that don't already have them. The updated means of applying this fix should work as follows (note that this method can also save queries on the /blog page but this particular invocation will not cause that to happen):

function bp_no_wp_main() {
	if ( ! bp_is_blog_page() && ! bp_is_user_recent_posts() )
		add_filter( 'wp_main', 'bp_set_no_wp_main' );
}
add_action( 'init', 'bp_no_wp_main', 0 );

	function bp_set_no_wp_main() {
		return false;
	}

#5 @DJPaul
11 years ago

  • Keywords dev-feedback added; tested removed

Maybe we should be waiting outcome on the WordPress ticket, but this patch needs to account for people running the BP or a child theme on hosted blogs. I think this is unfortunately too late for 1.2 but leaving for Andy or John to comment.

#6 @johnjamesjacoby
11 years ago

  • Milestone changed from 1.2 to 1.3

If I remember correctly this can have implications in several places and would need a good round of dedicated testing before going live, right?

I think we're running out of time to safely stuff this in and adequately test before the release of 1.2.

I'm going to bump this to a future release. Feel free to keep working on this though as it would be great to see this in core.

#7 @cnorris23
10 years ago

  • Component set to Core

#8 @paulhastings0
10 years ago

  • Summary changed from unnecessary extra query caused by bp_dtheme_show_home_blog to [patch] unnecessary extra query caused by bp_dtheme_show_home_blog

#9 @DJPaul
10 years ago

  • Resolution set to wontfix
  • Status changed from new to closed

This was relevant for only the 1.1 theme, but thank you junsuijin for the patch regardless.

Note: See TracTickets for help on using tickets.