Hey:

On Wed, Aug 3, 2016 at 10:48 PM, Benjamin Coutu <ben.co...@zeyos.com> wrote:

> Hello Xinchen,
>
> Thanks for changing array_pad and array_rand accordingly, that's very good.
>
> I noticed a small improvement we could make to array_slice for the packed
> case:
>
> We can change line 3003:
> if ((Z_ARRVAL_P(input)->u.flags & HASH_FLAG_PACKED) && !preserve_keys)
> =>
> if ((Z_ARRVAL_P(input)->u.flags & HASH_FLAG_PACKED) && (!preserve_keys ||
> offset == 0))
>
> So whenever the input array is packed, we can of course safely use a
> packed array for the output array when preserve_keys is false, but also
> when preserve_keys is true and if (only if) the offset is zero, because
> offset=0 on a packed array will yield the same result no matter whether
> preserved_keys is true or false.
>
packed array doesn't means alwasy start with index 0,

like:

    $array = array(1 => 1, 2, 3, 4, 5, 6);

this is also a packed array

so,  we can not do such opt only if offset == 0

thanks

>
> Ben
>
>
> ========== Original ==========
> From: Xinchen Hui <xinche...@zend.com>
> To: Benjamin Coutu <ben.co...@zeyos.com>
> Date: Tue, 02 Aug 2016 08:32:14 +0200
> Subject: Re: [PHP-DEV] More packed hash optimizations in array.c
>
> Hey:
>
> On Tue, Aug 2, 2016 at 2:21 PM, Benjamin Coutu <ben.co...@zeyos.com>
> wrote:
>
>> Hello Xinchen,
>>
>> Thanks for implementing.
>> Unfortunatly we just broke array_column by mistake. My description might
>> not have been clear enough.
>>
>> We can only create a packed array when $index_key=null AND if the input
>> array was already a packed array, because array_column must preserve
>> keys if index_key=null.
>>
>  Hmm, it should not, I reviewed the origin implementation:
>
>      if (zkey) {
>             zkeyval = array_column_fetch_prop(data, zkey, &rvk);
>         }
>
>         Z_TRY_ADDREF_P(zcolval);
>         if (zkeyval && Z_TYPE_P(zkeyval) == IS_STRING) {
>             zend_symtable_update(Z_ARRVAL_P(return_value),
> Z_STR_P(zkeyval), zcolval);
>         } else if (zkeyval && Z_TYPE_P(zkeyval) == IS_LONG) {
>             add_index_zval(return_value, Z_LVAL_P(zkeyval), zcolval);
>         } else if (zkeyval && Z_TYPE_P(zkeyval) == IS_OBJECT) {
>             zend_string *key = zval_get_string(zkeyval);
>             zend_symtable_update(Z_ARRVAL_P(return_value), key, zcolval);
>             zend_string_release(key);
>         } else {
>             add_next_index_zval(return_value, zcolval);
>         }
>
>     if zkey is NULL(index_key),  then only add_next_index_zval is executed.
>
>     do you have any reproduce script to show the broken?
>
> thanks
>
>>
>> Cheers,
>>
>> Benjamin
>>
>> ========== Original ==========
>> From: Xinchen Hui <xinche...@zend.com>
>> To: Benjamin Coutu <ben.co...@zeyos.com>
>> Date: Tue, 02 Aug 2016 06:45:18 +0200
>> Subject: Re: [PHP-DEV] More packed hash optimizations in array.c
>>
>>
>> Hey
>>
>> On Thu, Jul 28, 2016 at 4:59 PM, Benjamin Coutu <ben.co...@zeyos.com>
>> wrote:
>>
>>> Hello Xinchen,
>>>
>>> I have noticed two more cases where we could easily use packed arrays.
>>>
>>> 1. array_merge($packed1, $packed2, ...):
>>>
>>> In the quite common case where all arguments are packed arrays, the
>>> resulting array can also be a packed array (as per documentation: "if the
>>> input arrays [...] contain numeric keys, the later value will not overwrite
>>> the original value, but will be appended"), thereby ensuring that packed
>>> arrays stay packed when appended to one another.
>>> In general we should try to preserve the packed characteristics wherever
>>> possible.
>>>
>> I will need do some benchmark, as it is already very simple
>> implementation now.
>>
>>>
>>> 2. array_column($input, $column_key, $index_key=null):
>>>
>>> For the common case when $index_key=null, we can just create a packed
>>> array.
>>> Also (not related to packing), we could pre-initialize the return_value
>>> array size with array_init_size() if, and only if, $column_key=null.
>>>
>> this one is committed here:
>> https://github.com/php/php-src/commit/fea2042a47dbda7210306c881a9f0a82b8503a45
>>
>> thanks
>>
>>>
>>> Please let me know your thoughts.
>>>
>>> Thanks,
>>>
>>> Ben
>>>
>>> --
>>>
>>> Bejamin Coutu
>>> ben.co...@zeyos.com
>>>
>>> ZeyOS, Inc.
>>> http://www.zeyos.com
>>>
>>>
>>
>>
>> --
>> Xinchen Hui
>> @Laruence
>> http://www.laruence.com/
>>
>
>
>
> --
> Xinchen Hui
> @Laruence
> http://www.laruence.com/
>



-- 
Xinchen Hui
@Laruence
http://www.laruence.com/

Reply via email to