Hi

On 4/6/23 00:12, Vorisek, Michael wrote:
> Hello,
> 
> I would like to open a discussion for 
> https://github.com/php/php-src/issues/10791 .
> [https://opengraph.githubassets.com/a23cb565cc8acac6a33ecab5d9ee68a46f046a1ffe215501673156e506695430/php/php-src/issues/10791]<https://github.com/php/php-src/issues/10791>
> Array spread append · Issue #10791 · 
> php/php-src<https://github.com/php/php-src/issues/10791>
> Description Currently spread operator can be used for almost anything. But 
> not for array append. I propose the following to be supported: <?php $arr = 
> [1, 2]; $arr2 = [3, 4]; $arr[...] = $arr2; // ...
> github.com
> Appending N elements to an array is quite common language usage pattern and I 
> belive it should be supported natively for shorter syntax, language 
> consistency and performance.
> 
> I am unable to implement it, but I am ready to help with RFC.
> 
> Michael Vorisek
> 

As far as I understand, this is somewhat of a continuation of 
https://github.com/php/php-src/issues/9881, in which the concern about the 
array_merge performance was posted. This in turn resulted in this feature 
proposal if I understand correctly.

I think $x = array_merge($x, ...); is probably a common pattern. Creating a new 
function or a new syntax will make it more difficult for users to know when to 
use the array_merge approach or when to use the new append approach.

I saw the original comments on the array_merge GitHub thread, and read about 
the optimization difficulty.
The reason there's a performance penalty is due the first array always being 
copied because:
- it may be modified (i.e. keys made sequential)
- it may be shared in more places

However, I think it might be worth adding an optimization for the most common 
$x = array_merge($x, ...) case.
To do this:
- we need to check if the array is sequential. For packed array this can be 
done with HT_IS_WITHOUT_HOLES().
- we need to detect that coding pattern. This can be done by checking if 
there's a ZEND_ASSIGN after the call that assigns the result to the first 
passed array variable.
- array must not be immutable
- array must have refcount 2: once as variable and once as argument

I implemented this optimization in the following commit on my fork: 
https://github.com/nielsdos/php-src/commit/269d547043c9d3c59d7c9d773eb1ef174dcc5c44
For now it's limited to packed arrays, and it's only a proof-of-concept. I'm 
also not sure if it's 100% correct, I'd need to do some more debugging / 
testing to know for sure, but I'm short on time now.
The performance improvement for the array_merge case in 
https://github.com/php/php-src/issues/10993#issuecomment-1493166391 is big:
1.32s without my patch, vs 0.008s with my patch

I think this could be made more generic, and be cleaned up.
But I don't know if something like this is desired in PHP.

Kinds regards
Niels

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php

Reply via email to