Block themes: Use a cache for block template files

[flixos90]

Updated Feb 28 by @joemcgill

On every page load for a block theme, several block templates, template parts, and patterns are used, each of which results in a file_get_contents() operation to get the block contents before parsing and rendering the blocks included in each. Getting these file contents is an expensive operation, resulting in a performance bottleneck which gets greater the more block template parts are being used in a theme.

Since these files are unlikely to change often in production, we should attempt to avoid these redundant operations by the use of the caching strategy outlined in #59719.

Solutions need to respect WP_DEVELOPMENT_MODE settings, so that persistent caching does not happen when developing a theme, and should also be forgiving for use cases where theme files are modified directly in production through various methods.

Original issue description

While reviewing the performance impact of #59583, I noticed that a large chunk of the cost of parsing block templates and block template parts comes from simply the file_get_contents() calls for those files.

On every page load for a block theme, several block templates and template parts are used, each of which results in a file_get_contents() operation. Getting these file contents is an expensive operation, that results in a performance bottleneck which gets greater the more block template parts are being used in a theme.

I have prepared a draft PR with a solution for this and based on Server-Timing benchmarks, it notably improves the overall response time by a solid 5-10% for various block themes.

Given this is a major performance win and relies on a pattern already established via #59490 / [56765], I think it is worth prioritizing even this late in the release cycle.

[flixos90]

See PR ​https://github.com/WordPress/wordpress-develop/pull/5463 for the proposed solution and benchmarks showing its impact.

[flixos90]

​https://github.com/WordPress/wordpress-develop/pull/5463 now has unit tests added, to ensure the cache is relied upon and set correctly, but only when it should.

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

@spacedmonkey This approach caches more data than my PR #5463, but the concerns that you shared in ​https://github.com/WordPress/wordpress-develop/pull/5463#pullrequestreview-1673559100 apply to this PR as well, as far as I can tell.

Can you please clarify what the benefits of this PR are over #5463?

[flixos90]

Per the PR conversations, punting this to 6.5 to give ourselves a bit more time to consider different approaches.

[flixos90]

Marking this as a high priority due to having significant performance impact with the additional caching.

Note that this is blocked by making a decision on #59719 first.

[flixos90]

Reassigning in relation to https://core.trac.wordpress.org/ticket/59719#comment:13. Let's figure out the direction there first, and then we can work on this ticket as well as #60120. Maybe best to do #60120 first as it would be about migrating existing functionality to the new approach, i.e. slightly more urgent than this one here, which is a new enhancement.

[joemcgill]

Moving this to 6.6 so we can land caching for this and pattern files early in the release cycle and not just before RC. I'm updating the issue title to also cover persistent caching of block patterns, which was discussed in 60120, but not implemented in that issue.

@thekt12 already has ​an approach in progress for caching patterns, which is currently related to #59719, but should be updated to point to this issue. That way we can close out #59719, since it's purpose was to agree on a consistent strategy for caching to block theme files, which was documented in this comment.

Further implementation details can be discussed related to the PRs for this issue.

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

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

Oh, this is interesting. I'm going to think on it a bit.

My knee-jerk reaction is that making such a big change the the main wp_cache_ functions, is likely not the right approach, but there is something kind of clever about trying to make it easier to optionally back caches in non-persistent environments with transients without a lot of repeated code.

I wonder if it would be easier to handle from the opposite perspective, given that transients already get saved to external object cache when available? I think the main things that are missing for transients are the ability to set a cache group besides 'transient' which is where they all get stored now, and a way to ensure transients that are stored in the DB options table get flushed whenever the cache group they belong to gets flushed.

The reason I was inclined to the changes here is :

• Consistency - mainly in naming convention. If we have a consistent name, in future it will be easy to implement flushing.
• Secondly, we will have too many filters to manage and a lot of repetitive code.

I was analysing both the functions wp_cache_ side functions and transient side functions.
The way I have written this code is transient cache is conditionally enabled only when $transient_cache flag is set and object_cache is not enabled. So older code will work the same way. This shift of logic applies only to places where we want that logical check, so it's easy to ensure BC.

The problem I felt with making changes to transient side functions is that;

• We will have to be extra careful while adding support for groups in caching code as there might be some sites reliant on object cache. Ensuring BC might be a bit more complex here as compared to wp_cache base approach.
• Handling the expiry situation might become complex (I cannot say for sure though) as we will have to add a bunch of code to differentiate between regular transient and transient for caching.
• We will have to replace wp_cache_ at all the places with transient functions, which might get confusing. Also, key name logic will be bit more complex inside transient functions.

Lastly, I felt it was more of a caching problem than a transient problem (even though they mean the same), so I added my code to wp_cache_.

