wp_get_global_settings: add object cache

[oandregal]

Trac ticket: https://core.trac.wordpress.org/ticket/57502
Related to WordPress/gutenberg#45171

This PR backports:

• Update which origins are queried for gutenberg_get_global_settings gutenberg#45971
• Add WP_Object_Cache to the gutenberg_get_global_settings method gutenberg#45372
• Make theme.json object caches non persistent gutenberg#46150

Props @felixarntz, @aristath, @ramonjd, @spacedmonkey, @anton-vlasenko, @jorgefilipecosta, @mmtr, @mcsf

What

This PR adds object cache to the existing wp_get_global_settings.

Why

This PR improves the performance of a request by:

• 40% for themes with theme.json (from 876.05ms to 520.80ms, a total of -355.25ms)
• 10% for themes without theme.json (from 500.26ms to 450.66ms, a total of -49.60ms)

After this PR, the time a theme with theme.json takes to process a request is almost the same (70ms slower) than a theme without theme.json. See full dataset.

How

Adds a cache to the high-level public method that consumers use to query the settings coming from theme.json APIs (WordPress, block, theme, and user data).

Test

• Make sure automated test pass.
• Test block theme (TwentyTwentyThree)
  • Create new colors as user from the GS sidebar and verify they are shown in the color components in the block sidebar (site editor, post editor).
  • Modify theme.json settings of the theme
• Test classic theme (TwentyTwenty)
  • Verify colors defined by the theme via add_theme_support( 'editor-color-palette', $palette ) work as expected.
  • Disable custom colors by adding add_theme_support( 'disable-custom-colors' ); to the functions.php of the theme. Verify that users cannot add custom colors.

Performance analysis

How performance was measured

I've tested this using the same mechanism I use for all performance work I've been doing, so the impact of each PR can be compared against each other:

• I have loaded the "Hello World!" post as a logged-out user. Conducted the test 9 times and then calculated the median (see full dataset).
• Compared this PR vs trunk at e27c5a38e371e1db85efeea239accf535227ed39 (svn 55005)
• Used TwentyTwentyThree for theme with theme.json and TwentyTwenty for a theme with no theme.json.

I'm also sharing the raw data I extracted (cachegrind files) so anyone can open them with their tool of choice and inspect the results. Feel free to DM me if you are interested in setting this up for running the test yourself or anything.

• Data for TwentyTwenty:
  • cachegrind.out._2022_12_19_hello-world_.twentytwenty.pr.zip
  • cachegrind.out._2022_12_19_hello-world_.twentytwenty.trunk.zip
• Data for TwentyTwentyThree:
  • cachegrind.out._2022_12_19_hello-world_.twentytwentythree.pr.zip
  • cachegrind.out._2022_12_19_hello-world_.twentytwentythree.trunk.zip

How to reproduce the experiment yourself

You don't need to, as I've made available the cachegrind data (see section above). Though you may still want to run this yourself for peer review:

1. Configure docker and enviroment by applying this patch. This tells docker to enable the xdebug profiler, output the performance files by URL, and set WP_DEBUG to false:

diff --git a/.env b/.env
index 63a8169f64..f3a87fbb08 100644
--- a/.env
+++ b/.env
@@ -18,7 +18,7 @@ LOCAL_DIR=src
 LOCAL_PHP=latest
 
 # Whether or not to enable Xdebug.
-LOCAL_PHP_XDEBUG=false
+LOCAL_PHP_XDEBUG=true
 
 ##
 # The Xdebug features to enable.
@@ -31,7 +31,7 @@ LOCAL_PHP_XDEBUG=false
 #
 # For a full list of accepted values, see https://xdebug.org/docs/all_settings#mode.
 ##
-LOCAL_PHP_XDEBUG_MODE=develop,debug
+LOCAL_PHP_XDEBUG_MODE=profile
 
 # Whether or not to enable Memcached.
 LOCAL_PHP_MEMCACHED=false
@@ -54,7 +54,7 @@ LOCAL_DB_TYPE=mysql
 LOCAL_DB_VERSION=5.7
 
 # The debug settings to add to `wp-config.php`.
-LOCAL_WP_DEBUG=true
+LOCAL_WP_DEBUG=false
 LOCAL_WP_DEBUG_LOG=true
 LOCAL_WP_DEBUG_DISPLAY=true
 LOCAL_SCRIPT_DEBUG=true
diff --git a/docker-compose.yml b/docker-compose.yml
index 739c65f4a8..b781c3fe94 100644
--- a/docker-compose.yml
+++ b/docker-compose.yml
@@ -39,6 +39,7 @@ services:
     environment:
       - LOCAL_PHP_XDEBUG=${LOCAL_PHP_XDEBUG-false}
       - XDEBUG_MODE=${LOCAL_PHP_XDEBUG_MODE-develop,debug}
+      - XDEBUG_CONFIG=profiler_output_name=cachegrind.out.%R
       - LOCAL_PHP_MEMCACHED=${LOCAL_PHP_MEMCACHED-false}
       - PHP_FPM_UID=${PHP_FPM_UID-1000}
       - PHP_FPM_GID=${PHP_FPM_GID-1000}

2. Build the environment: npm install && npm run build:dev.
3. Start docker: npm run env:start && npm run env:install.
4. Visit the site and load the "Hello World!" post. It should be something along the lines of http://localhost:8889/2022/12/19/hello-world/
5. Take the cachegrind data generated by xdebug out of docker:

# List and find the latest timestamp of cachegrind.out.* files
docker exec -it `docker ps -f name=wordpress-develop_php -q` ls -lat --full-time /tmp/
# Copy from docker to manipulate with any tool in your computer
docker cp `docker ps -f name=wordpress-develop_php -q`:'/tmp/cachegrind.out._2022_12_19_hello-world_.gz' ~/Desktop/

