Make WordPress Core

Opened 2 weeks ago

Closed 3 hours ago

Last modified 3 hours ago

#58394 closed enhancement (fixed)

Performance of wp_maybe_inline_styles

Reported by: spacedmonkey's profile spacedmonkey Owned by: spacedmonkey's profile spacedmonkey
Milestone: 6.3 Priority: normal
Severity: normal Version:
Component: Script Loader Keywords: has-patch
Focuses: performance Cc:

Description

The function wp_maybe_inline_styles is called twice on templates. Hooked into wp_head and wp_footer. wp_maybe_inline_styles loops through all register styles to inlines them. But a the file operations, like filesize or file_get_contents can be expensive. On the first run, there should be some level or caching or flag set, to ensure that this function can be run multiple times without run the expensive file operations more than once.

Change History (9)

This ticket was mentioned in PR #4500 on WordPress/wordpress-develop by @spacedmonkey.


2 weeks ago
#1

  • Keywords has-patch added

#2 @spacedmonkey
2 weeks ago

Brenchmark 1000 runs on TT3

Trunk PR
Response Time (median) 107.41 97.72
wp-load-alloptions-query (median) 0.87 0.86
wp-before-template (median) 51.6 50.04
wp-before-template-db-queries (median) 4.35 4.25
wp-template (median) 49.58 41.54
wp-total (median) 101.94 92.05
wp-template-db-queries (median) 2.51 2.44

#3 @flixos90
13 days ago

  • Milestone changed from Future Release to 6.3
  • Owner set to spacedmonkey
  • Status changed from new to assigned
  • Version 5.8 deleted

This is an excellent find!

I just conducted a benchmark for both TT3 (block theme) and TT1 (classic theme), and can report that there is definitely a notable impact on performance for both of them, even classic themes:

  • For TT3, wp-template is ~21% faster, making overall response time ~7% faster.
  • For TT1, wp-template is ~5% faster, making overall response time ~2% faster.

That all makes sense, given that classic themes don't rely on the affected logic as heavily, e.g. don't commonly have stylesheets without a $src enqueued. Still, even there the changes of caching the file size brings a performance benefit that is clear to see in the data.

Here are the full results.

Given this is a major performance win, I'll milestone this for 6.3.

Last edited 13 days ago by flixos90 (previous) (diff)

@spacedmonkey commented on PR #4500:


12 days ago
#4

@joemcgill @felixarntz I am going to add some unit tests to wp_maybe_inline_styles. Add it seems to have none at all :(

@spacedmonkey commented on PR #4500:


2 days ago
#5

@joemcgill @felixarntz Actioned feedback on tests. If you are happy, then I will commit.

@peterwilsoncc commented on PR #4500:


10 hours ago
#6

This is a nice enhancement. What's unclear to me is why this is being hooked to both the wp_head and wp_footer action. It doesn't seem like this function was designed to be run twice. In fact, wp_enqueue_style doesn't even support an in_footer param like wp_enqueue_script,

I presume this is to print styles that were registered after the header action fires, similar to print_late_styles().

https://github.com/WordPress/wordpress-develop/blob/c9f3a682812c4df3fde221969619acfdf529ec72/src/wp-includes/script-loader.php#L2228-L2238

#7 @spacedmonkey
3 hours ago

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

In 55888:

Script Loader: Improve performance of wp_maybe_inline_styles function.

The wp_maybe_inline_styles function is called twice on the average page load. On it's second run however, it did not check to see if the style had already been processed on the first run. This resulted in calling filesize and get_file_contents unnecessarily, which was bad for performance. Now, the loop around the queued styles, checks to see if the source is set to false, meaning it has already been processed. This change also replaces calls to filesize with the core function wp_filesize, which improves extensibility.

Props spacedmonkey, flixos90, peterwilsoncc, joemcgill.
Fixes #58394.

Note: See TracTickets for help on using tickets.