Improve caching in `wp_get_global_stylesheet`

[spacedmonkey]

Update: Originally this ticket also covered the wp_get_global_styles_svg_filters function which is subject to a similar change, but since that is a separate effort it makes sense to break it out in a separate ticket.

The wp_get_global_stylesheet (and wp_get_global_styles_svg_filters) functions cache data in transient for 1 minute. This means that one high traffic sites, the cache will be warmed and performance improved. But for sites with lower traffic or CDN caching ( where less traffic hits the origin ) this cache will not be warned and performance will suffer. Global styles only change if the owner of the site, makes a change or a theme is changed. These caches should be set forever and correctly invalidated. This will improve performance and mean that front end users who hit a page will not suffer poor performance.

[ironprogrammer]

Thanks for this suggestion, @spacedmonkey! Looking forward to seeing an update like this!

Updating to an enhancement, but if I've missed something here, please feel free to provide any additional info and correction.

[andrewserong]

Great idea to tweak the caching logic to be more performant!

Global styles only change if the owner of the site, makes a change or a theme is changed

Some of the other times that the output will be changed is when a plugin is installed or updated, or when core files change, since any of these can alter the output of the core theme.json processing.

There are reports over in https://core.trac.wordpress.org/ticket/56970 and ​https://github.com/WordPress/gutenberg/issues/45713 that sound related to core (and its theme.json file and processing logic) being updated, but new styles from the core theme.json file (and the associated processing logic) not being included. So, I think any updates to the caching logic should ideally factor in WP updates or plugin installations and upgrades, too.

I suppose there's also the possibility of plugins allowing configuration over things that might affect global styles output, too, so personally, I'd still probably lean toward there being some kind of fallback timeout for cache invalidation rather than caching indefinitely, since this area is difficult to predict. However, things like plugin installations or updates happen pretty infrequently, so I agree that the timeout could be much longer than one minute.

Trac tickets: https://core.trac.wordpress.org/ticket/56910
Related to ​https://github.com/WordPress/gutenberg/issues/45171

Backports the relevant parts of these PRs:

• ​https://github.com/WordPress/gutenberg/pull/45679
• ​https://github.com/WordPress/gutenberg/pull/46150

This PR:

• provides a fix to an issue reported for 6.1.1 (see trac ticket and ​gutenberg issue)
• backports part of the work of introducing object cache to theme.json APIs (​see)

Props @mmtr @felixarntz @spacedmonkey @aristath

As @felixarntz pointed out, this PR needs a rebase and potentially a rebuild of the individual commits to remove what is no longer relevant.

That said, there's an open discussion still happening on the caching solution (due to fatal errors in the admin area) (See the Trac ticket https://core.trac.wordpress.org/ticket/56975). Until a solution is committed, might be best to wait on the rebase.

Rebased and removed the no longer relevant parts. Changes for src/wp-includes/load.php, src/wp-includes/ms-blogs.php, tests/phpunit/includes/abstract-testcase.php are still relevant here as they were reverted from core at ​https://github.com/WordPress/wordpress-develop/pull/3864

I was unable to reproduce the bug raised at https://core.trac.wordpress.org/ticket/56975#comment:32 in this PR. I'll need to go back there and see if I can have some testing steps. I want to understand how to reproduce to either introduce the fix in this PR directly or that we don't block this from landnig if it doesn't.

As I shared in other places, this week I'm unable to contribute for more than half an hour here and there. So, go ahead when you feel is ready and don't wait for my feedback. Or ping me via DM if there's something I must absolutely review.

@felixarntz @oandregal I have created a PR for making SVG function use this same caching. ​https://github.com/oandregal/wordpress-develop/pull/1

That's a good next step, thanks for working on it. As it is now, it is based on this PR. I'd recommend preparing a separate PR instead as they are unrelated changes (though, sensibly, going in the same direction). Ideally, that work happens in Gutenberg first, so it is tested by users before merging it into core.

[hellofromTonya]

