Cache parse of block in block patterns #5421
Conversation
|
In my testing #5399, I also extracted the logic for updating the |
|
This is a promising approach 🤔 Maybe I closed the ticket too early 😅 (although arguably, this PR does something different than what the ticket says, so it might be worth filing a new one). Let's maybe run some numbers on this one, and rebase it once @gziolo's #5399 lands (in order to use the extracted parsing and hooked blocks insertion logic). |
| return array_values( $patterns ); | ||
| } | ||
|
|
||
| protected function get_parse_blocks( $pattern ) { |
There was a problem hiding this comment.
Nitpick, we might want a different name for the method, as it not only parses the content but also (conditionally) inserts hooked blocks, and returns re-serialized markup.
(Per #5421 (comment), we might just use pepare_content from #5399 as name.)
|
@spacedmonkey Now that #5399 has been committed, there is a merge conflict here. Could you please address it? Curious to run benchmarks on this once it's up to date with latest |
I will provide benchmarks tomorrow. |
|
So after some testing, it seems like there is little performance benefit from this change now. All the performance benefit seems to have been committed in #5399. This PR might be useful for times where there the same pattern is used multiple times on a page. Not in all the example themes I found, I could not find an example where it did this. I am also worried that the filters would not run if this result was cached. I still think there is something here, but it needs more work and is unlikely to make it's way into core in this release. Closing for now. |
|
I see that the ticket got reopened. It would be great to run the benchmarks again with the latest |
Trac ticket: https://core.trac.wordpress.org/ticket/59541
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.