Improve `get_block_templates` performance #4097

oandregal · 2023-02-17T12:33:19Z

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

What

This PR improves the performance of block template resolution.

Why

It provides a performance improvement for block themes. This change should not affect themes that don't have block templates, given they bail early.

The effect of this PR according to the performance CI job:

Home classic theme

Metric (ms)	This PR	Base	Improvement
wpBeforeTemplate	17.02	19.47	+12.58%
wpTemplate	41.06	44.40	+7.52%
wpTotal	57.92	64.23	+9.82%

Home block theme

Metric (ms)	This PR	Base	Improvement %
wpBeforeTemplate	23.34	24.42	+4.42%
wpTemplate	44.27	55.24	+19.85%
wpTotal	67.63	79.56	+14.99%

How

The get_block_templates function is responsible to find block templates that match a given search. The function is provided a query parameter that is used to find the relevant user templates (database) and theme templates (file directory). The query parameter includes data such the slugs of the templates, the areas of the template parts, etc.

I've found that the get_block_templates retrieves and processes all block templates provided by the theme, to filter out the ones that don't match after. This function can be more performant if it filters out the ones that don't match before processing them, so it only computes the data to be used.

The following profile shows the impact of this modification:

Before	After

The time it takes as measured by XDebug profiler: 55ms (before) vs 27ms (after).
The number of times the _build_block_template_result_from_file is called: 22 (before) vs 5 (after).

How to test

Start the bundled environment.
Set .env to production by applying the following patch:

diff --git a/.env b/.env
index 63a8169f64..9b1d52ac50 100644
--- a/.env
+++ b/.env
@@ -54,10 +54,10 @@ LOCAL_DB_TYPE=mysql
 LOCAL_DB_VERSION=5.7
 
 # The debug settings to add to `wp-config.php`.
-LOCAL_WP_DEBUG=true
-LOCAL_WP_DEBUG_LOG=true
-LOCAL_WP_DEBUG_DISPLAY=true
-LOCAL_SCRIPT_DEBUG=true
+LOCAL_WP_DEBUG=false
+LOCAL_WP_DEBUG_LOG=false
+LOCAL_WP_DEBUG_DISPLAY=false
+LOCAL_SCRIPT_DEBUG=false
 LOCAL_WP_ENVIRONMENT_TYPE=local
 
 # The URL to use when running e2e tests.

Run 100 test of the homepage: seq 100 | xargs -Iz curl -o /dev/null -H 'Cache-Control: no-cache' -s -w "%{time_starttransfer}\n" http://localhost:8889 | xclip -selection clipboard.
Copy the results to a spreadsheet like this one to measure the changes.

Notes

I submitted this PR to core instead of Gutenberg because it modifies functions that are no longer there:

_get_block_templates_files: no longer present.
get_block_templates: the corresponding gutenberg_get_block_templates is still present, but it'll be removed once WordPress 6.2 is published. It won't be present in Gutenberg when this PR lands (WordPress 6.3 cycle).

src/wp-includes/block-template-utils.php

oandregal · 2023-02-17T17:24:57Z

src/wp-includes/block-template-utils.php

-			if ( $should_include ) {
-				$query_result[] = $template;
-			}
+			$query_result[] = _build_block_template_result_from_file( $template_file, $template_type );


This is the key change of this PR: instead of building the template object for all items and filter them after, we filter before this step, so we have to build less items. We can do this because all the data used to filter the items is already available in the step before.

The impact of this change can be seen in the XDebug profiler by looking at the number of times the _build_block_template_result_from_file function is called:

Before (22 times) After (5 times)

oandregal · 2023-02-17T17:47:13Z

ok, this is now ready to review :) cc @youknowriad @ockham @creativecoder @carlomanf as people who has worked in this area, according to #1267

oandregal · 2023-03-10T13:17:13Z

