From 40d4f4a0d71b3129282e58b9a2f9b7f930ad13d3 Mon Sep 17 00:00:00 2001 From: Jameson Nash Date: Mon, 5 Apr 2021 20:30:25 -0400 Subject: [PATCH] searchsorted ranges: simplify code, fix bug with Unsigned needle This avoids computing `length` unnecessarily, which often requires a division, and is therefore often expensive. --- base/sort.jl | 82 ++++++++++++++++--------------------------------- test/sorting.jl | 45 +++++++++++++++++---------- 2 files changed, 54 insertions(+), 73 deletions(-) diff --git a/base/sort.jl b/base/sort.jl index 6a3883215c0b4..1a994398bea04 100644 --- a/base/sort.jl +++ b/base/sort.jl @@ -231,92 +231,62 @@ end function searchsortedlast(a::AbstractRange{<:Real}, x::Real, o::DirectOrdering)::keytype(a) require_one_based_indexing(a) - if step(a) == 0 - lt(o, x, first(a)) ? 0 : length(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 - n = round(Integer, clamp((x - first(a)) / step(a) + 1, 1, length(a))) + n = round(Integer, (x - f) / h + 1) lt(o, x, a[n]) ? n - 1 : n end end function searchsortedfirst(a::AbstractRange{<:Real}, x::Real, o::DirectOrdering)::keytype(a) require_one_based_indexing(a) - if step(a) == 0 - lt(o, first(a), x) ? length(a) + 1 : 1 + 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 - n = round(Integer, clamp((x - first(a)) / step(a) + 1, 1, length(a))) + n = round(Integer, (x - f) / h + 1) lt(o, a[n], x) ? n + 1 : n end end function searchsortedlast(a::AbstractRange{<:Integer}, x::Real, o::DirectOrdering)::keytype(a) require_one_based_indexing(a) - h = step(a) - if h == 0 - lt(o, x, first(a)) ? 0 : length(a) - elseif h > 0 && x < first(a) - firstindex(a) - 1 - elseif h > 0 && x >= last(a) - lastindex(a) - elseif h < 0 && x > first(a) - firstindex(a) - 1 - elseif h < 0 && x <= last(a) - lastindex(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 - fld(floor(Integer, x) - first(a), h) + 1 + fld(floor(Integer, x) - f, h) + 1 else - fld(ceil(Integer, x) - first(a), h) + 1 + fld(ceil(Integer, x) - f, h) + 1 end end end function searchsortedfirst(a::AbstractRange{<:Integer}, x::Real, o::DirectOrdering)::keytype(a) require_one_based_indexing(a) - h = step(a) - if h == 0 - lt(o, first(a), x) ? length(a)+1 : 1 - elseif h > 0 && x <= first(a) - firstindex(a) - elseif h > 0 && x > last(a) - lastindex(a) + 1 - elseif h < 0 && x >= first(a) - firstindex(a) - elseif h < 0 && x < last(a) - lastindex(a) + 1 + 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 - -fld(floor(Integer, -x) + Signed(first(a)), h) + 1 + cld(ceil(Integer, x) - f, h) + 1 else - -fld(ceil(Integer, -x) + Signed(first(a)), h) + 1 + cld(floor(Integer, x) - f, h) + 1 end end end -function searchsortedfirst(a::AbstractRange{<:Integer}, x::Unsigned, o::DirectOrdering)::keytype(a) - require_one_based_indexing(a) - if lt(o, first(a), x) - if step(a) == 0 - length(a) + 1 - else - min(cld(x - first(a), step(a)), length(a)) + 1 - end - else - 1 - end -end - -function searchsortedlast(a::AbstractRange{<:Integer}, x::Unsigned, o::DirectOrdering)::keytype(a) - require_one_based_indexing(a) - if lt(o, x, first(a)) - 0 - elseif step(a) == 0 - length(a) - else - min(fld(x - first(a), step(a)) + 1, length(a)) - end -end - searchsorted(a::AbstractRange{<:Real}, x::Real, o::DirectOrdering) = searchsortedfirst(a, x, o) : searchsortedlast(a, x, o) diff --git a/test/sorting.jl b/test/sorting.jl index 6bf7d60bd859d..718a7f819e203 100644 --- a/test/sorting.jl +++ b/test/sorting.jl @@ -105,7 +105,7 @@ end @test searchsorted(fill(R(1), 15), T(1), 6, 10, Forward) == 6:10 end - for (rg,I) in [(49:57,47:59), (1:2:17,-1:19), (-3:0.5:2,-5:.5:4)] + for (rg,I) in Any[(49:57,47:59), (1:2:17,-1:19), (-3:0.5:2,-5:.5:4)] rg_r = reverse(rg) rgv, rgv_r = [rg;], [rg_r;] for i = I @@ -144,7 +144,7 @@ end @testset "issue 32568" begin for R in numTypes, T in numTypes - for arr in [R[1:5;], R(1):R(5), R(1):2:R(5)] + for arr in Any[R[1:5;], R(1):R(5), R(1):2:R(5)] @test eltype(searchsorted(arr, T(2))) == keytype(arr) @test eltype(searchsorted(arr, T(2), big(1), big(4), Forward)) == keytype(arr) @test searchsortedfirst(arr, T(2)) isa keytype(arr) @@ -164,35 +164,46 @@ end @test searchsorted([1,2], Inf) === 3:2 @test searchsorted(1:2, Inf) === 3:2 - for coll in [ + for coll in Any[ Base.OneTo(10), 1:2, + 0x01:0x02, -4:6, 5:2:10, [1,2], 1.0:4, [10.0,20.0], ] - for huge in [Inf, 1e300] + for huge in Any[Inf, 1e300, typemax(Int64), typemax(UInt64)] @test searchsortedfirst(coll, huge) === lastindex(coll) + 1 - @test searchsortedfirst(coll, -huge)=== firstindex(coll) @test searchsortedlast(coll, huge) === lastindex(coll) - @test searchsortedlast(coll, -huge) === firstindex(coll) - 1 @test searchsorted(coll, huge) === lastindex(coll)+1 : lastindex(coll) - @test searchsorted(coll, -huge) === firstindex(coll) : firstindex(coll) - 1 - - @test searchsortedfirst(reverse(coll), huge, rev=true) === firstindex(coll) - @test searchsortedfirst(reverse(coll), -huge, rev=true) === lastindex(coll) + 1 - @test searchsortedlast(reverse(coll), huge, rev=true) === firstindex(coll) - 1 - @test searchsortedlast(reverse(coll), -huge, rev=true) === lastindex(coll) - @test searchsorted(reverse(coll), huge, rev=true) === firstindex(coll):firstindex(coll) - 1 - @test searchsorted(reverse(coll), -huge, rev=true) === lastindex(coll)+1:lastindex(coll) + if !(eltype(coll) <: Unsigned) + @test searchsortedfirst(reverse(coll), huge, rev=true) === firstindex(coll) + @test searchsortedlast(reverse(coll), huge, rev=true) === firstindex(coll) - 1 + @test searchsorted(reverse(coll), huge, rev=true) === firstindex(coll):firstindex(coll) - 1 + end + + if !(huge isa Unsigned) + @test searchsortedfirst(coll, -huge)=== firstindex(coll) + @test searchsortedlast(coll, -huge) === firstindex(coll) - 1 + @test searchsorted(coll, -huge) === firstindex(coll) : firstindex(coll) - 1 + if !(eltype(coll) <: Unsigned) + @test searchsortedfirst(reverse(coll), -huge, rev=true) === lastindex(coll) + 1 + @test searchsortedlast(reverse(coll), -huge, rev=true) === lastindex(coll) + @test searchsorted(reverse(coll), -huge, rev=true) === lastindex(coll)+1:lastindex(coll) + end + end end end - @testset "issue ##34408" begin + + @test_broken length(reverse(0x1:0x2)) == 2 + @testset "issue #34408" begin r = 1f8-10:1f8 # collect(r) = Float32[9.999999e7, 9.999999e7, 9.999999e7, 9.999999e7, 1.0e8, 1.0e8, 1.0e8, 1.0e8, 1.0e8] - @test_broken searchsorted(collect(r)) == searchsorted(r) + for i in r + @test_broken searchsorted(collect(r), i) == searchsorted(r, i) + end end end @testset "issue #35272" begin @@ -329,7 +340,7 @@ end @test insorted(T(10), R.(collect(1:10)), by=(>=(5))) end - for (rg,I) in [(49:57,47:59), (1:2:17,-1:19), (-3:0.5:2,-5:.5:4)] + for (rg,I) in Any[(49:57,47:59), (1:2:17,-1:19), (-3:0.5:2,-5:.5:4)] rg_r = reverse(rg) rgv, rgv_r = collect(rg), collect(rg_r) for i = I