Re: Inconsistent implementation of array sorting (old zend_qsort in array_multisort)

From: Date: Thu, 23 Jun 2016 10:08:12 +0000
Subject: Re: Inconsistent implementation of array sorting (old zend_qsort in array_multisort)
References: 1  Groups: php.internals 
Request: Send a blank email to internals+get-94220@lists.php.net to get a copy of this message
Hey: On Thu, Jun 23, 2016 at 5:04 PM, Benjamin Coutu <ben.coutu@zeyos.com> wrote: > Hello everyone, > > While reviewing the array.c code base, I have noticed that the > array_multisort function still uses the old zend_qsort instead of the new > zend_sort algorithm that was introduced with PHP 7. > > That represents a clear inconsistency, because all other array sorting > functions utilize the advanced implementation of quicksort (through > zend_hash_sort) that was derived from LLVM's libc++ implementation of > std::sort (https://marc.info/?l=php-internals&m=142047779209352). > > Not only is zend_qsort less efficient, but moreover, because it doesn't > rely on the insertion sort fallback for small array chunks that zend_sort > provides (which yields a more stable sorting order), the result might in > effect actually differ from that of all the other array sorting functions. > > IMHO this inconsistency can be considered a bug. > > Also, array_multisort is the only remnant in PHP core that still uses > zend_qsort (https://github.com/php/php-src/search?q=zend_qsort). If we > would eliminate that inconsistency, we could probably dispense with the > then obsolete zend_qsort all together. > > Please let me know your thoughts. > Thanks for the notice, it is long time, I don't remember why I didn't change that, but it does looks like a oversight to me now. I will change it to use zend_sort in 7.1 , however, we can not remove zend_qsort, because its a ZEND_API , some third extensions may use it. thanks > Thanks, > > Ben > > -- > > Bejamin Coutu > ben.coutu@zeyos.com > > ZeyOS, Inc. > http://www.zeyos.com > > -- Xinchen Hui @Laruence http://www.laruence.com/

« previous php.internals (#94220) next »