Site-kit-wp: Some wild (20s+) performance slowdowns loading pages with WP admin bar likely linked to Site Kit

Created on 14 Dec 2020  路  7Comments  路  Source: google/site-kit-wp

Bug Description

We have a fairly large site (~10mln monthly PVs) and switched to Site Kit a few months ago. I have a CS background and performance issues are of specific interest to me. I have been noticing more and more lately that any pages with the WP admin bar, such as the front pages when logged in or the backend pages, occasionally have colossal delays in server-side page generation time.

It's not uncommon for me to wait for a story to load for 20s+ that usually takes less than 0.5s. This doesn't affect regular visitors as they don't see the admin bar.

We turned on Query Monitor but all it did was confirm the issue is not in the SQL part of the page-building process.

We ended up building a custom debug system that attempts to analyze hooks and various modules and it consistently pointed to wp_head and wp_enqueue_scripts. This puzzled us so we then extended the system further, and this Site Kit closure is what it consistently points out now:
https://github.com/google/site-kit-wp/blob/develop/includes/Core/Admin_Bar/Admin_Bar.php#L90-L109.

Unfortunately, I don't have further details yet on what exactly is causing this delay, but it's driving me absolutely nuts as pages randomly take 10-20s+ to load on extremely beefy hardware (4x AMD EPYC 7601 32-Core Processor, 64GB RAM servers).

  1. Have you had any reports of similar performance issues or observed them yourself?
  2. Do you have further debugging steps or ideas?

Steps to reproduce

  1. Make sure you're logged into WP.
  2. Visit the site after a period of inactivity, frontend or backend.
  3. Observe random major delays.

Screenshots

image

Additional Context

  • PHP Version: 7.4
  • OS: Windows, Android
  • Browser: Chrome
  • Plugin Version: 1.22.0
  • Device: PC, Android

_Do not alter or remove anything below. The following sections will be managed by moderators only._

Acceptance criteria

  • The admin bar menu should be loaded/displayed for every public post, regardless of whether it has Search Console or Analytics data. In other words, the PHP checks should be removed.
  • This issue should be kept open for tracking, but let's focus on #2508 for the actual work which includes the fix for this one.

Implementation Brief

  • N/A (see #2508)

QA Brief

  • There should not be any server-side calls to the Search Console or Analytics APIs to check whether there's data for a post in order to load the Site Kit admin bar menu.

Changelog entry

  • Remove server-side API requests to determine whether a post has Search Console or Analytics data as it could significantly slow down WP admin response time.
Analytics Search Console P1 Eng Rollover Enhancement

All 7 comments

We have newrelic installed on one of the servers, and I just found an example with lots of details that proves Site Kit is the culprit and will hopefully let you come up with a solution. Please take a look.

To me, it looks like for whatever reason, Site Kit calls has_data_for_url for Analytics and Search Console synchronously with the page load, and then actually executes external Guzzle HTTP requests to get remote data, which ends up slowing down the main page load by many many seconds. Why is this happening? These sorts of requests should never happen synchronously and should be either loaded via Ajax or cron.

image
image
image
image
image

@archon810 Thank you for raising this issue and for the detailed traces from New Relic. Ironically, we do the has_data_for_url call you mentioned in an effort to _reduce_ page load impact (by not loading the admin-bar menu code for pages that lack data).

We cache this has data attribute, so you are probably seeing a load where the cache has expired. Some possible ideas:

  • One fix would be to change the caching strategy, and serve the expired (stale) state while refetching in the background to see if data is available.
  • Alternately we could consider always loading the Site Kit menu-bar menu, then checking for data when the user interacts/opens it. This async approach is probably preferable; we will confer with @felixarntz and the rest of the team to decide the best path forward.

@adamsilverstein Thanks for replying. Yeah, the typical Transient strategy is quite faulty in my opinion because it doesn't regenerate while serving stale cache and can therefore result in long load times as well as race conditions reloading the data. We've typically stayed away from this approach and served stale cache while regenerating.

Having said that, the approach in your second suggestion sounds fine, especially since it can probably be implemented and rolled out very quickly.

Moreover, having the Site Kit entry in the admin bar for some posts but not others has always confused me very much. I'd rather it be always present and say why the data isn't there or offer a way to sync it on the fly via Ajax.

@eclarke1 @fhollis This issue doesn't require its own IB or PR since the fix will be covered via #2508. We should still keep it open for tracking purposes because the bug described here is worth its own issue distinct from the #2508 enhancement. So basically we can consider this one an estimate of 0 and just move it along #2508 in the board.

Given the importance of this issue, could we maybe bump it up to https://github.com/google/site-kit-wp/milestone/62 instead of https://github.com/google/site-kit-wp/milestone/63? https://github.com/google/site-kit-wp/milestone/63 hasn't been started yet and is due in 10 days.

@aaemnnosttv This is coupled to #2508 - please move it along when done with CR (at least by the time this has been merged).

Was this page helpful?
0 / 5 - 0 ratings