Clean up the pivot selection for QuickSort

[pabloferz]

This is and improvement to the current selectpivot!, that selects the pivot for the partition in the QuickSort algorithm.

Currently the method always orders the three elements and then swaps the first and middle elements. This change saves a little by ordering in such a way that v[hi] is the greatest and v[mi] the smallest, so there is no need of a swapping at the end.

For comparison the current implementation generates the following machine code

;julia> @code_native selectpivot!(v, i, j, Base.Order.Forward)
    .text
Filename: sort.jl
Source line: 247
    pushq   %rbp
    movq    %rsp, %rbp
Source line: 247
    movq    (%rdi), %rcx
    movq    -8(%rcx,%rsi,8), %r8
Source line: 243
    leaq    (%rdx,%rsi), %rax
    shrq    %rax
Source line: 247
    movq    -8(%rcx,%rax,8), %r9
    cmpq    %r8, %r9
    jge L46
Source line: 248
    movq    %r8, -8(%rcx,%rax,8)
    movq    (%rdi), %rcx
    movq    %r9, -8(%rcx,%rsi,8)
Source line: 250
L46:    movq    (%rdi), %rcx
    movq    -8(%rcx,%rax,8), %r8
    movq    -8(%rcx,%rdx,8), %r9
    cmpq    %r8, %r9
    jge L121
Source line: 251
    movq    -8(%rcx,%rsi,8), %r10
    cmpq    %r10, %r9
    jge L108
Source line: 252
    movq    %r9, -8(%rcx,%rsi,8)
    movq    (%rdi), %rcx
    movq    %r10, -8(%rcx,%rax,8)
    movq    (%rdi), %rcx
    movq    %r8, -8(%rcx,%rdx,8)
    jmpq    L121
Source line: 254
L108:   movq    %r8, -8(%rcx,%rdx,8)
    movq    (%rdi), %rcx
    movq    %r9, -8(%rcx,%rax,8)
Source line: 259
L121:   movq    (%rdi), %rdx
    movq    -8(%rdx,%rsi,8), %r8
    movq    -8(%rdx,%rax,8), %rcx
    movq    %rcx, -8(%rdx,%rsi,8)
    movq    (%rdi), %rcx
    movq    %r8, -8(%rcx,%rax,8)
Source line: 260
    movq    (%rdi), %rax
    movq    -8(%rax,%rsi,8), %rax
Source line: 264
    popq    %rbp
    ret

while the change produces

;julia> @code_native selectpivot!(v, i, j, Base.Order.Forward)
    .text
Filename: none
Source line: 4
    pushq   %rbp
    movq    %rsp, %rbp
Source line: 4
    movq    (%rdi), %rcx
    movq    -8(%rcx,%rsi,8), %r8
Source line: 3
    leaq    (%rdx,%rsi), %rax
    shrq    %rax
Source line: 4
    movq    -8(%rcx,%rax,8), %r9
    cmpq    %r9, %r8
    jge L46
Source line: 5
    movq    %r9, -8(%rcx,%rsi,8)
    movq    (%rdi), %rcx
    movq    %r8, -8(%rcx,%rax,8)
Source line: 7
L46:    movq    (%rdi), %rcx
    movq    -8(%rcx,%rsi,8), %r9
    movq    -8(%rcx,%rdx,8), %r8
    cmpq    %r9, %r8
    jge L116
Source line: 8
    movq    %r9, -8(%rcx,%rdx,8)
    movq    (%rdi), %rcx
    movq    %r8, -8(%rcx,%rsi,8)
Source line: 9
    movq    (%rdi), %rcx
    movq    -8(%rcx,%rax,8), %rdx
    movq    -8(%rcx,%rsi,8), %r8
    cmpq    %rdx, %r8
    jge L116
Source line: 10
    movq    %rdx, -8(%rcx,%rsi,8)
    movq    (%rdi), %rcx
    movq    %r8, -8(%rcx,%rax,8)
Source line: 13
L116:   movq    (%rdi), %rax
    movq    -8(%rax,%rsi,8), %rax
Source line: 15
    popq    %rbp
    ret

This seems to give near a 10% speed up for sort and sortperm in some tests I made.

CC @StefanKarpinski

[pabloferz]

The partition function could also be improved for the case in which there is a considerable number of duplicates in the array. But that would need to use (and to have) both isless and isequal defined for each type that might be wanted to be quicksorted.

[JeffBezanson]

Cool. This looks like a perfect use case for the nanosoldier sorting benchmarks!

[pabloferz]

Ok, I'll give it a try. @nanosoldier runbenchmarks("sort", vs = ":master")

[KristofferC]

You need to be a contributor for it to work I think.

[pabloferz]

I see. Then I leave to the appropriate party.

[JeffBezanson]

runbenchmarks("sort", vs = ":master")

[KristofferC]

Also need to cc nanosoldier. :)

[jrevels]

@nanosoldier runbenchmarks("sort", vs = ":master")

[nanosoldier]

Your benchmark job has completed - no performance regressions were detected. A full report can be found here. cc @jrevels

[pabloferz]

I believe I benchmarked something wrong the first time. It seems now like a insignificant improvement and more like a cosmetic change. For the following function I get

function test(v, n)
    o = Base.Order.Forward
    pivot = zero(eltype(v))
    for i = 1:n
        i, j = minmax(rand(1:length(v)), rand(1:length(v)))
        pivot = selectpivot!(v, i, j, o)
    end
    return pivot
end

v = rand(10^7);
@benchmark test(v, 10^7)

Trial(1.68 s) # master
Trial(1.67 s) # this PR
Trial(1.70 s) # 0.4.5
Trial(1.69 s) # 0.4.5 + this change

Anyway, I still believe the partition! function can be improved for the cases of duplicate keys by using a 3-way partition. Or maybe even more by a dual or triple pivot partition (http://epubs.siam.org/doi/abs/10.1137/1.9781611973198.6)

[kmsquire]

@pabloferz, it would be great if you coded up the version of quicksort with a three way partition and submitted a pull request to SortingAlgorithms.jl.

[pabloferz]

@kmsquire Done in JuliaCollections/SortingAlgorithms.jl#19.