Merge consideration of block-templates with classic template hierarchy

[joemcgill]

For block themes, WordPress will first attempt to locate the appropriate PHP template in the theme for the request before considering a block template in get_query_template. However, any block template that is found at an equal or higher priority takes precedence over the PHP templates. This means that block themes perform several unnecessary file_exists checks to look for a PHP template, when a block template is the desired result.

Design considerations

To fix this, the logic from locate_block_template that attempts to find an appropriate block template would need to be integrated into the foreach loop in locate_template so that the appropriate block template would be found prior to searching for a PHP template, in get_query_template if current_theme_supports( 'block-templates' ) returns true. Alternatively, the order of those two functions could be reversed so that locate_block_template is called first if the theme supports block templates.

Benefits

The benefit to this approach is that wasteful file operations would not be called if a more appropriate block template is available, speeding up template rendering. For example, when a block theme responds to a standard single post, locate_template will end up calling file_exists 9 times before even considering whether an appropriate block template exists.

Caveats and risks

While file_exists checks have been shown to have a noticeable performance cost in lab profiling, the results of these calls are often cached internally by PHP (see: ​https://www.php.net/manual/en/function.clearstatcache.php), so the performance impact in the field may be negligible.

Additionally, there are backwards compatibility considerations that will need to be accounted for with this implementation, as plugins are already making use of direct calls to locate_template and locate_block_template independently of how they’re used in get_query_template.

[joemcgill]

Moving this to Future Release, since we're so close to the 6.4 beta release.

[joemcgill]

Moving to 6.5 for consideration. Initial discovery should account for the impact of the internal PHP stat cache on these file operations and the potential back-compat concerns with modifying the loading order of template files, as noted in the ticket description.

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

This PR is under development. For the template we have consider below scenarios:

• Block themes
• Classic themes
• hybrid themes

The $template in locate_block_template is not removed at the moment instead use empty string in get_query_template for BC.

@joemcgill and @felixarntz, the challenge I'm facing is that when I attempt to reproduce the current unit test failure locally using the following methods:

1) Filtering a specific test function. i.e. npm run test:php -- --filter test_switch_theme
2) Filtering the theme class. i.e. npm run test:php -- --filter Tests_Theme
3) Filtering the theme group test. i.e. npm run test:php -- --group themes

I can't seem to reproduce the issue. To reproduce it, I need to run the full unit tests, which is time-consuming. Do you have any suggestions for a quicker way to reproduce the same issue locally?

@mukeshpanchal27 it sounds like the issue you're having indicates a bug with the test suite instead of with your code, since it's only reproducible by running the whole suite. It's likely that some other test is effecting a global variable and not resetting it after the tests are run. Given that 89% of the tests run before the one that is failing, it's hard to know which one is causing the issue. We'll need to find and fix that test, but I wouldn't let it block you until you think you've found an approach that addresses the issue you're fixing.

That said, given the research that @kt-12 did on https://core.trac.wordpress.org/ticket/59314#comment:19, I'm thinking that this exploration is likely to end in a similar conclusion, that avoiding a few unnecessary file lookups do not have a large enough performance impact to risk introducing bugs here. To confirm, could you run some benchmarks of a block theme (e.g., Twenty Twenty-three) in trunk versus when locate_template() is bypassed completely in get_query_template() to see if there is a measurable improvement?

Thank you, @joemcgill. I conducted some benchmarking using the POC code with dummy data for home, page and post pages and have shared the results below. For the full results, you can refer to this ​Google Sheets document.

• Home and page show regression.
• The post page, on the other hand, demonstrates an improvement of approximately 20%.

### TT3 ( Block theme )

|   | Before | After | Diff % | Diff abs.

-- | -- | -- | -- | -- | --
HOME( 10 posts ) | wp-before-template (p50) | 72.04 | 80.62 | 11.91% | 8.58
HOME( 10 posts ) | wp-template (p50) | 56.58 | 64.77 | 14.48% | 8.19
HOME( 10 posts ) | wp-total (p50) | 127.91 | 146.48 | 14.52% | 18.57
PAGE ( /page-image-alignment/ page from dummy data ) | wp-before-template (p50) | 59.42 | 76.82 | 29.28% | 17.40
PAGE ( /page-image-alignment/ page from dummy data ) | wp-template (p50) | 38.14 | 46.76 | 22.60% | 8.62
PAGE ( /page-image-alignment/ page from dummy data ) | wp-total (p50) | 99.24 | 126.8 | 27.77% | 27.56
POST( block-image post page from dummy data ) | wp-before-template (p50) | 78.57 | 56.49 | -28.10% | -22.08
POST( block-image post page from dummy data ) | wp-template (p50) | 63.43 | 44.61 | -29.67% | -18.82
POST( block-image post page from dummy data ) | wp-total (p50) | 144.26 | 101.88 | -29.38% | -42.38

### TT1 ( Classic theme )

|   | Before | After | Diff % | Diff abs.

-- | -- | -- | -- | -- | --
HOME( 10 posts ) | wp-before-template (p50) | 44.97 | 44.22 | -1.67% | -0.75
HOME( 10 posts ) | wp-template (p50) | 52.84 | 59.37 | 12.36% | 6.53
HOME( 10 posts ) | wp-total (p50) | 99.05 | 104.29 | 5.29% | 5.24
PAGE ( /page-image-alignment/ page from dummy data ) | wp-before-template (p50) | 40.31 | 57.36 | 42.30% | 17.05
PAGE ( /page-image-alignment/ page from dummy data ) | wp-template (p50) | 34.78 | 49.42 | 42.09% | 14.64
PAGE ( /page-image-alignment/ page from dummy data ) | wp-total (p50) | 75.92 | 105.19 | 38.55% | 29.27
POST( block-image post page from dummy data ) | wp-before-template (p50) | 53.42 | 39.42 | -26.21% | -14.00
POST( block-image post page from dummy data ) | wp-template (p50) | 44.71 | 35.81 | -19.91% | -8.90
POST( block-image post page from dummy data ) | wp-total (p50) | 97.58 | 75.94 | -22.18% | -21.64

