Re: com php-src: Faster sorting algo: UPGRADING Zend/Makefile.am Zend/tests/methods-on-non-objects-usort.phpt Zend/zend_API.c Zend/zend_hash.c
Zend/zend_hash.h Zend/zend_ini.c Zend/zend_llist.c Zend/zend_qsort.c Zend/zend_qsort.h Zend/zend_sort.c Zend/zend_sort.h Zend/zend_ts_hash.c
Zend/zend_types.h configure.in ext/ereg/ereg.c ext/intl/collator/collator_sort.c ext/phar/dirstream.c ext/phar/phar_internal.h ext/standard/array.c
ext/standard/info.c ex

From: Date: Sat, 17 Jan 2015 05:24:21 +0000
Subject: Re: com php-src: Faster sorting algo: UPGRADING Zend/Makefile.am Zend/tests/methods-on-non-objects-usort.phpt Zend/zend_API.c Zend/zend_hash.c
Zend/zend_hash.h Zend/zend_ini.c Zend/zend_llist.c Zend/zend_qsort.c Zend/zend_qsort.h Zend/zend_sort.c Zend/zend_sort.h Zend/zend_ts_hash.c
Zend/zend_types.h configure.in ext/ereg/ereg.c ext/intl/collator/collator_sort.c ext/phar/dirstream.c ext/phar/phar_internal.h ext/standard/array.c
ext/standard/info.c ex
References: 1 2 3 4 5 6 7  Groups: php.internals 
Request: Send a blank email to [email protected] to get a copy of this message
Hey:

On Sat, Jan 17, 2015 at 1:16 PM, Xinchen Hui <[email protected]> wrote:
> Hey:
>
> On Sat, Jan 17, 2015 at 12:37 PM, Rasmus Lerdorf <[email protected]> wrote:
>> On Jan 16, 2015, at 19:18, Xinchen Hui <[email protected]> wrote:
>>>
>>> Hey:
>>>
>>>
>>>> On Sat, Jan 17, 2015 at 8:20 AM, Rasmus Lerdorf <[email protected]> wrote:
>>>>> On 01/16/2015 03:54 PM, Simon J Welsh wrote:
>>>>> That’s an invalid comparison function. It causes the current usort to reverse
>>>>> sort the array and I see no problem with that changing (you’re saying that a value for $a less
>>>>> than $b is in fact greater than it, and a value of $b less than or equal to $a is equal to it).
>>>>
>>>> Your didn't make much sense. But yes, I am aware it is a weird
>>>> comparison function taken from an existing app which is now broken under
>>>> PHP7 but it only breaks if the array grows beyond 16 elements.
>>>> If we are going to make a BC change here, it shouldn't be in a way that
>>>> is dependent on the size of the array being sorted.
>>>
>>> I suggest you use :
>>>
>>> function cmp($a,$b) {
>>>    return strtotime($a['date']) - strtotime($b['date']);
>>> }
>>>
>>> as our usort doc said:
>>>
>>> "The comparison function must return an integer less than, equal to,
>>> or greater than zero if the first argument is considered to be
>>> respectively less than, equal to, or greater than the second."
>>>
>>> otherwise, it's not a defined behavior
>>
>> Yes, like I said, I am aware that the comparison function is flaky but it is still code
>> that has worked for 15 years so we have to be clear about the BC break. The fact that it works up
>> until the array gets more than 16 elements makes it a tricky one to track down for people. With the
>> massive volume of legacy code out there we have to be careful with changes like this that can break
>> existing code, even if that code is questionable.
>>
>> At the very least we need a clear note in the upgrading doc reminding people to check all
>> their user comparison functions.
> Hey:
>
> hmm, I am not sure about "BC break previous undefined behavior"
>
> but okey, I agree we need write some note(actually I noted in UPGRADE
> of we are using hybrid sorting algo now)..
>
> but what kindof note  I mean how details it should be? I am not a
> native english speaker, could anyone here to help me to write that ?
and to explained my concern . that is, the 16 is my picked number, I
did some benches, then use 16..

but maybe we could change that later...

like std::sort doesn't use a fixed limit, but choose 10 or 30
according the how heavy the swap fucntion is..

so...

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



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


Thread (23 messages)

« previous php.internals (#80678) next »