Blocks: Introduce helper function to retrieve hooked blocks

[gziolo]

In order to implement Block Hooks (see #59313), we added block_hooks field to the WP_Block_Type class, as well as to block registration related functions. In this follow-up, new helper function gets introduced that is going to compute the list of hooked blocks by other registered blocks for a given block type.

This allows for a more modular approach that will eventually be extended to implement automatic insertion of hooked blocks (see #59313).

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

In order to implement Block Hooks (see #59313), we added block_hooks field to the WP_Block_Type class, as well as to block registration related functions. In this follow-up, new helper function gets introduced that is going to compute the list of hooked blocks by other registered blocks for a given block type.

Extracted from ​https://github.com/WordPress/wordpress-develop/pull/5158 and covered with unit tests.

The ticket is https://core.trac.wordpress.org/ticket/59383.

[gziolo]

In 56610:

Blocks: Introduce helper function to retrieve hooked blocks

In order to implement Block Hooks (see #59313), we added block_hooks field to the WP_Block_Type class, as well as to block registration related functions. In this follow-up, new helper function gets introduced that is going to compute the list of hooked blocks by other registered blocks for a given block type.

Extracted from ​https://github.com/WordPress/wordpress-develop/pull/5158 and covered with unit tests.

Props ockham.
Fixes #59383.

Committed with https://core.trac.wordpress.org/changeset/56610.

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

[spacedmonkey]

Reopenning as performance issue with the get_hooked_blocks function. See screenshots.

@gziolo Created a ​https://github.com/WordPress/wordpress-develop/pull/5293 that might fix the issue.

Performance issue.

### Before

### After

[gziolo]

Great catch @spacedmonkey.

Do you think we should also compute the result of processing all registered blocks once and cache it? get_hooked_blocks is going to be called multiple times on every page that is served using each theme template shipped with the theme. It's something that we still need to profile to understand better the implications.

[spacedmonkey]

@gziolo There are lots of places where register blocks are looked up. There are filters and blocks are can be unregistered, caching it worries me.

[gziolo]

One alternative solution I was contemplating is to pass the structure for all hooked blocks to the callback that is called when serializing every block. Instead of computing it here:

​https://github.com/WordPress/wordpress-develop/blob/de9e06a4c021295af3ac11fdd08ea29a57529668/src/wp-includes/blocks.php#L807

​https://github.com/WordPress/wordpress-develop/blob/de9e06a4c021295af3ac11fdd08ea29a57529668/src/wp-includes/blocks.php#L827

We could do it once for a template in:

​https://github.com/WordPress/wordpress-develop/blob/de9e06a4c021295af3ac11fdd08ea29a57529668/src/wp-includes/block-template-utils.php#L605-L606

and once per pattern:

​https://github.com/WordPress/wordpress-develop/blob/de9e06a4c021295af3ac11fdd08ea29a57529668/src/wp-includes/class-wp-block-patterns-registry.php#L170-L171

[spacedmonkey]

In 56677:

Themes: Use instanceof in get_hooked_blocks.

In [56610], the get_hooked_blocks function was introduced. However, using property_exists within this function negatively impacted its performance. This commit replaces the property_exists function call with instanceof WP_Block_Type, resulting in improved performance.

Props gziolo, spacedmonkey.
See #59383.

Committed ​https://github.com/WordPress/wordpress-develop/commit/f9299148b7b7e94ac2742f5e249dd7f342ca6669

[hellofromTonya]

Should this new utility function be wp_ prefixed?

I realize the most of the other functions in the same file are not prefixed. However, as this is a brand new Core function, prefixing could avoid a name collision if a plugin, theme, or mu-use script has the same function name.

What is the likelihood of another global function being named the same?

• A search of ​plugins and ​themes on wp.org did not reveal any instances.
• A ​search of GitHub did not find any instances other than WordPress.

That's good. It's still possible in the wild for things not hosted in either of these mediums.

Thoughts? I'll mark this with 2nd-opinion to open the discussion.

[Bernhard Reiter]

@hellofromTonya FWIW, we've only recently introduced the term "Block Hooks" to denote this new feature, and the related/"reverse" concept of hooked blocks. I'm inclined to think that the name is obscure enough that it hasn't sprung up elsewhere in this context or a different one (as somewhat confirmed by your research).

(My personal preference would be to omit the wp_ prefix, but since I'm the one who first suggested the function name, mine shouldn't probably count as a second opinion :)

See the unit test diff to see how the return format has changed.

The main reason for doing is is that all existing calls of get_hooked_blocks in production code used to need an extra array_keys call. This seemed beside the point.

Furthermore, this allows us to remove the extra $relative_position argument from the function again, as the same data can now be simply fetched via array access.

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

[Bernhard Reiter]

In 56704:

Blocks: Have get_hooked_blocks() return blocks grouped by position.

All existing calls of get_hooked_blocks() in non-test code are currently wrapped in an extra array_keys() call. This changeset absorbs that logic into the function and changes the structure of the return value accordingly.

Furthermore, this allows us to remove the extra $relative_position argument (introduced in [56673]) from the function again, as the same data can now be simply fetched via array access.

Props gziolo, spacedmonkey, mukesh27.
See #59383.

Committed to Core in https://core.trac.wordpress.org/changeset/56704.

[hellofromTonya]

Replying to Bernhard Reiter:

@hellofromTonya FWIW, we've only recently introduced the term "Block Hooks" to denote this new feature, and the related/"reverse" concept of hooked blocks. I'm inclined to think that the name is obscure enough that it hasn't sprung up elsewhere in this context or a different one (as somewhat confirmed by your research).

(My personal preference would be to omit the wp_ prefix, but since I'm the one who first suggested the function name, mine shouldn't probably count as a second opinion :)

Thanks for Bernie for sharing the reasoning and preference. I'm okay with the name as long as there are no naming conflicts that impact users. I'll remove the 2nd-opinion keyword with a disclaimer to keep a watchful eye through the rest of the 6.4 cycle just in case there are reports of naming issues.

[hellofromTonya]

This ticket was reopened due to performance concerns which seem to be resolved with [56677]. Is there work remaining? Or can this ticket be reclosed?

cc @bernhard-reiter @spacedmonkey

[gziolo]

In 56759:

Tests: Cover Block Hooks integration with a custom block theme

Adds a simplified version of Twenty Twenty-Three theme that helps testing Block Hooks integration. The theme contains:

• The required index.html template.
• The optional single.html template used with tests.
• 3 template parts where two of them reference patterns.
• 3 patterns referenced in the templates and the template parts.

New tests automatically register 4 custom blocks with the test theme where each of them hooks into another block using all four target relative positions: before, after, firstChild, lastChild.

The tests verify that the block gets hooked into the correct positions when targeting:

• template
• template part
• pattern

Props ockham, costdev.
See #59313, #59383.
Follow-up [56610].

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

Performance results from the CI job ​https://github.com/WordPress/wordpress-develop/actions/runs/6405613311/job/17389573828?pr=5399:

The first time, it didn't look so promising. I will re-trigger it a few more times and I will keep updating this comment.

I'm playing with local benchmarking by following instructions from ​https://make.wordpress.org/performance/handbook/measuring-performance/benchmarking-php-performance-with-server-timing/. I created a test post with the content copied from the same page. I also added 5 comments. I'm using this branch which uses Twenty Twenty-Four theme without any changes applied.

$ npm run research -- benchmark-server-timing -u http://localhost:8889/2023/10/05/benchmarking-php-performance-with-server-timing/ -n 100 -p 


╔════════════════════════════════╤═══════════════════════════════════════════════════════════════════════════════════╗
║ URL                            │ http://localhost:8889/2023/10/05/benchmarking-php-performance-with-server-timing/ ║
╟────────────────────────────────┼───────────────────────────────────────────────────────────────────────────────────╢
║ Success Rate                   │ 100%                                                                              ║
╟────────────────────────────────┼───────────────────────────────────────────────────────────────────────────────────╢
║ Response Time (p10)            │ 297.74                                                                            ║
╟────────────────────────────────┼───────────────────────────────────────────────────────────────────────────────────╢
║ Response Time (p25)            │ 305.66                                                                            ║
╟────────────────────────────────┼───────────────────────────────────────────────────────────────────────────────────╢
║ Response Time (p50)            │ 322.18                                                                            ║
╟────────────────────────────────┼───────────────────────────────────────────────────────────────────────────────────╢
║ Response Time (p75)            │ 347.98                                                                            ║
╟────────────────────────────────┼───────────────────────────────────────────────────────────────────────────────────╢
║ Response Time (p90)            │ 365.45                                                                            ║
╟────────────────────────────────┼───────────────────────────────────────────────────────────────────────────────────╢
║ wp-load-alloptions-query (p10) │ 2.35                                                                              ║
╟────────────────────────────────┼───────────────────────────────────────────────────────────────────────────────────╢
║ wp-load-alloptions-query (p25) │ 2.41                                                                              ║
╟────────────────────────────────┼───────────────────────────────────────────────────────────────────────────────────╢
║ wp-load-alloptions-query (p50) │ 2.5                                                                               ║
╟────────────────────────────────┼───────────────────────────────────────────────────────────────────────────────────╢
║ wp-load-alloptions-query (p75) │ 2.7                                                                               ║
╟────────────────────────────────┼───────────────────────────────────────────────────────────────────────────────────╢
║ wp-load-alloptions-query (p90) │ 2.85                                                                              ║
╟────────────────────────────────┼───────────────────────────────────────────────────────────────────────────────────╢
║ wp-before-template (p10)       │ 122.17                                                                            ║
╟────────────────────────────────┼───────────────────────────────────────────────────────────────────────────────────╢
║ wp-before-template (p25)       │ 125                                                                               ║
╟────────────────────────────────┼───────────────────────────────────────────────────────────────────────────────────╢
║ wp-before-template (p50)       │ 132.04                                                                            ║
╟────────────────────────────────┼───────────────────────────────────────────────────────────────────────────────────╢
║ wp-before-template (p75)       │ 142.56                                                                            ║
╟────────────────────────────────┼───────────────────────────────────────────────────────────────────────────────────╢
║ wp-before-template (p90)       │ 150.72                                                                            ║
╟────────────────────────────────┼───────────────────────────────────────────────────────────────────────────────────╢
║ wp-template (p10)              │ 171.11                                                                            ║
╟────────────────────────────────┼───────────────────────────────────────────────────────────────────────────────────╢
║ wp-template (p25)              │ 175.33                                                                            ║
╟────────────────────────────────┼───────────────────────────────────────────────────────────────────────────────────╢
║ wp-template (p50)              │ 184.7                                                                             ║
╟────────────────────────────────┼───────────────────────────────────────────────────────────────────────────────────╢
║ wp-template (p75)              │ 201.2                                                                             ║
╟────────────────────────────────┼───────────────────────────────────────────────────────────────────────────────────╢
║ wp-template (p90)              │ 205.28                                                                            ║
╟────────────────────────────────┼───────────────────────────────────────────────────────────────────────────────────╢
║ wp-total (p10)                 │ 294.5                                                                             ║
╟────────────────────────────────┼───────────────────────────────────────────────────────────────────────────────────╢
║ wp-total (p25)                 │ 301.72                                                                            ║
╟────────────────────────────────┼───────────────────────────────────────────────────────────────────────────────────╢
║ wp-total (p50)                 │ 317.81                                                                            ║
╟────────────────────────────────┼───────────────────────────────────────────────────────────────────────────────────╢
║ wp-total (p75)                 │ 343.66                                                                            ║
╟────────────────────────────────┼───────────────────────────────────────────────────────────────────────────────────╢
║ wp-total (p90)                 │ 357.73                                                                            ║
╚════════════════════════════════╧═══════════════════════════════════════════════════════════════════════════════════╝

I did another test based on the approach used by @felixarntz in ​https://github.com/WordPress/wordpress-develop/pull/5413#issuecomment-1749752437.

TT3 Hello world post (with Like Button plugin active):

222.09ms (PR) vs 227.4ms (trunk)
221.11ms (PR) vs 226.27ms (trunk)
221.57ms (PR) vs 226.69ms (trunk)

@gziolo @ockham Sharing my benchmark summary here.

I used the benchmark-server-timing command, with 100 runs each, and I did that 3 times (i.e. 3 medians, each based on 100 runs). All benchmarks without any plugins active (other than Performance Lab for the Server-Timing headers). Below is a summary of the wp-total metrics my data.

TL;DR Other than #5413 (which was rather neutral), here I can see notable benefits, in some cases _massive_ benefits! 🎉

With TT4 home page:

• 84.97ms (PR) vs 107.8ms (trunk) --> 21.2% faster 🎉
• 84.85ms (PR) vs 110.21ms (trunk) --> 23.0% faster 🎉
• 84.87ms (PR) vs 107.8ms (trunk) --> 21.3% faster 🎉

With TT3 home page:

• 77.87ms (PR) vs 79.2ms (trunk) --> 1.7% faster 👍
• 78.3ms (PR) vs 79.4ms (trunk) --> 1.4% faster 👍
• 78.91ms (PR) vs 78.95ms (trunk) --> 0.1% faster 🆗

Especially for TT4, that looks incredible, and brings it much closer in performance to TT3, despite the so much richer layout and content.

To make sure it doesn't have negative impact on classic themes, I also benchmarked with TT1 - in hopes it's pretty much a zero sum game, which it indeed is.

With TT1 home page:

• 49ms (PR) vs 48.67ms (trunk) --> 0.7% slower 🆗
• 48.23ms (PR) vs 48.45ms (trunk) --> 0.5% faster 🆗
• 48.64ms (PR) vs 48.42ms (trunk) --> 0.5% slower 🆗

The difference here is so tiny and goes into both directions, so we can assume this to be mostly due to variance.

I wanted to see whether more complex block theme content means more benefit, so I also ran a benchmark against the 3P theme "Frost".

With Frost home page:

• 74.21ms (PR) vs 76.1ms (trunk) --> 2.5% faster 👍
• 74.22ms (PR) vs 75.96ms (trunk) --> 2.3% faster 👍
• 74.57ms (PR) vs 75.74ms (trunk) --> 1.5% faster 👍

Interestingly, the benefit here is by far not as high as TT4, but it's still notably positive, close to TT3.

---

I am curious why TT4 sees such a high benefit here. Since I don't have much understanding of the logic behind all this, can you shed some light on this? Do you have any idea? Based on the impact, this looks totally worth prioritizing even this late in the 6.4 cycle, particularly as it could have a major impact on TT4 performance, which would alleviate some of the "problems" related to https://core.trac.wordpress.org/ticket/59465.

[flixos90]

Reopening this due to a seemingly major performance opportunity in ​https://github.com/WordPress/wordpress-develop/pull/5399, particularly for the new default theme TT4. ​See my latest benchmarks.

@gziolo @bernhard-reiter Do you have any idea why this has such a massive impact on TT4 but not as much on the other block themes tested? Of course even for the other block themes there's a notable positive impact, but for TT4 it's the kind of "worth dropping everything else" impact :D

I'm mostly asking for clarification to ensure we know what we're doing here, and to rule out that something random in my setup led to the TT4 performance potentially seeming better than it is.

I'm seeing similarly large performance improvements with this PR, when I run XHProf with this PR against Trunk, when the Twenty Twenty-four (TT4) theme is active.

## TL;DR

On the homepage of TT4, this change reduces the total number of calls to get_hooked_blocks() from 620 to 16, decreasing the inclusive wall time (IWT) from 23,182 µs to 1,293 µs. Representing ~2% improvement in total execution time.

## Trunk

## This PR
_Note: Prior to running profiles, I merged trunk back into the branch to ensure no other difference would affect the measurements._

nice marked improvement. these results measured on a 2.0 GHz EPYC server

I am curious why TT4 sees such a high benefit here. Since I don't have much understanding of the logic behind all this, can you shed some light on this? Do you have any idea? Based on the impact, this looks totally worth prioritizing even this late in the 6.4 cycle, particularly as it could have a major impact on TT4 performance, which would alleviate some of the "problems" related to https://core.trac.wordpress.org/ticket/59465.

I spent some time trying to understand how TT4 got assembled to better understand the results we see. It's a very interesting discovery to see how this theme got structured for the homepage. The template used is a simple one line reference to a pattern shipped in the theme:

​https://github.com/WordPress/wordpress-develop/blob/eb0ae5679a3477c4507b09a283c05b67b87130ab/src/wp-content/themes/twentytwentyfour/templates/home.html#L1

The pattern itself isn't very complex, but it references two template parts and another pattern:

​https://github.com/WordPress/wordpress-develop/blob/eb0ae5679a3477c4507b09a283c05b67b87130ab/src/wp-content/themes/twentytwentyfour/patterns/template-home-business.php#L11-L19

When peeking at the nested pattern, we can see even more patterns used:

​https://github.com/WordPress/wordpress-develop/blob/eb0ae5679a3477c4507b09a283c05b67b87130ab/src/wp-content/themes/twentytwentyfour/patterns/page-01-business-home.php#L13-L18

The footer template part also references a pattern:

​https://github.com/WordPress/wordpress-develop/blob/eb0ae5679a3477c4507b09a283c05b67b87130ab/src/wp-content/themes/twentytwentyfour/parts/footer.html#L1

I didn't dig deeper, but this way, we can see that we have at least the following files referenced that need to get processed when seeking places to inject hooked blocks:

• 1 template
• 2 template parts
• 9 patterns

Thanks a lot @felixarntz and @joemcgill for benchmarking and profiling, and @gziolo for explaining the reasons for the improvements we're seeing!

Looks like we have a winner 😄 -- I've closed my #5406 and #5413 which have done little to nothing to help improve performance.

I'll review this PR now so we can get it merged in time for Beta 3 😊

For context, I worked on a similar ticket that would cache / only parse once the block pattern. See ​https://github.com/WordPress/wordpress-develop/pull/5421. I wonder if there are any lessons left there and we could add some of this to this PR.

For context, I worked on a similar ticket that would cache / only parse once the block pattern. See #5421. I wonder if there are any lessons left there and we could add some of this to this PR.

I think that's a promising optimization! I was thinking to keep the change separate so we can benchmark it to see its impact in an isolated manner, but maybe it's fine to err on the "optimistic" side and include it in this PR. Curious to hear everyone's thoughts!

For context, I worked on a similar ticket that would cache / only parse once the block pattern. See ​https://github.com/WordPress/wordpress-develop/pull/5421. I wonder if there are any lessons left there and we could add some of this to this PR.

My hypothesis (take it with a grain of salt) is that for the front end, it might not have an expected impact, as it looks like there are over 50 patterns in TT4, and we see 16 calls of get_hooked_blocks reported when profiling the homepage. In effect, I read it as only these patterns used in templates get processed, and it doesn't look like they are used more than once. It might be a different story in the admin, though. I would definitely follow up with some further validation of that PR to learn more about how these patterns are consumed on the front end. I might be simply wrong with my working hypothesis. In general, I agree with @spacedmonkey that the whole process of loading theme patterns is a good candidate for further optimization.

For context, I worked on a similar ticket that would cache / only parse once the block pattern. See ​https://github.com/WordPress/wordpress-develop/pull/5421. I wonder if there are any lessons left there and we could add some of this to this PR.

My hypothesis (take it with a grain of salt) is that for the front end, it might not have an expected impact, as it looks like there are over 50 patterns in TT4, and we see 16 calls of get_hooked_blocks reported when profiling the homepage. In effect, I read it as only these patterns used in templates get processed, and it doesn't look like they are used more than once. It might be a different story in the admin, though. I would definitely follow up with some further validation of that PR to learn more about how these patterns are consumed on the front end. I might be simply wrong with my working hypothesis. In general, I agree with @spacedmonkey that the whole process of loading theme patterns is a good candidate for further optimization.

I think that ​https://github.com/WordPress/wordpress-develop/pull/5421 and this PR do very similar things. Instead of parsing the block everytime, it is only done on demand. My PR parses the blocks once and caches the result. This PR only does it if there are block hooks. Very similar.

I think that #5421 and this PR do very similar things. Instead of parsing the block everytime, it is only done on demand. My PR parses the blocks once and caches the result. This PR only does it if there are block hooks. Very similar.

They look rather different to me, TBH 🤔 I'll try to elaborate:

Note that hooked blocks aren’t only inserted into patterns, but also into templates and template parts (where it happens ​upon loading from the respective block theme files — so caching at read time isn’t needed there).

This PR does two things:

• It runs get_hooked_blocks only once per template (or template part, or pattern), rather than for each block encountered. This is likely the biggest factor in reducing overhead when there are hooked blocks.
• It only even applies parsing, hooked block insertion, and re-serializing if there are any hooked blocks at all. This is an optimization on top of the other one for the probably "default" case of no hooked blocks whatsoever.

However, it doesn't cache patterns' markup after hooked blocks have been inserted.

All checks are green. I will wait until tomorrow morning to ensure that the feedback from @spacedmonkey is taken into account. If anyone with the committ access would like to land this patch today after reaching the consensus, I would be more than happy to see that happen. Thank you everyone for guiding me through the performance analysis and your invaluable help in confirming the results initially reported 🙌🏻

TT3 theme - 1000 runs.

| | Trunk | PR |

Response Time (median)	85.46	85.45
wp-load-alloptions-query (median)	0.65	0.65
wp-before-template (median)	35.14	34.83
wp-template (median)	46.88	46.95
wp-total (median)	82.02	81.91

TT4 theme - 1000 runs.
| | Trunk | PR |

Response Time (median)	100.63	95.83
wp-before-template (median)	38.47	37.67
wp-template (median)	58.42	54.4
wp-total (median)	97.15	92.2

I am not seeing much of an improve with TT3, but I am seeing a benefit from TT4.

[joemcgill]

Adding the performance focus, for visibilty.

I'm seeing comments / discussions about function parameter order and function naming. Is there consensus to keep the order and naming in this PR?

I ask because: once it ships in Core, really really hard to make changes due to BC.

I'm seeing comments / discussions about function parameter order and function naming. Is there consensus to keep the order and naming in this PR?

I ask because: once it ships in Core, really really hard to make changes due to BC.

AFAICT, Grzegorz ​implemented my suggestions with regard to the param order. As for the callback naming, neither of us could come up with a better name, so we were thinking ​to leave as-is.

[Bernhard Reiter]

In 56805:

Blocks: Call get_hooked_blocks only once per template/part/pattern.

Prior to this changeset, get_hooked_blocks was called four times for every parsed block in each template, template part, and pattern. With this changeset applied, get_hooked_blocks is called only once per template, template part, or pattern.

Additionally, get_hooked_blocks is called only once when returning the list of all registered patterns. (The latter modification brings the implementation closer to its state prior to Block Hooks.)

Finally, when there are no registered hooked blocks or hooked_block_types filters, parsing, hooked block insertion, and re-serializing is skipped altogether.

Props gziolo, flixos90, joemcgill, dmsnell, spacedmonkey, hellofromtonya.
Fixes #59383.

Thanks again everybody, really great collab on this one!

Committed to Core in https://core.trac.wordpress.org/changeset/56805.

Looks like we introduced a regression: ​https://github.com/WordPress/gutenberg/issues/55202 😕

Work on a fix is underway in ​https://github.com/WordPress/wordpress-develop/pull/5450.

[codente]

I'm reviewing tickets with @webcommsat for dev notes, if there is not already a dev note started for this, could someone use the information in https://core.trac.wordpress.org/ticket/59383#comment:49 to draft one?

We did not see this one on the Performance Dev Notes tracker:
​https://github.com/WordPress/performance/issues/840

The dev note for review and publishing purposes is being tracked at this ​https://github.com/WordPress/Documentation-Issue-Tracker/issues/1185

Thanks for your help.

We have noted that performance have clustered a number of tickets together for the dev notes.
We will cross-reference the issues to reflect this.