Mean time i'm going to check the regression for home and pages.

@joemcgill I did the performance benchmark for the function.

The current version of get_query_template from trunk and my modified version have been compared using benchmarks. To eliminate variations caused by server resource allocations as much as feasible, both versions of each benchmark are run on the same computer from within the same process, with only the relative change being recorded. Here is result of 10 run.

Average time spent for 10 x 18 iterations.

### TT3 ( Block theme )

get_query_template() - 0.04080924987793 new_get_query_template() - 0.039115571975708 Change: -4.2%
get_query_template() - 0.036437177658081 new_get_query_template() - 0.033733105659485 Change: -7.4%
get_query_template() - 0.03502893447876 new_get_query_template() - 0.033513689041138 Change: -4.3%
get_query_template() - 0.035555481910706 new_get_query_template() - 0.033641672134399 Change: -5.4%
get_query_template() - 0.036332750320435 new_get_query_template() - 0.034693217277527 Change: -4.5%
get_query_template() - 0.035776114463806 new_get_query_template() - 0.034015727043152 Change: -4.9%
get_query_template() - 0.035825991630554 new_get_query_template() - 0.03460590839386 Change: -3.4%
get_query_template() - 0.036041331291199 new_get_query_template() - 0.033895254135132 Change: -6%
get_query_template() - 0.035417985916138 new_get_query_template() - 0.033571100234985 Change: -5.2%
get_query_template() - 0.03657169342041 new_get_query_template() - 0.034809374809265 Change: -4.8%

### TT1 ( Classic theme )

get_query_template() - 0.001415491104126 new_get_query_template() - 0.0013654470443726 Change: -3.5%
get_query_template() - 0.0014647960662842 new_get_query_template() - 0.0013648748397827 Change: -6.8%
get_query_template() - 0.0014342069625854 new_get_query_template() - 0.0013604879379272 Change: -5.1%
get_query_template() - 0.0014017581939697 new_get_query_template() - 0.0013597965240479 Change: -3%
get_query_template() - 0.0014153242111206 new_get_query_template() - 0.0013453006744385 Change: -4.9%
get_query_template() - 0.0014492034912109 new_get_query_template() - 0.0013694286346436 Change: -5.5%
get_query_template() - 0.0013886690139771 new_get_query_template() - 0.0013503074645996 Change: -2.8%
get_query_template() - 0.0015052080154419 new_get_query_template() - 0.0014845132827759 Change: -1.4%
get_query_template() - 0.0013949394226074 new_get_query_template() - 0.0013380289077759 Change: -4.1%
get_query_template() - 0.0014493227005005 new_get_query_template() - 0.0014261722564697 Change: -1.6%

The benchmark summary states that this improvement should speed up the get_query_template function by ~3% under typical use conditions. The test script I've used to run the benchmarks can be found here: ​https://gist.github.com/mukeshpanchal27/8a087f67a90e0526c703ec8e26514e0a

[mukesh27]

Based on the comment above, it seems that the changes made offer only a minimal performance gain, but they alter get_query_template() in a way that could potentially lead to issues in the future. This suggests that the trade-off between performance and potential future problems should be carefully considered when evaluating these changes.

Thanks for conducting some profiling for this, @mukeshpanchal27. For the first set of benchmarks you conducted (​link), it looks like the code in this PR led to an _increase_ in response time. Is that correct, or am I misunderstanding the data? If so, that would be a very surprising conclusion given that this is intended to avoid calling unnecessary code.

For the second set of data, it does look like the modified function improves processing time, but at such a small scale that is likely not worth pursuing. Looking an your testing process, I do think it would be helpful to separate both cases into separate CLI runs so that each iteration is run with a clean opcode cache.

I did a quick profile run with XHProf on the new Twenty Twenty-four theme, to see how much time the traditional locate_template() function was taking relative to the whole function and it is in line with the 3% improvement that your benchmarks show, but given that it's is a %3 improvement on a function that is only 4% of the overall server response time, we're talking about a micro-optimization at best, so I think we should likely close this as a wontfix, if you agree.

Thanks for conducting some profiling for this, @mukeshpanchal27. For the first set of benchmarks you conducted (​link), it looks like the code in this PR led to an _increase_ in response time. Is that correct, or am I misunderstanding the data? If so, that would be a very surprising conclusion given that this is intended to avoid calling unnecessary code.

For the second set of data, it does look like the modified function improves processing time, but at such a small scale that is likely not worth pursuing. Looking an your testing process, I do think it would be helpful to separate both cases into separate CLI runs so that each iteration is run with a clean opcode cache.

I did a quick profile run with XHProf on the new Twenty Twenty-four theme, to see how much time the traditional locate_template() function was taking relative to the whole function and it is in line with the 3% improvement that your benchmarks show, but given that it's is a %3 improvement on a function that is only 4% of the overall server response time, we're talking about a micro-optimization at best, so I think we should likely close this as a wontfix, if you agree.

For the first benchmark, I initially used live site data (InstaWP), but it didn't provide as consistent results as the local benchmark. Therefore, I reverted to the old method for benchmarking the function.

I agree with you, @joemcgill, to close this issue with a wontfix resolution.

[joemcgill]

Based on more exploration, the overall cost of file lookups for PHP template files are minimal and not worth trying to refactor. Closing this as a wontfix.

Closed as wontfix