Make WordPress Core

Opened 3 months ago

Last modified 3 months ago

#57789 new enhancement

Make theme.json related caches persistent

Reported by: flixos90's profile flixos90 Owned by:
Milestone: 6.3 Priority: normal
Severity: normal Version:
Component: Themes Keywords: needs-patch
Focuses: performance Cc:

Description (last modified by flixos90)

The theme_json cache group is currently non-persistent, which means that, even when a site uses a persistent object cache, the data in this cache group will only be cached within a single request. In other words, there is only a benefit to this caching when a function is called several times, at least more than once, in a single request.

Needless to say, we have noticed in the past that parsing theme.json and running related logic can be expensive, and the preferable outcome would be to make those caches persistent so that they also can be cached beyond requests.

The biggest challenge to this is to implement proper cache invalidation. Originally, within Gutenberg, the cache group was persistent, but it led to problems due to:

  • cache invalidation hard to figure out (what are all the events can lead to the cached values becoming stale?)
  • caching logic, rather than database values, is not yet a common practice in WordPress, and one of the quirks is that e.g. action/filter hooks may no longer be run when the logic that contains them is cached, which can lead to a clunky developer experience

See related comment https://core.trac.wordpress.org/ticket/57648#comment:17

Change History (7)

#1 @flixos90
3 months ago

  • Description modified (diff)
  • Milestone changed from Awaiting Review to 6.3

This tracking ticket will likely involve several issues and PRs in the Gutenberg repository to identify the best approach for the various usages of caching related to theme.json and the theme_json cache group.

I'm hopeful we can at least make some notable progress here for WordPress 6.3, so I'll milestone it for that release for now, acknowledging though that most of the work will need to happen in the Gutenberg repository, so it'll probably be a while until we get a PR here.

cc @oandregal @spacedmonkey

#2 @iCaleb
3 months ago

One approach could be to mainly just utilize the persistent cache only during frontend requests (where it'd get the most performance benefit).

Each logged-in non-ajax admin requests could, at least once, always fetch the theme.json. If there are changes from what is in the persistent cache then it can update. As a backup (in case a theme is updated but nobody logs in), could also allow the same invalidation-checks during cron as well.

Worst case scenario: no cron is running and no admin-side log-ins are occurring. In which case it'd just have to rely on the TTL expiring and a frontend request would have to do the work to populate the cache again.

This gives the benefits of persistent cache, while solving for invalidation in a pretty reasonable way.

Last edited 3 months ago by iCaleb (previous) (diff)

#3 @oandregal
3 months ago

It's worth noting that these caches were implemented as persistent in Gutenberg, and they were removed to unblock this work from landing into core. See https://github.com/WordPress/gutenberg/pull/46150 for details, and links to the conversations. The PR can also serve to bring everything back quickly.

The cache invalidation covered all the following scenarios:

  • upgrader_process_complete: for WordPress, plugin, and theme updates.
  • switch_theme: the active theme is changed.
  • start_previewing_theme: previews for customizer and theme directory.
  • save_post_wp_global_styles: user updates theme.json data via the site editor.
  • activated_plugin and deactivated_plugin: any plugin can hook into the existing theme.json filters, so when they are activated/deactivated the filters may change.

The disagreement was specific to the following two points:

  • Consumers that hook into theme.json data using dynamic mechanisms (deciding whether to hook or not based on an option or user meta value). For these cases, the alternatives would be to add more cache invalidation points (option add/update/delete, etc.), or ask the consumers to clear the cache themselves.
  • Updates that are not done via the regular WordPress mechanism (FTP updates, updates managed via git repositories, etc.). The alternative is asking consumers to clear the cache themselves.

Note these two cases only if they use a persistent object cache plugin, as core, by default, doesn't persist.

#4 @spacedmonkey
3 months ago

WordPress core already has the cache group of theme. I wonder if we could use them. This is a special cache, that is an opt-in to persistent. See.

This means, for site owner that have control, their can make this group persistent, as they can cache clear when the theme is updated. Otherwise, it is a non persistent groups. This could be a interesting model to follow, as it would allow for production sites, that hosting companies know are not going to change, to persistent otherwise just be an in memory cache.

#5 @mukesh27
3 months ago

  • Keywords needs-patch added

#6 @flixos90
3 months ago

@iCaleb Your idea certainly sounds interesting, however there is still the concern about how that affects filter callback logic that runs due to the filters from the logic that generates the data which is then cached. Whenever the cached data is returned, those filters aren't run. Which maybe is fine for some, but is not an expected situation, and with the scale of WordPress I'm sure will lead to breakage.

So the problems here are partially invalidation, partially filters in the logic that is no longer executed when cached (see @oandregal's comment for more details).

@spacedmonkey Regarding the existing theme cache group, I think we should avoid intertwining with that. There are different implications for that cache group, and opting in to making it persistent can be a good idea. Whereas if you now opted in to making the theme-json cache group persistent, you would effectively opt in into broken behavior, which IMO is not something we should offer or encourage.

#7 @spacedmonkey
3 months ago

This cache could also be global cache group, like the theme group.

Note: See TracTickets for help on using tickets.