Skip to content

Commit

Permalink
improve performance of searchsorted(range, lt=<) (#50365)
Browse files Browse the repository at this point in the history
No behavior change, just a performance improvement: searchsorted(range,
lt=<) now performs exactly as fast as searchsorted(range).

Passing lt=< allows searchsorted to find floating point values such as
-0.0, so it's useful to make it fast - see
#44102 (comment).
  • Loading branch information
aplavin authored Sep 30, 2023
1 parent d988f8f commit 5006db1
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 26 deletions.
21 changes: 9 additions & 12 deletions base/ordering.jl
Original file line number Diff line number Diff line change
Expand Up @@ -125,18 +125,15 @@ lt(o::Lt, a, b) = o.lt(a,b)
(lt(p.order, da, db)::Bool) | (!(lt(p.order, db, da)::Bool) & (a < b))
end

_ord(lt::typeof(isless), by::typeof(identity), order::Ordering) = order
_ord(lt::typeof(isless), by, order::Ordering) = By(by, order)

function _ord(lt, by, order::Ordering)
if order === Forward
return Lt((x, y) -> lt(by(x), by(y)))
elseif order === Reverse
return Lt((x, y) -> lt(by(y), by(x)))
else
error("Passing both lt= and order= arguments is ambiguous; please pass order=Forward or order=Reverse (or leave default)")
end
end

_ord(lt::typeof(isless), by, order::Ordering) = _by(by, order)
_ord(lt::typeof(isless), by, order::ForwardOrdering) = _by(by, order) # disambiguation
_ord(lt::typeof(isless), by, order::ReverseOrdering{ForwardOrdering}) = _by(by, order) # disambiguation
_ord(lt, by, order::ForwardOrdering) = _by(by, Lt(lt))
_ord(lt, by, order::ReverseOrdering{ForwardOrdering}) = reverse(_by(by, Lt(lt)))
_ord(lt, by, order::Ordering) = error("Passing both lt= and order= arguments is ambiguous; please pass order=Forward or order=Reverse (or leave default)")
_by(by, order::Ordering) = By(by, order)
_by(::typeof(identity), order::Ordering) = order

"""
ord(lt, by, rev::Union{Bool, Nothing}, order::Ordering=Forward)
Expand Down
17 changes: 10 additions & 7 deletions base/sort.jl
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,10 @@ function searchsorted(v::AbstractVector, x, ilo::T, ihi::T, o::Ordering)::UnitRa
return (lo + 1) : (hi - 1)
end

function searchsortedlast(a::AbstractRange{<:Real}, x::Real, o::DirectOrdering)::keytype(a)

const FastRangeOrderings = Union{DirectOrdering,Lt{typeof(<)},ReverseOrdering{Lt{typeof(<)}}}

function searchsortedlast(a::AbstractRange{<:Real}, x::Real, o::FastRangeOrderings)::keytype(a)
require_one_based_indexing(a)
f, h, l = first(a), step(a), last(a)
if lt(o, x, f)
Expand All @@ -238,7 +241,7 @@ function searchsortedlast(a::AbstractRange{<:Real}, x::Real, o::DirectOrdering):
end
end

function searchsortedfirst(a::AbstractRange{<:Real}, x::Real, o::DirectOrdering)::keytype(a)
function searchsortedfirst(a::AbstractRange{<:Real}, x::Real, o::FastRangeOrderings)::keytype(a)
require_one_based_indexing(a)
f, h, l = first(a), step(a), last(a)
if !lt(o, f, x)
Expand All @@ -251,39 +254,39 @@ function searchsortedfirst(a::AbstractRange{<:Real}, x::Real, o::DirectOrdering)
end
end

function searchsortedlast(a::AbstractRange{<:Integer}, x::Real, o::DirectOrdering)::keytype(a)
function searchsortedlast(a::AbstractRange{<:Integer}, x::Real, o::FastRangeOrderings)::keytype(a)
require_one_based_indexing(a)
f, h, l = first(a), step(a), last(a)
if lt(o, x, f)
0
elseif h == 0 || !lt(o, x, l)
length(a)
else
if o isa ForwardOrdering
if !(o isa ReverseOrdering)
fld(floor(Integer, x) - f, h) + 1
else
fld(ceil(Integer, x) - f, h) + 1
end
end
end

function searchsortedfirst(a::AbstractRange{<:Integer}, x::Real, o::DirectOrdering)::keytype(a)
function searchsortedfirst(a::AbstractRange{<:Integer}, x::Real, o::FastRangeOrderings)::keytype(a)
require_one_based_indexing(a)
f, h, l = first(a), step(a), last(a)
if !lt(o, f, x)
1
elseif h == 0 || lt(o, l, x)
length(a) + 1
else
if o isa ForwardOrdering
if !(o isa ReverseOrdering)
cld(ceil(Integer, x) - f, h) + 1
else
cld(floor(Integer, x) - f, h) + 1
end
end
end

searchsorted(a::AbstractRange{<:Real}, x::Real, o::DirectOrdering) =
searchsorted(a::AbstractRange{<:Real}, x::Real, o::FastRangeOrderings) =
searchsortedfirst(a, x, o) : searchsortedlast(a, x, o)

for s in [:searchsortedfirst, :searchsortedlast, :searchsorted]
Expand Down
25 changes: 18 additions & 7 deletions test/ordering.jl
Original file line number Diff line number Diff line change
Expand Up @@ -2,21 +2,24 @@

using Test

import Base.Order: Forward, Reverse
import Base.Order: Forward, Reverse, ord, Lt, By, ReverseOrdering

# every argument can flip the integer order by passing the right value. Here,
# we enumerate a few of these combinations and check that all these flips
# compound so that in total we either have an increasing or decreasing sort.
for (s1, rev) in enumerate([true, false])
for (s2, lt) in enumerate([>, <, (a, b) -> a - b > 0, (a, b) -> a - b < 0])
for (s2, lt) in enumerate([(a, b)->isless(b, a), isless, >, <, (a, b) -> a - b > 0, (a, b) -> a - b < 0])
for (s3, by) in enumerate([-, +])
for (s4, order) in enumerate([Reverse, Forward])
if iseven(s1 + s2 + s3 + s4)
target = [1, 2, 3]
else
target = [3, 2, 1]
end
is_fwd = iseven(s1 + s2 + s3 + s4)
target = is_fwd ? (1:3) : (3:-1:1)
# arrays, integer and float ranges sometimes have different code paths
@test target == sort([2, 3, 1], rev=rev, lt=lt, by=by, order=order)

@test target == sort(1:3, rev=rev, lt=lt, by=by, order=order)
@test target == sort(3:-1:1, rev=rev, lt=lt, by=by, order=order)
@test float(target) == sort(1.0:3, rev=rev, lt=lt, by=by, order=order)
@test float(target) == sort(3.0:-1:1, rev=rev, lt=lt, by=by, order=order)
end
end
end
Expand All @@ -40,3 +43,11 @@ struct SomeOtherOrder <: Base.Order.Ordering end

@test reverse(Forward) === Reverse
@test reverse(Reverse) === Forward

@test ord(isless, identity, false, Forward) === Forward
@test ord(isless, identity, true, Forward) === Reverse
@test ord(<, identity, false, Forward) === Lt(<)
@test ord(isless, abs, false, Forward) === By(abs)
@test ord(<, abs, false, Forward) === By(abs, Lt(<))
@test ord(<, abs, true, Forward) === ReverseOrdering(By(abs, Lt(<)))
@test ord(<, abs, true, Reverse) === By(abs, Lt(<))
10 changes: 10 additions & 0 deletions test/sorting.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1225,6 +1225,16 @@ end
@test searchsorted(v, 0.1, rev=true) === 4:3
end
end

@testset "ranges issue #44102, PR #50365" begin
# range sorting test for different Ordering parameter combinations
@test searchsorted(-1000.0:1:1000, -0.0) === 1001:1000
@test searchsorted(-1000.0:1:1000, -0.0; lt=<) === 1001:1001
@test searchsorted(-1000.0:1:1000, -0.0; lt=<, by=x->x) === 1001:1001
@test searchsorted(reverse(-1000.0:1:1000), -0.0; lt=<, by=-) === 1001:1001
@test searchsorted(reverse(-1000.0:1:1000), -0.0, rev=true) === 1002:1001
@test searchsorted(reverse(-1000.0:1:1000), -0.0; lt=<, rev=true) === 1001:1001
end
end
# The "searchsorted" testset is at the end of the file because it is slow.

Expand Down

0 comments on commit 5006db1

Please sign in to comment.