Re: [PHP-DEV] Re: Faster zend sorting implementation
Hi! I made a PR here: https://github.com/php/php-src/pull/999 for reviewing in benchmark this can brings more than 30% performance gain in array_sort etc functions. tests fails are related to non-stable vs stable sorting difference. anyway, I feel it's better to ask you to do a final review, what do you think? is there any objections to merge this? I think the sort order of equal elements was never defined, so changing it would not be a big issue. The tests, of course, need to be fixed and note in UPGRADING should be provided, but otherwise it's fine. -- Stas Malyshev smalys...@gmail.com -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Re: Faster zend sorting implementation
On Wed, Jan 14, 2015 at 9:09 PM, Stanislav Malyshev smalys...@gmail.com wrote: Hi! I made a PR here: https://github.com/php/php-src/pull/999 for reviewing in benchmark this can brings more than 30% performance gain in array_sort etc functions. tests fails are related to non-stable vs stable sorting difference. anyway, I feel it's better to ask you to do a final review, what do you think? is there any objections to merge this? I think the sort order of equal elements was never defined, so changing it would not be a big issue. The tests, of course, need to be fixed and note in UPGRADING should be provided, but otherwise it's fine. Same here, all good as long as a notice is present and tests are updated. -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Re: Faster zend sorting implementation
Hey: On Thu, Jan 15, 2015 at 7:52 AM, Pierre Joye pierre@gmail.com wrote: On Wed, Jan 14, 2015 at 9:09 PM, Stanislav Malyshev smalys...@gmail.com wrote: Hi! I made a PR here: https://github.com/php/php-src/pull/999 for reviewing in benchmark this can brings more than 30% performance gain in array_sort etc functions. tests fails are related to non-stable vs stable sorting difference. anyway, I feel it's better to ask you to do a final review, what do you think? is there any objections to merge this? I think the sort order of equal elements was never defined, so changing it would not be a big issue. The tests, of course, need to be fixed and note in UPGRADING should be provided, but otherwise it's fine. Same here, all good as long as a notice is present and tests are updated. thanks for the reviewing, I am going to merge it. I have got some ideas to improve based on this patch.. so I'd like to merge it first. thanks -- Xinchen Hui @Laruence http://www.laruence.com/ -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Re: Faster zend sorting implementation
Hey:
I made a PR here: https://github.com/php/php-src/pull/999 for reviewing
in benchmark this can brings more than 30% performance gain in
array_sort etc functions.
tests fails are related to non-stable vs stable sorting difference.
anyway, I feel it's better to ask you to do a final review, what
do you think?
is there any objections to merge this?
thanks
On Tue, Jan 6, 2015 at 1:08 AM, Xinchen Hui larue...@php.net wrote:
Hey:
I was working on zend_qsort improvement. but I got a problem need
to be disscussed with you fist..
as we know, previously zend_qsort is not a stable sorting algo.
my draft patch (which already get 0.1% IRs reduce in wordpress)
is kindof a stable sorting algo, you can find it here
(https://github.com/laruence/php-src/compare/zend_sort)
so, there is a bc break, like for :
$array = array(o, O);
sort($array, SORT_STRING|SORT_FLAG_CASE);
var_dump($array);
previously implementation does the swap:
array(2) {
[0]=
string(1) O
[1]=
string(1) o
}
but new implementation doesn't not:
array(2) {
[0]=
string(1) o
[1]=
string(1) O
}
do you think such BC break is acceptable? or I still need a RFC? :
thanks
--
Xinchen Hui
@Laruence
http://www.laruence.com/
--
Xinchen Hui
@Laruence
http://www.laruence.com/
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Re: Faster zend sorting implementation
I don't see big problems committing this.
In some cases new sort() functions may provide different results, but they
are still valid, because the order of equal elements after sort is not
defined.
Thanks. Dmitry.
On Wed, Jan 14, 2015 at 12:59 PM, Xinchen Hui larue...@php.net wrote:
Hey:
I made a PR here: https://github.com/php/php-src/pull/999 for
reviewing
in benchmark this can brings more than 30% performance gain in
array_sort etc functions.
tests fails are related to non-stable vs stable sorting difference.
anyway, I feel it's better to ask you to do a final review, what
do you think?
is there any objections to merge this?
thanks
On Tue, Jan 6, 2015 at 1:08 AM, Xinchen Hui larue...@php.net wrote:
Hey:
I was working on zend_qsort improvement. but I got a problem need
to be disscussed with you fist..
as we know, previously zend_qsort is not a stable sorting algo.
my draft patch (which already get 0.1% IRs reduce in wordpress)
is kindof a stable sorting algo, you can find it here
(https://github.com/laruence/php-src/compare/zend_sort)
so, there is a bc break, like for :
$array = array(o, O);
sort($array, SORT_STRING|SORT_FLAG_CASE);
var_dump($array);
previously implementation does the swap:
array(2) {
[0]=
string(1) O
[1]=
string(1) o
}
but new implementation doesn't not:
array(2) {
[0]=
string(1) o
[1]=
string(1) O
}
do you think such BC break is acceptable? or I still need a RFC? :
thanks
--
Xinchen Hui
@Laruence
http://www.laruence.com/
--
Xinchen Hui
@Laruence
http://www.laruence.com/
Re: [PHP-DEV] Re: Faster zend sorting implementation
On Mon, Jan 5, 2015 at 6:09 PM, Xinchen Hui larue...@php.net wrote:
On Tue, Jan 6, 2015 at 1:08 AM, Xinchen Hui larue...@php.net wrote:
Hey:
I was working on zend_qsort improvement. but I got a problem need
to be disscussed with you fist..
first
as we know, previously zend_qsort is not a stable sorting algo.
my draft patch (which already get 0.1% IRs reduce in wordpress)
is kindof a stable sorting algo, you can find it here
(https://github.com/laruence/php-src/compare/zend_sort)
so, there is a bc break, like for :
$array = array(o, O);
sort($array, SORT_STRING|SORT_FLAG_CASE);
var_dump($array);
previously implementation does the swap:
array(2) {
[0]=
string(1) O
[1]=
string(1) o
}
but new implementation doesn't not:
does not
array(2) {
[0]=
string(1) o
[1]=
string(1) O
}
Hum, I dont think such a BC is acceptable.
There are tons of userland code out there relying on alpha case sorting
that could get impacted
IMO :-)
Q: why extract the swap function from the qsort algo ? Is there an interest
of replacing it at runtime ?
Julien.P
[PHP-DEV] Re: Faster zend sorting implementation
On Tue, Jan 6, 2015 at 1:08 AM, Xinchen Hui larue...@php.net wrote:
Hey:
I was working on zend_qsort improvement. but I got a problem need
to be disscussed with you fist..
first
as we know, previously zend_qsort is not a stable sorting algo.
my draft patch (which already get 0.1% IRs reduce in wordpress)
is kindof a stable sorting algo, you can find it here
(https://github.com/laruence/php-src/compare/zend_sort)
so, there is a bc break, like for :
$array = array(o, O);
sort($array, SORT_STRING|SORT_FLAG_CASE);
var_dump($array);
previously implementation does the swap:
array(2) {
[0]=
string(1) O
[1]=
string(1) o
}
but new implementation doesn't not:
does not
array(2) {
[0]=
string(1) o
[1]=
string(1) O
}
do you think such BC break is acceptable? or I still need a RFC? :
thanks
--
Xinchen Hui
@Laruence
http://www.laruence.com/
--
Xinchen Hui
@Laruence
http://www.laruence.com/
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Re: Faster zend sorting implementation
Hey：
On Jan 6, 2015, at 1:27 AM, Julien Pauli jpa...@php.net wrote:
On Mon, Jan 5, 2015 at 6:09 PM, Xinchen Hui larue...@php.net wrote:
On Tue, Jan 6, 2015 at 1:08 AM, Xinchen Hui larue...@php.net wrote:
Hey:
I was working on zend_qsort improvement. but I got a problem need
to be disscussed with you fist..
first
as we know, previously zend_qsort is not a stable sorting algo.
my draft patch (which already get 0.1% IRs reduce in wordpress)
is kindof a stable sorting algo, you can find it here
(https://github.com/laruence/php-src/compare/zend_sort)
so, there is a bc break, like for :
$array = array(o, O);
sort($array, SORT_STRING|SORT_FLAG_CASE);
var_dump($array);
previously implementation does the swap:
array(2) {
[0]=
string(1) O
[1]=
string(1) o
}
but new implementation doesn't not:
does not
array(2) {
[0]=
string(1) o
[1]=
string(1) O
}
Hum, I dont think such a BC is acceptable.
I am not sure if you get the problem?
O and o are equal is that script
The flags means using case-insensitive string sorting mean
Thanks
There are tons of userland code out there relying on alpha case sorting that
could get impacted
IMO :-)
Q: why extract the swap function from the qsort algo ? Is there an interest
of replacing it at runtime ?
Julien.P
Re: [PHP-DEV] Re: Faster zend sorting implementation
Sounds more like a bugfix to me and def'ly an acceptable BC break in
either case. The behavior isn't specified and if anything I would
assume there _wouldn't_ be a swap with SORT_FLAG_CASE on. Interesting
though that many sorting examples (across languages) sidestep this
clearly common case.
-- S.
On Mon, Jan 5, 2015 at 12:36 PM, Xinchen Hui larue...@gmail.com wrote:
Hey：
On Jan 6, 2015, at 1:27 AM, Julien Pauli jpa...@php.net wrote:
On Mon, Jan 5, 2015 at 6:09 PM, Xinchen Hui larue...@php.net wrote:
On Tue, Jan 6, 2015 at 1:08 AM, Xinchen Hui larue...@php.net wrote:
Hey:
I was working on zend_qsort improvement. but I got a problem need
to be disscussed with you fist..
first
as we know, previously zend_qsort is not a stable sorting algo.
my draft patch (which already get 0.1% IRs reduce in wordpress)
is kindof a stable sorting algo, you can find it here
(https://github.com/laruence/php-src/compare/zend_sort)
so, there is a bc break, like for :
$array = array(o, O);
sort($array, SORT_STRING|SORT_FLAG_CASE);
var_dump($array);
previously implementation does the swap:
array(2) {
[0]=
string(1) O
[1]=
string(1) o
}
but new implementation doesn't not:
does not
array(2) {
[0]=
string(1) o
[1]=
string(1) O
}
Hum, I dont think such a BC is acceptable.
I am not sure if you get the problem?
O and o are equal is that script
The flags means using case-insensitive string sorting mean
Thanks
There are tons of userland code out there relying on alpha case sorting that
could get impacted
IMO :-)
Q: why extract the swap function from the qsort algo ? Is there an interest
of replacing it at runtime ?
Julien.P
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
