Skip to content

Commit

Permalink
Remove specialized sort! method for PartialQuickSort{Int} (fixes Juli…
Browse files Browse the repository at this point in the history
…aLang#12833)

There were two sort methods for PartialQuickSort, depending
on whether it was parameterized by an Int (representing the
last index needing to be sorted) or a Range.  The version
parameterized by an Int has one less comparison per branch,
but it causes inference to fail to infer the return type of
sort/sort! under many circumstances.  Removing this version
and only providing the generic function restores proper type
inference.

If there's an underlying bug that is fixed, this can be reverted
(though the test should remain.)
  • Loading branch information
kmsquire committed Aug 28, 2015
1 parent ff77f73 commit 9e8fcd3
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 26 deletions.
61 changes: 35 additions & 26 deletions base/sort.jl
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,11 @@ immutable PartialQuickSort{T <: Union{Int,OrdinalRange}} <: Algorithm
k::T
end

Base.first(a::PartialQuickSort{Int}) = 1
Base.last(a::PartialQuickSort{Int}) = a.k
Base.first(a::PartialQuickSort) = first(a.k)
Base.last(a::PartialQuickSort) = last(a.k)

const InsertionSort = InsertionSortAlg()
const QuickSort = QuickSortAlg()
const MergeSort = MergeSortAlg()
Expand Down Expand Up @@ -338,38 +343,42 @@ function sort!(v::AbstractVector, lo::Int, hi::Int, a::MergeSortAlg, o::Ordering
return v
end

function sort!(v::AbstractVector, lo::Int, hi::Int, a::PartialQuickSort{Int},
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!{T<:OrdinalRange}(v::AbstractVector, lo::Int, hi::Int, a::PartialQuickSort{T},
## TODO: When PartialQuickSort is parameterized by an Int, this version of sort
## has one less comparison per loop than the version below, but enabling
## it causes return type inference to fail for sort/sort! (#12833)
##
# function sort!(v::AbstractVector, lo::Int, hi::Int, a::PartialQuickSort{Int},
# 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::Int, hi::Int, 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 <= first(a.k)
if j <= first(a)
lo = j+1
elseif j >= last(a.k)
elseif j >= last(a)
hi = j-1
else
if j-lo < hi-j
Expand Down
3 changes: 3 additions & 0 deletions test/sorting.jl
Original file line number Diff line number Diff line change
Expand Up @@ -346,3 +346,6 @@ Base.isless(x :: Twain, y :: Twain) = x.a < y.a
let x = Twain(2,3), y = Twain(2,4)
@test (min(x,y), max(x,y)) == (x,y) == minmax(x,y)
end

# issue #12833 - type stability of sort
@test Base.return_types(sort, (Vector{Int},)) == [Vector{Int}]

0 comments on commit 9e8fcd3

Please sign in to comment.