Improve performance of the WP_Theme_JSON class

[aristath]

As pointed out in other tickets during the 6.1 release cycle, the WP_Theme_JSON is one of the biggest bottlenecks (if not the biggest) when loading a page using a block theme.
Some of the methods inside WP_Theme_JSON call a lot of expensive array functions - and their effect is compounded by the fact that they usually run hundreds of times as part of deeply-nested loops.
We can improve the performance of these methods by simplifying all these calls to faster, simpler, and usually more readable alternatives.

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

This PR simplifies expensive array-functions calls where appropriate.

Attaching some profiling results to showcase the impact.

Before (trunk):

After (with this PR):

All numbers in the screenshots above use milliseconds, and the impact of these seemingly simple changes is bigger than expected, shaving off more than 200ms. Of course those numbers are a bit inflated due to the fact that profiling slows down a page significantly, but the overall improvement on the page-load is more than 3%.

[spacedmonkey]

Related: ​https://github.com/WordPress/gutenberg/issues/45171

[matt]

Cool!

While doing some profiling, I noticed there are still calls to _wp_array_get. This calls array_key_exists, which is expensive. I wonder if we could avoid these somehow.

I wonder if we remove some uses of _wp_array_get. This function calls array_key_exists internally. If we can remove a couple of uses of this, it would give us a performance win here.

I wonder if we remove some uses of _wp_array_get. This function calls array_key_exists internally. If we can remove a couple of uses of this, it would give us a performance win here.

While doing some profiling, I noticed there are still calls to _wp_array_get. This calls array_key_exists, which is expensive. I wonder if we could avoid these somehow.

I wouldn't worry (or overestimate) the impact of array_key_exists().

• array_key_exists() is _not_ one of the typically expensive array_*() function calls.
• Yes, isset() is generally speaking faster...
• ... BUT array_key_exists() has been ​opcode optimized in PHP since PHP 7.4 and what with the ​fast majority of WP users being on PHP 7.4 (or above), the performance difference will be negligible for most users.

Note: isset() and array_key_exists() also have a _functional_ difference in how they treat a null value, so for array_key_exists() to be equivalent to isset(), it MUST be accompanied by a && $var !== null.

Note: isset() and array_key_exists() also have a _functional_ difference in how they treat a null value, so for array_key_exists() to be equivalent to isset(), it MUST be accompanied by a && $var !== null.

Yeah, that's an important distinction!
In the comments I added for future reference to replace array_key_exists() with isset() (// Note: array_key_exists() should be replaced with an isset() check once WordPress drops support for PHP 5.6.), I checked the values in each case and only added that comment in cases where the value can not be null.
In one case the value _can_ be null, and I added a comment in that one that // We need to use array_key_exists() instead of isset() because the value can be null. 👍

In trunk.

In this brunch.

@jrfnl Following up on array_key_exists: Makes sense not to overthink this in terms of performance impact compared to isset, however I had one follow up thought I wanted to at least raise here:

• Most of the usage of array_key_exists here could be replaced with isset as the additional null consideration is irrelevant for most (if not all) of the code where it's used here.
• Given that, there would be the alternative option of assigning the result to a variable and then calling isset on that variable.

For example, instead of:
{{{php
if ( array_key_exists( $element, static::EXPERIMENTAL_ELEMENT_CLASS_NAMES ) ) {
}
}}}

Do:
{{{php
$element_class_names = static::EXPERIMENTAL_ELEMENT_CLASS_NAMES;
if ( isset( $element_class_names[ $element ] ) {
}
}}}

Again, not saying this is necessarily worth doing (and certainly not a blocker), but I'd be curious if the first or the last is generally faster. :)

There's bound to be more room for improvement, but yes, let's take this step by step and I do believe this is a valuable step in that.

Re: assign+isset vs array_key_exists: as most array_key_exists() are only in place for PHP 5.6, might be more effective to re-open the discussion on dropping support for PHP 5.6 ... 😇

On that note - PHP 8.3 will bring another very useful improvement ​`json_validate()` (validate a JSON input before decoding without running the risk of the JSON tying up all the memory), but that is no doubt still a bridge too far ;-)

[flixos90]

In 54804:

Editor: Improve performance of WP_Theme_JSON class by reducing usage of expensive array functions.

In many scenarios array functions are more expensive than using simpler for or foreach loops.

This changeset results in roughly 4% faster wp_head execution time for both block themes and classic themes. While this may seem like a small win, it is a worthwhile enhancement and only one part of several other little performance tweaks which are being worked on to improve performance of theme.json parsing further.

Props aristath, desrosj, jrf, spacedmonkey.
Fixes #56974.
See #57067.

Committed in https://core.trac.wordpress.org/changeset/54804.

[flixos90]

Reopening for backport to 6.1.1.

[flixos90]

In 54805:

Editor: Improve performance of WP_Theme_JSON class by reducing usage of expensive array functions.

In many scenarios array functions are more expensive than using simpler for or foreach loops.

This changeset results in roughly 4% faster wp_head execution time for both block themes and classic themes. While this may seem like a small win, it is a worthwhile enhancement and only one part of several other little performance tweaks which are being worked on to improve performance of theme.json parsing further.

Props aristath, desrosj, jrf, spacedmonkey.
Merges [54804] to the 6.1 branch.
Fixes #56974.
See #57067.

[flixos90]

In 55142:

Editor: Fix undefined variable following [54805].

Props mamaduka, costdev, mukesh27.
See #56974, #57067.

[flixos90]

@jeffpaul Do we need to backport the fix from [55142] to the 6.1 branch? If so, should this ticket be reopened for that purpose? Not sure what the timeline for a potential 6.1.2 is.

[JeffPaul]

No immediate 6.1.2 that I have planned / I know is planned. @priethor @desrosj @mikeschroder @davidbaumwald @czapla @bernhard-reiter @cbravobernal as other 6.1 RC/Core&Editor Tech Leads and @hellofromTonya @webcommsat as core team reps, are any of you aware of / planning a 6.1.2?

This was raised as part of #56974, which reviewed several of these checks.isset is slightly preferable overarray_key_exists in terms of performance (with the caveat that the two behave differently when it comes to null values).

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

Gutenberg PR: ​https://github.com/WordPress/gutenberg/pull/53098

Committed in https://core.trac.wordpress.org/changeset/56345.