Memoize wp_get_wp_version

[Cybr]

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;
}

[Cybr]

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.

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.

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.

[SergeyBiryukov]

In 58827:

General: Memoize the return value in wp_get_wp_version().

This aims to optimize performance by saving the return value to a static variable, so that the version.php file is not unnecessarily required on each function call.

Follow-up to [58813].

Props Cybr, debarghyabanerjee, mukesh27.
Fixes #61782. See #61627.

Thanks for the PR! Merged in r58827.

[costdev]

@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.

[SergeyBiryukov]

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!

[peterwilsoncc]

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.

[Cybr]

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.

[peterwilsoncc]

In 58848:

General: Removing static from wp_get_wp_version().

Removes the static storing the version number in wp_get_wp_version() to ensure the version number is reported correctly after a WordPress upgrade is completed.

Reverts [58827].

Props costdev, SergeyBiryukov, Cybr.
See #61782.

[peterwilsoncc]

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.

[TobiasBg]

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.

[desrosj]

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.

[peterwilsoncc]

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.

[azaozz]

Replying to costdev:
Having some second thoughts about [58848].

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.

Right, but that is the old behavior? I mean, once the global $wp_version was set it is kept in memory and would not change even if the version.php file is updated and the version string there changes?

It seems that catching the version as it exists at the time of first run of wp_get_wp_version() would match the old behavior better? Not completely sure if that can be changed without causing any inconsistencies. Then the functions that require the version as it appears after updating version.php would still have to require it locally.

May be overthinking this a bit but still would like to reopen it just for visibility.

[peterwilsoncc]

@azaozz The upgrade routine returns the new version but I am not sure if it assigned to the global on the current request or not. If not, then you are correct that caching the value would match the current behaviour.

Colin's Slack status indicates he is has recently decided to take a break from contributing to Core so I'll post a link to your comment in the upgrade channel.

[azaozz]

Replying to peterwilsoncc:

@azaozz The upgrade routine returns the new version but I am not sure if it assigned to the global on the current request or not. If not, then you are correct that caching the value would match the current behaviour.

Yea, seems the two calls to wp_get_wp_version() that are in update-core.php would need to be removed as that file cannot contain functions that don't exist in the (old) version that is being updated, #62165. That seems to be the only place that may be "sensitive" about old vs. new version, and looks like it's best handled by including the appropriate version.php file.

Not really sure if anything else might be affected, seems quite unlikely. Nevertheless seems that maintaining the old behavior would be preferable just in case.

Trac ticket: Core-61782

Hi @peterwilsoncc , Hope you are doing well!

I had raised a similar PR before.
Reference: ​https://github.com/WordPress/wordpress-develop/pull/7105

[peterwilsoncc]

In 59192:

General: Memoize the return value in wp_get_wp_version().

Cache the unmodified $wp_version value as a static. This retains the current behaviour during the upgrade process $wp_version referencing the version of WordPress being upgraded from.

Follow up to [58848].

Props Cybr, debarghyabanerjee, mukesh27, costdev, SergeyBiryukov, TobiasBg, desrosj, azaozz.
Fixes #61782.

Merged r59192 / ​https://github.com/WordPress/wordpress-develop/commit/3528598f5a92a36122b728515bd96f58b99b82bc

@Debarghya-Banerjee Yeah, I based this off yours as it was easier to create a new one that deal with merge errors. There's been a bit of back and forth as to whether the value ought to be cached or not.