Optimize spread operator for packed arrays (fix GH-9794)

[jhdxr]

Current implementation doesn't optimize for packed arrays as point out in #9794, and this PR adds the optimization.

I used the benchmark script provided in #9794 (with small modification).

const ARRAY_SIZE = 50;

$a = range(1,ARRAY_SIZE); $b = range(1,ARRAY_SIZE); $t = microtime(true); for ( $i = 0; $i < 100000; $i++ ) { $c = [...$a, ...$b]; }
echo "array unpack:", microtime(true)-$t, PHP_EOL;

$a = range(1,ARRAY_SIZE); $b = range(1,ARRAY_SIZE); $t = microtime(true); for ( $i = 0; $i < 100000; $i++ ) { $c = array_merge($a,$b); }
echo "array_merge:", microtime(true)-$t, PHP_EOL;

Before patching:
array unpack:0.3077449798584
array_merge:0.13217878341675

After patching:
array unpack:0.12740397453308
array_merge:0.13788986206055

[jhdxr]

The failed AppVeyor CI looks not related to this PR.

[TysonAndre]

Suggested change

	if (HT_IS_PACKED(ht) && (zend_hash_num_elements(result_ht) == 0 || HT_IS_PACKED(result_ht))) {
	if (HT_IS_PACKED(ht) && (!HT_IS_INITIALIZED(result_ht) || HT_IS_PACKED(result_ht))) {

I'm pretty sure there can be hash tables that are initialized but associative(not packed), e.g. $globalXyz = ['a' => 'val']; unset($globalXyz['a']); has zend_hash_num_elements === 0

1. oh, right, this is a temporary array being constructed and only being added to, that suggestion isn't technically needed here, but I expect my suggestion to be be faster anyway since both macros are checking HT_FLAGS from the same location in memory and the compiler could optimize that
2. HT_IS_INITIALIZED is a better example for other code and I'd personally strongly recommend that

Seems like it should behave correctly

[jhdxr]

Yeah this is just to check if the temp array needs to be mark as packed and/or extend to necessary size. zend_hash_num_elements is for new array.

I'll do a benchmark later to see there is any performance improvement, although I won't expect much difference. zend_hash_num_elements is inline and simple.

[TysonAndre]

since both macros are checking HT_FLAGS from the same location in memory and the compiler could optimize that

And following up, unsurprisingly, the compiler does optimize that (one less branch to predict, so I'd expect it to be better, though for it to be that noticeable you'd use a random mix of lists of length 0 and 1)

https://godbolt.org/z/sdaPEf8z9

void xyz(int n) {
    if ((n & (1<<2)) || !(n&(1<<3))) {
        puts("Matches one flag and not the other");
    }
}

e.g. the condition compiles to (x&12) != 8

        and     edi, 12
        cmp     edi, 8
        jne     .LBB0_2

[TysonAndre]

EDIT: I mixed up the packed check for result_ht and ht again, this still looks good.

---

Things that are correct but I double checked (no action is required):

• UNEXPECTED macro seems reasonable enough to add
• result_ht != ht is guaranteed by the fact that [$x, $y, ...$tmp] has a top level that's not yet assigned to value (the original code relies on the same fact)
• zend_hash_extend is correct and doesn't overflow for checking the sum of lengths of 2 valid arrays

[iluuu1994]

Looks correct to me.

[TysonAndre]

Is this something that other maintainers think would be a good idea to include in php 8.2 as well? (merged into 8.3-dev and realized this later)