The perf job reports the following (copy/pasted here in a single table, otherwise it's difficult to compare the results):

Home classic theme

Metric (ms)	This PR	Base	Improvement
wpBeforeTemplate	24.32	25.33	+3.98%
wpTemplate	50.67	56.97	+11.05%
wpTotal	75.28	80.32	+6.25%

Home block theme

Metric (ms)	This PR	Base	Improvement
wpBeforeTemplate	28.33	31.80	+10.91%
wpTemplate	53.37	71.39	+25.24%
wpTotal	81.34	102.74	+20.82%

src/wp-includes/block-template-utils.php

youknowriad · 2023-03-21T12:17:53Z

Can't talk about the performance impact but this looks great to me. Thanks for the updates @oandregal

mukeshpanchal27

Thanks @oandregal for the update. I take another round of review and left some review and questions.

In my test i found:

The changes only affect block themes, so they did not improve performance for the classic themes. That performance comparison for classic themes is due to GHA variance.
For block theme(Fresh setup with a post):
- After the PR changes, the _build_block_template_result_from_file function called 7 times on the home page and 5 times on the post page, compared to 40 times on the home page and 22 times on the post page before the changes.

It's good improvement for the block theme.

src/wp-includes/block-template-utils.php

mukeshpanchal27

Thanks @oandregal for the changes. Can you share updated performance number?

Only return the theme templates that match the slugs we are looking for, instead of returning them all.

Only return template pats that match the area we are looking for, instead of returning them all.

Only return the theme templates that match the given postTypes, instead of returning them all.

Only return items that don't have a user-defined equivalent, instead of returning them all.

Co-authored-by: Jonny Harris <spacedmonkey@git.wordpress.org>

Co-authored-by: Mukesh Panchal <mukeshpanchal27@users.noreply.github.com>

This reverts commit 6f62ae2.

Co-authored-by: Mukesh Panchal <mukeshpanchal27@users.noreply.github.com>

oandregal · 2023-04-19T09:50:31Z

Thanks @oandregal for the changes. Can you share updated performance number?

I've rebased this upon trunk in preparation for commiting, and updated the issue description with the numbers reported by the perf CI job.

felixarntz · 2023-04-21T19:05:21Z

@oandregal How did you get the numbers from the perf CI job? I'm asking since they only really run for trunk commits as far as I know. Did you trigger it yourself and look at the workflow run logs? The fact that classic themes see a notable improvement strikes me as potentially incorrect, since the change here should only affect block themes like you're saying.

felixarntz

@oandregal Outstanding work here! It's fair to say this is the most impressive single PR with major performance impact I have seen to date. 🥳

Based on a benchmark I just ran, this PR improves overall load time performance with block themes by an impressive ~15%.

Apologies for the delay in review, but it's safe to say this will be a major win for performance in WordPress 6.3 already - almost as good as all the improvements we got from WP 6.1 to 6.2 altogether.

mukeshpanchal27

Thanks @oandregal, Awesome work.

oandregal · 2023-04-24T08:05:54Z

@oandregal How did you get the numbers from the perf CI job? I'm asking since they only really run for trunk commits as far as I know. Did you trigger it yourself and look at the workflow run logs?

🤔

Oh. I looked at the performance CI job that ran for this PR. My understanding was that the numbers under "Print performance test results" was this PR and the numbers under "Print base line test results" was the baseline to compare to. Is that how should I look at them @felixarntz ?

The first time I ran my own numbers they were on the same range that the CI job, so I didn't bother doing my own again. Happy to if I'm reading this wrong.

The fact that classic themes see a notable improvement strikes me as potentially incorrect, since the change here should only affect block themes like you're saying.

Yeah, I don't understand this either. I presumed it was related to the variability of the tests.

felixarntz · 2023-04-24T17:39:43Z

@oandregal

Oh. I looked at the performance CI job that ran for this PR. My understanding was that the numbers under "Print performance test results" was this PR and the numbers under "Print base line test results" was the baseline to compare to. Is that how should I look at them @felixarntz ?

Ah that may be a misunderstanding and slightly confusing. The "base line test results" are not the current stats for trunk, but rather a re-run of getting metrics for trunk in the same baseline state that was originally measured (which I believe we have set to the last commit hash of WP 6.1.1). So this isn't the state of latest trunk.

Looking at the printed "performance test results" is a great idea though. What you need to compare that too though is the similar data for the last commit in trunk that is in your branch here.

All of that is of course not how this should work, but to be fair the performance CI workflow in its current stage was not yet meant to be used to assess PRs. Reporting diffs between the last commit in the branch and the PR automatically in a PR comment is high on the todo-list though :)

oandregal · 2023-04-26T14:48:13Z

This has been committed at https://core.trac.wordpress.org/changeset/55687

oandregal · 2023-07-31T09:29:27Z

This has created an issue with child themes, see instructions at WordPress/gutenberg#53138 I'm looking into this.

oandregal · 2023-07-31T14:52:50Z

Fix at #4940

oandregal · 2023-07-31T14:56:00Z

src/wp-includes/block-template-utils.php

