mirror of
https://github.com/php/php-src.git
synced 2025-08-15 21:48:51 +02:00
array_find: Remove unnecessary refcounting (#17536)
* array_find: Fix data type for `retval_true` * array_find: Remove unnecessary refcounting In a post on LinkedIn [1], Bohuslav Šimek reported that the native implementation of `array_find()` was about 3× slower than the equivalent userland implementation. While I was not able to verify this claim, due to a lack of reproducer provided, I could confirm that the native `array_find()` was indeed slower than the equivalent userland implementation. For the following example script: <?php function my_array_find(array $array, callable $callback): mixed { foreach ($array as $key => $value) { if ($callback($value, $key)) { return $value; } } return null; } $array = range(1, 10000); $result = 0; for ($i = 0; $i < 5000; $i++) { $result += array_find($array, static function ($item) { return $item === 5000; }); } var_dump($result); with the `array_find()` call appropriately replaced for each case, a PHP-8.4 release build provided the following results: Benchmark 1: /tmp/before native.php Time (mean ± σ): 765.9 ms ± 7.9 ms [User: 761.1 ms, System: 4.4 ms] Range (min … max): 753.2 ms … 774.7 ms 10 runs Benchmark 2: /tmp/before userland.php Time (mean ± σ): 588.0 ms ± 17.9 ms [User: 583.6 ms, System: 4.1 ms] Range (min … max): 576.0 ms … 633.3 ms 10 runs Summary /tmp/before userland.php ran 1.30 ± 0.04 times faster than /tmp/before native.php Running `native.php` with perf reports that a third of the time is spent in `zend_call_function()` and another 20% in `execute_ex()`, however `php_array_find()` comes next at 14%: # Samples: 3K of event 'cpu_core/cycles/' # Event count (approx.): 2895247444 # # Overhead Command Shared Object Symbol # ........ ....... ................. ........................................... # 32.47% before before [.] zend_call_function 20.63% before before [.] execute_ex 14.06% before before [.] php_array_find 7.89% before before [.] ZEND_IS_IDENTICAL_SPEC_CV_CONST_HANDLER 7.31% before before [.] zend_init_func_execute_data 6.50% before before [.] zend_copy_extra_args which was surprising, because the function doesn’t too all that much. Looking at the implementation, the refcounting stood out and it turns out that it is not actually necessary. The `array` is passed by value to `array_find()` and thus cannot magically change within the callback. This also means that the array will continue to hold a reference to string keys and values, preventing these values from being collected. The refcounting inside of `php_array_find()` thus will never do anything useful. Comparing the updated implementation against the original implementation shows that this change results in a 1.14× improvement: Benchmark 1: /tmp/before native.php Time (mean ± σ): 775.4 ms ± 29.6 ms [User: 771.6 ms, System: 3.5 ms] Range (min … max): 740.2 ms … 844.4 ms 10 runs Benchmark 2: /tmp/after native.php Time (mean ± σ): 677.3 ms ± 16.7 ms [User: 673.9 ms, System: 3.1 ms] Range (min … max): 655.9 ms … 705.0 ms 10 runs Summary /tmp/after native.php ran 1.14 ± 0.05 times faster than /tmp/before native.php Comparing the native implementation against the userland implementation with the new implementation shows that while the native implementation still is slower, the difference reduced to 15% (down from 30%): Benchmark 1: /tmp/after native.php Time (mean ± σ): 670.4 ms ± 9.3 ms [User: 666.7 ms, System: 3.4 ms] Range (min … max): 657.1 ms … 689.0 ms 10 runs Benchmark 2: /tmp/after userland.php Time (mean ± σ): 576.7 ms ± 7.6 ms [User: 572.5 ms, System: 3.7 ms] Range (min … max): 563.9 ms … 588.1 ms 10 runs Summary /tmp/after userland.php ran 1.16 ± 0.02 times faster than /tmp/after native.php Looking at the updated perf results shows that `php_array_find()` now only takes up 8% of the time: # Samples: 2K of event 'cpu_core/cycles/' # Event count (approx.): 2540947159 # # Overhead Command Shared Object Symbol # ........ ....... .................... ........................................... # 34.77% after after [.] zend_call_function 18.57% after after [.] execute_ex 12.28% after after [.] zend_copy_extra_args 10.91% after after [.] zend_init_func_execute_data 8.77% after after [.] php_array_find 6.70% after after [.] ZEND_IS_IDENTICAL_SPEC_CV_CONST_HANDLER 4.68% after after [.] zend_is_identical [1] https://www.linkedin.com/posts/bohuslav-%C5%A1imek-kambo_the-surprising-performance-of-php-84-activity-7287044532280414209-6WnA * array_find: Clean up exception handling This change has no effect on performance, but greatly improves readability of the implementation.
This commit is contained in:
parent
3786b721e8
commit
c39d112620
1 changed files with 20 additions and 28 deletions
|
@ -6629,16 +6629,19 @@ static zend_result php_array_find(const HashTable *array, zend_fcall_info fci, z
|
||||||
if (!str_key) {
|
if (!str_key) {
|
||||||
ZVAL_LONG(&args[1], num_key);
|
ZVAL_LONG(&args[1], num_key);
|
||||||
} else {
|
} else {
|
||||||
ZVAL_STR_COPY(&args[1], str_key);
|
ZVAL_STR(&args[1], str_key);
|
||||||
}
|
}
|
||||||
|
|
||||||
ZVAL_COPY(&args[0], operand);
|
ZVAL_COPY_VALUE(&args[0], operand);
|
||||||
|
|
||||||
zend_result result = zend_call_function(&fci, &fci_cache);
|
zend_result result = zend_call_function(&fci, &fci_cache);
|
||||||
if (EXPECTED(result == SUCCESS)) {
|
ZEND_ASSERT(result == SUCCESS);
|
||||||
int retval_true;
|
|
||||||
|
|
||||||
retval_true = zend_is_true(&retval);
|
if (UNEXPECTED(EG(exception))) {
|
||||||
|
return FAILURE;
|
||||||
|
}
|
||||||
|
|
||||||
|
bool retval_true = zend_is_true(&retval);
|
||||||
zval_ptr_dtor(&retval);
|
zval_ptr_dtor(&retval);
|
||||||
|
|
||||||
/* This negates the condition, if negate_condition is true. Otherwise it does nothing with `retval_true`. */
|
/* This negates the condition, if negate_condition is true. Otherwise it does nothing with `retval_true`. */
|
||||||
|
@ -6653,18 +6656,7 @@ static zend_result php_array_find(const HashTable *array, zend_fcall_info fci, z
|
||||||
ZVAL_COPY(result_key, &args[1]);
|
ZVAL_COPY(result_key, &args[1]);
|
||||||
}
|
}
|
||||||
|
|
||||||
zval_ptr_dtor(&args[0]);
|
break;
|
||||||
zval_ptr_dtor(&args[1]);
|
|
||||||
|
|
||||||
return SUCCESS;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
zval_ptr_dtor(&args[0]);
|
|
||||||
zval_ptr_dtor(&args[1]);
|
|
||||||
|
|
||||||
if (UNEXPECTED(result != SUCCESS)) {
|
|
||||||
return FAILURE;
|
|
||||||
}
|
}
|
||||||
} ZEND_HASH_FOREACH_END();
|
} ZEND_HASH_FOREACH_END();
|
||||||
|
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue