inline to_indices

JuliaArrays · Feb 28, 2021 · 21956e7 · 21956e7
1 parent e26f0cf
commit 21956e7
Show file tree

Hide file tree

Showing 4 changed files with 27 additions and 57 deletions.
diff --git a/benchmark/benchmarks.jl b/benchmark/benchmarks.jl
@@ -3,13 +3,13 @@ using OffsetArrays
 
 const dim = 1000
 
-x = Array{Float64}(undef, 2*dim)
-y = OffsetArray{Float64}(undef, -dim + 1 : dim)
-x2d = Array{Float64}(undef, 2*dim, 2*dim)
-y2d = OffsetArray{Float64}(undef, -dim + 1 : dim, -dim + 1 : dim)
+x = Array{Float64}(undef, 2*dim);
+y = OffsetArray{Float64}(undef, -dim + 1 : dim);
+x2d = Array{Float64}(undef, 2*dim, 2*dim);
+y2d = OffsetArray{Float64}(undef, -dim + 1 : dim, -dim + 1 : dim);
 
-r = OffsetVector(1:dim, 0)
-s = OffsetVector(1:dim, 0)
+r = OffsetVector(1:dim, 0);
+s = OffsetVector(1:dim, 0);
 
 fill1d(x) = for i in axes(x,1); x[i] = i; end
 fill2d(x) = for j in axes(x,2); for i in axes(x,1); x[i,j] = i + j; end; end
@@ -23,8 +23,9 @@ unsafe_update(x) = @inbounds(for i in axes(x,1); x[i] = x[i] + i; end)
 unsafe_update2d(x) = @inbounds(for j in axes(x,2); for i in axes(x,1); x[i,j] = x[i,j] + i + j; end; end)
 unsafe_update_eachindex(x) = @inbounds(for i in eachindex(x); x[i] = x[i] + i; end)
 
-vectorindexing(a, s) = a[s]
-nestedvectorindexing(a, r, s) = a[r[s]]
+vectorlinearindexing(a, ax) = a[ax]
+vectorCartesianindexing(a, ax1, ax2) = a[ax1, ax2]
+nestedvectorlinearindexing(a, ax1, ax2) = a[ax1[ax2]]
 
 macro showbenchmark(ex)
 	print(ex, " : ")
@@ -55,5 +56,10 @@ end
 @showbenchmark unsafe_update_eachindex(x)
 @showbenchmark unsafe_update_eachindex(y)
 
-@showbenchmark vectorindexing(x, s)
-@showbenchmark nestedvectorindexing(x, r, s)
+# Benchmarks of vector indexing using OffsetRanges as axes
+@showbenchmark vectorlinearindexing(x, s)
+@showbenchmark vectorlinearindexing(x, parent(s))
+@showbenchmark vectorCartesianindexing(x2d, s, s)
+@showbenchmark vectorCartesianindexing(x2d, parent(s), parent(s))
+@showbenchmark nestedvectorlinearindexing(x, r, s)
+@showbenchmark nestedvectorlinearindexing(x, parent(r), parent(s))
diff --git a/src/OffsetArrays.jl b/src/OffsetArrays.jl
@@ -344,27 +344,27 @@ Base.checkindex(::Type{Bool}, inds::AbstractUnitRange, or::OffsetRange) = Base.c
 @inline function Base.getindex(a::OffsetRange, r::OffsetRange)
     @boundscheck checkbounds(a, r)
     @inbounds pr = a.parent[r.parent .- a.offsets[1]]
-    _maybewrapoffset(pr, axes(r,1))
+    OffsetArray(pr, axes(r,1))
 end
 @inline function Base.getindex(a::OffsetRange, r::IdOffsetRange)
     @boundscheck checkbounds(a, r)
     @inbounds pr = a.parent[r.parent .+ (r.offset - a.offsets[1])]
-    _maybewrapoffset(pr, axes(r,1))
+    OffsetArray(pr, axes(r,1))
 end
 @inline function Base.getindex(a::OffsetRange, r::AbstractRange)
     @boundscheck checkbounds(a, r)
     @inbounds pr = a.parent[r .- a.offsets[1]]
-    _maybewrapoffset(pr, axes(r,1))
+    OffsetArray(pr, axes(r,1))
 end
 @inline function Base.getindex(a::AbstractRange, r::OffsetRange)
     @boundscheck checkbounds(a, r)
     @inbounds pr = a[parent(r)]
-    _maybewrapoffset(pr, axes(r,1))
+    OffsetArray(pr, axes(r,1))
 end
 
 # An OffsetUnitRange{<:Integer} has an equivalent IdOffsetRange with the same values and axes
 # We may replace the former with the latter in an indexing operation to obtain a performance boost
