Now that WP_Query is cached, just load all templates.

[spacedmonkey]

What?

See #45309

Now that WP_Query is cached WordPress/wordpress-develop@7f7d616, we should just load all template parts and use php to filter.

Before

After

Why?

How?

Testing Instructions

Testing Instructions for Keyboard

Screenshots or screencast

[felixarntz]

@spacedmonkey Can you explain what the benefits of this would be?

Without additional context, just from looking at this I have some concerns:

• I don't see the benefits as it is still one query, like before.
• The query however now queries all template parts instead of just one.
• The unbound query with posts_per_page = -1 has me concerned about potential problems.
• The extra logic to go through all found posts also seems unnecessary, the previous logic of querying just for the one post is more efficient.

So I would like to understand better why we should be doing this.

[peterwilsoncc]

I'm guessing the intended benefit here is for pages with multiple template parts, for example a header and footer.

On the first call both template-parts would be queried from the database (presuming no persistent object cache) but on the second call there will not be a database query as the result is cached.

I guess the concern is for sites that do not have a persistent cache and use different template-parts in a variety of templates (for example a different header on the home and internal pages). With this change, there would be more data returned on each page load.

Pros

• All templates parts stored in memory for sites with a persistent cache upon the first function call
• One uncached wp_query call per page for sites using multiple template parts per page

Cons

• Unused data queried on sites with a variety of template parts without a persistent cache.
• One sites with fewer than four templates on a page wp#57416 could be more efficient

[felixarntz]

Thanks @peterwilsoncc, makes sense.

I like the idea behind it, but I'm also concerned about a few things: As you're saying, depending on how sites use block template parts, this PR may make performance better or worse.

About sites with a persistent cache, it would seem to me that the benefits are higher than for sites without one, since the query would allow avoiding queries on the other block templates. On the other hand, one could argue that this benefit is tiny or even non existent when looking at the big picture. Consider the following:

• Assuming that a site with a persistent object cache does not have an excessive amount of template parts (let's say 10 for the sake of this example)...
• Without this PR, there is one query for each template part. This is 10 queries in total, likely across a few different templates/URLs, and all those results are cached.
• Once cached (majority of requests), no queries occur.
• With this PR, there is only 1 query now for all the templates. That seems great at first.
• But at large, I question the impact: Once cached (majority of requests), no queries occur (but that was already the case before). But now there is this foreach loop on every page load. So for any requests with cache hits (realistically the vast majority), this PR actually introduces a (tiny, potentially negligible, but certainly present) regression, due to the loop that has to run for every request.

We may need data for those different scenarios to better assess overall impact. Right now I'm questioning whether this change is worthwhile.

[spacedmonkey]

So this is the point, how well does this scale.

Sites with lots of template parts are more likely to need one more than template part. So header, footer, sidebar, comments etc. So more likely to need more template parts.

But now there is this foreach loop on every page load. So for any requests with cache hits (realistically the vast majority), this PR actually introduces a (tiny, potentially negligible, but certainly present) regression

Looping through data in PHP is extremely quick, to the point of not mention it's effect on performance. The much bigger hit to performance is the database query and the overhead of that.

Not sure this should end up in core, we may need to be a single query to get all template parts we know are in core. Doing lots of single queries means the following.

• A single call to prime a single template parts, meta, terms. Which can result in 3 data queries. One for post meta, one to get term ids and another get terms.
• More caches stored.

This PR needs testing but I think still worth exploring.

[peterwilsoncc]

I thought I'd do some testing with a mini plugin to compare the proposed with the current code.

https://gist.github.com/peterwilsoncc/50222356f2549797db8e224af46b1128

When using a subset of template parts, two of eight, I see a consistently improved performance on the test with a cold cache but slower performance on the test with a warm cache.

I'm testing with Chassis (vagrant on VirtualBox 6) which may be affecting it. Vagrant has taken the faster boot speed, slower to run compromise which is the reverse of Docker.

Playing around with the constants should give you an idea of various cases. When changing the number of templates or number of templates used, I recommend ignoring the first result as it will have done a bunch of work up-front that may affect the rest of the run.

Also, please let me know if the mini plugin is flawed, it's always possible I've fluffed something up.

[jordesign]

@spacedmonkey - just looping back to this stale PR to see if it is something you're still pursuing or if it can be closed?

[github-actions]

Warning: Type of PR label mismatch

To merge this PR, it requires exactly 1 label indicating the type of PR. Other labels are optional and not being checked here.

• Type-related labels to choose from: [Type] Automated Testing, [Type] Breaking Change, [Type] Bug, [Type] Build Tooling, [Type] Code Quality, [Type] Copy, [Type] Developer Documentation, [Type] Enhancement, [Type] Experimental, [Type] Feature, [Type] New API, [Type] Task, [Type] Technical Prototype, [Type] Performance, [Type] Project Management, [Type] Regression, [Type] Security, [Type] WP Core Ticket, Backport from WordPress Core.
• Labels found: [Status] Stale.

Read more about Type labels in Gutenberg. Don't worry if you don't have the required permissions to add labels; the PR reviewer should be able to help with the task.