Skip to content

Commit 54aa57c

Browse files
authored
Make QuickerSort efficient for non-homogonous eltype (#47973)
* set `v[j] = pivot` in partition rather than returning pivot to caller to make partition! type stable for non-concrete eltype
1 parent 0913cbc commit 54aa57c

File tree

2 files changed

+22
-9
lines changed

2 files changed

+22
-9
lines changed

base/sort.jl

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -998,7 +998,8 @@ select_pivot(lo::Integer, hi::Integer) = typeof(hi-lo)(hash(lo) % (hi-lo+1)) + l
998998
#
999999
# returns (pivot, pivot_index) where pivot_index is the location the pivot
10001000
# should end up, but does not set t[pivot_index] = pivot
1001-
function partition!(t::AbstractVector, lo::Integer, hi::Integer, offset::Integer, o::Ordering, v::AbstractVector, rev::Bool)
1001+
function partition!(t::AbstractVector, lo::Integer, hi::Integer, offset::Integer, o::Ordering,
1002+
v::AbstractVector, rev::Bool, pivot_dest::AbstractVector, pivot_index_offset::Integer)
10021003
pivot_index = select_pivot(lo, hi)
10031004
@inbounds begin
10041005
pivot = v[pivot_index]
@@ -1016,14 +1017,16 @@ function partition!(t::AbstractVector, lo::Integer, hi::Integer, offset::Integer
10161017
offset += fx
10171018
lo += 1
10181019
end
1020+
pivot_index = lo-offset + pivot_index_offset
1021+
pivot_dest[pivot_index] = pivot
10191022
end
10201023

1021-
# pivot_index = lo-offset
1022-
# t[pivot_index] is whatever it was before
1023-
# t[<pivot_index] <* pivot, stable
1024-
# t[>pivot_index] >* pivot, reverse stable
1024+
# t_pivot_index = lo-offset (i.e. without pivot_index_offset)
1025+
# t[t_pivot_index] is whatever it was before unless t is the pivot_dest
1026+
# t[<t_pivot_index] <* pivot, stable
1027+
# t[>t_pivot_index] >* pivot, reverse stable
10251028

1026-
pivot, lo-offset
1029+
pivot_index
10271030
end
10281031

10291032
function _sort!(v::AbstractVector, a::QuickerSort, o::Ordering, kw;
@@ -1037,9 +1040,11 @@ function _sort!(v::AbstractVector, a::QuickerSort, o::Ordering, kw;
10371040
end
10381041

10391042
while lo < hi && hi - lo > SMALL_THRESHOLD
1040-
pivot, j = swap ? partition!(v, lo+offset, hi+offset, offset, o, t, rev) : partition!(t, lo, hi, -offset, o, v, rev)
1041-
j -= !swap*offset
1042-
@inbounds v[j] = pivot
1043+
j = if swap
1044+
partition!(v, lo+offset, hi+offset, offset, o, t, rev, v, 0)
1045+
else
1046+
partition!(t, lo, hi, -offset, o, v, rev, v, -offset)
1047+
end
10431048
swap = !swap
10441049

10451050
# For QuickerSort(), a.lo === a.hi === missing, so the first two branches get skipped

test/sorting.jl

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -918,6 +918,14 @@ end
918918
@test bsqs() === bsqs(missing, missing, InsertionSort)
919919
end
920920

921+
@testset "QuickerSort allocations on non-concrete eltype" begin
922+
v = Vector{Union{Nothing, Bool}}(rand(Bool, 10000))
923+
@test 4 == @allocations sort(v)
924+
@test 4 == @allocations sort(v; alg=Base.Sort.QuickerSort())
925+
# it would be nice if these numbers were lower (1 or 2), but these
926+
# test that we don't have O(n) allocations due to type instability
927+
end
928+
921929
function test_allocs()
922930
v = rand(10)
923931
i = randperm(length(v))

0 commit comments

Comments
 (0)