Phpinspectionsea: array_merge() being used on looping => docs

Created on 13 Feb 2017  路  16Comments  路  Source: kalessil/phpinspectionsea

EDIT: needs to be documented here


I'm getting the reporting about using of array_merge() in loopings. My question is: what is the best way to not do that?

For instance: the code below stores some data from $user, that is an array of int.

$dupesIds = [];

foreach ($users as $user) {
    // ...
    $dupesIds = array_merge($dupesIds, $user->some_ids_array);
}
question fixed

Most helpful comment

Okay, I think opcache is not used during the benchmarking. In the case opcache does pretty neat optimizations )

All 16 comments

Itshould be like this (saves looooots of memory allocation rounds, what leads to better performace as well)

$dupesIds = [];
foreach ($users as $user) {
    $dupesIds []= $user->some_ids_array;
}
$dupesIds = call_user_func_array('array_merge', $dupesIds)

I do some tests, and the problem is not the memory (in this case, inside looping is better), but the CPU usage (it's a looooooooot worse).

Can you put your example on plugin (description)? It can help to users to follow it. Currently description is very unclear about other options. 馃槃

From plugin: _Analyzes loops and reports if any resource-greedy array functions are being used (e.g. 'array_merge(...)')._


Some benchmarking (_7.000 iterations_):

Case 1: with array_merge() on looping (http://pastebin.com/acEdVLSE).
Note: over 7.000 iterations is impossible run this code on my PC. haha

Time: 953.20ms
Memory: 8.00Mb
Mem. Peak: 12.00Mb

Case 2: with array_merge() outside the looping with call_user_func_array() - _following your example_ (http://pastebin.com/QAqSDfEK).

Time: 3.00ms (-99.68% compared to case 1)
Memory: 2.00Mb (-75.00%)
Mem. Peak: 6.00Mb (-50.00%)


Extra case: with array_merge() outside the looping with call_user_func_array() - 250.000 iterations.
You can note that the memory usage is high, because you are storing a big array, instead of merging it in each looping, but it is more fast than first case.

Time: 112.80ms
Memory: 12.00Mb
Mem. Peak: 82.00Mb

You also can try prefixing call_user_func_array with root NS: '\call_user_func_array(...)' ;)

It can gain some extra execution time benefits in PHP7+

No changes. I guess that it's a micro-micro optimization haha. Running on PHP 7.1.

\count(\call_user_func_array('array_merge', $idsArray));

Time: 112.80ms (same time)
Memory: 12.00Mb (same memory)
Mem. Peak: 82.00Mb (same memory peak)

Okay, I think opcache is not used during the benchmarking. In the case opcache does pretty neat optimizations )

Documented here, feedback is welcome.

Sorry for posting in a closed issue but I think it's worth mentioning that PHP will throw a Fatal error (ArgumentCountError) if the foreach loop isn't executed at least once:

array_merge() expects at least 1 parameter, 0 given

something like

if (!empty($options) {
    $options = array_merge(...$options);
}

could be inserted

No problems, it's pretty handy actually as the context already here - I'll update the docs.

    $options = [[]]; // the inner array prevents errors when invoking the array_merge after loop
    ...

@rieschl : I updated the documentation, thanks for reporting =)

How about rewriting something like this without array_merge?

foreach ($results as $key => $value) {
    $results[$key]['breakdown'] = \array_merge($results[$key]['breakdown'], $extraAccessorials);
}

Basically, merging $extraAccessorials to all elements of $results

@akrz depending of your PHP version (requires 5.6+), you could do something like that:

$breakdownRaw = [];

foreach ($results as $key => $value) {
    // $extraAccessorials should be assigned at some point before.
    $breakdownRaw[] = $extraAccessorials;
}

$results[$key]['breakdown'] = \array_merge($results[$key]['breakdown'], ... $breakdownRaw);

Example: https://ideone.com/rmZ5jW (will reproduces [1, 2, 3, 4, 5, 6, 7, 8, 9]).

@rentalhost Thanks for responding.
That will merge only to the last element of $results. I want to merge to ALL elements.

Example of what I want: https://ideone.com/jgWMWo

With your suggestion, $extraAccessorials will end up in the last element of $results:
https://ideone.com/f8yF5k

Oh! Now I understand your case.

So I think that it should be just ignored by @noinspection. Except if we could find a pattern where it will not kill this inspection. (_Maybe ignore if array_merge() is merging an most external scope variable?_)

@akrz : this is definitely a false-positive, I created #1348 bug-report.

Spread comes to save the world.

Was this page helpful?
0 / 5 - 0 ratings