Make WordPress Core

Opened 3 months ago

Closed 2 months ago

Last modified 8 weeks ago

#59383 closed enhancement (fixed)

Blocks: Introduce helper function to retrieve hooked blocks

Reported by: gziolo's profile gziolo Owned by: gziolo's profile gziolo
Milestone: 6.4 Priority: normal
Severity: normal Version: 6.4
Component: Themes Keywords: has-patch has-unit-tests needs-dev-note gutenberg-merge
Focuses: performance Cc:

Description

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).

Attachments (2)

Screenshot 2023-09-25 at 11.42.43.png (571.9 KB) - added by spacedmonkey 2 months ago.
Screenshot 2023-09-25 at 11.41.16.png (419.4 KB) - added by spacedmonkey 2 months ago.

Download all attachments as: .zip

Change History (54)

This ticket was mentioned in PR #5239 on WordPress/wordpress-develop by @gziolo.


3 months ago
#1

  • Keywords has-patch has-unit-tests added

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.

#2 @gziolo
3 months ago

  • Owner set to gziolo

#4 @gziolo
3 months ago

  • Resolution set to fixed
  • Status changed from assigned to closed

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.

#6 @spacedmonkey
2 months ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

#8 @spacedmonkey
2 months ago

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.

@spacedmonkey commented on PR #5293:


2 months ago
#9

Performance issue.

### Before
https://i0.wp.com/github.com/WordPress/wordpress-develop/assets/237508/8ce978c1-b982-4489-98ef-424542b1a22a

### After
https://i0.wp.com/github.com/WordPress/wordpress-develop/assets/237508/ef227d89-79d3-4d9a-83b6-100f0891fa4a

#10 @gziolo
2 months ago

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.

I think we don't see the impact in WordPress core as there are no hooked blocks. It's the plugin's territory, so we would have to simulate that by registering a few blocks that pass block_hooks during the register_block_type call.

Last edited 2 months ago by gziolo (previous) (diff)

#11 @spacedmonkey
2 months ago

@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.

This ticket was mentioned in Slack in #core-performance by gziolo. View the logs.


2 months ago

#14 @spacedmonkey
2 months ago

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.

#16 @hellofromTonya
2 months ago

  • Keywords 2nd-opinion needs-dev-note added

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?

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.

This ticket was mentioned in Slack in #core by oglekler. View the logs.


2 months ago

#18 @gziolo
2 months ago

  • Keywords gutenberg-merge added

#19 follow-up: @Bernhard Reiter
2 months ago

@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 :)

This ticket was mentioned in PR #5297 on WordPress/wordpress-develop by @Bernhard Reiter.


2 months ago
#20

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

#21 @Bernhard Reiter
2 months ago

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.

#23 in reply to: ↑ 19 @hellofromTonya
2 months ago

  • Keywords 2nd-opinion removed

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.

#24 @hellofromTonya
2 months ago

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

#25 @spacedmonkey
2 months ago

  • Resolution set to fixed
  • Status changed from reopened to closed

#26 @gziolo
2 months ago

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].

@gziolo commented on PR #5399:


2 months ago
#28

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

https://i0.wp.com/github.com/WordPress/wordpress-develop/assets/699132/17631a05-154d-4e60-bb5d-b2af2e40fe88

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.

@gziolo commented on PR #5399:


2 months ago
#29

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                                                                            ║
╚════════════════════════════════╧═══════════════════════════════════════════════════════════════════════════════════╝

@gziolo commented on PR #5399:


2 months ago
#30

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)

@flixos90 commented on PR #5399:


2 months ago
#31

@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.

#32 @flixos90
2 months ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

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.

joemcgill commented on PR #5399:


2 months ago
#33

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

https://i0.wp.com/github.com/WordPress/wordpress-develop/assets/801097/b1726416-1480-47ff-9ddd-5fadf6eba2f3

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

https://i0.wp.com/github.com/WordPress/wordpress-develop/assets/801097/a20ff393-9034-401a-9805-d177ebaa7879

@dmsnell commented on PR #5399:


2 months ago
#34

https://i0.wp.com/github.com/WordPress/wordpress-develop/assets/5431237/2366c597-bd49-445c-bddf-79818f635778

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

@gziolo commented on PR #5399:


2 months ago
#35

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

@Bernhard Reiter commented on PR #5399:


2 months ago
#36

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 😊

@spacedmonkey commented on PR #5399:


2 months ago
#37

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.

@Bernhard Reiter commented on PR #5399:


2 months ago
#38

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!

@gziolo commented on PR #5399:


2 months ago
#39

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.

@gziolo commented on PR #5399:


2 months ago
#40

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.

@spacedmonkey commented on PR #5399:


2 months ago
#41

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.

@Bernhard Reiter commented on PR #5399:


2 months ago
#42

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.

@gziolo commented on PR #5399:


2 months ago
#43

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 🙌🏻

@spacedmonkey commented on PR #5399:


2 months ago
#44

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.

This ticket was mentioned in Slack in #core-performance by flixos90. View the logs.


2 months ago

#46 @joemcgill
2 months ago

  • Focuses performance added

Adding the performance focus, for visibilty.

@hellofromTonya commented on PR #5399:


2 months ago
#47

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.

@Bernhard Reiter commented on PR #5399:


2 months ago
#48

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.

#49 @Bernhard Reiter
2 months ago

  • Resolution set to fixed
  • Status changed from reopened to closed

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.

@Bernhard Reiter commented on PR #5399:


2 months ago
#50

Thanks again everybody, really great collab on this one!

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

#52 @codente
8 weeks ago

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.

Note: See TracTickets for help on using tickets.