Blocks: Remove inject_hooked_block_markup filter
#5292
Conversation
Per discussion in #59424, there's agreement that the new `hooked_block_types` filter (introduced in [56673]) covers conditional addition and removal of hooked blocks better and at a higher level than the `inject_hooked_block_markup` filter that was originally added in [56649] for that same purpose. Consequently, this changeset removes the latter filter. Props gziolo. Fixes #59439.
|
@felixarntz, I'm wondering how much we can trust this performance job: 
We don't have any hooked blocks in WordPress core, so there should be no difference between this branch and |
ockham
force-pushed
the
remove/inject-hooked-block-markup-filter
branch
from
9ffa37a to
9e53373
Compare
|
Committed to Core in https://core.trac.wordpress.org/changeset/56674. |
|
@gziolo It looks strange that this appears to have such an impact in classic themes indeed (see https://github.com/WordPress/wordpress-develop/actions/runs/6297076920). Though if this function is called very often on page load, it could have an actual impact to just remove the filter appliance - though I can't imagine that is the case, as it would need to be extremely often 😆 We are aware of sometimes notable variance in the performance test results, but generally Server-Timing has been much more stable than the Web Vitals, so this difference is confusing to see. @swissspidy Do you have any idea? |
|
I'm sure y'all have seen the In the meantime, best to compare the numbers with other runs on PR or trunk to get a better feeling for the variance. |
|
I just ran a benchmark for this locally using TT1, and it gave me:
That difference is small enough to be safely discarded as variance as there is also absolutely no way this could be slower as it's only removing a filter execution. Still, seeing this kind of huge difference in the CI workflow makes me doubt its reliability at the moment. While a centrally visible and reusable workflow is certainly the goal, there may unfortunately still be value in running benchmarks individually on a personal machine as it seems the environment in GitHub Actions is far less controlled than a regular machine 😞 |
|
It's known that GitHub Actions runners don't provide the most consistent environment. Results can change even depending on the time of day (and thus load). But that's all solvable to some degree :-) |
Per discussion in #5271, there's agreement that the new
hooked_block_typesfilter covers conditional addition and removal of hooked blocks better and at a higher level than theinject_hooked_block_markupfilter that was originally added in #5261 for that same purpose, and that the latter should thus be removed.Trac ticket: https://core.trac.wordpress.org/ticket/59439
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.