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 <
[email protected]>
To: Nikita Popov <
[email protected]>
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 12:10 PM, Nikita Popov <
[email protected]> wrote:
> > On Fri, Oct 27, 2017 at 4:16 PM, Sara Golemon <
[email protected]> wrote:
> >>
> >> On Fri, Oct 27, 2017 at 8:12 AM, Benjamin Coutu <
[email protected]>
> >> 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
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit:
http://www.php.net/unsub.php