Make WordPress Core

Opened 3 months ago

Closed 3 months ago

#56945 closed defect (bug) (fixed)

Avoid running unnecessary expensive logic around theme.json parsing for classic themes

Reported by: flixos90's profile flixos90 Owned by: flixos90's profile flixos90
Milestone: 6.1.1 Priority: normal
Severity: normal Version: 6.1
Component: Editor Keywords: has-patch has-unit-tests fixed-major
Focuses: css, performance Cc:

Description

I'm opening this ticket to address the performance problem identified in https://core.trac.wordpress.org/ticket/56467#comment:408 (which is the ticket it was introduced as part of, however it's a massive ticket that contains 100s of changes, so it's not reasonable to discuss a specific change in there).
Also see the relevant follow up comments https://core.trac.wordpress.org/ticket/56467#comment:411 and https://core.trac.wordpress.org/ticket/56467#comment:413. The performance problem was identified as part of a WordPress 6.1 performance analysis, which specifically in the wp_head action saw a major regression compared to WordPress 6.0.

To summarize here: [54408] (also see original Gutenberg PR https://github.com/WordPress/gutenberg/pull/42005) has removed a condition that now causes wp_add_global_styles_for_blocks() to be called for classic themes as well, where prior it was only called for FSE themes.

While this change was needed to support including certain block styles even for classic themes as they can be relevant there too, the logic triggered by wp_add_global_styles_for_blocks() does a lot more than that, most of which is only beneficial to FSE sites and thus essentially wasted resources for sites using a classic theme.

A first PR with a potential fix is available in https://github.com/WordPress/wordpress-develop/pull/3536, and it improves median wp_head execution time from 16.45ms to 11.92ms (~28% faster).

Change History (13)

#1 @flixos90
3 months ago

Paging @oandregal @scruffian @andraganescu here who were involved in the original change (see ticket description), this is a more specific continuation of what I originally pointed out in https://core.trac.wordpress.org/ticket/56467#comment:408.

#2 @spacedmonkey
3 months ago

On a related matter, I worked on improving the performance of the WP_Theme_JSON_Resolver class.

Se my WIP PR to convert class to use non-static methods and use a global class instance. See #3540.

This ticket was mentioned in Slack in #core by jeffpaul. View the logs.


3 months ago

#6 @spacedmonkey
3 months ago

  • Owner set to flixos90
  • Status changed from new to assigned

#7 @spacedmonkey
3 months ago

With WordPress 6.1.1 fast approaching, I want to see if we can get this fix merged. @flixos90 your PR is currently in draft. Is there any change in getting this PR finalized, so we can start testing.

#8 @flixos90
3 months ago

  • Keywords has-unit-tests added; 2nd-opinion needs-unit-tests removed

PR https://github.com/WordPress/wordpress-develop/pull/3536 is now ready for a full review and should be in shape to get committed and merged for 6.1.1.

I've broken out the more complex piece into a new Gutenberg issue https://github.com/WordPress/gutenberg/issues/45616 for later consideration.

This ticket was mentioned in Slack in #core by jeffpaul. View the logs.


3 months ago

#10 @flixos90
3 months ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 54799:

Editor: Avoid running certain logic around theme.json parsing unnecessarily for classic themes.

Here's what it does:

  • Do not load and parse theme-i18n.json schema if the theme does not have a theme.json file.
  • Fix the variable caching layer around the theme's theme.json parsing so that a parent's theme theme.json is cached as well.
  • Do not run a WP_Query for global styles for a user when the theme does not have a theme.json.

In a basic WordPress setup, this changeset improves wp_head execution time for classic themes by 10%, and overall response time for both block themes and classic themes by 4%. This may seem like a small win, but 4% reduced overall response time is actually quite a bit for one change, and it is worth mentioning that this is just one of several other little performance tweaks which are being worked on to improve performance of theme.json parsing further.

Props flixos90, manuilov, oandregal, peterwilsoncc, spacedmonkey.
Fixes #56945.

#11 @flixos90
3 months ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening for backport to 6.1.1.

#12 @peterwilsoncc
3 months ago

  • Keywords fixed-major added

#13 @flixos90
3 months ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 54800:

Editor: Avoid running certain logic around theme.json parsing unnecessarily for classic themes.

Here's what it does:

  • Do not load and parse theme-i18n.json schema if the theme does not have a theme.json file.
  • Fix the variable caching layer around the theme's theme.json parsing so that a parent's theme theme.json is cached as well.
  • Do not run a WP_Query for global styles for a user when the theme does not have a theme.json.

In a basic WordPress setup, this changeset improves wp_head execution time for classic themes by 10%, and overall response time for both block themes and classic themes by 4%. This may seem like a small win, but 4% reduced overall response time is actually quite a bit for one change, and it is worth mentioning that this is just one of several other little performance tweaks which are being worked on to improve performance of theme.json parsing further.

Props flixos90, manuilov, oandregal, peterwilsoncc, spacedmonkey.
Merges [54799] to the 6.1 branch.
Fixes #56945.

Note: See TracTickets for help on using tickets.