From f49cee2f190ff78b7e7cd2729788bab97505f7aa Mon Sep 17 00:00:00 2001 From: Andy Ferris Date: Sat, 21 Apr 2018 15:54:05 +1000 Subject: [PATCH 1/3] Make view of range with range be a range Typical `AbstractRange` types are compact and immutable, and we can remove the overhead of `SubArray` in this case while the cost of `view` remains O(1). Here we specialize only on valid combinations of integer ranges, otherwise still relying on `SubArray`. --- base/subarray.jl | 18 ++++++++++++++++++ test/ranges.jl | 7 +++++++ 2 files changed, 25 insertions(+) diff --git a/base/subarray.jl b/base/subarray.jl index 206abb46e1561..c3b338acb174b 100644 --- a/base/subarray.jl +++ b/base/subarray.jl @@ -134,6 +134,24 @@ function view(A::AbstractArray, I::Vararg{Any,N}) where {N} unsafe_view(_maybe_reshape_parent(A, index_ndims(J...)), J...) end +# Ranges tend to be compact & immutable +function view(r1::OneTo, r2::OneTo) + @_propagate_inbounds_meta + getindex(r1, r2) +end +function view(r1::AbstractUnitRange, r2::AbstractUnitRange{<:Integer}) + @_propagate_inbounds_meta + getindex(r1, r2) +end +function view(r1::AbstractUnitRange, r2::StepRange{<:Integer}) + @_propagate_inbounds_meta + getindex(r1, r2) +end +function view(r1::StepRange, r2::AbstractRange{<:Integer}) + @_propagate_inbounds_meta + getindex(r1, r2) +end + function unsafe_view(A::AbstractArray, I::Vararg{ViewIndex,N}) where {N} @_inline_meta SubArray(A, I) diff --git a/test/ranges.jl b/test/ranges.jl index 9b32428dea766..8899457598779 100644 --- a/test/ranges.jl +++ b/test/ranges.jl @@ -1263,3 +1263,10 @@ end @test step(x) == 0.0 @test x isa StepRangeLen{Float64,Base.TwicePrecision{Float64},Base.TwicePrecision{Float64}} end + +@testset "Views of ranges" begin + @test view(Base.OneTo(10), Base.OneTo(5)) === Base.OneTo(5) + @test view(1:10, 1:5) === 1:5 + @test view(1:10, 1:2:5) === 1:2:5 + @test view(1:2:9, 1:5) === 1:2:9 +end From 40d3bddd42a8ed0ede06d08e6d32bddfaecc8373 Mon Sep 17 00:00:00 2001 From: Matt Bauman Date: Tue, 28 Apr 2020 15:18:55 -0500 Subject: [PATCH 2/3] Cover all implementations of `getindex(::AbstractRange,...)` --- base/subarray.jl | 10 +++++++++- test/ranges.jl | 9 +++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/base/subarray.jl b/base/subarray.jl index d88912f24fe40..4e8af7a03e2ca 100644 --- a/base/subarray.jl +++ b/base/subarray.jl @@ -164,7 +164,7 @@ function view(A::AbstractArray, I::Vararg{Any,N}) where {N} unsafe_view(_maybe_reshape_parent(A, index_ndims(J...)), J...) end -# Ranges tend to be compact & immutable +# Ranges implement getindex to return recomputed ranges; use that for views, too (when possible) function view(r1::OneTo, r2::OneTo) @_propagate_inbounds_meta getindex(r1, r2) @@ -181,6 +181,14 @@ function view(r1::StepRange, r2::AbstractRange{<:Integer}) @_propagate_inbounds_meta getindex(r1, r2) end +function view(r1::StepRangeLen, r2::OrdinalRange{<:Integer}) + @_propagate_inbounds_meta + getindex(r1, r2) +end +function view(r1::LinRange, r2::OrdinalRange{<:Integer}) + @_propagate_inbounds_meta + getindex(r1, r2) +end function unsafe_view(A::AbstractArray, I::Vararg{ViewIndex,N}) where {N} @_inline_meta diff --git a/test/ranges.jl b/test/ranges.jl index dc5d1682ea84e..f2d6414e87942 100644 --- a/test/ranges.jl +++ b/test/ranges.jl @@ -1368,6 +1368,15 @@ end @test view(1:10, 1:5) === 1:5 @test view(1:10, 1:2:5) === 1:2:5 @test view(1:2:9, 1:5) === 1:2:9 + + # Ensure we don't hit a fallback `view` if there's a better `getindex` implementation + vmt = collect(methods(view, Tuple{AbstractRange, AbstractRange})) + for m in methods(getindex, Tuple{AbstractRange, AbstractRange}) + tt = Base.tuple_type_tail(m.sig) + tt == Tuple{AbstractArray,Vararg{Any,N}} where N && continue + vm = findfirst(sig->tt <: Base.tuple_type_tail(sig.sig), vmt) + @test vmt[vm].sig != Tuple{typeof(view),AbstractArray,Vararg{Any,N}} where N + end end @testset "Issue #26608" begin From 21b3e0d997801599d88fef84e228cce53c0a11d6 Mon Sep 17 00:00:00 2001 From: Matt Bauman Date: Tue, 28 Apr 2020 15:23:42 -0500 Subject: [PATCH 3/3] Add NEWS --- NEWS.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/NEWS.md b/NEWS.md index 45e89d4897e94..7c7b615c33d3e 100644 --- a/NEWS.md +++ b/NEWS.md @@ -143,6 +143,8 @@ Standard library changes * A 1-d `Zip` iterator (where `Base.IteratorSize` is `Base.HasShape{1}()`) with defined length of `n` has now also size of `(n,)` (instead of throwing an error with truncated iterators) ([#29927]). * The `@timed` macro now returns a `NamedTuple` ([#34149]) * New `supertypes(T)` function returns a tuple of all supertypes of `T` ([#34419]). +* Views of builtin ranges are now recomputed ranges (like indexing returns) instead of + `SubArray`s ([#26872]). * Sorting-related functions such as `sort` that take the keyword arguments `lt`, `rev`, `order` and `by` now do not discard `order` if `by` or `lt` are passed. In the former case, the order from `order` is used to compare the values of `by(element)`. In the latter case,