Make WordPress Core

Opened 2 years ago

Closed 2 years ago

Last modified 20 months ago

#55680 closed enhancement (fixed)

Replace `phpversion()` function calls with PHP_VERSION

Reported by: ayeshrajans's profile ayeshrajans Owned by: sergeybiryukov's profile SergeyBiryukov
Milestone: 6.1 Priority: normal
Severity: normal Version: 6.0
Component: General Keywords: has-patch has-unit-tests add-to-field-guide
Focuses: performance Cc:

Description

We use several phpversion() functions in WordPress core, which I propose to be replaced with PHP_VERSION constants.

phpversion() return value and PHP_VERSION constant value are identical, but the former is about 3 times slower because it is a function call compared to a direct constant value lookup.

Simple benchmark:

<?php

$start = microtime(true);
for ($i = 0; $i < 1000000; $i++) {phpversion();}
echo microtime(true) - $start;

echo "\r\n";

$start = microtime(true);
for ($i = 0; $i < 1000000; $i++) {PHP_VERSION;}
echo microtime(true) - $start;

Change History (12)

This ticket was mentioned in PR #2674 on WordPress/wordpress-develop by Ayesh.


2 years ago
#1

  • Keywords has-unit-tests added

Trac ticket: https://core.trac.wordpress.org/ticket/55680#ticket

We use several phpversion() function calls in WordPress core, which I propose to be replaced with PHP_VERSION constants.

phpversion() return value and PHP_VERSION constant value are identical, but the former is about 3 times slower because it is a function call compared to a direct constant value lookup.

Simple benchmark:

{{{php

$start = microtime(true);
for ($i = 0; $i < 1000000; $i++) {phpversion();}
echo microtime(true) - $start;

echo "\r\n";

$start = microtime(true);
for ($i = 0; $i < 1000000; $i++) {PHP_VERSION;}
echo microtime(true) - $start;

}}}

#2 @SergeyBiryukov
2 years ago

  • Milestone changed from Awaiting Review to 6.1

Ayesh commented on PR #2674:


2 years ago
#3

@costdev you are right, it absolutely makes sense to change the function_exists('phpversion) do the defined`, which I have now done.

Apart from this PR, I think we could use a good cleanup of these function_exists calls we have accumulated over the years, which are now either bundled into core, is past the version test.

jrfnl commented on PR #2674:


2 years ago
#4

@costdev you are right, it absolutely makes sense to change the function_exists('phpversion)do thedefined`, which I have now done.

Actually.... I think that check can probably be removed...

If I remember correctly, those kind of checks will have been put in place for silly hosts which think adding phpversion to the `disabled_functions` ini directive increases security (it does not).

I'm trying to think, but I can't remember any way (for hosts) to prevent PHP from declaring the PHP_VERSION constant.

Unless someone else comes up with a way that could be done, that function_exists()/defined() check is no longer needed.

Apart from this PR, I think we could use a good cleanup of these function_exists calls we have accumulated over the years, which are now either bundled into core, is past the version test. Makes the code cleaner, and a happy path for OPCache.

➕ Agreed. I remember doing such a check for WP 5.3 (after the PHP 5.2 version drop), but I doubt it's been done since.

Ayesh commented on PR #2674:


2 years ago
#5

Unless someone else comes up with a way that could be done, that function_exists()/defined() check is no longer needed.

Happily got rid of that check, thank you!

#6 @SergeyBiryukov
2 years ago

  • Owner set to SergeyBiryukov
  • Status changed from new to reviewing

SergeyBiryukov commented on PR #2674:


2 years ago
#7

phpversion() return value and PHP_VERSION constant value are identical, but the former is about 3 times slower because it is a function call compared to a direct constant value lookup.

Simple benchmark:

$start = microtime(true);
for ($i = 0; $i < 1000000; $i++) {phpversion();}
echo microtime(true) - $start;

echo "\r\n";

$start = microtime(true);
for ($i = 0; $i < 1000000; $i++) {PHP_VERSION;}
echo microtime(true) - $start;

I was curious about the results of this benchmark. On my install, after a few runs, PHP_VERSION on average appears to be 5 times faster than phpversion(), making the performance improvement even more noticeable:

phpversion(): 0.0367081165314
PHP_VERSION:  0.00715589523315

Should be good to go.

SergeyBiryukov commented on PR #2674:


2 years ago
#8

On my install, after a few runs, PHP_VERSION on overage appears to be 5 times faster than phpversion(), making the performance improvement even more noticeable

For reference, this was on PHP 8.0 and 8.1. On PHP 7.4, the difference is indeed closer to 3 times.

#9 @SergeyBiryukov
2 years ago

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

In 53426:

Code Modernization: Replace phpversion() function calls with PHP_VERSION constant.

phpversion() return value and PHP_VERSION constant value are identical, but the latter is several times faster because it is a direct constant value lookup compared to a function call.

Props ayeshrajans, jrf, mukesh27, costdev, hellofromTonya, SergeyBiryukov.
Fixes #55680.

Ayesh commented on PR #2674:


2 years ago
#11

Thank you so much!

#12 @milana_cap
20 months ago

  • Keywords add-to-field-guide added
Note: See TracTickets for help on using tickets.