Repository navigation
Cache the theme.json data merged from all origins - #81979
Conversation
18a323d to
aa55294
Compare
aa55294 to
fafc2b8
Compare
fafc2b8 to
3e4e20a
Compare
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueComment |
🤖 PR meta 🤖🎉 PropsIf you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. Updated as activity occurs, without notifying anyone named here. Add the ⚡ PerformanceShow the resultsClient side metrics exclude the server response time. front-end-block-theme
front-end-classic-theme
media-processing
media-upload
post-editor
site-editor
🏁 Flaky testsSome tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information. refuses the drop and uploads nothing in
|
320381f to
39122cd
Compare
|
@sirreal I assume the draft status is because it hasn't been human reviewed yet; are you planning to do so? Happy to give it a review and test run once it's marked ready. |
|
@tellthemachines Yep, this was waiting on my self-review and some numbers. I did some testing and here are the results. In summary, This seems to outperform trunk. Keeping the accessor cache adds a very small improvement that doesn't seem worth it. I'll perform self-review and mark as ready, then I'd love feedback or land if/when you're happy with it. Full LLM-driven perf measurementsMethod. Base Time in full merges per request.
About 1.9 ms is the first merge per origin and remains in every column. Callers that still merge on trunk. The accessor cache serves only REST
The editor perf suites miss this: their fixtures have no Trunk with #81889 reverted measures the same as trunk on every page: equal merge counts, merge time within 0.2 ms. The layout static served the same calls the accessor cache does. Request level.
Round spread is at most 0.6 ms on Accessor cache on top of the memo: no measurable difference. A hit saves one memoized call, 0.34 µs. Memory. Not measured. |
c964129 to
6440fcc
Compare
… old cache keys. Match the Gutenberg test from WordPress/gutenberg#81979: the key assertions forbid one past implementation by name and fail only if those exact keys return. Keep the assertion that the accessor reflects resolver data refreshed after a previous call.
tellthemachines
left a comment
There was a problem hiding this comment.
Thanks for the PR! The code LGTM and I couldn't find any issues in manual testing. I'm not an expert in this area though, so feel free to get a second opinion 😄
Looking at the perf numbers it makes a lot of sense to replace #81889 with this.
Just curious, any reason the tests in the first table were run on TT3 and TT4 but not TT5?
No conscious decision on my part to do that. I'm happy to run more numbers if we'd like. |
I don't think it's necessary, TT4 has a ton of blocks on its front page so if it works there it should be good for the other themes 😄 |
Me neither unfortunately 😅 I'll leave this open for another day or so in case other folks can have a look. |
get_merged_data() stored the merged data on the first call but nothing recorded the registered blocks for it, so the guard on the second call found an empty record, filled it, and rebuilt. The first hit was the third call. Run has_same_registered_blocks() before the isset() so the first call records the blocks while it builds, and the second call hits. The guard tracks block registration only. get_theme_data() reads the theme supports on every call, so a support added after the first merge stays out of the merged data until the blocks change or the cache is cleaned. Say so in the comment. Add a test that the second call returns the memoized data, through reflection on the merged property; it fails when the merged_* keys are missing from the block record. Warm the late-registration test with two calls so the guard runs against a seeded record.
test_clean_cached_data_refreshes_merged_data never called clean_cached_data(); it tested the switch_theme hook. Keep that as test_switching_theme_refreshes_merged_data, and have the clean test plant a sentinel, clean, and assert that the next call rebuilds. The mutation test switched to twentytwentyfour, which only the wp-env test config provides, from a zip. Use the in-repo fonts-block-theme fixture: its root font family references a font family preset, which resolve_variables() resolves. The duotone reference in block-theme does not resolve, because duotone presets emit no CSS variables.
#81889 added an object cache in front of the resolver. Nothing reads its resolve-variables keys: no caller in the plugin or in core passes that transform. A plain hit saves one clone and one array read over the memoized resolver. The cache sits in front of has_same_registered_blocks(), so a block registered after the first call stays invisible to this accessor for the rest of the request while every other consumer sees it, and WP_DEBUG bypasses it. Keep the layout static removal from #81889. Add a test that the accessor reflects a block registered after a previous call. The test suite runs with WP_DEBUG on, which bypassed the removed cache, so the test only fails against it with WP_DEBUG off.
block_core_gallery_render() held the global styles in a function static and read them through core's wp_get_global_styles(), which uses core's resolver, not the plugin's. A gap set in global styles after the first gallery rendered stayed invisible for the rest of the request, and the gallery saw different theme.json data than the layout support. Prefer gutenberg_get_global_styles() when it exists, the way the file already prefers gutenberg_resolve_style_state_aliases(). In the plugin that reads the memoized plugin resolver. In core the first branch is dead and wp_get_global_styles() runs, which becomes cheap once the resolver change syncs. The static came from #80030 as a workaround for the uncached merge.
The suite defines WP_DEBUG true in phpunit/bootstrap.php, and the cache this PR removes recomputed under WP_DEBUG, so the late-registration assertion passed against that cache. The test guarded nothing in CI. The cache was written on every call whatever WP_DEBUG said. Assert its four keys stay empty after the accessor runs, so putting the cache back fails the test under the settings CI uses. Renamed for what it now checks: the accessor reads through to the resolver.
The mutation isolation test switched to the fonts-block-theme fixture, which #82813 removed from phpunit/data/themedir1. Add a font family preset and a root font family that references it through the wp_theme_json_data_theme filter instead, so the test depends on no bundled theme. It still fails when the clone is removed from get_merged_data().
The block-library build lists wp_get_global_styles() in prefixFunctions, so the plugin build already rewrites the call to gutenberg_get_global_styles(). The IS_GUTENBERG_PLUGIN branch added in 1ece376 selected the same function in the plugin and was dead in core. Remove it and keep the per-render read.
Shorten the docblocks above the cache lookup, the clone helper and the accessor test to the reason each exists, list the merged_* keys that has_same_registered_blocks() accepts, and drop the "asserts nothing" commentary from the precondition assertions. Run the reuse and clearing tests over all four origins; removing one merged_* key from the registered-blocks record now fails the theme case. Assert a block-theme palette slug across the theme switch instead of comparing whole trees.
WordPress Core has no cache behind wp_get_global_styles() or the resolver, so removing the static would run a full theme.json merge per gallery render in Core once the package syncs. Keep it, explain why, and drop the changelog entry and the test that asserted the removal.
All merged origins depend on block registration identically, so one 'merged' entry in $blocks_cache suffices. Clear the whole merged array when new blocks appear, and reset $blocks_cache from its own keys in clean_cached_data() so the two lists cannot drift.
The resolver now caches the merged data, so this cache saved one array read per call and its WP_DEBUG bypass re-read the same cached object. Read through to the resolver on every call, as the styles accessor does, and cover the removal with the same read-through test.
The cache_merged_data() helper wrapped each early return to store the result and return a clone. Merge each origin behind a condition instead, so the method stores the result once and clones once, and the helper, its docblock and its visibility question go away. The helper was added in this branch and never shipped.
The file syncs into Core, where a comment naming WordPress 7.2 and promising removal after it would describe Core to itself. State the fact: the static avoids a merge per gallery in Core until the resolver cache is backported.
Match the docblocks in wordpress-develop#14071 so the port stays mechanical: the merged entry in $blocks_cache, the $merged property, the 'merged' origin of has_same_registered_blocks(), the cache and returned copy in get_merged_data(), and the reset in clean_cached_data().
The two accessor tests asserted that six removed wp_cache keys stay absent. That forbids one past implementation by key name and fails only if those exact keys return. Keep the assertions that the accessors reflect a block registered, or theme data changed, after a previous call, and name the tests after that behavior.
The test suite defines WP_DEBUG, which bypassed the removed accessor cache, so the test passes with or without that cache and cannot show its absence. Name it, like the styles test, for what it checks.
637f599 to
48923db
Compare
Match the test name and assertion message from WordPress/gutenberg#81979. The Gutenberg suite defines WP_DEBUG, which bypassed the plugin's removed cache, so the test there cannot show that cache's absence. Core's removed cache was bypassed only in theme development mode, so this test still fails against it.
…tion. Port the styles accessor test from WordPress/gutenberg#81979. The accessor reads the resolver's current data, so a block registered after a previous call appears in the next result.
`WP_Theme_JSON_Resolver::get_merged_data()` merged all origins on every call, including once for each rendered block with an `is-style-*` class. Cache the result until a block is registered or `wp_clean_theme_json_cache()` is called, and remove the redundant cache in `wp_get_global_settings()`. Theme support changes made after the first merge require `wp_clean_theme_json_cache()`. Backports WordPress/gutenberg#81979. Developed in: #14071 Props jonsurrell, peterwilsoncc, isabel_brison, ramonopoly. Fixes #66280. See #62249. git-svn-id: https://develop.svn.wordpress.org/trunk@64354 602fd350-edb4-49c9-b593-d223f7449a82
`WP_Theme_JSON_Resolver::get_merged_data()` merged all origins on every call, including once for each rendered block with an `is-style-*` class. Cache the result until a block is registered or `wp_clean_theme_json_cache()` is called, and remove the redundant cache in `wp_get_global_settings()`. Theme support changes made after the first merge require `wp_clean_theme_json_cache()`. Backports WordPress/gutenberg#81979. Developed in: WordPress/wordpress-develop#14071 Props jonsurrell, peterwilsoncc, isabel_brison, ramonopoly. Fixes #66280. See #62249. Built from https://develop.svn.wordpress.org/trunk@64354 git-svn-id: http://core.svn.wordpress.org/trunk@63504 1a063a9b-81f0-0310-95a4-ce76da25c4cd
What?
Cache
WP_Theme_JSON_Resolver_Gutenberg::get_merged_data()per origin for the request, so repeated calls reuse the merged data. Remove the request caches ingutenberg_get_global_settings()andgutenberg_get_global_styles(), which now read through to the resolver.Supersedes #81883. Core backport: WordPress/wordpress-develop#14071.
Why?
The accessor cache added in #81889 avoids repeated merges for
gutenberg_get_global_styles(), but direct resolver callers still merge on every call. These include block style variations, stylesheets, settings and duotone. Block style variations account for most of the remaining work: one merge per block with anis-style-*class.Caching in the resolver serves all callers and applies the existing block-registration check to every read. Keeping the accessor cache as well produced no measurable request-time improvement.
How?
The resolver caches each origin separately in
$merged. Amergedentry in$blocks_cachereuseshas_same_registered_blocks(), so a block registered after the first merge discards the cached data, as it already does for the per-origin caches.clean_cached_data()resets$mergedand rebuilds$blocks_cachefrom its own keys. An unknown origin resolves tocustom, which the docblock already documents as the fallback.Each call returns a clone, so callers can modify the result without changing the cache.
WP_Theme_JSON_Gutenberg::resolve_variables()relies on that.The gallery block keeps its per-request static. The static is redundant in the plugin, where the build maps
wp_get_global_styles()to the plugin accessor, but WordPress Core has no cache behind the function until the resolver change is backported. Only the comment changes.Compatibility:
_gutenberg_clean_theme_json_caches()before they appear in merged data. Thewp_theme_json_data_*filters already behaved this way through the per-origin caches. Normally both are registered onafter_setup_theme.theme_jsongroup, so nothing changes across requests. TheirWP_DEBUGbypass only affected the current request.backport-changelog/7.2/14071.mdlists Try caching gutenberg_get_global_styles #81889 as well, since this PR removes the cache it added and Core's layout support never had the static it removed. The entry for wordpress-develop#13214 is dropped; that PR has nothing left to port.Performance
Measured on 2026-10-07 on an earlier revision of this branch with the same caching behavior, against trunk
4791f174f20. WordPress 7.2-alpha-64225,WP_DEBUGoff, median of 20 requests after one warm-up. Values are total time inget_merged_data()per request, including the first merge.A separate perf-tool run on WordPress 7.1.3, with three interleaved rounds of 16 samples, reduced median
wpTemplatetime from 15.73 to 13.49 ms for Twenty Twenty-Three and from 12.04 to 10.86 ms for Twenty Twenty-One. Retaining the accessor cache alongside the resolver cache made no difference beyond run-to-run variation.Testing Instructions
With built assets and the PHP test environment running:
npm run test:unit:php:base -- --filter 'WP_Theme_JSON_Resolver_Gutenberg_Test|WP_Theme_Json_Test'Tests cover a separate instance per call, the cached data per origin, modification of the returned instance, late block registration,
clean_cached_data(), theme switching, and the accessors reflecting a block registered or theme data changed after a previous call.Local run on 2026-10-07 at the current head, including
Tests_Blocks_Render_Gallery: 77 tests, 253 assertions, passing.Use of AI Tools
Claude generated the implementation, tests, review fixes and this description under @sirreal's direction, including work with Claude Fable 5.1. Codex edited an earlier description. Human review of the generated code is pending.