Project

General

Profile

Bug #14909

Reducing DB queries and asset loading on the main site

Added by Raymond Hoh about 1 month ago. Updated 29 days ago.

Status:
New
Priority name:
Normal
Assignee:
Category name:
Performance
Target version:
Start date:
2021-10-27
Due date:
% Done:

0%

Estimated time:

Description

Since we are redesigning the homepage, it's a good opportunity to audit the number of database queries that run on the homepage (and the main site in general).

With the following code snippets, this can drop the DB query count by 11:

/**
 * Stop unnecessary DB queries for Classic Editor plugin.
 *
 * Classic Editor settings are only needed in the admin area.
 */
add_filter( 'classic_editor_plugin_settings', function( $retval ) {
    if ( is_admin() ) {
        return $retval;
    }

    return [];
} );

// No need to check for mapped domain redirects on the main site.
add_action( 'send_headers', function() {
    if ( is_main_site() ) {
        remove_action( 'send_headers', 'Mercator\Redirect\handle_redirect' );
    }
}, 0 );

// Only allow bbPress to initialize its roles on a group forum page.
add_action( 'wp_roles_init', function() {
    if ( is_admin() ) {
        return;
    }

    /*
     * bbPress initializes its roles on every page load.
     *
     * This runs five uncached DB queries, so let's only enable this on group
     * forum pages where this might be necessary.
     */
    if ( is_main_site() && false === strpos( $_SERVER['REQUEST_URI'], '/forum/' ) ) {
        remove_action( 'wp_roles_init', 'bbp_roles_init' );
    }
}, 0 );

Boone, I'm running this in wp-content/mu-plugins/ray.php at the moment if you need to disable anything.

The Classic Editor and Mercator mods should be pretty straightforward. With the bbPress one, I briefly tested by logging in as a regular user and attempting to create a new group topic and this still worked fine.

Just wanted to post as a FYI. I'll add more improvements as I find them.

History

#1 Updated by Boone Gorges about 1 month ago

Awesome! I noticed that bbPress one the other day, scratched my head for a moment, but decided I didn't feel like descending into the bowels of bbPress to figure it out. Thanks for taking one for the team :-D

I see no reason why the homepage shouldn't be able to load without any database queries at all (when hitting the cache). Let's keep this ticket to see if we can add caching layers where appropriate around the various "latest..." queries on the homepage.

#3 Updated by Boone Gorges about 1 month ago

https://github.com/cuny-academic-commons/cac/commit/2accf094c36b523d6df523d043367421d810cc73 adds some caching for the site activity query for logged-out users.

#4 Updated by Boone Gorges about 1 month ago

https://github.com/cuny-academic-commons/cac/commit/ace8733b1eed81d5b1333872857bb6ef005c69ad (follow-up https://github.com/cuny-academic-commons/cac/commit/f33799b79962fb72d73f526fa5946972f6cc107a) reworks the Members blade query so that it's more efficient and cacheable. Unfortunately, BP_User_Query always fetches WP user objects, even if you only want user_ids. So I've done a direct query instead, and added some caching to it.

The remaining queries on the logged-out page are linked to WP: the main query (hard to short-circuit) and some new stuff related to block templates.

#5 Updated by Raymond Hoh about 1 month ago

Thanks for looking into caching other parts of the homepage loops!

Regarding:

Here are a few tricks that prevent some of the initial Cavalcade queries:

https://github.com/cuny-academic-commons/cac/commit/13d4fd618aa37185957b5ee33579b771e0ec546d
https://github.com/cuny-academic-commons/cac/commit/32aca9c91a861a075d49775bb604e9b8451e6a4b

The Cavalcade ones shouldn't be necessary. See https://github.com/cuny-academic-commons/cac/blob/73972fcffa41678b03d35247c55e78c562bd2ed7/wp-content/mu-plugins/cavalcade.php#L55-L61.

I think we need to remove the older jobs from cdev's Cavalcade DB tables like we did on production. I'll look into this.

#6 Updated by Raymond Hoh about 1 month ago

Ahh, I see that the 'pre_get_scheduled_event' is the hook we should be using if we want to cancel any cron tasks from running instead of attempting to bail on the 'schedule_event' hook.

Let's move the following tasks over to 'pre_get_scheduled_event' and return false:
https://github.com/cuny-academic-commons/cac/blob/73972fcffa41678b03d35247c55e78c562bd2ed7/wp-content/mu-plugins/cavalcade.php#L55-L73

#7 Updated by Raymond Hoh about 1 month ago

We can save an additional five DB queries by bailing out of queries where WordPress is looking for block templates:

Here's the code:

/**
 * Don't query for block templates.
 *
 * We don't use block templates in bp-nelo.
 */
add_filter( 'posts_pre_query', function( $retval, $query ) {
    if ( is_admin() || ! is_main_site() || $query->is_main_query() ) {
        return $retval;
    }

    // Bail if we're not querying for a block template.
    if ( 'wp_template' !== $query->get( 'post_type' ) ) {
        return $retval;
    }

    // Set default properties as recommended in the 'posts_pre_query' DocBlock.
    $query->found_posts   = 0;
    $query->max_num_pages = 0;

    // Return something other than a null value to bypass WP_Query.
    return array();
}, 10, 2 );

