Skip to:
Content

BuddyPress.org

Opened 11 years ago

Closed 11 years ago

#5362 closed defect (bug) (fixed)

Utilize caching during BP init process

Reported by: denis-de-bernardy's profile Denis-de-Bernardy Owned by:
Milestone: 2.0 Priority: normal
Severity: normal Version: 1.9.1
Component: Core Keywords:
Cc: ddebernardy@…

Description

While probing into what was throwing queries all over the place on my dev site (WPMU w/ two sites, a blank theme, BP, and an object-cache), I noted unnecessary queries getting thrown absolutely all over the place.

To see them, add a mu-plugin with the following code:

add_action('shutdown', function() {
    global $wpdb;
    if (defined('SAVEQUERIES') && SAVEQUERIES) var_dump($wpdb->queries);
}, 1000);

For instance, some of them get thrown multiple times on the same page, e.g this one on the secondary site could probably use calls to wp_cache_get():

SELECT * FROM wp_blogs WHERE blog_id = 1 /* get_blog_details */

This one gets run each and every time on a static front page, and should likely get ignored altogether (since it's a static front page that should theoretically never correspond to a BP page) or cached:

SELECT * FROM wp_users WHERE user_nicename = 'home'

I'd advise that the above also hints at a potential security problem if a user can potentially register with a matching username: could his profile take over the front page on a site with a static front page that is configured with BP_ENABLE_ROOT_PROFILES set to true?

These get done over and over on every page load. They could probably be cached too:

SELECT ID, post_name, post_parent, post_title FROM wp_posts WHERE ID IN (9,10) AND post_status = 'publish'
SELECT * FROM wp_bp_notifications WHERE user_id IN (1) AND component_name IN ('activity') AND is_new = 1

Same for notifications. memcache increment (new notifications)/delete (notifications read) might be a good idea here to avoid the query altogether:

SELECT * FROM wp_bp_notifications WHERE user_id IN (1) AND component_name IN ('activity') AND is_new = 1

The last three on the secondary site give me the impression that the last_activity user_meta is updated on every page load. This really should be throttled somewhat:

SELECT umeta_id FROM wp_usermeta WHERE meta_key = 'last_activity' AND user_id = 1
UPDATE `wp_usermeta` SET `meta_value` = '2014-01-30 17:59:50' WHERE `user_id` = 1 AND `meta_key` = 'last_activity'
SELECT user_id, meta_key, meta_value FROM wp_usermeta WHERE user_id IN (1) ORDER BY umeta_id ASC

On the primary site, there's an obvious missed opportunity to use wp_cache here:

SELECT meta_key AS name, meta_value AS value FROM wp_sitemeta WHERE meta_key IN ( 'tags_blog_id', 'sitewide_tags_blog', 'registration', 'fileupload_maxk' ) AND site_id = 1

Not sure what this one is supposed to do (it runs on both sites):

SELECT option_name AS name, option_value AS value FROM wp_options WHERE option_name IN ( 'bp-deactivated-components', 'bb-config-location', 'bp-xprofile-base-group-name', 'bp-xprofile-fullname-field-name', 'bp-blogs-first-install', 'bp-disable-profile-sync', 'hide-loggedout-adminbar', 'bp-disable-avatar-uploads', 'bp-disable-account-deletion', 'bp-disable-blogforum-comments', '_bp_theme_package_id', 'bp_restrict_group_creation', '_bp_enable_akismet', '_bp_force_buddybar', 'registration', 'avatar_default

Change History (7)

#1 @boonebgorges
11 years ago

  • Milestone changed from Awaiting Review to 2.0

Thanks for the details, Denis-de-Bernardy.

All the queries you mention here are necessary for BuddyPress to function, though you are correct that all of them should be aggressively cached. Persistent caching has not been a focus for BP in the past, but it's something that we are planning to pursue vigorously starting with this development cycle. See eg #5350. I'll move this ticket into our next major release milestone, as I think you're correct that these are easy and high-impact areas for caching.

The last three on the secondary site give me the impression that the last_activity user_meta is updated on every page load.

The intent is for this to be throttled to every five minutes. See https://buddypress.trac.wordpress.org/browser/tags/1.9.1/bp-core/bp-core-functions.php#L914. If this is failing, it's a bug.

Not sure what this one is supposed to do (it runs on both sites):

A number of BP's options are stored in sitemeta when on Multisite, which is not auto-cached by WP. So we pre-load them.

I'd advise that the above also hints at a potential security problem if a user can potentially register with a matching username: could his profile take over the front page on a site with a static front page that is configured with BP_ENABLE_ROOT_PROFILES set to true?

We filter WP's 'illegal_names' option to prevent this kind of hijacking: https://buddypress.trac.wordpress.org/browser/tags/1.9.1/bp-members/bp-members-functions.php#L1023

Again, thanks for the helpful ticket. It's my personal priority for the 2.0 dev cycle to take care many of these wp_cache()-able items.

#2 @boonebgorges
11 years ago

  • Summary changed from many, many unnecessary queries when BP initializes to Utilize caching during BP init process

I'm going to hijack this ticket for more general caching issues related to the BP bootstrap.

#3 @boonebgorges
11 years ago

In 7809:

Store directory_pages data in the persistent cache.

This information is needed during every bootstrap, yet rarely changes, so there
should be significant performance benefit from caching it.

Invalidation takes place when one of the pages is updated, or when the bp-pages
option is changed.

Note that this change will likely be moot after migrating to the WP rewrite
API. See #4954.

See #5362

Props Denis-de-Bernardy

#4 @boonebgorges
11 years ago

In 7812:

Introduce bp_xprofile_fullname_field_id() and use in query building throughout BP

There are many places throughout BuddyPress where it's necessary to query for
user fullnames. When XProfile is active, part of this process involves
determining the ID of the fullname field. Historically, this has been done by
querying for the ID of a field corresponding to the 'name' bp_xprofile_fullname_field_name().
Creating a standalone function for this allows us to centralize persistent
caching for the field ID.

As a side note, the fact that we do this query at all is a little farcical.
The fullname field is essentially necessarily tied to field #1 (due to some
hardcoding in the xprofile schema definition as well as bp-xprofile-admin.php).
We rely on the Name setting only for historical reasons, which probably didn't
even make all that much sense at the time. At some point, a serious review
should be done, and references to the fullname field id standardized (either
all hardcoded, or all not hardcoded).

See #5362

#5 @boonebgorges
11 years ago

In 7819:

Add persistent caching for bp_core_get_root_options()

See #5362

#6 @Denis-de-Bernardy
11 years ago

  • Cc ddebernardy@… added

#7 @boonebgorges
11 years ago

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

Except in some edge cases, I think we've successfully cached all queries performed during our bootstrap. Let's handle specific failures in separate tickets. Denis-de-Bernardy, thanks again for prompting this discussion.

Note: See TracTickets for help on using tickets.