Improve theme file caching in class-wp-theme-json.php #6707
Conversation
Minor changes have been made to class-wp-theme-json.php to better cache the theme files. This includes adding an 'update_cache' functionality that sets 'str_start_with' and 'str_start_with_cache', and removing redundant checks for empty or non-numerical values. These improvements aim to optimize performance by updating cache when necessary.
|
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. |
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 array-to-string conversion in the class-wp-theme-json.php file has been modified for more optimized performance. The original simple implode function has been replaced with a loop to more efficiently concatenate the array elements. This approach also gives more flexible control in handling the dot separator.
|
Thanks @spacedmonkey. I did some profiling of this change against trunk and it does reduce the number of Trunk This PR That said, the PR we've been working on has a bigger impact by reducing the number of calls to this method altogether. It may be worth combining both approaches to maximize the benefit, but I'd like to prioritize getting #6392 into trunk first as it has the biggest potential impact. |
|
@spacedmonkey are ok to close this in favor of WordPress/gutenberg#62522? |
|
@joemcgill This is the backport PR? We'll need both? |
|
@ellatrix and @spacedmonkey, it's fine to use this PR as the backport from WordPress/gutenberg#62522. At present, it's not the same code though, so this will need to be updated once the GB PR is finalized. |
| } | ||
|
|
||
| /* | ||
| * Look up protected properties, keyed by value path. | ||
| * Skip protected properties that are explicitly set to `null`. | ||
| */ | ||
| if ( is_array( $value_path ) ) { | ||
| $path_string = implode( '.', $value_path ); | ||
| $path_string = ''; | ||
| for ( $i = 0; $i < count( $value_path ); $i++ ) { |
There was a problem hiding this comment.
is this bit faster than the call to implode?
There was a problem hiding this comment.
Marking as not ready until after this has been updated with the approach from WordPress/gutenberg#62522 (see related comment).
|
@spacedmonkey, I've approved the related GB PR. Once this PR is updated to reflect the latest changes, I'm happy to give another review. |
|
Closing in favour of #6895 |
Minor changes have been made to class-wp-theme-json.php to better cache the theme files. This includes adding an 'update_cache' functionality that sets 'str_start_with' and 'str_start_with_cache', and removing redundant checks for empty or non-numerical values. These improvements aim to optimize performance by updating cache when necessary.
Trac ticket: https://core.trac.wordpress.org/ticket/59595
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.