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/