Make WordPress Core

Opened 7 years ago

Closed 16 months ago

Last modified 5 months ago

#40731 closed enhancement (wontfix)

locate_template() performance improvement

Reported by: danielhuesken's profile danielhuesken Owned by:
Milestone: Priority: normal
Severity: normal Version:
Component: Themes Keywords: has-patch needs-testing
Focuses: template, performance Cc:

Description

If the theme uses massive the *_template functions it can come to a slow performance because the the files will be searched again and again.

We can improve the performance if we use a static cache like this:

function locate_template($template_names, $load = false, $require_once = true ) {
	static $template_cache;
	
	$located = '';
	foreach ( (array) $template_names as $template_name ) {
		if ( !$template_name )
			continue;
		if ( $template_cache[$template_name] ) {
			$located = $template_cache[$template_name];
			break;
		}
		if ( isset( $template_cache[$template_name] ) && ! $template_cache[$template_name] )
			continue;
		if ( file_exists(STYLESHEETPATH . '/' . $template_name)) {
			$located = STYLESHEETPATH . '/' . $template_name;
			$template_cache[$template_name] = $located;
			break;
		} elseif ( file_exists(TEMPLATEPATH . '/' . $template_name) ) {
			$located = TEMPLATEPATH . '/' . $template_name;
			$template_cache[$template_name] = $located;
			break;
		} elseif ( file_exists( ABSPATH . WPINC . '/theme-compat/' . $template_name ) ) {
			$located = ABSPATH . WPINC . '/theme-compat/' . $template_name;
			$template_cache[$template_name] = $located;
			break;
		}
		$template_cache[$template_name] = false;
	}

	if ( $load && '' != $located )
		load_template( $located, $require_once );

	return $located;
} 

Attachments (1)

40731.patch (1.6 KB) - added by danielhuesken 7 years ago.

Download all attachments as: .zip

Change History (24)

#1 @danielhuesken
7 years ago

  • Keywords dev-feedback added

@danielhuesken
7 years ago

#2 @danielhuesken
7 years ago

  • Keywords has-patch added

#3 @danielhuesken
7 years ago

  • Version 4.6.4 deleted

#4 @SergeyBiryukov
7 years ago

  • Summary changed from locate_template() preformance improvment to locate_template() performance improvement

#5 follow-up: @johnbillion
7 years ago

  • Keywords reporter-feedback added; dev-feedback removed

Thanks for the patch @danielhuesken.

Can you let us know in what situation this would improve performance please? It appears that performance is only improved with repeated calls to locate templates of the same name. Is this is common occurrence?

#6 @danielhuesken
7 years ago

Yes. If you have the same template for a post and you have a post list, that the template will be located every time again and the file_exists() calls will be many.

In our case we have WooCommerce and Products and in the product a plugin will do many things with templates that must be located every time again, with the expensive file_exits(). On that project is a page where the load will be increased by 1.5 seconds with this change.

I could see the calls in a xdebug profile.

#7 in reply to: ↑ 5 @Biont
7 years ago

Replying to johnbillion:

Can you let us know in what situation this would improve performance please? It appears that performance is only improved with repeated calls to locate templates of the same name. Is this is common occurrence?

This is definetely common. I routinely build wrappers for template functions to cache all those filesystem accesses wherever I can. A proper system-wide would speed up complex themes noticeably - and practically "for free". While this issue does not rank among the worst offenders when looking at an average xdebug profile, it certainly does show up an it's low-hanging fruit that's easy to tackle.

#8 @johnbillion
6 years ago

  • Keywords reporter-feedback removed
  • Milestone changed from Awaiting Review to 5.0
  • Owner set to johnbillion
  • Status changed from new to reviewing

#9 @dd32
6 years ago

Just wanted to note, that while this may speed things up ever so slightly, file_exists() calls are already internally cached by both PHP and the underlying filesystem and subsequent calls to the same file_exists() calls should return significantly faster than the initial call.

#10 @johnbillion
5 years ago

  • Milestone changed from 5.0 to 5.1

#11 @pento
5 years ago

  • Keywords needs-testing added
  • Milestone changed from 5.1 to Future Release

