count() optimization with !empty() check instead when possible

[samsonasik]

• Is this related to quality assurance?

[michalbundyra]

It fine, we can also use casting too bool:

return (bool) array_filter(...);

or:

return array_filter(...) ? true : false

not sure what is the most efficient... hah

[weierophinney]

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?

[samsonasik]

there is online benchmark for it http://maettig.com/code/php/php-performance-benchmarks.php#empty

[Ocramius]

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

[weierophinney]

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.

[samsonasik]

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%  |
+-----------------+---------------------+--------+--------+------+-----+------------+----------+----------+----------+----------+---------+--------+--------+

[weierophinney]

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

[samsonasik]

@weierophinney updated

[Ocramius]

🚢

[Ocramius]

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 👍