Thanks for explaining your thinking in a bit more detail. Still, I don't think caching block theme files warrants a major modification of the Cache API to make use of Transients as a first class way to provide persistence in non-persistent environments. I think doing so confuses the responsibilities of these two APIs somewhat. For context, here's how I would describe the purpose of both APIs:

​Cache API:

• Primarily stores computed data in memory during a single request so it can be reused without being recomputed in the same request
• Is non-persistent by default, but can optionally can be made persistent with the use of an external object cache

​Transients API:

• Primarily provides a standard way to persist some cached data between requests.
• Enhances the Options API with a way to allow cached data saved to options to expire to avoid DB bloat.
• In environments that support persistent object caches, transients will be offloaded to the cache system and not be in the DB at all.

Realistically, for sites that want cached data to persist, the solution is to install a persistent object cache. Backing these caches with transients is usually redundant and unnecessary. For data that should persist, regardless of an external object cache, the data should be stored in a transient, which will get offloaded to an external object cache if available. For block template files, if we want to ensure persistence, we could simply prioritize the use of transients. However, since we want to provide a way for environments with persistent caches to flush these caches using cache groups, we will need to manually check for whether wp_using_ext_object_cache() returns true, and if so, use the Cache API with the theme_files group.

@kt-12 @joemcgill I'm not sure https://core.trac.wordpress.org/ticket/59600 is the correct ticket this PR is addressing. That ticket is about block template files, while this PR addresses block patterns. I thought there was a dedicated ticket for that one too. If not, let's create one and point this PR to that one.

Caching block template files, while very similar in the approach, is a separate problem.

@felixarntz

I'm not sure https://core.trac.wordpress.org/ticket/59600 is the correct ticket this PR is addressing

The tickets related to theme_file caching began to get very hard to follow during the end of the last release, so I've updated the issue to consolidate all template file caching (including patterns) into the same ticket. See this comment.

@joemcgill

I'm not sure https://core.trac.wordpress.org/ticket/59600 is the correct ticket this PR is addressing

The tickets related to theme_file caching began to get very hard to follow during the end of the last release, so I've updated the issue to consolidate all template file caching (including patterns) into the same ticket. See this comment.

Thanks for clarifying. However, this PR still doesn't solve the original purpose of that ticket, it only addresses part of it (block patterns). So I don't think that ticket can be closed from only caching block patterns when it was originally about block templates. I'm only saying that because typically commits close tickets. I'm happy to keep this association as long as we're aware committing this will not _fix_ the ticket.

Trac ticket:

This was found while analysing https://core.trac.wordpress.org/ticket/59600