-				continue;
-			}
-
-			$is_not_custom   = false === array_search(


This logic checks for both templates coming from the user ("custom" as it was defined here) and previously inserted templates (for example, it checks that a parent template is not added if the child already defined it). This wasn't properly ported and caused an issue.

mukeshpanchal27 reviewed Feb 17, 2023

View reviewed changes

src/wp-includes/block-template-utils.php Outdated Show resolved Hide resolved

oandregal marked this pull request as draft February 17, 2023 15:24

oandregal commented Feb 17, 2023

View reviewed changes

oandregal marked this pull request as ready for review February 17, 2023 17:45

spacedmonkey reviewed Mar 10, 2023

View reviewed changes

src/wp-includes/block-template-utils.php Show resolved Hide resolved

spacedmonkey reviewed Mar 10, 2023

View reviewed changes

src/wp-includes/block-template-utils.php Outdated Show resolved Hide resolved

youknowriad reviewed Mar 21, 2023

View reviewed changes

src/wp-includes/block-template-utils.php Outdated Show resolved Hide resolved

src/wp-includes/block-template-utils.php Outdated Show resolved Hide resolved

src/wp-includes/block-template-utils.php Outdated Show resolved Hide resolved

youknowriad approved these changes Mar 22, 2023

View reviewed changes

mukeshpanchal27 reviewed Mar 27, 2023

View reviewed changes

mukeshpanchal27 reviewed Mar 30, 2023

View reviewed changes

oandregal and others added 15 commits April 18, 2023 16:18

Filter theme templates (wp_template & wp_template_part) by slug

edb0d2b

Only return the theme templates that match the slugs we are looking for, instead of returning them all.

Filter wp_template_part by area

5aa5a39

Only return template pats that match the area we are looking for, instead of returning them all.

Filter wp_template by postTypes

9173757

Only return the theme templates that match the given postTypes, instead of returning them all.

Filter theme templates (wp_template & wp_template_part) by user

0026719

Only return items that don't have a user-defined equivalent, instead of returning them all.

Add PHPDoc for _get_block_templates_files changes

86d533b

Do not query the template directory if it is the same as stylesheet dir.

8476944

Co-authored-by: Jonny Harris <spacedmonkey@git.wordpress.org>

Remove unused query param

2ffbb24

Convert the user_templates param into slugs__not_in query param

dc1e44c

Limit query results by number of slugs

b234e1d

Update src/wp-includes/block-template-utils.php

60896d3

Co-authored-by: Mukesh Panchal <mukeshpanchal27@users.noreply.github.com>

Update src/wp-includes/block-template-utils.php

91711a7

Co-authored-by: Mukesh Panchal <mukeshpanchal27@users.noreply.github.com>

Revert "Update src/wp-includes/block-template-utils.php"

208ee0c

This reverts commit 6f62ae2.

Update src/wp-includes/block-template-utils.php

2a59888

Co-authored-by: Mukesh Panchal <mukeshpanchal27@users.noreply.github.com>

Use in_array instead of array_search

12c2193

Make sure slugs are unique

c5cfb8e

Merge branch 'trunk' into improve/get-block-templates-performance

090547b

felixarntz approved these changes Apr 21, 2023

View reviewed changes

mukeshpanchal27 approved these changes Apr 22, 2023

View reviewed changes

oandregal closed this Apr 26, 2023

oandregal deleted the improve/get-block-templates-performance branch April 26, 2023 14:48

oandregal mentioned this pull request Apr 27, 2023

Docs: describe return type of _get_block_template_file(). #4388

Closed

oandregal mentioned this pull request Jul 31, 2023

Fix for template retrieval: do not include parent template if a child template is defined #4940

Closed

oandregal commented Jul 31, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve `get_block_templates` performance #4097

Improve `get_block_templates` performance #4097

oandregal commented Feb 17, 2023 •

edited

Loading

oandregal Feb 17, 2023 •

edited

Loading

oandregal Feb 17, 2023 •

edited

Loading

oandregal commented Feb 17, 2023

oandregal commented Mar 10, 2023 •

edited

Loading

youknowriad commented Mar 21, 2023

mukeshpanchal27 left a comment •

edited

Loading

mukeshpanchal27 left a comment

oandregal commented Apr 19, 2023

felixarntz commented Apr 21, 2023

felixarntz left a comment

mukeshpanchal27 left a comment

oandregal commented Apr 24, 2023

felixarntz commented Apr 24, 2023

oandregal commented Apr 26, 2023

oandregal commented Jul 31, 2023

oandregal commented Jul 31, 2023

oandregal Jul 31, 2023

Improve get_block_templates performance #4097

Improve get_block_templates performance #4097

Conversation

oandregal commented Feb 17, 2023 • edited Loading

What

Why

How

How to test

Notes

oandregal Feb 17, 2023 • edited Loading

Choose a reason for hiding this comment

oandregal Feb 17, 2023 • edited Loading

Choose a reason for hiding this comment

oandregal commented Feb 17, 2023

oandregal commented Mar 10, 2023 • edited Loading

youknowriad commented Mar 21, 2023

mukeshpanchal27 left a comment • edited Loading

Choose a reason for hiding this comment

mukeshpanchal27 left a comment

Choose a reason for hiding this comment

oandregal commented Apr 19, 2023

felixarntz commented Apr 21, 2023

felixarntz left a comment

Choose a reason for hiding this comment

mukeshpanchal27 left a comment

Choose a reason for hiding this comment

oandregal commented Apr 24, 2023

felixarntz commented Apr 24, 2023

oandregal commented Apr 26, 2023

oandregal commented Jul 31, 2023

oandregal commented Jul 31, 2023

oandregal Jul 31, 2023

Choose a reason for hiding this comment

Improve `get_block_templates` performance #4097

Improve `get_block_templates` performance #4097

oandregal commented Feb 17, 2023 •

edited

Loading

oandregal Feb 17, 2023 •

edited

Loading

oandregal Feb 17, 2023 •

edited

Loading

oandregal commented Mar 10, 2023 •

edited

Loading

mukeshpanchal27 left a comment •

edited

Loading