Given @dd32's comment, I'd like to see some performance measurements on modern PHP versions that show a significant boost from 40731.patch.

#12 @johnbillion
5 years ago

  • Owner johnbillion deleted

@mukesh27 commented on PR #3701:


16 months ago
#14

Thanks @costdev for the feedback.

@dd32 @pento I have measured performance for locate_template() against different PHP versions.

The current version of locate_template from trunk and my modified version have been compared using benchmarks. To eliminate variations caused by server resource allocations as much as feasible, both versions of each benchmark are run on the same computer from within the same process, with only the relative change being recorded. The benchmark results are displayed in a table here: https://docs.google.com/spreadsheets/d/1nWXA0BOafdHKXY9w-FTCy9D6IXch7GGSu6j9EexZNWE/edit?usp=sharing

The benchmark summary states that this improvement should speed up the locate_template function by ~91% under typical use conditions. The test script I've used to run the benchmarks can be found here: https://gist.github.com/mukeshpanchal27/b10f1360067e4429db1ef406f9e9dc02

Note: To run the benchmark, I use the Twenty Twenty-One theme and make duplicate (home-2.php, index-2.php, etc.) templates from existing ones so it will not used cached version.

cc.@johnbillion

#15 @mukesh27
16 months ago

  • Milestone changed from Future Release to 6.2

Thanks @danielhuesken for the ticket.

I have submitted PR 3701 with performance analysis results.

Moving ticket to 6.2.

@dd32 commented on PR #3701:


16 months ago
#16

While the performance numbers as a percent look impressive, how much is this really adding in a real-life scenario? Are we talking 10ms per call? Or 0.01ms per call speed up? Is the speed increase from caching an expensive lookup, or just because of a reduced number of PHP function calls?

  • file_exists() hits PHP's internal (and filesystem) stat cache.
  • How often does WordPress call locate_template() using previously-requested templates?
    • I'm guessing this might be a little more common with things like content-{$post_type}.php in archives or something.

@mukesh27 commented on PR #3701:


16 months ago
#17

@dd32 As locate_template() ultimately hits a lot of Core via get_header(), get_footer(), get_sidebar(), get_template_part(), get_search_form(), get_query_template(),their "Used by" chain, and extender usage.

Twenty Twenty-One theme default home page called locate_template() 13 times with different templates name. If we called all 13 templates for single time with analysis script then it speed up by average ~10% to ~30%.

Average time spent for 1 x 11 iterations: 

- locate_template() - 0.0094649791717529
- new_locate_template() - 0.0051949024200439 

Change: -45.1%

@peterwilsoncc commented on PR #3701:


16 months ago
#18

@mukeshpanchal27 What's the time scale (seconds, milliseconds, etc) in the raw numbers?

I'm a little worried there could be unintended consequences, etiher now or in the future, if the template path changes mid way through execution. If there is a change, then 'my-template-parts/pwcc.php' could differ between calls.

I know the customizer switches themes temporarily but not sure if this would be an issue here. A test with file name collisions would be needed.

@mukesh27 commented on PR #3701:


16 months ago
#19

The time scale is in milliseconds.

@peterwilsoncc commented on PR #3701:


16 months ago
#20

Given the time scale, I think it's better to close this off without a fix. I'll put some notes on the ticket.

#21 @peterwilsoncc
16 months ago

  • Milestone 6.2 deleted
  • Resolution set to wontfix
  • Status changed from reviewing to closed

Given the raw numbers show a 0.004270076751709 millisecond improvement in performance under ideal conditions (ie, repeatedly requesting the same set of templates), I think this ticket can be closed of without a fix.

Practically there is a very small performance gain but it alters locate_template() in such a way that it might become a problem at a later date.

Working on another performance ticket recently, I had to deal with complexities caused by the use of static variables so adding once here without a significant time improvement isn't worth the risk.

@peterwilsoncc commented on PR #3701:


16 months ago
#22

Closing this off as the issue it's linked to has been closed.

This ticket was mentioned in Slack in #core-performance by joemcgill. View the logs.


5 months ago

Note: See TracTickets for help on using tickets.