Update transient cache for `wp_add_global_styles_for_blocks` by joemcgill · Pull Request #6879 · WordPress/wordpress-develop

joemcgill · 2024-06-21T22:01:59Z

This uses a consistent cache key with the cache busting hash saved to the cache value rather than as part of the key. This allows us to remove the expiry time for the cache and only invalidate when the hash changes.

This also swaps the use of a site transient to the use of a regular transient so that the option will be autoloaded since there is no expiration time.

Trac ticket: https://core.trac.wordpress.org/ticket/61679

This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

github-actions · 2024-06-21T22:13:53Z

Test using WordPress Playground

The changes in this pull request can previewed and tested using a WordPress Playground instance.

WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser.

Some things to be aware of

The Plugin and Theme Directories cannot be accessed within Playground.
All changes will be lost when closing a tab with a Playground instance.
All changes will be lost when refreshing the page.
A fresh instance is created each time the link below is clicked.
Every time this pull request is updated, a new ZIP file containing all changes is created. If changes are not reflected in the Playground instance,
it's possible that the most recent build failed, or has not completed. Check the list of workflow runs to be sure.

For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation.

Test this pull request with WordPress Playground.

spacedmonkey

One question. But what worries me more is that unit tests are currently failing.

src/wp-includes/global-styles-and-settings.php

joemcgill

Thanks @spacedmonkey. The unit test failures are expected currently, because I didn't update the new tests yet. I wanted to get feedback on the approach first, but will update the test before making this change (if at all).

src/wp-includes/global-styles-and-settings.php

spacedmonkey · 2024-06-25T21:54:22Z

@joemcgill On a side now, get_site_transient only result in one database query now. See

Same with get_transient

This uses a consistent cache key with the cache busting hash saved to the cache value rather than as part of the key. This allows us to remove the expiry time for the cache and only invalidate when the hash changes. This also swaps the use of a site transient to the use of a regular transient so that the option will be autoloaded since there is no expiration time.

github-actions · 2024-07-12T15:48:48Z

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 props-bot label.

Core Committers: Use this line as a base for the props when committing in SVN:

Props joemcgill, spacedmonkey, costdev, aaronrobertshaw, andrewserong, peterwilsoncc.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

src/wp-includes/global-styles-and-settings.php

tests/phpunit/tests/theme/wpAddGlobalStylesForBlocks.php

Co-authored-by: Colin Stewart <79332690+costdev@users.noreply.github.com>

joemcgill · 2024-07-12T17:41:31Z

src/wp-includes/global-styles-and-settings.php

