Re: Re: Improvements to array.c code base

Hey: On Tue, Jan 20, 2015 at 5:21 PM, Benjamin Coutu <ben.coutu@zeyos.com> wrote: > Sorry, your right of course as not preserving keys only applies to numeric indices. np, advise and questions on performance are always welcome :) thanks > > ========== Original ========== > From: Xinchen Hui <xinchen.h@zend.com> > To: Benjamin Coutu <ben.coutu@zeyos.com> > Date: Tue, 20 Jan 2015 10:13:04 +0100 > Subject: Re: [PHP-DEV] Re: Improvements to array.c code base > > > > Hey: > > On Tue, Jan 20, 2015 at 5:06 PM, Benjamin Coutu <ben.coutu@zeyos.com> wrote: >> Hi, >> >> Regarding ARRAY_SLICE(..., preserve_keys=false) my whole point was that the check for >> non-numeric key inputs is not necessary if preserve_keys is false. >> It should basically be analogue to array_values(), but of course respecting the offset and >> length boundaries. > I am not sure I understood your point. > > even if preserve_keys is false, it still use string key if origin is string key. > > >> >> One question regarding FAST_ZPP: Why is it not used more often, especially in the satndard >> libraries? Is there some particular drawback? I'd like to understand. > > hmm, https://wiki.php.net/rfc/fast_zpp :) > > thanks >> >> Thanks, >> >> Ben >> >> ========== Original ========== >> From: Xinchen Hui <xinchen.h@zend.com> >> To: Benjamin Coutu <ben.coutu@zeyos.com> >> Date: Tue, 20 Jan 2015 09:53:38 +0100 >> Subject: [PHP-DEV] Re: Improvements to array.c code base >> >> >> >> Hey: >> >> >> On Tue, Jan 20, 2015 at 4:30 PM, Benjamin Coutu <ben.coutu@zeyos.com> wrote: >>> Hi Dmitry, >>> >>> I was doing some code review of ext/standard/array.c and have recognized some potential >>> for a few performance improvements: >>> >>> === ARRAY_SLICE(..., preserve_keys=false) === >>> >>> array_slice() can always construct a packed array if preserve_keys is false, >>> restricting it to inputs with packed flag does not make much sense. >>> Removing the check for packed inputs on line 2376 would improve performance if used on >>> non-packed inputs with the default preserve_keys=false. >> This is check for non-numeric key inputs >> >>> Furthermore, ZEND_HASH_FOREACH_VAL should be used instead of >>> ZEND_HASH_FOREACH_NUM_KEY_VAL on line 2379. >> yeah, I will fix it. >>> It also think range() should use FAST_ZPP as it is a basic language feature (other >>> languages even have extra operators for it, e.g. [0..10]) >>> >>> === RANGE(...) === >>> >>> range() always returns a numerically indexed array [0..count-1]. The resulting array >>> therefore should be constructed as a packed array (ZEND_HASH_FILL_PACKED+ZEND_HASH_FILL_ADD instead >>> of zend_hash_next_index_insert_new). >>> >>> === ARRAY_FILL(start_key=0, ...) >>> === >>> >>> Just like with range(), array_fill() always returns a numerically indexed array >>> [0..count-1] if start_key is 0. In this special but very common case the resulting array can be >>> constructed as a packed array (ZEND_HASH_FILL_PACKED+ZEND_HASH_FILL_ADD instead of >>> zend_hash_next_index_insert_new). >>> Another common case is for start_key to be 1. One could refine the proposed >>> packed-array branch to just set the first bucket to undefined in this case. >>> >>> === COUNT(...) === >>> >>> count() is so ubiquitous! Giving it an opcode and making it part of the VM seams >>> reasonable. >>> >>> Please let me know your thoughts. >> hmm, we currently do optimization based on profile against some real >> life apps, like wordpress. >> >> range and array_fill etc doesn't used very common.. so I didn't look into it.. >> >> thanks for the advise, I will take a look . >> >> thanks >>> >>> Cheers, >>> >>> Ben >>> >>> -- >>> >>> Benjamin Coutu >>> Zeyon Technologies Inc. >>> http://www.zeyos.com >>> >> > -- Xinchen Hui @Laruence http://www.laruence.com/