Fix performance regression in the block-level custom CSS feature#11686
Fix performance regression in the block-level custom CSS feature#11686mukeshpanchal27 wants to merge 23 commits into
Conversation
Add early return for empty custom CSS class string
|
Based on @westonruter’s findings (https://gist.github.com/westonruter/5ae4155059d2135197976f324d00645c#file-spx-claude-analysis-md), it looks like this runs for every block. Since most blocks don’t have a custom class, we should return early so we don’t end up calling |
Test using WordPress PlaygroundThe 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
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
|
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 Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
I asked Claude Opus 4.7 to review this change with the SPX profile reports loaded as context: Looking at the PR's actual change vs. my profile data — it's a real micro-optimization but the gain is tiny in absolute terms. Here's the math: What the PR doesAdds an early What the profile said about this functionFrom my three runs:
The Realistic per-request savings
So on the order of 100µs per page request in the homepage case, more like 10–20µs on a small page. Well under 1% of the wp70 regression, and probably less than that in production (profiler overhead inflates per-call costs). VerdictIt's a legitimate, zero-risk fix — six added lines, no behavior change since The bigger lesson: the regression in this code path lives one level up.
The structural fix would be to not register the filter at all unless there's at least one block with custom CSS support enabled (which is rare — it's an opt-in feature). A check at Same pattern applies to Worth landing this PR as-is, but I'd encourage the contributor (or you) to also propose conditional filter registration as the larger follow-up. |
Add check for empty custom CSS before block support validation.
|
@westonruter Thanks for the report 😄 While we’re figuring out the best way to register the filter, I updated the PR with a change that avoids calling the Block Registry: a331593 |
Here's Claude's analysis with this addition: The new revision adds a second, more impactful optimization. Now there are two changes: Change 1 (unchanged):
|
|
I obtained the SPX report before and after.
Per Claude, that accounts for:
The 891 number is the smoking gun — it matches exactly the count of Just to put the magnitude in perspective: 0.48% fewer function dispatches per request. If average dispatch cost is ~1µs (reasonable order-of-magnitude on warm opcache), that's ~3.5ms of savings — and that's why call counts are a more sensitive instrument than wall time for changes this size. It's also why the wall-time delta is invisible: ~3.5ms of expected savings is right in the same ballpark as the ±2ms run-to-run drift in your 500-iteration medians. (Weston here) I ran Without the fix applied:
With the fix applied:
Claude's analysis of this data: Honest reading: the PR's measured wall-time impact is statistically zero at this scale. Both runs land at ~231ms
A 1.63ms median delta on a 230ms baseline is +0.7% — well within run-to-run drift between two separate 500-iteration runs. Background CPU, thermal state, MySQL/opcache warmth, and container scheduling all contribute multiple-ms of systematic variance between sessions, so I would not read +1.63ms as "the PR is slower." It's noise. (The +29.8ms Response Time delta confirms this — that's whole-request including network overhead and is even noisier; it's drifting in the same direction as wp-total at ~5×, consistent with environmental drift rather than the PR.) Reconciling with the call-count evidenceThe SPX run showed the optimization is structurally working (−891 lookup calls per page). My estimate was the savings would be ~100–1000µs. The benchmark confirms that the upper bound of the actual savings is under the noise floor of a 500-iteration median — which fits cleanly with my estimate. If the PR were saving 5ms per request it would show; saving 0.1–0.5ms is invisible at this signal-to-noise ratio. So the PR is:
What this means for the bigger regression huntThis is the uncomfortable truth about the wp70-vs-wp69 regression: none of the individual fixes from my earlier list will measurably move the needle on their own. The ~25–30ms total regression is genuinely composed of many ~0.1–1ms costs. Killing them one at a time will keep producing benchmark results that look exactly like this one — within noise. That has implications for the workflow:
The structural wins I called out earlier — conditionally registering the Recommendation for what to say on the PRSomething like:
The PR is correct and worth merging. Just don't expect any individual benchmark to vindicate it. |
Co-authored-by: Weston Ruter <westonruter@gmail.com>
There was a problem hiding this comment.
Pull request overview
Addresses a performance regression in block-level Custom CSS rendering by adding early-exit checks to avoid unnecessary work on blocks that don’t use the feature.
Changes:
- Return early in
wp_render_custom_css_support_styles()when nostyle.csscustom CSS is present, avoiding block type lookup and support checks. - Return early in
wp_render_custom_css_class_name()whenclassNameis empty, avoiding an unnecessarypreg_match().
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Commit a291f83 also addresses the following PHPStan rule level 10 errors: |
|
Commit d637df3 fixes these PHPStan issues which are possible when |
|
And fba41c3 fixes these PHPStan rule level 10 issues: |
|
@mukeshpanchal27 please review my additions |
| * @param WP_Block_Type $block_type Block type to check for support. | ||
| * @param string|array $feature Feature slug, or path to a specific feature to check support for. | ||
| * @param mixed $default_value Optional. Fallback value for feature support. Default false. | ||
| * @param WP_Block_Type|null $block_type Block type to check for support. |
There was a problem hiding this comment.
This change is because the return value of WP_Block_Type_Registry::get_instance()->get_registered() is often passed right into block_has_support(). And the function specifically is checking for whether the arg is a WP_Block_Type.
Thanks @westonruter, The changes look good to me. Are you going to commit this? |
… substring check. Add a str_contains() fast path before the preg_match() in wp_render_custom_css_class_name() so blocks with a className but no wp-custom-css-* class avoid the regex on the render_block filter, and fold the match check into the guard condition. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
| if ( ! is_string( $class_name_attr ) || ! str_contains( $class_name_attr, 'wp-custom-css-' ) ) { | ||
| return $block_content; | ||
| } | ||
|
|
||
| if ( empty( $matches ) ) { | ||
| // Parse out the 'wp-custom-css-*' class name added by wp_render_custom_css_support_styles(). | ||
| $tag = new WP_HTML_Tag_Processor( '<div>' ); | ||
| $tag->next_tag(); // Advance to the DIV. | ||
| $tag->set_attribute( 'class', $class_name_attr ); | ||
| $custom_class_name = null; | ||
| foreach ( $tag->class_list() as $class_name ) { | ||
| if ( str_starts_with( $class_name, 'wp-custom-css-' ) ) { | ||
| $custom_class_name = $class_name; | ||
| break; | ||
| } | ||
| } |
There was a problem hiding this comment.
@dmsnell I'd appreciate your feedback on this approach.
This ensures correctness, as it won't incorrectly match my-wp-custom-css-0acafc53 as previously due to the \b word boundary matching at hyphens.
There was a problem hiding this comment.
Note the additional computation involved for WP_HTML_Tag_Processor is guarded against via the str_contains( $class_name_attr, 'wp-custom-css-' ) check above.
There was a problem hiding this comment.
Should we also add a unit test to validate the edge cases that WP_HTML_Tag_Processor covers (vs the regex)?
There was a problem hiding this comment.
@ramonjd citing your comment here from WordPress/gutenberg#78217 (review) to keep reviews on the canonical PR:
Tentative approval from me pending feedback on the use of
WP_HTML_Tag_ProcessorWP_HTML_Tag_Processor-based lookup of the generated wp-custom-css-* class, so token matching matches how the class was added and how add_class() reads it.
Just so I understand the intention here, is idea that
class_list()is the canonical tokenizer for HTML class attributes, andadd_class()uses that same tokenization internally to avoid duplicates? In other words, same input/output for all operations?For custom CSS the class gets written to
$block['attrs']['className']ingutenberg_render_custom_css_support_styles()via plain string concatenation.I guess the
WP_HTML_Tag_Processorapproach does future-proof things if write side changes later.
Yes, the intention is to avoid reinventing the wheel for parsing class names. Leveraging the HTML Tag Processor as a utility like this is something I learned from @sirreal when populating text in a script or style tag. I recall @dmsnell also intended to expose a static method that provided a wrapper to do this parsing.
Alternatively, I first tried just improving the regex in bd93fe8 but then I decided to go down the route of using the tag processor.
There was a problem hiding this comment.
I recall … a static method that provided a wrapper to do this parsing.
I believe that's #10043 proposing a class list utility.
There was a problem hiding this comment.
I took a look at this and did some testing. I think this HTML round-trip to extract the class name is safe to avoid here.
Consider this non-blocking feedback. There are merits to using the tag processor for greater consistency and correctness guarantees.
Given the performance focus of this PR, I'll leave feedback on alternative strictly for the purpose of your consideration. I'm happy to leave the final call up to @westonruter. There shouldn't be any downside to using the tag processor here aside from some possibly redundant overhead.
$block_contentcontains HTML. The class attribute there appears to have passed throughesc_attr().$class_name_attris a plain string, it does not include escaped HTML and is not encoded.- Going through the tag processor will ensure that the class names are correctly split. There's a round-trip to HTML encoding and decoding that should not be necessary here. There's a hidden normalization here that should not be necessary since the
$class_name_attris a plain text representation. - Using the tag processor will do a number of things:
- Deduplicate class names (not relevant)
- Normalize null bytes (not relevant)
- Split the string correctly 👍
- Lowercase class names in quirks mode (not relevant, not using quirks mode)
Given we don't need normalization at this phase and we'll later add the plaintext class name using the add_class() method, I think we can use some simpler string whitespace splitting that matches HTML rules for a set of space-separated tokens (like the class attribute).
In short, I think it's safe to save some additional overhead and split like this:
$class_name_attr = "custom&super GREAT\n\r\f\t <swell>\nwp-custom-css-f493a295";
$space_separated_token_delimiter = " \t\f\r\n";
$class_token = strtok( $class_name_attr, $space_separated_token_delimiter );
while ( false !== $class_token ) {
var_dump( $class_token ); // Test for matching class name here.
$class_token = strtok( $space_separated_token_delimiter );
}strtok() has some pitfalls, so a regular expression pattern should also work, it just needs to align with how class names are matched according to standards:
// Named pattern match
$pattern = '/(?:\A|[ \t\f\r\n])(?P<CLASS_NAME>wp-custom-css-[^ \t\f\r\n]+)(?=[ \t\f\r\n]|\z)/';
// Same result using `\K` "reset match"
$pattern = '/(?:\A|[ \t\f\r\n])\Kwp-custom-css-[^ \t\f\r\n]+(?=[ \t\f\r\n]|\z)/';
$res = preg_match( $pattern, $class_name_attr, $matches );Again, please take this as non-blocking feedback. Any of the 3 approaches (tag processor or my suggestions here) seem correct and have different tradeoffs.
There was a problem hiding this comment.
|
Corresponding Gutenberg PR: WordPress/gutenberg#78217 |
Co-authored-by: ramonjd <ramonopoly@git.wordpress.org>
Co-Authored-By: Jon Surrell <jonsurrell@git.wordpress.org>
…om CSS rendering. Short-circuits the custom CSS support filter before the more expensive lookups so blocks without custom CSS return faster. Replaces the regex class name parsing in `wp_render_custom_css_class_name()` with a cheap `str_contains()` guard followed by an HTML spec-compliant `strtok()` walk over the className tokens. This avoids the regex engine for the common case where no `wp-custom-css-` class is present, and correctly handles tab/form-feed/CR/LF separators as well as classes such as `my-wp-custom-css-*` that merely contain the prefix as a substring after a hyphen. Also hardens both functions against malformed parsed blocks (non-string `className`, missing keys), tightens `@phpstan-param` array shapes, and corrects the `block_has_support()` `@param` to allow `WP_Block_Type|null`. Lastly, a `@return Generator<int, non-empty-string>` phpdoc tag is added to `WP_HTML_Tag_Processor::class_list()`. Developed in #11686 and WordPress/gutenberg#78217 Follow-up to r61678. Props mukesh27, westonruter, ramonopoly, jonsurrell. See #64544, #64238. git-svn-id: https://develop.svn.wordpress.org/trunk@62359 602fd350-edb4-49c9-b593-d223f7449a82
|
Committed in 20b5d10 (62359) |
…om CSS rendering. Short-circuits the custom CSS support filter before the more expensive lookups so blocks without custom CSS return faster. Replaces the regex class name parsing in `wp_render_custom_css_class_name()` with a cheap `str_contains()` guard followed by an HTML spec-compliant `strtok()` walk over the className tokens. This avoids the regex engine for the common case where no `wp-custom-css-` class is present, and correctly handles tab/form-feed/CR/LF separators as well as classes such as `my-wp-custom-css-*` that merely contain the prefix as a substring after a hyphen. Also hardens both functions against malformed parsed blocks (non-string `className`, missing keys), tightens `@phpstan-param` array shapes, and corrects the `block_has_support()` `@param` to allow `WP_Block_Type|null`. Lastly, a `@return Generator<int, non-empty-string>` phpdoc tag is added to `WP_HTML_Tag_Processor::class_list()`. Developed in WordPress/wordpress-develop#11686 and WordPress/gutenberg#78217 Follow-up to r61678. Props mukesh27, westonruter, ramonopoly, jonsurrell. See #64544, #64238. Built from https://develop.svn.wordpress.org/trunk@62359 git-svn-id: http://core.svn.wordpress.org/trunk@61640 1a063a9b-81f0-0310-95a4-ce76da25c4cd
…om CSS rendering. Short-circuits the custom CSS support filter before the more expensive lookups so blocks without custom CSS return faster. Replaces the regex class name parsing in `wp_render_custom_css_class_name()` with a cheap `str_contains()` guard followed by an HTML spec-compliant `strtok()` walk over the className tokens. This avoids the regex engine for the common case where no `wp-custom-css-` class is present, and correctly handles tab/form-feed/CR/LF separators as well as classes such as `my-wp-custom-css-*` that merely contain the prefix as a substring after a hyphen. Also hardens both functions against malformed parsed blocks (non-string `className`, missing keys), tightens `@phpstan-param` array shapes, and corrects the `block_has_support()` `@param` to allow `WP_Block_Type|null`. Lastly, a `@return Generator<int, non-empty-string>` phpdoc tag is added to `WP_HTML_Tag_Processor::class_list()`. Developed in #11686 and WordPress/gutenberg#78217 Follow-up to r61678. Reviewed by ellatrix. Merges [62359] to the 7.0 branch. Props mukesh27, westonruter, ramonopoly, jonsurrell. See #64544, #64238. git-svn-id: https://develop.svn.wordpress.org/branches/7.0@62376 602fd350-edb4-49c9-b593-d223f7449a82
…om CSS rendering. Short-circuits the custom CSS support filter before the more expensive lookups so blocks without custom CSS return faster. Replaces the regex class name parsing in `wp_render_custom_css_class_name()` with a cheap `str_contains()` guard followed by an HTML spec-compliant `strtok()` walk over the className tokens. This avoids the regex engine for the common case where no `wp-custom-css-` class is present, and correctly handles tab/form-feed/CR/LF separators as well as classes such as `my-wp-custom-css-*` that merely contain the prefix as a substring after a hyphen. Also hardens both functions against malformed parsed blocks (non-string `className`, missing keys), tightens `@phpstan-param` array shapes, and corrects the `block_has_support()` `@param` to allow `WP_Block_Type|null`. Lastly, a `@return Generator<int, non-empty-string>` phpdoc tag is added to `WP_HTML_Tag_Processor::class_list()`. Developed in WordPress/wordpress-develop#11686 and WordPress/gutenberg#78217 Follow-up to r61678. Reviewed by ellatrix. Merges [62359] to the 7.0 branch. Props mukesh27, westonruter, ramonopoly, jonsurrell. See #64544, #64238. Built from https://develop.svn.wordpress.org/branches/7.0@62376 git-svn-id: http://core.svn.wordpress.org/branches/7.0@61657 1a063a9b-81f0-0310-95a4-ce76da25c4cd
Performance regression for #10777
Trac ticket: https://core.trac.wordpress.org/ticket/64544
By checking
$block['attrs']['className']first, it ensure that for the 90% of blocks that don't have custom CSS, the function returns in microseconds without ever triggering the preg_match.Use of AI Tools
N/A
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.