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

On Sun, Jan 18, 2015 at 3:08 PM, Rasmus Lerdorf <rasmus@lerdorf.com> wrote: > On 01/18/2015 11:45 AM, Stanislav Malyshev wrote: >> Hi! >> >>> 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 >> >> Looking at that function, I'm not sure the fact it worked is not pure >> luck. It is not an order function - it produces different result when >> parameters are switched, i.e. if you call f($a, $b) it would say $a > >> $b, but if you call f($b, $a) it would say $a == $b. Of course, even >> broken functions can produce valid results, with some luck, but I'm not >> sure we can guarantee non-order functions to work when order function is >> required. > > Sure, I understand it is flaky. But it is still code that has always > worked in every single version of PHP (and HHVM) ever released going > back 15+ years. > > eg. http://3v4l.org/7prWJ > > and I have a php3 build here that it works on as well. > > We have to be really really careful with this "oh, that code is wrong, > so we can break it argument". This will break hundreds if not thousands > of sites in a hard-to-debug way. It took me less than a minute of > looking on Github to find a case that will break: > > > https://github.com/chenboking/service.downloadmanager.amule/blob/cda510415f9a58660e096a7de42b3ea6f39ee851/webserver/php-default/amuleweb-main-search.php#L121 > > It is extremely common to just do a less-than or a greater-than check in > a user comparison function. Of course it isn't textbook-correct, but > since it has always worked, people do it. I have no qualms about breaking code that is not even returning the correct type when the documentation explains how it is supposed to work[1]. The integer value returned by a comparison function is not even boolean-like, which means that using < or <= is fundamentally incorrect. With that said, I'm not sure the kind of performance numbers being reported are worth changing the behavior. I haven't looked at the patch, but if the code isn't cleaner or more maintainable then I'd have to say that the performance numbers alone really aren't worth changing the code. [1] At least on the pages I checked, such as usort and uksort