Marking gutenberg-merge as some (or all) of the work here is to backport/copy from Gutenberg PR(s) into Core (such as ​PR 3712. This keyword helps to track backports.

Noting: Similar work is being done in wp_get_global_settings() in Trac 57502 and ​https://github.com/WordPress/wordpress-develop/pull/3789. That scope of work captures the latest conversations and consensus. I'm thinking it might be worth landing PR #3789 first, then rebasing and updating this PR to remove the duplicate code, and then copy the code patterns from it.

@felixarntz @spacedmonkey @oandregal Do you agree?

I see two blocking reviews, though I think I've addressed all the feedback. Please, let me know if I should look at anything to unblock this.

# Test Report

As @felixarntz notes ​https://github.com/WordPress/wordpress-develop/pull/3712#discussion_r1088143732, need to test the PR in the WP Admin area to determine if a fatal error happens from not having wp_cache_*() functions available when load-styles.php runs.

## Testing Instructions

• Step 1: Added the following constants to wp-config.php:

{{{php
define( 'CONCATENATE_SCRIPTS', true );
define( 'WP_DEBUG', false );
define( 'SCRIPT_DEBUG', false );
}}}

• Step 2: Pull or apply this PR
• Step 3: Start your localhost environment.

If using Docker, then do:

npm install
npm run build:dev
npm run env:start
npm run env:install

• Step 4: Log in and open the WP Admin area.

Does a fatal error occur?
Is the load-styles.css stylesheet loading?

If no, navigate through the screens. Does a fatal error occur?
If no, then this PR is not impacting load-styles.php ✅

## Testing Results

Env:

• Localhost: Docker with npm
• OS: macOS
• Browser: Chrome, Firefox, and Edge
• WP Version: trunk
• Plugins: none
• Theme: TT3

No fatal error ✅

Opened the Network tab in Dev Tools: yes, load-styles.php is being requested and has a 200 status ✅

Works as expected ✅

Does this cache need to be reset in wp_clean_theme_json_cache()?

[flixos90]

@spacedmonkey I am changing this ticket to focus just on the wp_get_global_stylesheet() function, since ​https://github.com/WordPress/wordpress-develop/pull/3712 is pretty much ready, and those two efforts are separate, even though the work is very similar.

Can you open a separate ticket to make the change to wp_get_global_styles_svg_filters()? I see that work is still pending a Gutenberg PR ​https://github.com/WordPress/gutenberg/pull/47460, and only then we can backport it to core, so it may take a bit longer.

@hellofromtonya

Does this cache need to be reset in wp_clean_theme_json_cache()?

Good catch, I missed that 🙃

Can you please add the matching wp_cache_delete call to that function?

Can you please add the matching wp_cache_delete call to that function?

@felixarntz done in ​https://github.com/WordPress/wordpress-develop/pull/3712/commits/d9c0909951b2a894d137324c1082a50167a2d114 ✅

Retested in WP Admin. No issues 💹

## Test Report
Tested with wp-config.php adjustments and steps outlined in ​https://github.com/WordPress/wordpress-develop/pull/3712#issuecomment-1405464211 above.

Patch tested: this PR since ​https://github.com/WordPress/wordpress-develop/commit/d9c0909951b2a894d137324c1082a50167a2d114.

### Environment

• Hardware: MacBook Pro Apple M1 Pro
• OS: macOS 12.6.2
• Browser: Safari 16.2, Mozilla Firefox 109.0
• Server: nginx/1.23.3
• PHP: 7.4.33
• WordPress: 6.2-alpha-54642-src
• Theme: twentytwentythree v1.0
• Gutenberg: tested both enabled/disabled

### Actual Results

• ✅ No fatal errors occurred while navigating through or refreshing WP admin.
• ✅ Requests to load-styles.php returned successfully, and CSS loaded in WP admin.

Thanks @ironprogrammer for testing it!

@felixarntz should be okay to commit.

Thank you everyone for your contributions!

[flixos90]

A final brief performance benchmark on this one:

• The use of WP cache API makes the code more performant than when using a transient, for when the value for the transient is not set (~4.5ms compared to ~8.8ms).
• When the transient has a value set, it is of course much faster than the WP cache API since we are now using a non-persistent cache (~0.6ms compared to ~4.5ms).

This is expected since the transient is cached between requests, making subsequent lookups much faster - however it's only cached for 1 minute, which is a hacky approach to get some sort of "semi-persistent" cache for something where we haven't yet figured out cache invalidation.

Overall the function is called only 1 time per regular page load, so the non-persistent cache here does not improve performance. However, it still makes more sense than the transient due to the above. As we are working towards a persistent cache solution, this is a step in the right direction, even though performance-wise it is a zero-sum game.

[flixos90]

In 55148:

Editor: Use a non-persistent object cache instead of transient in wp_get_global_stylesheet().

This changeset is part of a greater effort to enhance the caching strategy for theme.json based data. Similar to [55138], the cache is currently ignored when WP_DEBUG is on to avoid interrupting the theme developer's workflow.

Props oandregal, spacedmonkey, hellofromtonya, flixos90, ironprogrammer, azaozz, aristath, costdev, mcsf.
Fixes #56910.

@peterwilsoncc Apologies, this feedback came a bit too late, I had already committed this in https://core.trac.wordpress.org/changeset/55148.

If it's crucial, we can do a follow up commit?

@felixarntz I think it should be fine without. I noticed my terrible timing ;)

Just FYI: ideally testing should happen when WP runs from /build as it is closest to "production" (concatenated and minified js and css, etc.). Docker/wp-env can be set to run from there. The testing instructions are slightly different:

1. WP_DEBUG and SCRIPT_DEBUG should not be set (or commented out) in wp-config.php.
2. In the terminal:
   • npm run build or just grunt. No need to do npm install as Grunt will run it if needed.
   • LOCAL_DIR=build npm run env:start to start from /build
   • npm run env:install only if this is the first time using wp-env here
3. Don't forget to do npm run env:stop at the end.

I was able to reproduce with the ​steps Tonya provided.

@azaozz I tried to use ​the steps you suggested, but npm run build doesn't work for me:

Running "verify:old-files" task
Warning: build/wp-includes/blocks/heading/editor.css should not be present in the $_old_files array. Use --force to continue.

Aborted due to warnings.

I tried a few things, including clearing the build folder or passing --force to the build command. None worked.

---

It seems this PR has been committed, so I guess it can be closed/deleted.

[spacedmonkey]

@flixos90 Created a breakout ticket here #57568

Thanks all for the additional feedback. No actionable change in terms of committing per the above, so I'll indeed close this for now.

tried to use the steps you suggested, but npm run build doesn't work

Uhh, sorry, this is a bug in core. Generally when you build to /src, it leaves the source in a "dirty" state. Trying to actually build to /build copies some of these left-over files and the build fails.

There's a ticket for that already, will try to fix it soon.