From 72f2491210374dc7828929de1c24aa6c9e87edd2 Mon Sep 17 00:00:00 2001 From: Lilith Hafner Date: Wed, 11 Jan 2023 15:32:38 -0600 Subject: [PATCH 1/2] stop using `rand(lo:hi)` for QuickerSort pivot selection --- base/sort.jl | 14 ++++---------- stdlib/Random/src/Random.jl | 6 ------ 2 files changed, 4 insertions(+), 16 deletions(-) diff --git a/base/sort.jl b/base/sort.jl index 252d5b962d83b..f51fb95d9c575 100644 --- a/base/sort.jl +++ b/base/sort.jl @@ -983,21 +983,15 @@ QuickerSort(lo::Union{Integer, Missing}, hi::Union{Integer, Missing}) = QuickerS QuickerSort(lo::Union{Integer, Missing}, next::Algorithm=SMALL_ALGORITHM) = QuickerSort(lo, lo, next) QuickerSort(r::OrdinalRange, next::Algorithm=SMALL_ALGORITHM) = QuickerSort(first(r), last(r), next) -# select a pivot for QuickerSort -# -# This method is redefined to rand(lo:hi) in Random.jl -# We can't use rand here because it is not available in Core.Compiler and -# because rand is defined in the stdlib Random.jl after sorting is used in Base. -select_pivot(lo::Integer, hi::Integer) = typeof(hi-lo)(hash(lo) % (hi-lo+1)) + lo - # select a pivot, partition v[lo:hi] according # to the pivot, and store the result in t[lo:hi]. # -# returns (pivot, pivot_index) where pivot_index is the location the pivot -# should end up, but does not set t[pivot_index] = pivot +# sets `pivot_dest[pivot_index+pivot_index_offset] = pivot` and returns that index. function partition!(t::AbstractVector, lo::Integer, hi::Integer, offset::Integer, o::Ordering, v::AbstractVector, rev::Bool, pivot_dest::AbstractVector, pivot_index_offset::Integer) - pivot_index = select_pivot(lo, hi) + # Ideally we would use `pivot_index = rand(lo:hi)`, but a dependency on Random.jl + # complicates things and some folks object to mutating the global RNG in sorting. + pivot_index = typeof(hi-lo)(hash(lo) % (hi-lo+1)) + lo @inbounds begin pivot = v[pivot_index] while lo < pivot_index diff --git a/stdlib/Random/src/Random.jl b/stdlib/Random/src/Random.jl index bc016fc1cd057..8da2dd6f3e9c7 100644 --- a/stdlib/Random/src/Random.jl +++ b/stdlib/Random/src/Random.jl @@ -434,10 +434,4 @@ true """ seed!(rng::AbstractRNG, ::Nothing) = seed!(rng) -# Randomize quicksort pivot selection. This code is here because of bootstrapping: -# we need to sort things before we load this standard library. -# TODO move this into Sort.jl -Base.delete_method(only(methods(Base.Sort.select_pivot))) -Base.Sort.select_pivot(lo::Integer, hi::Integer) = rand(lo:hi) - end # module From a809c2227cada8a9fb3597d74af051a0b83e10b9 Mon Sep 17 00:00:00 2001 From: Lilith Orion Hafner Date: Wed, 11 Jan 2023 16:47:51 -0600 Subject: [PATCH 2/2] Update base/sort.jl Co-authored-by: Fredrik Ekre --- base/sort.jl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/base/sort.jl b/base/sort.jl index f51fb95d9c575..485b9d7fe1d14 100644 --- a/base/sort.jl +++ b/base/sort.jl @@ -989,8 +989,8 @@ QuickerSort(r::OrdinalRange, next::Algorithm=SMALL_ALGORITHM) = QuickerSort(firs # sets `pivot_dest[pivot_index+pivot_index_offset] = pivot` and returns that index. function partition!(t::AbstractVector, lo::Integer, hi::Integer, offset::Integer, o::Ordering, v::AbstractVector, rev::Bool, pivot_dest::AbstractVector, pivot_index_offset::Integer) - # Ideally we would use `pivot_index = rand(lo:hi)`, but a dependency on Random.jl - # complicates things and some folks object to mutating the global RNG in sorting. + # Ideally we would use `pivot_index = rand(lo:hi)`, but that requires Random.jl + # and would mutate the global RNG in sorting. pivot_index = typeof(hi-lo)(hash(lo) % (hi-lo+1)) + lo @inbounds begin pivot = v[pivot_index]