Theme JSON: cached resolved URIs #66155
Conversation
|
Flaky tests detected in fb5e0f3. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/11397729500
|
ramonjd
added
[Type] Enhancement
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If 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. |
Theme JSON: cached resolved URIs
ramonjd
requested review from
oandregal,
tellthemachines,
andrewserong and
aaronrobertshaw
| @@ -850,6 +861,13 @@ public static function get_resolved_theme_uris( $theme_json ) { | |||
| return $resolved_theme_uris; | |||
| } | |||
|
|
|||
| $theme_json_data = $theme_json->get_raw_data(); | |||
| $resolved_theme_uris_cache_key = md5( wp_json_encode( $theme_json_data ) ); | |||
There was a problem hiding this comment.
Not yet sure if this will make things better or worse 😄
There was a problem hiding this comment.
It'd be great if WordPress had minimum PHP 8 support for faster hashing: https://php.watch/articles/php-hash-benchmark
There was a problem hiding this comment.
md4 outperforms md5 for file integrity on many benchmarks I've seen but hash( 'md4', wp_json_encode( $theme_json_data ) ) doesn't move the needle at all for me in performance tests.
There was a problem hiding this comment.
If we had a minimum PHP version of 8, we could avoid the hashing and use a WeakMap where the theme_json data/object was the key.
|
As this is performance related, CC @felixarntz @joehoyle |
There was a problem hiding this comment.
Thanks for following up on this one @ramonjd 👍
Bonus points for the theme.json snippet in the test instructions. Definitely made this easier to test.
✅ Code changes make sense
✅ Unit tests are passing
✅ Background images set with relative paths display correctly
✅ Background images update correctly when adding new path or switching ones around
Screenshot of resolved image paths rendering
I think caching this data for a single request is fine for now. It would be an easy update to move it to a transient or something.
That said, I'm happy to defer to the performance experts in sanity checking the overall approach.
The CI test failure appears to be an unrelated time out. I've kicked that off again.
| @@ -850,6 +861,13 @@ public static function get_resolved_theme_uris( $theme_json ) { | |||
| return $resolved_theme_uris; | |||
| } | |||
|
|
|||
| $theme_json_data = $theme_json->get_raw_data(); | |||
| $resolved_theme_uris_cache_key = md5( wp_json_encode( $theme_json_data ) ); | |||
There was a problem hiding this comment.
Before the caching we should check theme development mode. WDYT?
$can_use_cached = ! wp_is_development_mode( 'theme' );
if ( $can_use_cached ) {
// Add caching here.
}
There was a problem hiding this comment.
Good point, but I'm not sure whether it's required here just yet.
The method is called several times in a single session and the main performance enhancement here is to avoid subsequent (and expensive) processing in the same session. This applies to the frontend and editor.
Nothing is saved to the database so I don't expect theme developers to run into any gotchas when testing sites given that the cache is wiped every time the page refreshes.
Also it's being used in gutenberg_get_global_stylesheet and gutenberg_get_global_stylesheet which are both using caches as well.
I could be missing some subtleties so please let me know!
|
I run profiling for TT4 theme: 
|
ramonjd
force-pushed
the
try/cache-resolved-theme-uris
branch
from
9d34c56 to
b9762eb
Compare
What?
Implement "pretty basic caching" for resolved theme URIs in
WP_Theme_JSON_Resolver::get_resolved_theme_uris.Why?
The intention to improve performance, even if slightly.
WP_Theme_JSON_Resolver::get_resolved_theme_uris, andWP_Theme_JSON_Resolver::resolve_theme_file_uriswhich calls it, is fired multiple times in a single session to:Results will become more pronounced as other relative paths are introduced, e.g., fonts, and more blocks support background images.
Cached data is not be stored persistently across page loads.
How?
Simple object caching.
Testing Instructions
Run
npm run test:unit:php:base -- --filter WP_Theme_JSON_Resolver_Gutenberg_TestCreate a theme.json with background image paths defined for several blocks.
Ensure these appear as expected in the editor and frontend.
In the theme.json file, replace one or several paths with new asset paths. Check that the styles have been updated and the editor/frontend looks good.
Here is some test JSON based on current assets in TT5:
{ "$schema": "../../schemas/json/theme.json", "version": 3, "settings": { "appearanceTools": true }, "styles": { "background": { "backgroundImage": { "url": "file:./assets/images/category-anthuriums.webp" } }, "blocks": { "core/group": { "background": { "backgroundImage": { "url": "file:./assets/images/category-cactus.webp" } } }, "core/post-content": { "background": { "backgroundImage": { "url": "file:./assets/images/hero-podcast.webp" } } }, "core/quote": { "background": { "backgroundImage": { "url": "file:./assets/images/category-sunflowers.webp" } } }, "core/pullquote": { "background": { "backgroundImage": { "url": "file:./assets/images/dallas-creek-square.webp" } } } } } }