Apply substr() optimization to array_slice()

Yes, abstracting such additional checks through something like zend_hash_duplicate() would make sense, but IMHO it should be named differently, e.g. zend_hash_copy_lazy. So to be analogous to zend_string_copy(), but not to be confused with the existing zend_hash_copy().

By the way: array_pad() in L4320, array_unique() in L4476 and array_diff() in L5415 also return the array without those kind of checks (analougous to what I have proposed for the array_slice() case). These existing bugs would have to be fixed as well.

• Ben -

========== Original ==========
From: Sara Golemon pollita@php.net
To: Nikita Popov nikita.ppv@gmail.com
Date: Fri, 27 Oct 2017 18:34:15 +0200
Subject: Re: [PHP-DEV] Apply substr() optimization to array_slice()

On Fri, Oct 27, 2017 at 8:12 AM, Benjamin Coutu ben.coutu@zeyos.com
wrote:

Now, array_slice() could be optimized similarly, but currently
(unless the resulting array is expected to be empty) we always
create a new array no matter if we actually have to.

Pushed, with an additional escape hatch for vector-like arrays (which
are implicitly like preserve_keys). In the future though, please use
the PR process. Thanks.

Unfortunately these optimizations are subtly incorrect in the current form,
because arrays have a bunch of additional hidden state. See
https://bugs.php.net/bug.php?id=75433 for a (not yet fixed) issue that
resulted from similar optimizations in 7.2. We'll have to review all the
places where we apply optimizations like these and make sure that we're not
introducing incorrect behavior wrt the next free element or internal array
pointer.

It took awhile to unwind the repro case given in the bug report, but
this seems to be a simpler example:
https://3v4l.org/rqphD

Basically, that next insert index for the output of array_values()
should be reset, and with the optimization it's not.

For array_values() the quick and dirty fix is adding "&& nextIndex ==
num_elements" psuedocode. In the case of array_slice, the same will
be true, so I agree we should be careful about applying such
optimizations.

I'll clean up these uses now, and would suggest something like:

zend_array* zend_hash_duplicate(zend_array* input, zend_bool
preserve_keys); type API which can be a certral place for making that
kind of short-circuit versus slow-copy decision when called from
places like array_values() and array_slice().

-Sara