Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FSE Post Meta Queries Performance #24377

Closed
tomjn opened this issue Aug 5, 2020 · 7 comments · Fixed by #27016
Closed

FSE Post Meta Queries Performance #24377

tomjn opened this issue Aug 5, 2020 · 7 comments · Fixed by #27016
Labels
[Feature] Templates API Related to API powering block template functionality in the Site Editor [Type] Performance Related to performance efforts

Comments

@tomjn
Copy link
Contributor

tomjn commented Aug 5, 2020

Describe the bug
Template part posts use a theme post meta key value. The problem is that FSE then searches for posts that have this value in the future, specifically on save.

While this happens in a lot of WP sites, the performance is poor, and it does not scale. Such a query will outright fail on a large site, and could bring down a database server on sites handling lots of concurrent visitors.

The posts meta table was never intended for those kind of queries, if it was then tags and categories would be stored in post meta. That table is designed for finding values when you know the post ID and key, not the other way around.

The problem is trivial to sidestep though, and even has precedent elsewhere in core, use a taxonomy.

The performance of that query if a taxonomy were used would be orders of magnitude faster. Post meta queries do not scale as the site gets more content and traffic, taxonomies do. Take a look at Nav menus for an example of taxonomies in core. I wouldn't recommend copying it, but a simple wp_theme taxonomy for the template and template parts would give massive performance gains for minimal effort.

To reproduce
Steps to reproduce the behavior:

  1. Try to save a template part on a site that has a lot of posts and posts meta
  2. Observe degraded performance
@aristath
Copy link
Member

aristath commented Aug 5, 2020

Just an idea, but why not introduce a new table for templates & parts?
It would be orders of magnitude faster if properly implemented, and we'd be free to optimize things without being limited by a db architecture that wasn't built for these things.

@tomjn
Copy link
Contributor Author

tomjn commented Aug 5, 2020

I actually think we gain a huge amount of flexibility by keeping them as posts, I have lots of ideas it enables, but perhaps that would make a good new issue? I'm only interested in the avoidable post meta query for this issue

@aristath
Copy link
Member

aristath commented Aug 6, 2020

Between a post-meta and a tax query, tax is definitely faster. There's no doubt about that. Adding a wp_theme makes perfect sense 👍

@annezazu annezazu added [Feature] Full Site Editing [Type] Performance Related to performance efforts [Feature] Templates API Related to API powering block template functionality in the Site Editor labels Aug 14, 2020
@draganescu draganescu added this to Inbox in Full site editing Aug 19, 2020
aristath added a commit to aristath/gutenberg that referenced this issue Aug 21, 2020
@aristath
Copy link
Member

aristath commented Aug 21, 2020

This was discussed in the last core-editor meeting

@tomjn I started working on a prototype on #24720 for how this might work.
Still untested, I'm currently going through the code and checking where we call the meta etc. Pushed it so we have something to talk about and make some decisions where needed.

@mtias
Copy link
Member

mtias commented Aug 26, 2020

Absolutely. There are also plans to have user styles saved — through the global styles project — which might also need a reference to a theme. Those would generally be single entities, but it's still worth thinking through the design and whether a taxonomy might be shared. More generally, the full reference from templates to themes (the ability to mix parts from different themes) needs a thorough revision as the pieces get more established.

@carlomanf
Copy link

There is yet another potential approach other than post meta or taxonomy or table. It might be a bad option, I don't know, but maybe worth considering.

The idea is to dynamically insert the current theme name into the post type slug. So wp_t_twentytwenty, wp_tp_twentytwenty, wp_t_twentynineteen and wp_tp_twentynineteen would all be separate post types, and the only two types registered at any given time would be the two types for the current theme.

Would this work?

@noahtallen
Copy link
Member

Would this work?

I think it would be challenging if you were to have user-provided templates and template parts. For example, the current implementation lets you create your own template in the database. If you name that such that it fits into the template hierarchy (e.g. "Front Page"), it will be resolved instead of whatever the theme provides. I think having extra CPTs per theme makes it more challenging to manage all of that -- we'd still have to keep the basic wp_template CPT and also have to generate the theme-provided CPT to add into every query. But I guess that is mostly an ergonomics things, I'm not sure what it would mean for performance.

Anyways, the taxonomy approach has been merged, so we have a chance to try it out!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Templates API Related to API powering block template functionality in the Site Editor [Type] Performance Related to performance efforts
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

6 participants