bp-nelo doesn't use block templates so we should be good. This is currently running on cdev for the main site only.

In the future, if we decide to use block templates on the frontend, we'll need to remove this block.

#8 Updated by Boone Gorges about 1 month ago

Awesome!

#9 Updated by Raymond Hoh 29 days ago

  • Subject changed from Reducing DB queries on the main site to Reducing DB queries and asset loading on the main site

I've committed what I've talked about above in the following commits: https://github.com/cuny-academic-commons/cac/compare/b5985b9...e43e413.

For Cavalcade, I've come up with this solution: https://github.com/cuny-academic-commons/cac/commit/3420a2aa9590b3a9fb9e49c00eb1d2640162c0fc. It takes our existing jobs that we want to cancel and applies them to the 'pre_schedule_event' and 'pre_get_scheduled_event' hooks, which Cavalcade uses to query for jobs. We can bail from these jobs early on these hooks.


I'm also going to co-opt this ticket to talk about limiting network resources.

Most of the recent commits are pretty straightforward:

However, I want to talk about removing all BuddyPress blocks and widgets from being loaded. With the new homepage, we no longer rely at all on any BuddyPress widgets. So I think it will be safe to stop loading BuddyPress widget code entirely. Similarly, regarding BuddyPress blocks, we're not utilizing block widgets in bp-nelo, so we do not need to load the code for that. Removing both BP widgets and blocks will save us quite a few network resources.

Boone, I've tentatively committed this here: https://github.com/cuny-academic-commons/cac/commit/b5985b91d766d5afb3d3604452c99c65b6ba2647. This applies across the entire network. However, if we feel we might want to limit this to the main site only, we can do that as well.

Other than that, there are a few other assets we could remove like wp-embed, bp-moment, bp-livestamp. The latter two can be kept if we want to implement dynamic timestamps on the homepage. Is this something we want to do? It's not really pertinent and would only be useful for the activity blade for the logged-in homepage.

#10 Updated by Boone Gorges 29 days ago

I'm on board 100% with not loading static assets (JS, etc) related to BP widgets.

I'm less comfortable with preventing them from loading in PHP. For one thing, I think that I've seen some of the widgets actually being used elsewhere in the network, so I think we ought to limit to the main site only. Furthermore, BP has a history of not being super modular, and I worry that things will break if we start unhooking them selectively. Blocks and Widgets are probably fine - hard to imagine that other things would depend on them - but it's something I'd like to be cautious about.

Other than that, there are a few other assets we could remove like wp-embed, bp-moment, bp-livestamp. The latter two can be kept if we want to implement dynamic timestamps on the homepage. Is this something we want to do? It's not really pertinent and would only be useful for the activity blade for the logged-in homepage.

IIRC we introduced the dynamic timestamps because we were caching the HTML of the widgets, but the "ago" text would be wrong in the cached markup. We're no longer caching HTML, just the database objects, so I think it's safe to let the "ago" text be generated on the server. Does that sound right to you?

#11 Updated by Raymond Hoh 29 days ago

Blocks and Widgets are probably fine - hard to imagine that other things would depend on them - but it's something I'd like to be cautious about.

WordPress is pretty good about this. If the widget is no longer available, WordPress will just omit that widget from display. Same thing occurs for blocks; in the post editor, there will be a warning about the block no longer being available and will ask you if you want to remove it.

For one thing, I think that I've seen some of the widgets actually being used elsewhere in the network, so I think we ought to limit to the main site only.

Okay, I'll remove all BuddyPress widgets for the main site only.

I think we should consider removing the Sitewide Notices widget across the network as sitewide notices are only useful on the main site and we are not really utilizing this BuddyPress feature.


Regarding BuddyPress blocks, I think we should take a look at which blocks might actually be useful for those on the network. Here's the list of blocks that BuddyPress offers:

  • Login Form
  • Member
  • Members
  • Dynamic Members List
  • Online Members
  • Recently Active Members
  • Latest Activities
  • Embed activity
  • Recent Networkwide Posts
  • Friends List
  • Group
  • Groups
  • Dynamic Groups List
  • Sitewide Notices

Of those, we can probably remove the "Sitewide Notices" and "Embed activity" blocks. Regarding the latter, it is possible to use the existing WordPress "Embed" block to embed BuddyPress activity items and I'm also not sure users would know how to get the BuddyPress Activity URL in order to use the "Embed activity" block as well.

The "Member" and "Group" blocks are similar to their plural block, but they also offer the ability to display the cover image as well. We do not utilize cover images in bp-nelo, so I think it's safe to remove these blocks.

The rest of the blocks have some utility in some form or another, but we could be aggressive and remove more if we wanted.


We're no longer caching HTML, just the database objects, so I think it's safe to let the "ago" text be generated on the server. Does that sound right to you?

Yes, that sounds right to me. I'll look into removing bp-moment and bp-livestamp.

Also available in: Atom PDF