WP_Theme::get_post_templates() is extremely inefficient for large themes

[gschoppe]

WP_Theme::get_post_templates() uses file_get_contents() to read the complete contents of each theme php file in the root of a theme into memory, twice, and processes the result with a custom Regular Expression.

As a result, significantly more data than necessary is processed when loading large themes.

In addition, because the standard get_file_data() function is not called (as it is with all other header comments), the "extra_{$context}_headers" filter is never called, which is inconsistent behavior with other file_header related operations, such as the WP_Theme constructor.

[gschoppe]

Patch refactoring WP_Theme::get_post_templates() to use get_file_data

[gschoppe]

To make it easier to review the issue without having to create a server and simulate high load, I've created a pathological theme that, when activated, should demonstrate the increased loading time. The file size was too large to upload directly to this ticket, so I have hosted it on google drive:
​https://drive.google.com/open?id=1t0S_yErQXQJZGlgDBu_Uo9UP3xIJvVJb

The load time should be significant.

[birgire]

@gschoppe Welcome to WordPress trac.

This seems like a good catch.

This will probably need a unit test as well. There's one already for the get_post_templates() method in Tests_Admin_includesTheme::test_get_post_templates_child_theme() so maybe it would cover this as well, instead of a new one?

I think I've spotted another part that could benefit from this, namely the get_file_description() function, that contains:

$template_data = implode( '', file( $file_path ) );

where file() reads the entire file into an array.

The theme editor uses (4.9+) wp_print_theme_file_tree() recursively and therefore we have multiple get_file_description() function calls.

So with your suggestion of using get_file_data() it seems that we could adjust get_file_description() as well to be more efficient. Best create a new ticket for that, if this is the way to go here.

[gschoppe]

In the process of writing unit tests, I discovered that get_file data() seems to not support single line file header declarations, such as

<?php // Template Name: Test ?>

This may be why the code was not previously using get_file_data(). I am now looking into whether it is feasible to add support for single line headers to get_file_data() as this lack of support seems to be an oversight, and this format needs uniform syntax across all use cases.

[gschoppe]

I have opened a related ticket to refactor the regex used to parse file header comments, so that get_file_data will support the single line variant.

Related Ticket: #42517

[swissspidy]

Not too worried about the performance impact here as the result is stored in cache.

Being able to use get_file_data() is blocked by #42517.
Calling file_get_contents() only once would be a trivial change that makes sense to do.

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