Re: Inconsistent implementation of array sorting (old zend_qsort in array_multisort)
| From: | Xinchen Hui | 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/