-		if ( ! is_array( $cached ) ) {
-			$cached = array();
+		// Hash the merged WP_Theme_JSON data to bust cache on settings or styles change.
+		$cache_hash = md5( wp_json_encode( $tree->get_raw_data() ) );


This is a much simpler hash generation, and accounts for a much broader set of changes that might happen to the merged Theme JSON data, which could affect global block styles, which is less risky.

aaronrobertshaw

This has been updated to also fix a bug that was reported in https://core.trac.wordpress.org/ticket/59595#comment:71 with testing instructions.

Thanks @joemcgill for updating this PR with a fix for the reported issue 👍

After applying the changes here, the global block styles update as expected on the frontend.

Before Revert (`ad135d5`)	After
Screen.Recording.2024-07-15.at.10.17.40.AM.mp4	Screen.Recording.2024-07-15.at.10.15.54.AM.mp4

It looks like this PR needs a rebase but in general it's looking pretty close to me.

andrewserong

but in general it's looking pretty close to me.

Same here, I like how this makes for a simpler and broader hash, it sounds like this will be a fair bit safer 👍

joemcgill · 2024-07-15T14:13:30Z

Thanks all, I'll update this PR so it applies cleanly after the revert of the original caching so we can try again for 6.6.1

joemcgill · 2024-07-16T21:24:23Z

I've refreshed this PR against trunk, which adds back the caching that was reverted in https://core.trac.wordpress.org/changeset/58710 using the updated approach.

I spent some time trying to write a new PHPUnit test that will confirm that the block meta will change whenever the wp_global_styles post is updated but discovered that the query in WP_Theme_JSON_Resolver::get_user_data_from_wp_global_styles( wp_get_theme() ) does not return the same post that is created if you first call WP_Theme_JSON_Resolver::get_user_data_from_wp_global_styles( wp_get_theme(), true ) to create the post. This seems like an unrelated bug so will report it upstream, but wanted to mention in case anyone had any insight into why this might be expected behavior.

costdev

Thanks for the work on this @joemcgill! The current tests look good. I've added some standards suggestions in this review.

tests/phpunit/tests/theme/wpAddGlobalStylesForBlocks.php

Co-authored-by: Colin Stewart <79332690+costdev@users.noreply.github.com>

joemcgill · 2024-07-18T18:06:58Z

Thanks for the review, @costdev. This is ready for another look.

joemcgill · 2024-07-18T20:27:49Z

I spent some time trying to write a new PHPUnit test that will confirm that the block meta will change whenever the wp_global_styles post is updated but discovered that the query in WP_Theme_JSON_Resolver::get_user_data_from_wp_global_styles( wp_get_theme() ) does not return the same post that is created if you first call WP_Theme_JSON_Resolver::get_user_data_from_wp_global_styles( wp_get_theme(), true ) to create the post. This seems like an unrelated bug so will report it upstream, but wanted to mention in case anyone had any insight into why this might be expected behavior.

Gutenberg issue opened, along with a unit test that demonstrates the behavior.

costdev

Thanks for the updates @joemcgill! Looks good to me 👍

peterwilsoncc · 2024-10-07T23:34:15Z

src/wp-includes/global-styles-and-settings.php

+			if ( isset( $cached['blocks'][ $cache_node_key ] ) ) {
+				$block_css = $cached['blocks'][ $cache_node_key ];
+			} else {
+				$block_css                           = $tree->get_styles_for_block( $metadata );


Just to make sure, can the meta data include block settings in the editor that will effect the output styles?

If so, then this may have incorrect data when a block is updated. If not then it's dandy.

I'm not sure I fully grasp the question, sorry.

Global styles for blocks set in the editor are saved to the Global Styles CPT, which are retrieved and merged with base and theme data via WP_Theme_JSON_Resolver::get_merged_data(); above. While in the editor, if global styles for a block type are adjusted without saving, there's a JS hook that outputs updated styles overriding those enqueued here.

My understanding is that the styles generated here only need to reflect what is saved for Global Styles.

Not sure if that helps, answers the question, or just confuses things 😅

My understanding is that the styles generated here only need to reflect what is saved for Global Styles.

That helps and was what I was asking, thanks Aaron.

joemcgill · 2024-10-18T20:09:17Z

I've refreshed this PR and added an additional unit test to confirm that the cache gets invalidated whenever the global-styles post is updated in the editor.

joemcgill · 2024-10-18T21:49:11Z

I've run a set of benchmarks on this locally, which compares 40 runs to the homepage of Twenty Twenty Four.

URL	trunk	This PR	Difference	Percentage Difference
Response Time (p10)	70.75	69.21	-1.54	-2.22%
Response Time (p25)	71.51	69.77	-1.74	-2.50%
Response Time (p50)	72.75	70.65	-2.10	-2.97%
Response Time (p75)	74.45	72.05	-2.40	-3.33%
Response Time (p90)	76.19	76.28	0.09	0.12%
wp-before-template (p10)	31.48	31.21	-0.27	-0.86%
wp-before-template (p25)	31.83	31.54	-0.29	-0.92%
wp-before-template (p50)	32.25	32.17	-0.08	-0.25%
wp-before-template (p75)	33.25	33.13	-0.12	-0.36%
wp-before-template (p90)	33.8	34.42	0.62	1.80%
wp-template (p10)	36.39	35.08	-1.31	-3.73%
wp-template (p25)	36.91	35.43	-1.48	-4.18%
wp-template (p50)	37.83	35.96	-1.87	-5.20%
wp-template (p75)	39.14	37.75	-1.39	-3.55%
wp-template (p90)	40.15	39.07	-1.08	-2.69%
wp-total (p10)	68.47	66.85	-1.62	-2.42%
wp-total (p25)	69.49	67.45	-2.04	-3.02%
wp-total (p50)	70.48	68.3	-2.18	-3.19%
wp-total (p75)	71.56	69.64	-1.92	-2.68%
wp-total (p90)	73.91	74.3	0.39	0.53%

joemcgill · 2024-10-18T21:54:58Z

Merged in https://core.trac.wordpress.org/changeset/59256

joemcgill changed the title ~~DRAFT: Update cache for wp_add_global_styles_for_blocks~~ Update cache for wp_add_global_styles_for_blocks Jun 21, 2024

joemcgill changed the title ~~Update cache for wp_add_global_styles_for_blocks~~ Update transient cache for wp_add_global_styles_for_blocks Jun 21, 2024

joemcgill requested review from peterwilsoncc and spacedmonkey June 21, 2024 22:17

spacedmonkey reviewed Jun 25, 2024

View reviewed changes

src/wp-includes/global-styles-and-settings.php Outdated Show resolved Hide resolved

joemcgill commented Jun 25, 2024

View reviewed changes

src/wp-includes/global-styles-and-settings.php Outdated Show resolved Hide resolved

joemcgill added 2 commits July 12, 2024 10:23

Hash the full theme.json data for cache invalidation

3a66386

joemcgill force-pushed the update/global-styles-for-blocks-cache branch from d5fda60 to 3a66386 Compare July 12, 2024 15:40

Update unit tests

424ca9d

joemcgill marked this pull request as ready for review July 12, 2024 15:48

joemcgill requested review from aaronrobertshaw, andrewserong and tellthemachines July 12, 2024 16:07

joemcgill mentioned this pull request Jul 12, 2024

Global Styles: Invalidate global block styles cache on style change #7017

Closed

costdev reviewed Jul 12, 2024

View reviewed changes

src/wp-includes/global-styles-and-settings.php Outdated Show resolved Hide resolved

tests/phpunit/tests/theme/wpAddGlobalStylesForBlocks.php Outdated Show resolved Hide resolved

fix typo

4058da4

Co-authored-by: Colin Stewart <79332690+costdev@users.noreply.github.com>

joemcgill commented Jul 12, 2024

View reviewed changes

aaronrobertshaw reviewed Jul 15, 2024

View reviewed changes

andrewserong reviewed Jul 15, 2024

View reviewed changes

joemcgill added 2 commits July 16, 2024 13:30

Merge branch 'trunk' into update/global-styles-for-blocks-cache

238c9a2

Update cache when hash changes

4553b60

joemcgill requested review from aaronrobertshaw, andrewserong and costdev July 16, 2024 21:19

costdev requested changes Jul 16, 2024

View reviewed changes

joemcgill and others added 3 commits July 18, 2024 13:00

Add message arguments to assertions

f53dcbe

Co-authored-by: Colin Stewart <79332690+costdev@users.noreply.github.com>

Use strict comparisons in assertions

683d07a

Update test dockblock descriptions and ticket refs

6c35708

joemcgill requested a review from costdev July 18, 2024 18:07

costdev approved these changes Jul 18, 2024

View reviewed changes

peterwilsoncc reviewed Oct 7, 2024

View reviewed changes

aaronrobertshaw mentioned this pull request Oct 16, 2024

Theme JSON: remove redundant check and relocate $selectors assignment WordPress/gutenberg#66154

Merged

ramonjd mentioned this pull request Oct 16, 2024

Performance Benchmarks for WordPress 6.7 WordPress/performance#1572

Closed

joemcgill added 2 commits October 18, 2024 13:21

Merge branch 'trunk' into update/global-styles-for-blocks-cache

6d6a35e

Add unit test to confirm user styles changes

059d080

Fix PHP 7.2 test failures

e7a0bcd

joemcgill self-assigned this Oct 18, 2024

joemcgill closed this Oct 18, 2024

aaronrobertshaw mentioned this pull request Oct 23, 2024

Backport: Caching of global styles for blocks from core WordPress/gutenberg#66349

Merged

Conversation

joemcgill commented Jun 21, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Jun 21, 2024

Test using WordPress Playground

Some things to be aware of

Uh oh!

spacedmonkey left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

joemcgill left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

spacedmonkey commented Jun 25, 2024

Uh oh!

github-actions bot commented Jul 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

joemcgill Jul 12, 2024

Choose a reason for hiding this comment

Uh oh!

aaronrobertshaw left a comment

Choose a reason for hiding this comment

Uh oh!

andrewserong left a comment

Choose a reason for hiding this comment

Uh oh!

joemcgill commented Jul 15, 2024

Uh oh!

joemcgill commented Jul 16, 2024

Uh oh!

costdev left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

joemcgill commented Jul 18, 2024

Uh oh!

joemcgill commented Jul 18, 2024

Uh oh!

costdev left a comment

Choose a reason for hiding this comment

Uh oh!

peterwilsoncc Oct 7, 2024

Choose a reason for hiding this comment

Uh oh!

aaronrobertshaw Oct 8, 2024

Choose a reason for hiding this comment

Uh oh!

peterwilsoncc Oct 9, 2024

Choose a reason for hiding this comment

Uh oh!

joemcgill commented Oct 18, 2024

Uh oh!

joemcgill commented Oct 18, 2024

Uh oh!

joemcgill commented Oct 18, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

joemcgill commented Jun 21, 2024 •

edited

Loading

github-actions bot commented Jul 12, 2024 •

edited

Loading