Remove specialized PartialQuickSort{<:Integer} method (#36896)

This specialized method is actually considerably slower than the one that also
works for `PartialQuickSort{<:OrdinalRange}`. It had been removed before in
9e8fcd3 due to an inference issue, and reintroduced by c41d5a3 (#31632) once
that issue got fixed, but apparently without benchmarks. It turns out that saving
one comparison per iteration isn't worth it. The gain is especially large when
looking for a value among the largest in the array, as the specialized method always
sorted all values lower than the requested one.

Author: nalimilan

base/sort.jl

@@ -650,40 +650,20 @@ function sort!(v::AbstractVector, lo::Integer, hi::Integer, a::MergeSortAlg, o::
     return v
 end
 
-function sort!(v::AbstractVector, lo::Integer, hi::Integer, a::PartialQuickSort{<:Integer},
+function sort!(v::AbstractVector, lo::Integer, hi::Integer, a::PartialQuickSort,
                o::Ordering)
     @inbounds while lo < hi
         hi-lo <= SMALL_THRESHOLD && return sort!(v, lo, hi, SMALL_ALGORITHM, o)
         j = partition!(v, lo, hi, o)
-        if j >= a.k
-            # we don't need to sort anything bigger than j
-            hi = j-1
-        elseif j-lo < hi-j
-            # recurse on the smaller chunk
-            # this is necessary to preserve O(log(n))
-            # stack space in the worst case (rather than O(n))
-            lo < (j-1) && sort!(v, lo, j-1, a, o)
-            lo = j+1
-        else
-            (j+1) < hi && sort!(v, j+1, hi, a, o)
-            hi = j-1
-        end
-    end
-    return v
-end
-
-
-function sort!(v::AbstractVector, lo::Integer, hi::Integer, a::PartialQuickSort{T},
-               o::Ordering) where T<:OrdinalRange
-    @inbounds while lo < hi
-        hi-lo <= SMALL_THRESHOLD && return sort!(v, lo, hi, SMALL_ALGORITHM, o)
-        j = partition!(v, lo, hi, o)
 
         if j <= first(a.k)
             lo = j+1
         elseif j >= last(a.k)
             hi = j-1
         else
+            # recurse on the smaller chunk
+            # this is necessary to preserve O(log(n))
+            # stack space in the worst case (rather than O(n))
             if j-lo < hi-j
                 lo < (j-1) && sort!(v, lo, j-1, a, o)
                 lo = j+1

test/sorting.jl

@@ -294,14 +294,15 @@ Base.step(r::ConstantRange) = 0
     end
 
     @testset "unstable algorithms" begin
-        for alg in [QuickSort, PartialQuickSort(length(a))]
-            b = sort(a, alg=alg)
-            @test issorted(b)
-            b = sort(a, alg=alg, rev=true)
-            @test issorted(b, rev=true)
-            b = sort(a, alg=alg, by=x->1/x)
-            @test issorted(b, by=x->1/x)
-        end
+        b = sort(a, alg=QuickSort)
+        @test issorted(b)
+        @test last(b) == last(sort(a, alg=PartialQuickSort(length(a))))
+        b = sort(a, alg=QuickSort, rev=true)
+        @test issorted(b, rev=true)
+        @test last(b) == last(sort(a, alg=PartialQuickSort(length(a)), rev=true))
+        b = sort(a, alg=QuickSort, by=x->1/x)
+        @test issorted(b, by=x->1/x)
+        @test last(b) == last(sort(a, alg=PartialQuickSort(length(a)), by=x->1/x))
     end
 end
 @testset "insorted" begin
@@ -362,14 +363,26 @@ end
 @testset "PartialQuickSort" begin
     a = rand(1:10000, 1000)
     # test PartialQuickSort only does a partial sort
+    let alg = PartialQuickSort(1:div(length(a), 10))
+        k = alg.k
+        b = sort(a, alg=alg)
+        c = sort(a, alg=alg, by=x->1/x)
+        d = sort(a, alg=alg, rev=true)
+        @test issorted(b[k])
+        @test issorted(c[k], by=x->1/x)
+        @test issorted(d[k], rev=true)
+        @test !issorted(b)
+        @test !issorted(c, by=x->1/x)
+        @test !issorted(d, rev=true)
+    end
     let alg = PartialQuickSort(div(length(a), 10))
         k = alg.k
         b = sort(a, alg=alg)
         c = sort(a, alg=alg, by=x->1/x)
         d = sort(a, alg=alg, rev=true)
-        @test issorted(b[1:k])
-        @test issorted(c[1:k], by=x->1/x)
-        @test issorted(d[1:k], rev=true)
+        @test b[k] == sort(a)[k]
+        @test c[k] == sort(a, by=x->1/x)[k]
+        @test d[k] == sort(a, rev=true)[k]
         @test !issorted(b)
         @test !issorted(c, by=x->1/x)
         @test !issorted(d, rev=true)
@@ -417,7 +430,8 @@ end
             # stable algorithms
             for alg in [MergeSort]
                 p = sortperm(v, alg=alg, rev=rev)
-                @test p == sortperm(float(v), alg=alg, rev=rev)
+                p2 = sortperm(float(v), alg=alg, rev=rev)
+                @test p == p2
                 @test p == pi
                 s = copy(v)
                 permute!(s, p)
@@ -427,9 +441,10 @@ end
             end
 
             # unstable algorithms
-            for alg in [QuickSort, PartialQuickSort(n)]
+            for alg in [QuickSort, PartialQuickSort(1:n)]
                 p = sortperm(v, alg=alg, rev=rev)
-                @test p == sortperm(float(v), alg=alg, rev=rev)
+                p2 = sortperm(float(v), alg=alg, rev=rev)
+                @test p == p2
                 @test isperm(p)
                 @test v[p] == si
                 s = copy(v)
@@ -438,6 +453,22 @@ end
                 invpermute!(s, p)
                 @test s == v
             end
+            for alg in [PartialQuickSort(n)]
+                p = sortperm(v, alg=alg, rev=rev)
+                p2 = sortperm(float(v), alg=alg, rev=rev)
+                if n == 0
+                    @test isempty(p) && isempty(p2)
+                else
+                    @test p[n] == p2[n]
+                    @test v[p][n] == si[n]
+                    @test isperm(p)
+                    s = copy(v)
+                    permute!(s, p)
+                    @test s[n] == si[n]
+                    invpermute!(s, p)
+                    @test s == v
+                end
+            end
         end
 
         v = randn_with_nans(n,0.1)