Make `theme.json` object caches non persistent #46150

oandregal · 2022-11-29T11:17:22Z

What?

This PR makes the object caches in use for theme.json related data non-persistent.

Why?

Upon some conversations at #45372 people find that there are a couple of cases for which the invalidation of cache may be not enough:

Consumers that hook into theme.json data using dynamic data (hooking or not based on an options or user meta value). The alternative would be to invalidate the cache upon certain events (plugin de/activation, option ad/update/delete, etc.).
Updates that are not done via the regular WordPress mechanism (FTP updates, updates managed via git repositories, etc.). The alternative is asking consumers to clear the cache manually if they use a persistent cache plugin.

To unblock that PR, this makes the data non-persistent.

How?

Adds theme_json to the list of cache groups not to persist.
Removes the existing filters that took care of invalidating the cache for across-request situations: plugin de/activation, global styles post update.

Testing Instructions

Go to the global styles sidebar in site editor, do some changes (e.g.: change background color) and save.
Go to the front-end and verify that the changes are reflected immediately.

oandregal · 2022-11-29T11:19:37Z

Personally, I'd like to revisit this decision later, and try using a persistent approach plus flushing the cache upon certain site events (plugin de/activation, options add/update/remove, etc.). Given the great gains of #45372 I'd rather prioritize shipping it and look into this later.

oandregal · 2022-11-29T11:20:14Z

lib/compat/wordpress-6.2/get-global-styles-and-settings.php

+function _gutenberg_add_non_persistent_theme_json_cache_group() {
+	wp_cache_add_non_persistent_groups( 'theme_json' );
+}
+add_action( 'init', '_gutenberg_add_non_persistent_theme_json_cache_group' );


I'm not sure what hook would be best to set this group to non-persistent. Any ideas?

Maybe plugins_loaded

🤔 At that point, any plugin that implement persistent cache should be loaded, hence calling this method should be effective. Sounds good.

…ersistent

- upon plugin de/activation - upon upgrading core, plugins, themes - upon saving global styles posts

oandregal · 2022-11-29T11:57:55Z

lib/compat/wordpress-6.2/default-filters.php

+ * When backporting to core, the existing filters hooked to WP_Theme_JSON_Resolver::clean_cached_data()
+ * need to be removed.
+ */
+add_action( 'start_previewing_theme', '_gutenberg_clean_theme_json_caches' );


I've only left the start_previewing_theme and switch_theme events as they are necessary for certain flows (preview themes, theme directory) that may access the theme data before they are dispatched.

These new events were introduced in 14.7 (stylesheet PR) when we aimed to make this persistent and can be removed: activated_plugin, deactivated_plugin, upgrader_process_complete.

With this change, all caches need the same invalidation logic, so I've consolidated it into a single place as per feedback at #45372 (comment)

If we merge #45955, it will mean that no cache invalidation will be needed for start_previewing_theme / switch_theme, as the act of changing / filter the stylesheet option, will cache the cache key.

It requires more changes than that: data cached by wp_theme_has_theme_json, gutenberg_get_global_styles, and WP_Theme_JSON_Resolver all needs to be cached by stylesheet. If/When we do that, we can remove these hooks.

Switching themes, mean we do not have cache to clear, as each theme would have it's own cache.

Switching themes, mean we do not have cache to clear, as each theme would have it's own cache.

I agree. However... clearing the cache won't be a bad thing since we'll be saving some memory 👍

My point is that to be able to remove these hooks, we need we make changes to those three places. It cannot be done in this PR or #45955 that only addresses wp_theme_has_theme_json as far as I could see.

oandregal · 2022-11-29T12:00:58Z

lib/compat/wordpress-6.2/get-global-styles-and-settings.php

-/**
- * Clean the cache used by the `gutenberg_get_global_stylesheet` function.
- */
-function gutenberg_get_global_stylesheet_clean_cache() {


This was introduced at #45679 and is slated for the 14.7 release. I've marked this PR for that release as well, so we can remove it. If this does not make it to the 14.7 we should deprecate it instead.

oandregal · 2022-11-29T12:01:46Z

lib/compat/wordpress-6.2/get-global-styles-and-settings.php

-		) {
-			wp_theme_has_theme_json_clean_cache();
-		}
+	function wp_theme_has_theme_json_clean_cache() {


This was introduced in 14.5 at #45543 so I'd rather deprecate it to be safe.

spacedmonkey

LGTM

lib/compat/wordpress-6.2/default-filters.php

oandregal · 2022-12-01T08:33:13Z

The main bulk of this PR is being backported at WordPress/wordpress-develop#3712

Adds `wp_theme_has_theme_json()` for public consumption, to replace the private internal Core-only `WP_Theme_JSON_Resolver::theme_has_support()` method. This new global function checks if a theme or its parent has a `theme.json` file. For performance, results are cached as an integer `1` or `0` in the `'theme_json'` group with `'wp_theme_has_theme_json'` key. This is a non-persistent cache. Why? To make the derived data from `theme.json` is always fresh from the potential modifications done via hooks that can use dynamic data (modify the stylesheet depending on some option, settings depending on user permissions, etc.). Also adds a new public function `wp_clean_theme_json_cache()` to clear the cache on `'switch_theme'` and `start_previewing_theme'`. References: * [WordPress/gutenberg#45168 Gutenberg PR 45168] Add `wp_theme_has_theme_json` as a public API to know whether a theme has a `theme.json`. * [WordPress/gutenberg#45380 Gutenberg PR 45380] Deprecate `WP_Theme_JSON_Resolver:theme_has_support()`. * [WordPress/gutenberg#46150 Gutenberg PR 46150] Make `theme.json` object caches non-persistent. * [WordPress/gutenberg#45979 Gutenberg PR 45979] Don't check if constants set by `wp_initial_constants()` are defined. * [WordPress/gutenberg#45950 Gutenberg PR 45950] Cleaner logic in `wp_theme_has_theme_json`. Follow-up to [54493], [53282], [52744], [52049], [50959]. Props oandregal, afragen, alexstine, aristath, azaozz, costdev, flixos90, hellofromTonya, mamaduka, mcsf, ocean90, spacedmonkey. Fixes #56975. git-svn-id: https://develop.svn.wordpress.org/trunk@55086 602fd350-edb4-49c9-b593-d223f7449a82

Adds `wp_theme_has_theme_json()` for public consumption, to replace the private internal Core-only `WP_Theme_JSON_Resolver::theme_has_support()` method. This new global function checks if a theme or its parent has a `theme.json` file. For performance, results are cached as an integer `1` or `0` in the `'theme_json'` group with `'wp_theme_has_theme_json'` key. This is a non-persistent cache. Why? To make the derived data from `theme.json` is always fresh from the potential modifications done via hooks that can use dynamic data (modify the stylesheet depending on some option, settings depending on user permissions, etc.). Also adds a new public function `wp_clean_theme_json_cache()` to clear the cache on `'switch_theme'` and `start_previewing_theme'`. References: * [WordPress/gutenberg#45168 Gutenberg PR 45168] Add `wp_theme_has_theme_json` as a public API to know whether a theme has a `theme.json`. * [WordPress/gutenberg#45380 Gutenberg PR 45380] Deprecate `WP_Theme_JSON_Resolver:theme_has_support()`. * [WordPress/gutenberg#46150 Gutenberg PR 46150] Make `theme.json` object caches non-persistent. * [WordPress/gutenberg#45979 Gutenberg PR 45979] Don't check if constants set by `wp_initial_constants()` are defined. * [WordPress/gutenberg#45950 Gutenberg PR 45950] Cleaner logic in `wp_theme_has_theme_json`. Follow-up to [54493], [53282], [52744], [52049], [50959]. Props oandregal, afragen, alexstine, aristath, azaozz, costdev, flixos90, hellofromTonya, mamaduka, mcsf, ocean90, spacedmonkey. Fixes #56975. Built from https://develop.svn.wordpress.org/trunk@55086 git-svn-id: http://core.svn.wordpress.org/trunk@54619 1a063a9b-81f0-0310-95a4-ce76da25c4cd

Adds `wp_theme_has_theme_json()` for public consumption, to replace the private internal Core-only `WP_Theme_JSON_Resolver::theme_has_support()` method. This new global function checks if a theme or its parent has a `theme.json` file. For performance, results are cached as an integer `1` or `0` in the `'theme_json'` group with `'wp_theme_has_theme_json'` key. This is a non-persistent cache. Why? To make the derived data from `theme.json` is always fresh from the potential modifications done via hooks that can use dynamic data (modify the stylesheet depending on some option, settings depending on user permissions, etc.). Also adds a new public function `wp_clean_theme_json_cache()` to clear the cache on `'switch_theme'` and `start_previewing_theme'`. References: * [WordPress/gutenberg#45168 Gutenberg PR 45168] Add `wp_theme_has_theme_json` as a public API to know whether a theme has a `theme.json`. * [WordPress/gutenberg#45380 Gutenberg PR 45380] Deprecate `WP_Theme_JSON_Resolver:theme_has_support()`. * [WordPress/gutenberg#46150 Gutenberg PR 46150] Make `theme.json` object caches non-persistent. * [WordPress/gutenberg#45979 Gutenberg PR 45979] Don't check if constants set by `wp_initial_constants()` are defined. * [WordPress/gutenberg#45950 Gutenberg PR 45950] Cleaner logic in `wp_theme_has_theme_json`. Follow-up to [54493], [53282], [52744], [52049], [50959]. Props oandregal, afragen, alexstine, aristath, azaozz, costdev, flixos90, hellofromTonya, mamaduka, mcsf, ocean90, spacedmonkey. Fixes #56975. Built from https://develop.svn.wordpress.org/trunk@55086 git-svn-id: https://core.svn.wordpress.org/trunk@54619 1a063a9b-81f0-0310-95a4-ce76da25c4cd

Adds `wp_theme_has_theme_json()` for public consumption, to replace the private internal Core-only `WP_Theme_JSON_Resolver::theme_has_support()` method. This new global function checks if a theme or its parent has a `theme.json` file. For performance, results are cached as an integer `1` or `0` in the `'theme_json'` group with `'wp_theme_has_theme_json'` key. This is a non-persistent cache. Why? To make the derived data from `theme.json` is always fresh from the potential modifications done via hooks that can use dynamic data (modify the stylesheet depending on some option, settings depending on user permissions, etc.). Also adds a new public function `wp_clean_theme_json_cache()` to clear the cache on `'switch_theme'` and `start_previewing_theme'`. References: * [WordPress/gutenberg#45168 Gutenberg PR 45168] Add `wp_theme_has_theme_json` as a public API to know whether a theme has a `theme.json`. * [WordPress/gutenberg#45380 Gutenberg PR 45380] Deprecate `WP_Theme_JSON_Resolver:theme_has_support()`. * [WordPress/gutenberg#46150 Gutenberg PR 46150] Make `theme.json` object caches non-persistent. * [WordPress/gutenberg#45979 Gutenberg PR 45979] Don't check if constants set by `wp_initial_constants()` are defined. * [WordPress/gutenberg#45950 Gutenberg PR 45950] Cleaner logic in `wp_theme_has_theme_json`. Follow-up to [54493], [53282], [52744], [52049], [50959]. Props oandregal, afragen, alexstine, aristath, azaozz, costdev, flixos90, hellofromTonya, mamaduka, mcsf, ocean90, spacedmonkey. Fixes #56975. Built from https://develop.svn.wordpress.org/trunk@55086 git-svn-id: http://core.svn.wordpress.org/trunk@54619 1a063a9b-81f0-0310-95a4-ce76da25c4cd

oandregal requested a review from spacedmonkey as a code owner November 29, 2022 11:17

oandregal requested review from jorgefilipecosta, aristath, mmtr, felixarntz and anton-vlasenko November 29, 2022 11:17

oandregal self-assigned this Nov 29, 2022

oandregal added the Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json label Nov 29, 2022

oandregal commented Nov 29, 2022

View reviewed changes

This was referenced Nov 29, 2022

Add WP_Object_Cache to the gutenberg_get_global_settings method #45372

Merged

Code quality check for theme.json related APIs #45171

Open

oandregal added 7 commits November 29, 2022 12:55

Make theme_json group non persistent

5a9921d

Sort hooks for readibility

6802de1

No need to delete cache keys upon upgrading if it is non persistent

b93314e

No need to delete cache keys upon plugin de/activation if it is non p…

2f5b23a

…ersistent

No need to delete cache keys upon post saving if it is non persistent

6616e9c

Remove filters for clearing WP_Theme_JSON_Resolver cache

cf99597

- upon plugin de/activation - upon upgrading core, plugins, themes - upon saving global styles posts

Deprecate old methods to clear the cache

83af27a

oandregal force-pushed the update/make-theme-json-cache-non-persistent branch from bb0266f to 83af27a Compare November 29, 2022 11:55

oandregal commented Nov 29, 2022

View reviewed changes

Remove method as it was introduced in this same release

0dd6a78

oandregal commented Nov 29, 2022

View reviewed changes

oandregal added this to the Gutenberg 14.7 milestone Nov 29, 2022

Use plugins_loaded instead of init as hook

f654f6f

aristath approved these changes Nov 29, 2022

View reviewed changes

spacedmonkey approved these changes Nov 29, 2022

View reviewed changes

oandregal merged commit 41b0e39 into trunk Nov 30, 2022

oandregal deleted the update/make-theme-json-cache-non-persistent branch November 30, 2022 08:12

oandregal added the [Type] Performance Related to performance efforts label Nov 30, 2022

afragen reviewed Nov 30, 2022

View reviewed changes

lib/compat/wordpress-6.2/default-filters.php Show resolved Hide resolved

oandregal mentioned this pull request Dec 1, 2022

wp_get_global_stylesheet: substitute 1-minute transient by non-persistent object cache WordPress/wordpress-develop#3712

Closed

mpkelly pushed a commit to mpkelly/gutenberg that referenced this pull request Dec 7, 2022

Make theme.json object caches non persistent (WordPress#46150)

fc6170e

This was referenced Dec 13, 2022

wp_theme_has_theme_json: add public method to know whether the theme supports theme.json WordPress/wordpress-develop#3556

Closed

wp_get_global_settings: add object cache WordPress/wordpress-develop#3789

Closed

felixarntz added a commit that referenced this pull request Dec 26, 2022

Update behavior based on #46150.

787aa97

felixarntz mentioned this pull request Dec 26, 2022

Introduce function to control whether to use persistent cache for certain theme file-based logic #46791

Open

Mamaduka mentioned this pull request Jan 17, 2023

Plugin: Backport PHP changes for WordPress 6.2 release #47187

Closed

85 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make `theme.json` object caches non persistent #46150

Make `theme.json` object caches non persistent #46150

oandregal commented Nov 29, 2022 •

edited

Loading

oandregal commented Nov 29, 2022 •

edited

Loading

oandregal Nov 29, 2022

spacedmonkey Nov 29, 2022

oandregal Nov 29, 2022

oandregal Nov 29, 2022

oandregal Nov 29, 2022

spacedmonkey Nov 29, 2022

oandregal Nov 29, 2022

spacedmonkey Nov 29, 2022

aristath Nov 29, 2022 •

edited

Loading

oandregal Nov 29, 2022

oandregal Nov 29, 2022 •

edited

Loading

oandregal Nov 29, 2022

spacedmonkey left a comment

oandregal commented Dec 1, 2022

Make theme.json object caches non persistent #46150

Make theme.json object caches non persistent #46150

Conversation

oandregal commented Nov 29, 2022 • edited Loading

What?

Why?

How?

Testing Instructions

oandregal commented Nov 29, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aristath Nov 29, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oandregal Nov 29, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

spacedmonkey left a comment

Choose a reason for hiding this comment

oandregal commented Dec 1, 2022

Make `theme.json` object caches non persistent #46150

Make `theme.json` object caches non persistent #46150

oandregal commented Nov 29, 2022 •

edited

Loading

oandregal commented Nov 29, 2022 •

edited

Loading

aristath Nov 29, 2022 •

edited

Loading

oandregal Nov 29, 2022 •

edited

Loading