Backport: WP_Theme_JSON_Resolver changes

[Mamaduka]

Backport changes and fixes merged into a Gutenberg plugin after WP 6.1.

Related Gutenberg PRs:

• Make child themes inherit parent's style variations. gutenberg#46554
• Performance global styles WP_Query gutenberg#46043
• Merged data should consider origin to return early gutenberg#45969

I'm going to backport WordPress/gutenberg#46112 separately.

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

---

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.

[audrasjb]

small docblock change proposal

[Mamaduka]

Is it preferred sorting arrays before assertions, or can we use something like assertEqualsCanonicalizing?

[hellofromtonya]

Here the deeply nested array of arrays are being recursively key sorted before the assertion. assertSameSets() repeats an index sort at the first dimension of the arrays. That's not necessary.

Instead of assertSameSets(), use assertSame().

I'm wondering though why this needed. Pulling down to explore.

[Mamaduka]

Hi, @hellofromtonya

Let me know if any follow-ups are needed for PHPUnits tests on this PR.

[hellofromtonya]

Hey @Mamaduka, is this PR and Trac ticket still needed? I remember you were breaking up the GB PRs into smaller Trac tickets. Just checking before I dive into it.

[Mamaduka]

@hellofromtonya, that was for WP_Theme_JSON file. I decided to keep this one since I had already done the work, and most of the line changes were for PHPUnit tests.

[costdev]

Thanks for the PR @Mamaduka! Just a few suggestions. 🙂

[Mamaduka]

Thanks for the suggestions, @costdev!

[felixarntz]

Performance check ✅

I gave this some server-side performance tests for the frontend, measuring TT3 on the home page and a single post with the Gutenberg demo content, using https://gist.github.com/felixarntz/de5c697a1a16c2b892634b70216eb6c7.

TLDR / Summary / Conclusion

Results look inconclusive. Function-specific benchmarks show a clear improvement, while overall WordPress execution time shows inconsistent results between the two tests. This suggests that the changes in this PR probably enhance performance, but overall does not have a great impact on performance. At the same time, it also means that there is no concern about merging this, so we can proceed from that perspective.

Results

All times below are in milliseconds.

First I benchmarked overall WP load time. Every test row below is actually based on the median of 20 runs itself. So overall this is 120 runs per scenario.

	trunk @ [55218]	plus PR #3901	% diff
Home page (test 1)	193.99	206.9	6.65%
Gutenberg demo (test 1)	195.65	193.1	-1.30%
Home page (test 2)	194.31	198.11	1.96%
Gutenberg demo (test 2)	194.98	191.46	-1.81%
Home page (test 3)	198.5	203.47	2.50%
Gutenberg demo (test 3)	193.64	202.12	4.38%

Because the results didn't show a clear trend in either direction, I wanted to see at least more specifically whether certain WP_Theme_JSON_Resolver class methods showed performance increases or regressions here.

Here are the results for WP_Theme_JSON_Resolver::get_merged_data(), again based on 20 runs per scenario (for the home page):

	trunk @ [55218]	plus PR #3901	% diff
get_merged_data() time	14.63	13.99	-4.37%
get_merged_data() calls	50	50	0.00%
WP total time	199.01	202.04	1.52%

Here are the results for WP_Theme_JSON_Resolver::get_user_data_from_wp_global_styles(), again based on 20 runs per scenario (for the home page):

	trunk @ [55218]	plus PR #3901	% diff
get_user_data_from_...() time	2.5	1.57	-37.20%
get_user_data_from...() calls	2	2	0.00%
WP total time	198.64	196.6	-1.03%

Both of the above show improvements for their respective method's performance, so that's promising. We will have to see whether this has any impact in the field, but overall it certainly looks unproblematic, and if anything, there is a positive trend in WP_Theme_JSON_Resolver performance. The overall WP execution time differences are too inconsistent to mean anything, likely due to high variance.

Note: The get_style_variations() method was not covered as it is not invoked in the frontend and thus has no impact there.

[felixarntz]

@Mamaduka Thanks! Code-wise this all looks solid to me, and performance-wise as well per my above feedback.

[Mamaduka]

Thanks for the reviews and testing, folks 🙇

It would be great to get this committed before Beta 1.

[ntsekouras]

Committed via: https://core.trac.wordpress.org/changeset/55231