From 8a67789d551907b0d6291293d0c98c472f7fe1cf Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Wed, 26 Mar 2025 23:14:41 +0100 Subject: [PATCH 1/2] Improve performance of array_find() etc MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This avoids destruction logic for the common case, avoids some copy, and adds an optimization hint. On an intel i7 1185G7: ``` Benchmark 1: ./sapi/cli/php x.php Time (mean ± σ): 543.7 ms ± 3.8 ms [User: 538.9 ms, System: 4.4 ms] Range (min … max): 538.4 ms … 552.9 ms 10 runs Benchmark 2: ./sapi/cli/php_old x.php Time (mean ± σ): 583.0 ms ± 4.2 ms [User: 578.4 ms, System: 3.4 ms] Range (min … max): 579.3 ms … 593.9 ms 10 runs Summary ./sapi/cli/php x.php ran 1.07 ± 0.01 times faster than ./sapi/cli/php_old x.php ``` On an intel i7 4790: ``` Benchmark 1: ./sapi/cli/php x.php Time (mean ± σ): 828.6 ms ± 4.8 ms [User: 824.4 ms, System: 1.6 ms] Range (min … max): 822.8 ms … 839.0 ms 10 runs Benchmark 2: ./sapi/cli/php_old x.php Time (mean ± σ): 940.1 ms ± 26.4 ms [User: 934.4 ms, System: 2.5 ms] Range (min … max): 918.0 ms … 981.1 ms 10 runs Summary ./sapi/cli/php x.php ran 1.13 ± 0.03 times faster than ./sapi/cli/php_old x.php ``` --- ext/standard/array.c | 31 ++++++++++++++++++++----------- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/ext/standard/array.c b/ext/standard/array.c index 8462492651310..3fe059a426771 100644 --- a/ext/standard/array.c +++ b/ext/standard/array.c @@ -6597,7 +6597,7 @@ enum php_array_find_result { PHP_ARRAY_FIND_SOME = 1, }; -static enum php_array_find_result php_array_find(const HashTable *array, zend_fcall_info fci, zend_fcall_info_cache fci_cache, zval *result_key, zval *result_value, bool negate_condition) +static enum php_array_find_result php_array_find(const HashTable *array, zend_fcall_info fci, zend_fcall_info_cache *fci_cache, zval *result_key, zval *result_value, bool negate_condition) { zend_ulong num_key; zend_string *str_key; @@ -6620,23 +6620,32 @@ static enum php_array_find_result php_array_find(const HashTable *array, zend_fc if (!str_key) { ZVAL_LONG(&args[1], num_key); } else { + ZEND_ASSUME(!HT_IS_PACKED(array)); ZVAL_STR(&args[1], str_key); } ZVAL_COPY_VALUE(&args[0], operand); - zend_result result = zend_call_function(&fci, &fci_cache); + zend_result result = zend_call_function(&fci, fci_cache); ZEND_ASSERT(result == SUCCESS); if (UNEXPECTED(Z_ISUNDEF(retval))) { return PHP_ARRAY_FIND_EXCEPTION; } - bool retval_true = zend_is_true(&retval); - zval_ptr_dtor(&retval); - - /* This negates the condition, if negate_condition is true. Otherwise it does nothing with `retval_true`. */ - retval_true ^= negate_condition; + bool retval_true; + switch (Z_TYPE(retval)) { + case IS_TRUE: + retval_true = !negate_condition; + break; + case IS_FALSE: + retval_true = negate_condition; + break; + default: + retval_true = zend_is_true(&retval) ^ negate_condition; + zval_ptr_dtor(&retval); + break; + } if (retval_true) { if (result_value != NULL) { @@ -6667,7 +6676,7 @@ PHP_FUNCTION(array_find) Z_PARAM_FUNC(fci, fci_cache) ZEND_PARSE_PARAMETERS_END(); - php_array_find(array, fci, fci_cache, NULL, return_value, false); + php_array_find(array, fci, &fci_cache, NULL, return_value, false); } /* }}} */ @@ -6683,7 +6692,7 @@ PHP_FUNCTION(array_find_key) Z_PARAM_FUNC(fci, fci_cache) ZEND_PARSE_PARAMETERS_END(); - php_array_find(array, fci, fci_cache, return_value, NULL, false); + php_array_find(array, fci, &fci_cache, return_value, NULL, false); } /* }}} */ @@ -6699,7 +6708,7 @@ PHP_FUNCTION(array_any) Z_PARAM_FUNC(fci, fci_cache) ZEND_PARSE_PARAMETERS_END(); - RETURN_BOOL(php_array_find(array, fci, fci_cache, NULL, NULL, false) == PHP_ARRAY_FIND_SOME); + RETURN_BOOL(php_array_find(array, fci, &fci_cache, NULL, NULL, false) == PHP_ARRAY_FIND_SOME); } /* }}} */ @@ -6715,7 +6724,7 @@ PHP_FUNCTION(array_all) Z_PARAM_FUNC(fci, fci_cache) ZEND_PARSE_PARAMETERS_END(); - RETURN_BOOL(php_array_find(array, fci, fci_cache, NULL, NULL, true) == PHP_ARRAY_FIND_NONE); + RETURN_BOOL(php_array_find(array, fci, &fci_cache, NULL, NULL, true) == PHP_ARRAY_FIND_NONE); } /* }}} */ From 2fb5ea915d33eeb53a8a0fc6c879d01e10713728 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Thu, 27 Mar 2025 21:19:20 +0100 Subject: [PATCH 2/2] review --- ext/standard/array.c | 38 ++++++++++++++++++++------------------ 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/ext/standard/array.c b/ext/standard/array.c index 3fe059a426771..d26e238180262 100644 --- a/ext/standard/array.c +++ b/ext/standard/array.c @@ -6509,6 +6509,22 @@ PHP_FUNCTION(array_reduce) } /* }}} */ +/* Consumes `zv` */ +static bool php_is_true(zval *zv) +{ + switch (Z_TYPE_P(zv)) { + case IS_TRUE: + return true; + case IS_FALSE: + return false; + default: { + bool rv = zend_is_true(zv); + zval_ptr_dtor(zv); + return rv; + } + } +} + /* {{{ Filters elements from the array via the callback. */ PHP_FUNCTION(array_filter) { @@ -6571,9 +6587,7 @@ PHP_FUNCTION(array_filter) RETURN_THROWS(); } - bool retval_true = zend_is_true(&retval); - zval_ptr_dtor(&retval); - if (!retval_true) { + if (!php_is_true(&retval)) { continue; } } else if (!zend_is_true(operand)) { @@ -6620,6 +6634,8 @@ static enum php_array_find_result php_array_find(const HashTable *array, zend_fc if (!str_key) { ZVAL_LONG(&args[1], num_key); } else { + /* Allows copying the numeric branch, without this branch, into the iteration code + * that checks for the packed array flag. */ ZEND_ASSUME(!HT_IS_PACKED(array)); ZVAL_STR(&args[1], str_key); } @@ -6633,21 +6649,7 @@ static enum php_array_find_result php_array_find(const HashTable *array, zend_fc return PHP_ARRAY_FIND_EXCEPTION; } - bool retval_true; - switch (Z_TYPE(retval)) { - case IS_TRUE: - retval_true = !negate_condition; - break; - case IS_FALSE: - retval_true = negate_condition; - break; - default: - retval_true = zend_is_true(&retval) ^ negate_condition; - zval_ptr_dtor(&retval); - break; - } - - if (retval_true) { + if (php_is_true(&retval) ^ negate_condition) { if (result_value != NULL) { ZVAL_COPY_DEREF(result_value, &args[0]); }