Closed samsonasik closed 6 years ago
Are there any benchmarks on these optimizations?
Honestly, I find count() > 0
to be easier to understand when we're working with a suite of array operations. It clearly defines that we are looking for an array with at least one item in it when done. Because empty
has so many semantics in PHP, I have to look at the context (the array operations) to understand what empty
might be testing for.
I guess I'm wondering if the optimizations will offer any measurable benefit that warrants reviewing, merging, and releasing these dozens of patches, particularly when there may be good rationale to leave the code as-is. Can you provide the background that led to these pull requests, please?
there is online benchmark for it http://maettig.com/code/php/php-performance-benchmarks.php#empty
That's VERY old: don't base yourself on it.
If we want to go with benchmarks, we should use our own benchmarks: https://github.com/zendframework/zend-stdlib/tree/7c7663c7c580563a14ac03783a981452254d1052/benchmark
Based on that link, @samsonasik, I'd argue that the approach that is more semantically close to what we want to demonstrate then is <expression> !== []
, as that has comparable performance to ! empty(<expression>)
, but more correctly demonstrates that an array with no items is undesirable.
But as @Ocramius notes: we have a benchmark suite in this repo already. Add benchmarks so we can prove it.
I've added the benchmark, this is on count-op
branch result:
$ vendor/bin/phpbench run --revs=2 --iterations=2 --report=aggregate benchmark/ArrayUtilsBench.php
PhpBench 0.13.0. Running benchmarks.
Using configuration file: /Users/samsonasik/www/zf-components-contribute/zend-stdlib-1/phpbench.json
\ZendBench\Stdlib\ArrayUtilsBench
benchHasStringKeys R2 I1 P0 [μ Mo]/r: 8.500 8.500 (μs) [μSD μRSD]/r: 0.000μs 0.00%
benchHasIntegerKeys R2 I1 P0 [μ Mo]/r: 8.500 8.500 (μs) [μSD μRSD]/r: 0.000μs 0.00%
benchHasNumericKeys R2 I1 P0 [μ Mo]/r: 8.750 8.750 (μs) [μSD μRSD]/r: 0.250μs 2.86%
3 subjects, 6 iterations, 6 revs, 0 rejects
(best [mean mode] worst) = 8.500 [8.583 8.583] 8.500 (μs)
⅀T: 51.500μs μSD/r 0.083μs μRSD/r: 0.952%
suite: 133eee789834ba0dd50ca7a56cbb70793bb0a9b8, date: 2018-07-11, stime: 02:25:52
+-----------------+---------------------+--------+--------+------+-----+------------+---------+---------+---------+---------+---------+--------+--------+
| benchmark | subject | groups | params | revs | its | mem_peak | best | mean | mode | worst | stdev | rstdev | diff |
+-----------------+---------------------+--------+--------+------+-----+------------+---------+---------+---------+---------+---------+--------+--------+
| ArrayUtilsBench | benchHasStringKeys | | [] | 2 | 2 | 1,241,072b | 8.500μs | 8.500μs | 8.500μs | 8.500μs | 0.000μs | 0.00% | 0.00% |
| ArrayUtilsBench | benchHasIntegerKeys | | [] | 2 | 2 | 1,241,072b | 8.500μs | 8.500μs | 8.500μs | 8.500μs | 0.000μs | 0.00% | 0.00% |
| ArrayUtilsBench | benchHasNumericKeys | | [] | 2 | 2 | 1,241,072b | 8.500μs | 8.750μs | 8.750μs | 9.000μs | 0.250μs | 2.86% | +2.94% |
+-----------------+---------------------+--------+--------+------+-----+------------+---------+---------+---------+---------+---------+--------+--------+
cherry-picked to master
branch with count()
usage, got result:
$ vendor/bin/phpbench run --revs=2 --iterations=2 --report=aggregate benchmark/ArrayUtilsBench.php
PhpBench 0.13.0. Running benchmarks.
Using configuration file: /Users/samsonasik/www/zf-components-contribute/zend-stdlib-1/phpbench.json
\ZendBench\Stdlib\ArrayUtilsBench
benchHasStringKeys R2 I1 P0 [μ Mo]/r: 9.500 9.500 (μs) [μSD μRSD]/r: 0.000μs 0.00%
benchHasIntegerKeys R2 I1 P0 [μ Mo]/r: 10.250 10.250 (μs) [μSD μRSD]/r: 0.250μs 2.44%
benchHasNumericKeys R2 I1 P0 [μ Mo]/r: 9.500 9.500 (μs) [μSD μRSD]/r: 0.000μs 0.00%
3 subjects, 6 iterations, 6 revs, 0 rejects
(best [mean mode] worst) = 9.500 [9.750 9.750] 9.500 (μs)
⅀T: 58.500μs μSD/r 0.083μs μRSD/r: 0.813%
suite: 133eee7c335c63029c1f2ab2dadf5277a0d1f232, date: 2018-07-11, stime: 02:27:42
+-----------------+---------------------+--------+--------+------+-----+------------+----------+----------+----------+----------+---------+--------+--------+
| benchmark | subject | groups | params | revs | its | mem_peak | best | mean | mode | worst | stdev | rstdev | diff |
+-----------------+---------------------+--------+--------+------+-----+------------+----------+----------+----------+----------+---------+--------+--------+
| ArrayUtilsBench | benchHasStringKeys | | [] | 2 | 2 | 1,241,552b | 9.500μs | 9.500μs | 9.500μs | 9.500μs | 0.000μs | 0.00% | 0.00% |
| ArrayUtilsBench | benchHasIntegerKeys | | [] | 2 | 2 | 1,241,552b | 10.000μs | 10.250μs | 10.250μs | 10.500μs | 0.250μs | 2.44% | +7.89% |
| ArrayUtilsBench | benchHasNumericKeys | | [] | 2 | 2 | 1,241,552b | 9.500μs | 9.500μs | 9.500μs | 9.500μs | 0.000μs | 0.00% | 0.00% |
+-----------------+---------------------+--------+--------+------+-----+------------+----------+----------+----------+----------+---------+--------+--------+
@samsonasik Honestly, that demonstrates almost no measurable difference. I'd prefer we go with [] !== <expression>
over ! empty(<expression>)
as it's easier to understand in this context (which includes a variety of array_*()
operations in the expressions).
@weierophinney updated
Note: I'm merging the patch as-is, but the most effective performance optimisation here would be to loop and bail out early if a value is found. Would be interesting to have such functional construct in the library 👍