Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:100992
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: error (pb1.pair.com: domain zend.com from 104.47.37.125 cause and error)
To: Sara Golemon <pollita@php.net>, Nikita Popov <nikita.ppv@gmail.com>
CC: Benjamin Coutu <ben.coutu@zeyos.com>, PHP Internals
	<internals@lists.php.net>
Thread-Topic: [PHP-DEV] Apply substr() optimization to array_slice()
Thread-Index: AQHTTxz20/a4qJvkKEGmuNphC0E9pKL3vjIAgAAfsICAAAa+gIAEVjw+
Date: Mon, 30 Oct 2017 10:51:45 +0000
Message-ID: <BN6PR02MB3234032488352B1AE6E99C48BF590@BN6PR02MB3234.namprd02.prod.outlook.com>
References: <20171027121225.2AAC95FB05@mx.zeyos.com>
 <CAESVnVrbEYuFLAPFrkssL08KrEFrGyciQwPemaU06SHieT60=w@mail.gmail.com>
 <CAF+90c_xYPvE72s7pSnKdmrMcUOCTsjyPfr2VoM9t47utbG-EA@mail.gmail.com>,<CAESVnVrGUAmE2HD7M=R3vbora4S8-65FtEV0XrfrKBTsiOn--Q@mail.gmail.com>
In-Reply-To: <CAESVnVrGUAmE2HD7M=R3vbora4S8-65FtEV0XrfrKBTsiOn--Q@mail.gmail.com>
Accept-Language: en-US
Content-Language: en-US
received-spf: None (protection.outlook.com: zend.com does not designate
 permitted sender hosts)
spamdiagnosticoutput: 1:99
spamdiagnosticmetadata: NSPM
Content-Type: multipart/alternative;
	boundary="_000_BN6PR02MB3234032488352B1AE6E99C48BF590BN6PR02MB3234namp_"
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-originalarrivaltime: 30 Oct 2017 10:51:45.9293
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 32210298-c08b-4829-8097-6b12c025a892
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN6PR02MB3235
Subject: Re: [PHP-DEV] Apply substr() optimization to array_slice()
From: dmitry@zend.com (Dmitry Stogov)

--_000_BN6PR02MB3234032488352B1AE6E99C48BF590BN6PR02MB3234namp_
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable

The optimization also broke Zend/tests/bug72598.phpt and Zend/tests/bug7259=
8_2.phpt

________________________________
From: php@golemon.com <php@golemon.com> on behalf of Sara Golemon <pollita@=
php.net>
Sent: Friday, October 27, 2017 7:34:15 PM
To: Nikita Popov
Cc: Benjamin Coutu; PHP Internals; Dmitry Stogov
Subject: Re: [PHP-DEV] Apply substr() optimization to array_slice()

On Fri, Oct 27, 2017 at 12:10 PM, Nikita Popov <nikita.ppv@gmail.com> wrote=
:
> On Fri, Oct 27, 2017 at 4:16 PM, Sara Golemon <pollita@php.net> wrote:
>>
>> 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 for=
m,
> because arrays have a bunch of additional hidden state. See
> https://bugs.php.net/bug.php?id=3D75433 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 n=
ot
> introducing incorrect behavior wrt the next free element or internal arra=
y
> 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 =3D=3D
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

--_000_BN6PR02MB3234032488352B1AE6E99C48BF590BN6PR02MB3234namp_--