Performance regression in WP 6.1 for `theme.json` processing

[aristath]

Description

I'm currently trying to improve performance in WP-Core, and the thing that currently stands out as a bottleneck is the WP_Theme_JSON::remove_keys_not_in_schema method.
Posting a screenshot of a webgrind analysis below:

WP_Theme_JSON::remove_keys_not_in_schema runs 38228 times, and consumes 14.36% of resources/server-time.

Step-by-step reproduction instructions

I'll just post instructions on how I setup webgrind on my localhost using wp-env, so hopefully others can also test and debug performance issues:

1. I use the https://github.com/WordPress/wordpress-develop repo, but I think the same would apply for this one. Edit the .env file. Set LOCAL_PHP_XDEBUG=true, and LOCAL_PHP_XDEBUG_MODE=develop,debug,profile.
2. Download or clone https://github.com/jokkedk/webgrind locally. I do it in wp-content/plugins/webgrind but all that matters is you put it somewhere you can access it from a URL.
3. start wp-env normally. Xdebug will now be available and profiling will run. Which also means that page-loads will be significantly slower due to all the profiling happening in the background.
4. Visit your site, login, then refresh the frontend a few times. Doing multiple runs will allow you to get some more stats and you can ensure that the stats you see later are not a fluke but consistent between all your page-loads.
5. Visit the URL where you copied webgrind, and examine the results. In my case that's on http://localhost:8889/wp-content/plugins/webgrind/ but it will vary depending on where you copied the files etc.

Screenshots, screen recording, code snippet

No response

Environment info

• WordPress trunk
• No plugins activated
• Happens with and without the Gutenberg plugin active

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

[aristath]

I'm trying to understand why remove_keys_not_in_schema is necessary... 🤔
I understand that it cleans up the parsed JSON, but is there any actual problem if the JSON has some extra things in it?
Do extra/unused items in the json cause any problems? Or do we just use that function because we want to keep things tightly controlled & limited?

cc @getdave, if I'm not mistaken that was added in the sanitize() method in #41786 ?

[oandregal]

We merged two recent PRs in wordpress-develop that made the caching only be available for reading the JSON file from the file system, and removed the other processing we did before.

• Make Editor and Frontend reflect Block Styles in theme.json wordpress-develop#3408
• WP_Theme_JSON: Invalidate blocks metadata cache when needed wordpress-develop#3359

The rationale for this is that the theme.json classes are now used before all the blocks are registered (aka get template data, etc.), hence, the caching wasn't working as expected. See #44434 and #44619

I'd like to have a better understanding of a baseline before/after these PRs to see what's the impact and whether it's something we need to act for 6.1 or can do later.

[aristath]

Update: It looks like this commit is the culprit: WordPress/wordpress-develop@a937892
Reverting that commit fixes the issue locally.

Props @ockham and @oandregal for pointing me to that direction 👍

[ockham]

Thank you for tracking this down, @aristath. I had left a comment on the originating PR with some suggestions for more caching: WordPress/wordpress-develop#3408 (comment). Maybe applying these could mitigate the issue?

Unfortunately, I'll only have limited availability myself until Tuesday. Pinging @oandregal @andrewserong @ajlende @talldan @glendaviesnz who were involved in the original research for assistance 😅

[ockham]

I had left a comment on the originating PR with some suggestions for more caching: WordPress/wordpress-develop#3408 (comment). Maybe applying these could mitigate the issue?

Ah, that might not cut the mustard 😕 (I had missed these two comments: #44772 (comment) and #44772 (comment).)

[aristath]

Posting 2 screenshots with the stats with and without that commit.

With the commit applied:

When reverting the commit:

The times shown there are all in milliseconds, and keep in mind that Xdebug + profiling slow down the site a lot. However, the differences are an indication of the commit's impact.
Reverting the commit reduces the time from 9473ms to 7528.
The WP_Theme_JSON::sanitize() method was called 633 times, and reverting the commit makes that number go down to 251. I believe that is the main issue... The sanitize method is what calls remove_keys_not_in_schema, so all those extra calls just make processing slower.

[ockham]

Thank you Ari!

@oandregal Can we cache sanitize based on the 2nd and 3rd argument (list of valid blocks and elements, respectively)? The keys of those arrays (block and element names) should be hopefully enough.