Cache duplicate calls to WP_Theme_JSON::sanitize

[josephscott]

A fresh out of the box WordPress site with the 2024 theme will make 53 calls to WP_Theme_JSON::sanitize() during a single request to the default home page. We can see the progress of duplicate $input calls with a simple addition to wp-includes/class-wp-theme-json.php:

    protected static function sanitize( $input, $valid_block_names, $valid_element_names, $valid_variations ) {

        static $inputs = array();
        $input_key = md5( serialize( $input ) );
        if ( ! isset( $inputs[ $input_key ] ) ) {
            $inputs[ $input_key ] = 1;
        } else {
            $inputs[ $input_key ]++;
        }
        error_log( print_r( $inputs, true ) );

        $output = array();

The final log entry:

Array
(
    [c81ffa6dbea2454497cd6bd1bb5328a0] => 2
    [c5ed022349e4a64a02171deb00a3d988] => 23
    [cb150b99ce54ad170f5d795b65f6a651] => 2
    [7aef88bb839b090be13dc01ab67a52e1] => 2
    [a277ebc729ba3b9e28c492d103043a2e] => 21
    [f8c99016466edd5a5377fdf68d6fa04d] => 1
    [4ec129bfa3c2182a1037990a0aad6232] => 1
    [6f5c486d28c1d96906e5237180daed22] => 1
)

Of the 53 calls, there are only 8 unique $input values. All but 3 have duplicates. With that ratio it we'd likely benefit from caching the results and skipping the work on cache hits. This would be a classic trade off of doing less work by using a bit more memory.

I'm including a patch to cache processed $inputs with a simple static variable. This allows for multiple calls in a single page view to benefit from the reduced amount of work. The exact implementation is not my main concern as much as reducing the amount of time spent processing WP_Theme_JSON::sanitize() calls.

Doing 1,000 sequential HTTP GET requests for the front page, here is the TTFB p75 before and after. While TTFB can vary because of any number of things taking longer, I'm including it to show even at that scale it can potentially move the needle. To show the difference in isolation for just this method call I'm including the exclusive processing time for a single home page view for WP_Theme_JSON::sanitize() as measured by php-spx.

Before patch

• TTFB p75: 44.28ms
• exclusive time: 1.74ms

After patch

• TTFB p75: 43.601ms
• exclusive time: 0.779ms

Change

• TTFB p75: 0.679ms ( 1.5% )
• exclusive time: 1.061ms ( 61% )

Setup:

• WP 6.7-alpha-59092
• PHP 8.3.10
• Theme: Twenty Twenty-Four
• No active plugins
• Default home page with 2024 theme

This applies the diff supplied by @josephscott in https://core.trac.wordpress.org/attachment/ticket/62126/cache-wp-theme-json-sanitize-input.diff to run CI against the proposed changes.

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

[joemcgill]

Thanks @josephscott. I've opened a PR from your diff in order to run CI against the proposed changes.

[joemcgill]

It looks like this is causing several unit tests to fail: ​https://github.com/WordPress/wordpress-develop/actions/runs/11220806461/job/31189817758?pr=7529

One thing to keep in mind is that WP_Theme_JSON data gets sanitized at different times during the application lifecycle so it's possible that code that modifies one of the origins (e.g., registered blocks) runs after the first time this method is called, which could cause these caches to become stale.

[josephscott]

Looks like I need to expand it to consider the other parameters as well.

If things can be altered after that fact then yeah, that is something that could be a problem as well.

[joemcgill]

Caching different aspects of the WP Theme JSON structure has been tricky for this very reason. If you're interested in a deep dive, you may find the discussion in #57789 useful.

[joemcgill]

#62187 was marked as a duplicate.

[joemcgill]

I've reverted setting #62187 as a duplicate since both issues seek to improve the performance of WP_Theme_JSON::sanitize in slightly different ways that are not mutually exclusive.

[joemcgill]

Going to set this, along with #62187 as issues to review for the 6.8 milestone. In the meantime, @josephscott do you want to investigate the PHPUnit test failures?

I also find it worth for some of the keys that are common in requested input when i profile the WP release.

[flixos90]

@joemcgill Are you mostly waiting for PR reviews at this point? Or anything else outstanding here? I'm happy to give this a review, if needed.

[joemcgill]

@flixos90 The PR here is just the applied patch from @josephscott cache-wp-theme-json-sanitize-input.diff.

Last update here is that it still needs to be updated. I'm going to remove the assignee myself so it's clear that someone else can pick this up. If there's not movement in the next couple of weeks, this is probably a candidate to punt to a future release.

[pbearne]

will update and add tests

…hin the WP_Theme_JSON class. Let's break down the changes and their impact.

Problem: The original sanitize method likely performed redundant computations when called multiple times with the same input. Sanitizing theme JSON data can be resource-intensive, especially with complex themes or frequent calls.

Solution: The added code addresses this inefficiency by implementing a simple caching mechanism. Before performing the sanitization, the method now checks if the input has already been processed.

How the Cache Works:

1. Hashing the Input: The md5 function generates a unique hash of the input array using wp_json_encode. This hash serves as the key for the cache. Using a hash ensures fast lookups and avoids potential issues with complex array keys.

2. Checking the Cache: The code checks if the generated hash key exists in the $sanitize_input_cache static property. If found, the cached sanitized output is returned directly, bypassing the sanitization process.

3. Caching the Result: If the hash is not found in the cache, the sanitization proceeds as before. After the sanitization is complete, the result is stored in the $sanitize_input_cache, associating it with the calculated hash key.

Benefits of this Change:

• Performance Improvement: By avoiding redundant calculations, this caching significantly improves performance, especially in scenarios where the same theme JSON data is sanitized repeatedly.
• Reduced Resource Consumption: Less processing means reduced CPU load and memory usage.

Potential Considerations:

• Cache Invalidation: The current implementation doesn't have a mechanism for cache invalidation. This means that if the underlying theme JSON data changes, the cached value might become stale. A more robust solution might consider ways to invalidate the cache when theme changes occur.
• Memory Usage: While generally beneficial, caching can consume memory. If the number of unique theme JSON variations is extremely large, the cache could potentially grow quite large. Monitoring memory usage is advisable, especially in memory-constrained environments.

Overall, this change is a positive improvement that addresses performance concerns in theme JSON sanitization. The simple caching strategy effectively reduces redundant processing and enhances efficiency. However, it's important to be aware of the potential need for cache invalidation in a production environment.

[pbearne]

I have added a function to reset the cache and added a call to the wp_add_global_styles_for_blocks function

This clears the test errors

[audrasjb]

As per today's bug scrub: It appears this ticket is still needs to be reviewed. As 6.8 beta 1 is very close, I'm moving it to 6.9. Feel free to move it back to 6.8 if it can be committed by Monday.