The execution time of the wp_get_theme_data_custom_templates function on the home page of TT4 was evaluated.
{{{PHP
function wp_get_theme_data_custom_templates() {

Capture the start time
$startTime = microtime(true);
$data = WP_Theme_JSON_Resolver::get_theme_data( array(), array( 'with_supports' => false ) )->get_custom_templates();
Capture the end time
$endTime = microtime(true);

Calculate the script execution time
$executionTime = $endTime - $startTime;
echo "<pre>Execution Time of wp_get_theme_data_custom_templates: " . $executionTime . " seconds.</pre>";

return $data;

}
}}}`

The performance improvement was as noted below:
Before PR

Description	Execution Time (seconds)
Run 1
Execution Time of wp_get_theme_data_custom_templates	0.0033679008483887
Execution Time of wp_get_theme_data_custom_templates	0.00087404251098633
Run 2
Execution Time of wp_get_theme_data_custom_templates	0.003180980682373
Execution Time of wp_get_theme_data_custom_templates	0.00077509880065918
Run 3
Execution Time of wp_get_theme_data_custom_templates	0.0039951801300049
Execution Time of wp_get_theme_data_custom_templates	0.00078392028808594
Run 4
Execution Time of wp_get_theme_data_custom_templates	0.0032379627227783
Execution Time of wp_get_theme_data_custom_templates	0.00077700614929199
Run 5
Execution Time of wp_get_theme_data_custom_templates	0.003154993057251
Execution Time of wp_get_theme_data_custom_templates	0.00075793266296387
Run 6
Execution Time of wp_get_theme_data_custom_templates	0.0032570362091064
Execution Time of wp_get_theme_data_custom_templates	0.00075292587280273

Mean of first call: 0.003366
Mean of static call: 0.000787

After PR

Description	Execution Time (seconds)
Run 1
Execution Time of wp_get_theme_data_custom_templates	0.0030369758605957
Execution Time of wp_get_theme_data_custom_templates	0.0004730224609375
Run 2
Execution Time of wp_get_theme_data_custom_templates	0.0031139850616455
Execution Time of wp_get_theme_data_custom_templates	0.00047683715820312
Run 3
Execution Time of wp_get_theme_data_custom_templates	0.002918004989624
Execution Time of wp_get_theme_data_custom_templates	0.00047898292541504
Run 4
Execution Time of wp_get_theme_data_custom_templates	0.0026509761810303
Execution Time of wp_get_theme_data_custom_templates	0.00044488906860352
Run 5
Execution Time of wp_get_theme_data_custom_templates	0.0027339458465576
Execution Time of wp_get_theme_data_custom_templates	0.00045418739318848
Run 6
Execution Time of wp_get_theme_data_custom_templates	0.0029489994049072
Execution Time of wp_get_theme_data_custom_templates	0.00049400329589844

Mean of first call: 0.002900
Mean of static call: 0.000470

@joemcgill Regarding cache invalidation, I think we had discussed those points already on the ticket. Trying to summarize the conclusion based on my understanding:

• For users that directly edit theme files (without bumping the version number):
  • If they edit using the WordPress built-in theme file editor, cache invalidation is automatically taken care of by WordPress.
  • If they edit elsewhere and upload or deploy somehow, they have three options:
    • Either use WP_DEVELOPMENT_MODE (e.g. set to "theme") to disable this caching entirely, permanently or temporarily.
    • Or, wait for the cache expiration (by default 30 minutes) for changes to take effect.
    • Or, if they want to have the changes reflected immediately, flush the respective cache (when using a persistent cache), or transients (when using a non-persistent cache).
      • Several plugins have a "Flush cache" button to do the first. For more fine grained control, they could only flush the theme_files group, which may be better for high-traffic sites, to not flush all other data too.
      • For flushing transients, I recently published ​https://wordpress.org/plugins/flush-transients/, which is essentially a "Flush cache" button, but for transients.
• It should be fair to assume that at scale, relatively speaking few users edit theme files directly. Those that do are often maintained by developers that often are aware of cache flushing. Though, even for when they're not, we're catering for that by using a short cache expiration time by default.

I believe that with all of these considerations we are catering for the majority of sites, while providing reasonable options for other sites. WordPress doesn't provide any opt-out filters for caching to my knowledge, and I'm not convinced we need to introduce one here. Last but not least, we're trying to optimize performance out of the box, even for sites without a persistent object cache, to make those benefits available to a majority of sites.

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

Committed in https://core.trac.wordpress.org/changeset/58025.

[joemcgill]

Commit 58025 did not properly sync to this Trac issue, but addresses adding persistence for Block Theme patterns.

This ticket should remain open until we've addressed Block Templates and Block Template Parts as well.

Thanks @kt-12. When running some profiling data on this approach, I'm not seeing much benefit to adding a cache at this point in the execution process.

Comparing runs of _build_block_template_result_from_file() with and without this cache in place, there is a marginal improvement and seems like the need to query the DB for these transients offsets any benefit in avoiding the file_get_contents() calls. In fact, this function is only responsible for ~1% of the overall execution time of the TT4 homepage without additional caching, so I'm not sure there is a benefit in adding caching here.

## _build_block_template_result_from_file()
Trunk

Current PR

## get_block_templates()
However, its parent function, get_block_templates(), does seem to be present a bigger opportunity for improvement, as it's profiling at 5% of iwt for the same requests. Of that time, most of the cost seems to from _get_block_templates_files(), with most of that processing eventually happening in wp_get_theme_data_custom_templates() when the WP_Theme_JSON_Resolver::get_theme_data() parses the customTemplates property from the WP_Theme_JSON data provided by the theme.

I think we could consider adding a cache to either _get_block_templates_files() or wp_get_theme_data_custom_templates() to have bigger impact—at least until we are able to handle some/all of https://core.trac.wordpress.org/ticket/57789.

I was investigating the performance opportunity get_block_templates() - Down the line, this function is calling WP_Theme_JSON_Resolver::get_theme_data which is the major reason for the performance.
At the moment, get_theme_data is static cached so, even if we persistent cache get_block_templates we won't get any benefit as get_theme_data is referred at two more places and the next function calling it will take the hit.

[thekt12]

@joemcgill I was investigating the performance opportunity in caching get_block_templates().

Down the line, this function WP_Theme_JSON_Resolver::get_theme_data is the major reason for the performance regression.

At the moment, results from get_theme_data is static cached. So even if we persistent cache result from get_block_templates we won't get any benefit as get_theme_data is called inside two other functions and the next function calling it will take the hit.

We could try to cache the get_theme_data output persistently instead with a lower expiry time. So far I feel it is possible but I am researching this as a part of Trac#57789

Another place where I found performance regression is inside render_block_core_template_part which is responsible for ~20% of load time (~110 ms). This function is one of the parents calling _get_block_template_file. But render_block_core_template_part is lot more dynamic and should be explored separately.

[joemcgill]

I'm reassigning to @thekt12 who is currently working on this ticket for WP 6.6.

@joemcgill With the latest changes we are getting a consistent 2.5% improvement without breaking anything.

Performance Metric	Trunk	PR	Improvement (%)
Response Time (median)	387.75	379.09	2.23%
wp-before-template (median)	151.2	145.18	3.98%
wp-before-template-db-queries (median)	0	0	0

Oh, by the way. It'd be nice if this change would have been done first in the Gutenberg codebase. The value that provides is immense to all of us, including easier synchronization and quick feedback from Gutenberg users if we've missed something. I have nothing against merging it in wordpress-develop first, though I'd kindly ask that the authors port the same PR to Gutenberg. Until we figure out something better, this is what we collectively have and need to care for.

Oh, by the way. It'd be nice if this change would have been done first in the Gutenberg codebase. The value that provides is immense to all of us, including easier synchronization and quick feedback from Gutenberg users if we've missed something. I have nothing against merging it in wordpress-develop first, though I'd kindly ask that the authors port the same PR to Gutenberg. Until we figure out something better, this is what we collectively have and need to care for.

Thank you @oandregal for your feedback. We clearly understand your feedback about Gutenberg first approach, hence we have already created a PR for Gutenberg (link in description of this PR). Once it's tested and approved on Gutenberg we can then plan to merge it for the core.

​https://github.com/WordPress/gutenberg/pull/61262

Thank you so much 🙏 I approved the Gutenberg PR.

By reviewing the Gutenberg PR I noticed that the core code is missing a check: see ​https://github.com/WordPress/wordpress-develop/pull/6271#discussion_r1589940324 and ​https://github.com/WordPress/gutenberg/pull/61262/files#r1589940214. I don't know why they have differed but that check is important and we should consolidate both codebases.

Merged in https://core.trac.wordpress.org/changeset/58185.

[ivailohristov]

It seems like the main performance benefit here comes from having the theme content grouped into a single object. I did a quick and dirty test of the same logic but instead of using transients in the DB the object is now saved as a simple file in the system's temp directory. In my tests this performs even better than the current transient based implementation. Do you think it is worth considering a file based implementation for this instead of putting further stress to the DB?

Please take a look here: ​https://github.com/felixarntz/wordpress-develop/compare/performance/theme-template-file-contents...ivailohristov:wordpress-develop:performance/theme-template-file-contents-cache

Furthermore this implementation gets rid of the need for timeouts and development mode. Theme changes are reflected immediately.

Here is how the averages for loading template paths look on my end (over 100 requests) when using transients vs file cache:

transients: 0.00062347412109375s
file cache: 0.00031973838806152s

[joemcgill]

As we're closing in on what can be done in this ticket for the 6.6 release, I'm reassigning to myself to close out any open tasks from this tracking issue and summarize what was completed for 6.6 and what remains.

[oglekler]

@joemcgill is there something that can be done in this milestone, or we should move the whole ticket to the next milestone? I don't see any commits 🤷

[joemcgill]

Replying to oglekler:

@joemcgill is there something that can be done in this milestone, or we should move the whole ticket to the next milestone? I don't see any commits 🤷

[58025] was committed during this milestone but didn't properly sync to this Trac ticket (see comment:32).

The need to add caching to templates and template parts showed less performance gains once some of the improvements to WP_Theme_JSON system were made (see #61112, #59595).

@ivailohristov Thanks for sharing your observations about the use of a file cache. We've generally avoided adding file caches to Core, since many hosting systems do not allow file writes, but it's something that could be implemented in a plugin.

Moving this to the next milestone for follow-up.

[ivailohristov]

@ivailohristov Thanks for sharing your observations about the use of a file cache. We've generally avoided adding file caches to Core, since many hosting systems do not allow file writes, but it's something that could be implemented in a plugin.

@joemcgill Makes sense. Are there cases where the system's temp directory is not writable? The proposed change uses the temp dir for storing the cache.

[joemcgill]

@ivailohristov I'm not an expert on all hosting setups, but I suspect that some do not allow writing and/or reading from the temp directory by the www user. However, another complication would be for hosting that scale their application servers horizontally, where these file caches would not persist consistently across the platform.

If we were to add a file cache to core, we would likely want to put it in the uploads directory, which often gets offloaded to a separate shared file system (e.g., S3, etc) in these scenarios. However, doing so is out of scope for this specific ticket. In addition, this has generally been considered best left to plugins to handle, rather than doing so in Core directly. See: #17055.

[joemcgill]

Moving this off of the 6.7 milestone as no additional work is planned this release.