-function Base.to_indices(A::AbstractArray, ax::Tuple, I::Tuple{OffsetUnitRange{<:Integer}, Vararg{Any}})
+@inline function Base.to_indices(A::AbstractArray, ax::Tuple, I::Tuple{OffsetUnitRange{<:Integer}, Vararg{Any}})
     or = first(I)
     r = parent(or)
     of = or.offsets[1]

diff --git a/src/utils.jl b/src/utils.jl
@@ -72,10 +72,10 @@ function _checkindices(N::Integer, indices, label)
     N == length(indices) || throw_argumenterror(N, indices, label)
 end
 
-_maybewrapoffset(r::AbstractUnitRange{<:Integer}, ::Base.OneTo) = no_offset_view(r)
-_maybewrapoffset(r::AbstractVector, ::Base.OneTo) = no_offset_view(r)
-function _maybewrapoffset(r::AbstractUnitRange{<:Integer}, ax)
+@inline _maybewrapoffset(r::AbstractUnitRange{<:Integer}, ::Base.OneTo) = no_offset_view(r)
+@inline _maybewrapoffset(r::AbstractVector, ::Base.OneTo) = no_offset_view(r)
+@inline function _maybewrapoffset(r::AbstractUnitRange{<:Integer}, ax)
 	of = first(ax) - 1
-	IdOffsetRange(UnitRange(r) .- of, of)
+	IdOffsetRange(UnitRange(first(r) - of, last(r) - of), of)
 end
-_maybewrapoffset(r::AbstractVector, ax) = OffsetArray(r, ax)
+@inline _maybewrapoffset(r::AbstractVector, ax) = OffsetArray(r, ax)
diff --git a/test/runtests.jl b/test/runtests.jl
@@ -871,18 +871,6 @@ end
             if r1 isa AbstractRange && axes(r2, 1) isa Base.OneTo
                 @test r1[r2] isa AbstractRange
             end
-
-            # This is a strong test to ensure that the indexing operation
-            # (::AbstractUnitRange{<:Integer})[::AbstractUnitRange{<:Integer}]
-            # returns an AbstractUnitRange{<:Integer} and not an AbstractVector.
-            # This is because the result may be used further in indexing, and indexing
-            # is more performant with AbstractUnitRanges than with AbstractVectors.
-            # This may not hold in general for custom AbstractUnitRanges
-            # depending on how they implement getindex,
-            # but should hold for the known types that are being tested here
-            if r1 isa AbstractUnitRange{<:Integer} && r2 isa AbstractUnitRange{<:Integer}
-                @test r1[r2] isa AbstractUnitRange{<:Integer}
-            end
         end
     end
 
@@ -915,26 +903,14 @@ end
             if axes(r2, 1) isa Base.OneTo
                 @test r1[r2] isa AbstractRange
             end
-
-            # This is a strong test to ensure that the indexing operation
-            # (::AbstractUnitRange{<:Integer})[::AbstractUnitRange{<:Integer}]
-            # returns an AbstractUnitRange{<:Integer} and not an AbstractVector.
-            # This is because the result may be used further in indexing, and indexing
-            # is more performant with AbstractUnitRanges than with AbstractVectors.
-            # This may not hold in general for custom AbstractUnitRanges
-            # depending on how they implement getindex,
-            # but should hold for the known types that are being tested here
-            if r1 isa AbstractUnitRange{<:Integer} && r2 isa AbstractUnitRange{<:Integer}
-                @test r1[r2] isa AbstractUnitRange{<:Integer}
-            end
         end
     end
 end
 
 @testset "Vector indexing with offset ranges" begin
     r = OffsetArray(8:10, -1:1)
     r1 = r[0:1]
-    @test r1 === 9:10
+    @test r1 == 9:10
     r1 = (8:10)[OffsetArray(1:2, -5:-4)]
     @test axes(r1) == (-5:-4,)
     @test no_offset_view(r1) == 8:9
@@ -1017,18 +993,6 @@ end
             ]
 
             test_indexing_axes_and_vals(r1, r2)
-
-            # This is a strong test to ensure that the indexing operation
-            # (::AbstractUnitRange{<:Integer})[::AbstractUnitRange{<:Integer}]
-            # returns an AbstractUnitRange{<:Integer} and not an AbstractVector.
-            # This is because the result may be used further in indexing, and indexing
-            # is more performant with AbstractUnitRanges than with AbstractVectors.
-            # This may not hold in general for custom AbstractUnitRanges
-            # depending on how they implement getindex,
-            # but should hold for the known types that are being tested here
-            if r1 isa AbstractUnitRange{<:Integer} && r2 isa AbstractUnitRange{<:Integer}
-                @test r1[r2] isa AbstractUnitRange{<:Integer}
-            end
         end
     end
 end