Opened 7 weeks ago
Closed 10 days ago
#61782 closed defect (bug) (maybelater)
Memoize wp_get_wp_version
Reported by: | Cybr | Owned by: | SergeyBiryukov |
---|---|---|---|
Milestone: | Priority: | normal | |
Severity: | normal | Version: | trunk |
Component: | General | Keywords: | has-patch dev-feedback close |
Focuses: | Cc: |
Description
wp_get_wp_version()
currently requires a file whenever it's being called. This file is being required every time the function is called.
<?php function wp_get_wp_version() { require ABSPATH . WPINC . '/version.php'; return $wp_version; }
We can memoize the return value using the static keyword. This will make the file-requiring happen only once. At just 1000 iterations, this memoization is already 160 times faster (from 0.1000s to 0.0006s execution time).
<?php function wp_get_wp_version() { static $wp_version; if ( empty( $wp_version ) ) require ABSPATH . WPINC . '/version.php'; return $wp_version; }
Change History (15)
This ticket was mentioned in PR #7105 on WordPress/wordpress-develop by @debarghyabanerjee.
7 weeks ago
#2
- Keywords has-patch added
Trac Ticket: #61782
## Problem Statement:
- The
wp_get_wp_version()
function currently requires the version.php file on every call. This results in unnecessary file inclusion, leading to inefficiencies especially when the function is invoked multiple times. Performance testing indicates that the execution time can be reduced dramatically with this change—memoization shows a performance improvement of approximately 160 times (from 0.1000s to 0.0006s) after 1000 iterations.
## Solution
Optimization
: Utilize the static keyword to cache the WordPress version within the wp_get_wp_version() function. This change ensures that the version.php file is required only once, and subsequent calls will return the cached version number without additional file operations.
Performance
: This modification results in a significant reduction in execution time by avoiding redundant file inclusions, leading to more efficient code execution and improved overall performance.
## Benefits
Performance Improvement
: Reduces the overhead of including the version.php file on each function call, resulting in faster execution times.
Resource Efficiency
: Decreases the number of file I/O operations, which is beneficial for high-traffic sites or applications that call wp_get_wp_version() frequently.
Cleaner Code
: Simplifies the function logic by leveraging caching to handle repeated requests more efficiently.
This ticket was mentioned in PR #7105 on WordPress/wordpress-develop by @debarghyabanerjee.
7 weeks ago
#3
Trac Ticket: #61782
## Problem Statement:
- The
wp_get_wp_version()
function currently requires the version.php file on every call. This results in unnecessary file inclusion, leading to inefficiencies especially when the function is invoked multiple times. Performance testing indicates that the execution time can be reduced dramatically with this change—memoization shows a performance improvement of approximately 160 times (from 0.1000s to 0.0006s) after 1000 iterations.
## Solution
Optimization
: Utilize the static keyword to cache the WordPress version within the wp_get_wp_version() function. This change ensures that the version.php file is required only once, and subsequent calls will return the cached version number without additional file operations.
Performance
: This modification results in a significant reduction in execution time by avoiding redundant file inclusions, leading to more efficient code execution and improved overall performance.
## Benefits
Performance Improvement
: Reduces the overhead of including the version.php file on each function call, resulting in faster execution times.
Resource Efficiency
: Decreases the number of file I/O operations, which is beneficial for high-traffic sites or applications that call wp_get_wp_version() frequently.
Cleaner Code
: Simplifies the function logic by leveraging caching to handle repeated requests more efficiently.
#5
@
7 weeks ago
- Owner set to SergeyBiryukov
- Resolution set to fixed
- Status changed from new to closed
In 58827:
@SergeyBiryukov commented on PR #7105:
7 weeks ago
#6
Thanks for the PR! Merged in r58827.
#7
@
7 weeks ago
- Keywords dev-feedback added
@SergeyBiryukov This new helper function may be used during the upgrade routines (or by plugins with post-upgrade hooks, for example), where wp-includes/version.php will have been replaced, and OPcache will have been invalidated. However, the return value of wp_get_wp_version()
won't match the actual current version until the next request. This may need some further thought and testing.
A $force
parameter may have been useful to include here, to force a fresh file load in such circumstances.
#8
@
7 weeks ago
- Resolution fixed deleted
- Status changed from closed to reopened
Replying to costdev:
This new helper function may be used during the upgrade routines (or by plugins with post-upgrade hooks, for example), where wp-includes/version.php will have been replaced, and OPcache will have been invalidated. However, the return value of
wp_get_wp_version()
won't match the actual current version until the next request. This may need some further thought and testing.
Good catch, thanks!
#9
@
6 weeks ago
My inclination is to revert [58827].
While 10,000 iterations shows a drop from 0.1000s to 0.0006s in execution time, it's more likely the function will only be called a few times in the typical execution of a WordPress request.
Given the risk of the incorrect version being reported pre- and post- upgrade a more real-world example of the improvement is needed.
#10
@
6 weeks ago
I concur with Peter. You can skip to the last paragraph to get the conclusion.
Still, a single parameter to force fetching the latest version could rewrite the cache (lest we not forget to use it):
<?php function wp_get_wp_version( $reset = false ) { static $wp_version; if ( ! isset( $wp_version ) || false !== $reset ) require ABSPATH . WPINC . '/version.php'; return $wp_version; }
I used false !== $reset
here so we can write wp_get_version( 'latest' )
or something, but that's a coding style preference. true
is not conveying much as a parameter, but that's probably best left for another discussion.
On WP 6.6.1 and WP 6.7-alpha-58846, on PHP 8.2 with OPcache, using Twenty Twenty-Two, no plugins, logged in, on the homepage, version.php
is included 3 times:
- 1x
wp_default_styles()
, - 1x
wp-settings.php
, - 1x
wp_scripts_get_suffix()
.
In that context, with just 3 plausible calls to wp_get_wp_version()
, the function, as per this comment, would save 0.04ms of load time compared to the non-memoizing function.
At /wp-admin/index.php
on 6.7-alpha-58846, wp_get_wp_version()
is called 2 times:
- 1x
_maybe_update_core()
, - 1x
core_update_footer()
.
The version.php
file is included 4 times:
- 1x
wp-settings.php
, - 1x
wp_scripts_get_suffix()
, - 1x
wp_default_styles()
, - 1x
wp_get_wp_version()
.
In that context, with just 2 plausible calls to wp_get_wp_version()
, the function would save 0.03ms.
The total load time of WordPress on the server I tested this with is about 60ms.
So, this can save anywhere from 0.05% to 0.06% of load time (one-1500th to one-2000th of a relative impact).
I'm pedantic and overly ambitious. Ignoring the law of diminishing returns and Zeno's paradox, with 1500 iterations of this kind of improvement, WP would load instantly.
Realistically, however, 0.04ms of total savings isn't a lot.
#12
@
6 weeks ago
Thanks for looking in the stats, @Cybr.
I've reverted the commit for now but am leaving the ticket open to consider performance improvements in the future as the function gains wider use within Core.
I considered using a non-persistent cache to store the value and flushing it during the upgrade but that failed the though experiment of managing plugins modifying the value: the security through obscurity advise would simply recommend changing both the global and the cached value.
Either a $force
parameter or caching a boolean to determine if the value has been populated may be possible.
#13
@
6 weeks ago
I don't think that a $force
parameter makes much sense. As a developer, when calling this function, I would always want the correct version, so I would be setting the $force
parameter. But if everybody uses that parameter anyways, there's not much benefit to even offering it. The $force
outcome should then be the default (as it is now after the revert) and would save the overhead of additional variable checks.
#14
@
2 weeks ago
- Keywords close added
I tend to agree that a $force
parameter feels a bit out of place. Plus, if the default value is false
, then anything expecting an accurate version to be returned may break when 6.7 is released.
I think this is just one of those improvements that changes old, long standing code that's just not worth the risk. I'm adding a close
suggestion based on the feedback from @TobiasBg and I.
#15
@
10 days ago
- Milestone 6.7 deleted
- Resolution set to maybelater
- Status changed from reopened to closed
I agree with @desrosj & @TobiasBg that using a static and a $force
parameter isn't worth the benefit a static or other caching provides.
As the use of the function increases, it may be worth reconsidering so I've closed this as maybelater.
Here's a plugin to test my claim: https://gist.github.com/sybrew/bd78076fabdd6ce67ce25e57a8fc7e72.
Just upload the PHP file as a plugin, activate it, and visit any public page with the "?test=1" query parameter.
Correction: I tested 10,000 iterations, not 1000 iterations. But testing 1000 will still yield the same ~160x performance improvement. At 10 iterations, we measure a ~60x performance improvement, and at 2 iterations ~2x.