6. Inspect the time it takes with any tool of your choice. I use kcachegrind, but there are alternatives such as qcachegrind, webgrind, etc. Note that kcachegrind shows times aggregated by 10ns, so the total included time of the request in the following screenshot would be 35 0442 02 * 10 = 350 442 020ns, which amounts to 350ms.

7. Repeat 9 times and calculate the median.

[oandregal]

cc @hellofromtonya @azaozz this is the other PR I mentioned I'd backport as early in the cycle as possible.

[azaozz]

Which Gutenberg PR(s) is/are covered by this?

[felixarntz]

Mostly looks good, however similar feedback from #3556 applies here. We should probably wait for that PR to land first.

[hellofromtonya]

Which Gutenberg PR(s) is/are covered by this?

@azaozz the PRs are listed in the description in the "This PR backports" section.

[oandregal]

Rebased from trunk and removed the no longer relevant changes. I didn't have the time to test this manually after the rebase, I'll share here when I do if nobody gets to it first.

[oandregal]

I've shared elsewhere that I've been frustrated with the high variability of XDebug results. I know that XDebug are no real numbers, as instrumenting the PHP code adds noise. However, up until this point, I presumed the percentage of variation (whether we improve or not and the order of magnitude) would be similar to production. I no longer think this is true. I'd recommend stop taking numbers directly from cachegrind files for reporting purposes and find ways to measure a production environment (either in PHP or the reported Time To First Byte data).

So I've run the same experiment (hello world post) but taking the times differently:

• make sure you use a site in production (WP_DEBUG false and XDEBUG disabled)
• load the "hello world" post
• take the time to first byte from the network panel

Run the experiment 9 times and calculated the median for these results:

• 25% improvement for themes with theme.json (from 51.98ms to 38.54ms, a total of -13.44ms)
• 10% improvement for themes without theme.json (from 17.79ms to 16.12ms, a total of -1.67ms)

[azaozz]

I've run the same experiment (hello world post) but taking the times differently

Yep, that seems quite better way to test, not just this change but anything that's expected to be more significant. One thing I'd like to suggest is to:

make sure you use a site in production (WP_DEBUG false and XDEBUG disabled), and it is running from /build, not /src. (Grunt watch can be used if making PHP changes).

Running from /build will make it as close to "production" as possible, i.e. minified js and css, etc.

[hellofromtonya]

Now that wp_theme_has_theme_json() and wp_clean_theme_json_cache() landed in Core in https://core.trac.wordpress.org/ticket/56975 with the static cache variable approach, it unblocks the work here.

This code is a clean backport from the collective work done in Gutenberg, with the merge conflicts and code requests resolved ✅

@felixarntz you previously blocked the PR. Can you please re-review and share your thoughts on if this achieves the goals and is ready for commit?

[hellofromtonya]

Also noting there's another PR #3712 opened in Trac 56910 which is doing something similar to the wp_get_global_settings() function. backspace backspace This comment is inaccurate 🤦‍♀️ Sorry for the confusion. The PRs are different.

[felixarntz]

@oandregal I think two things here should be changed to bring this in line with the slightly cleaner code in #3712.

Also note my concern voiced in #3712 (review) that may apply here too. Let's focus on #3712 first to get that across the finish line, and then we can follow the same approach here.

[felixarntz]

@oandregal This is almost good to go, however I just found one critical thing that is broken that I had missed before.

[felixarntz]

The cache clearing here is not working as it is the wrong key: We need to clear the correct cache keys for all possible origins, i.e. "wp_get_global_settings_{$origin}". What are the possible origins? We should probably loop through them here and clear those caches like that.

[oandregal]

Good catch, thanks! Fixed at 8a47ba6

[felixarntz]

Cross-posting here: https://core.trac.wordpress.org/ticket/57502#comment:12

This change results in an ~8% improvement in total WordPress server response time, which is amazing for a single change! 🎉

Excited to commit this once the last quirk above is addressed 😄

[oandregal]

Also note my concern voiced in #3712 (review) that may apply here too.

I tested following Tonya's instructions and no error happens. I verified that the load-styles.php is loaded and it has a 200 HTTP status code.

My env:

• Localhost: Docker with npm
• OS: Linux/Ubuntu
• Browser: Chrome
• WP Version: 6.2-alpha-54642-src
• Plugins: tested with none and with Gutenberg 15.0.1
• Theme: tested TT3 and TT1

[oandregal]

This change results in an ~8% improvement in total WordPress server response time, which is amazing for a single change! tada

@felixarntz In my tests, classic themes (TT1) improve by 10% and block themes (TT3) improve by 25% or 40%, depending on the test (see issue description & second test with TTFB). See raw data. 8% is quite a bit lower than I expected! Could you share how did you measure?

[felixarntz]

@oandregal Thanks for raising this. I am measuring Server-Timing of WordPress's own execution, which I typically do using the approach I outlined in this Gist: https://gist.github.com/felixarntz/de5c697a1a16c2b892634b70216eb6c7

So it is expected that those metrics are vastly different from TTFB. I would say both types of tests have their own justification, for different reasons:

• Server-Timing metrics are far more stable between requests, so they are better to rely on to get an idea on performance impact, especially for whether something is expected to bring a performance impact or not.
• TTFB is a "closer to the real world" metric though, which is a benefit, but it also shows more variance, so harder to rely on it, particularly if the performance difference is very little. At the same time, once we have developed an idea of the impact (e.g. via Server-Timing), I think TTFB is a much closer representation to how that difference will play out in the wild.

So I think those methodologies complement each other well.

[felixarntz]

Committed in https://core.trac.wordpress.org/changeset/55155 🎉

[oandregal]

Thanks everyone for helping to land all these PRs.