Make WordPress Core

Opened 5 years ago

Closed 5 years ago

Last modified 4 years ago

#14911 closed defect (bug) (fixed)

Don't fetch theme_roots on every page load.

Reported by: ryan Owned by:
Milestone: 3.1 Priority: normal
Severity: normal Version:
Component: Performance Keywords: has-patch
Focuses: Cc:


As part of setting up the directories for the current theme, get_theme_roots() and thus get_site_transient( 'theme_roots' ) is called on every page load. If you have many themes (typical for multisite setups) this means a lot of data is being pulled from the cache. If using a remote cache like memcached all of this data is going over the wire. An active multisite install with many themes and memcached can waste 100s of MBs of bandwidth pulling the same data over and over.

To avoid fetching theme_roots on every page load, lets store the roots for the current stylesheet and template in the options table as template_root and stylesheet_root. If these are present we can avoid calling get_theme_roots() for almost all page loads.

Attachments (2)

14911.diff (2.7 KB) - added by ryan 5 years ago.
14911.2.diff (3.4 KB) - added by ryan 5 years ago.
Don't bother caching at all if only one theme dir registered.

Download all attachments as: .zip

Change History (12)

5 years ago

5 years ago

Don't bother caching at all if only one theme dir registered.

#1 @scribu
5 years ago

  • Keywords has-patch added

#2 @ryan
5 years ago

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

(In [15641]) Don't fetch theme_roots transient on every page load. Avoid it altogether if there is only one theme dir. fixes #14911

#3 @westi
5 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Don't these need filtering for preview purposes otherwise anywhere a theme being previewed calls get_template_directory() we are going to return the current theme not the previewed one.

#4 @hakre
5 years ago

Looks like this is breaking backwards compability.

Additionally I think the function should be named with the raw at the end not after the get or according to what's written in the docblocks, to get_theme_root_unfiltered(). raw is a pretty broad term which makes it easy to misread.

Then I wonder why get_theme_root() calls get_raw_theme_root a.k.a. get_theme_root_unfiltered() because both functions look to be for the same thing (get theme root) but using a different approach (strategy: filtered and unfiltered). but mixing both destroys the first and the second strategy even with the cost of introducing a third one. That is making things complicated. My 2 cents.

#5 @markmcwilliams
5 years ago

In trying out the bbPress Plugin on 3.1-alpha (which JJJ hadn't as of yet) it'd seem that in registering the TwentyTen Child Theme using register_theme_directory(); it'll not work in a 3.1-alpha environment. It works on a 3.0.1 install, and I can confirm that!

/plugins/bbpress/bbp-themes/ is the directory being registered, and while it is in 3.1-alpha, when you either activate or want preview the theme, it'll not show TwentyTen Child Theme properly. And on viewing the source for the stylesheet URL, you will find it's looking for /wp-content/themes/bbp-twentyten/style.css when it should be looking for /wp-content/plugin/bbpress/bbp-themes/bbp-twentyten/style.css

So it's looking for a directory not there?

For more info you might want to take a quick read through #BB1334 from about the 20th comment down, and also #BB1358 the later being after I talked to JJJ on IRC last night, trying to diagnose what was happening.

#6 @ryan
5 years ago

(In [16424]) Register the default theme dir in wp-settings.php so that it is registered even when get_themes() doesn't run. Fix counting of theme dirs. Add option to get_raw_theme_root() to disregard the current theme root cache. see #14911

#7 @markmcwilliams
5 years ago

Just reporting back to say that change you made Ryan looks good, and works great on a live site anyway! :) As for in my local environment it's trying to locate http://localhost/wpdemo/wp-content/C:\xampp\htdocs\wpdemo\wp-content\plugins\bbpress/bbp-themes/bbp-twentyten/style.css purely because it uses __FILE__ to 'locate' the folder the plugin is in, as the later comments of #BB1334 are suggesting!

Are we looking at a(nother) possible WordPress error, or is this back in the hands of bbPress because the directory isn't hardcoded into the files? I suppose the fact it'll work everywhere else bar a local copy on Windows doesn't help?

#8 follow-up: @ryan
5 years ago

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

I think that's a separate issue. Let's close this ticket and look at that separately.

#9 in reply to: ↑ 8 @markmcwilliams
5 years ago

Replying to ryan:

I think that's a separate issue. Let's close this ticket and look at that separately.

Makes logical sense!

#10 @nacin
4 years ago

In [20001]:

Always return an absolute path in get_raw_theme_root() and get_theme_roots().

These functions were changed in [15641] to avoid any calculations when only one theme directory is registered. However, the short-circuit ended up being relative to WP_CONTENT_DIR, rather than absolute. This broke situations where theme roots are outside the content directory (technically allowed), and made return values inconsistent, as when multiple roots were registered, absolute paths were always returned.

fixes #17597. see #20103. see #14911.

Note: See TracTickets for help on using tickets.