Skip to content

Commit

Permalink
Orthogonalize re-indexing for FastSubArrays (#53369)
Browse files Browse the repository at this point in the history
By separating out the re-indexing step for `FastSubArray`s and
specializing this for `FastContiguousSubArray`s, we don't need to define
specialized `getindex`, `setindex!` and `isassigned` for
`FastContiguousSubArray`s anymore. The fallback method for
`FastSubArray`s will correctly handle the special case.
  • Loading branch information
jishnub authored Feb 18, 2024
1 parent b5221e1 commit 16871e7
Showing 1 changed file with 28 additions and 58 deletions.
86 changes: 28 additions & 58 deletions base/subarray.jl
Original file line number Diff line number Diff line change
Expand Up @@ -320,47 +320,46 @@ end

# But SubArrays with fast linear indexing pre-compute a stride and offset
FastSubArray{T,N,P,I} = SubArray{T,N,P,I,true}
# We define a convenience functions to compute the shifted parent index
# This differs from reindex as this accepts the view directly, instead of its indices
@inline _reindexlinear(V::FastSubArray, i::Int) = V.offset1 + V.stride1*i
@inline _reindexlinear(V::FastSubArray, i::AbstractUnitRange{Int}) = V.offset1 .+ V.stride1 .* i

function getindex(V::FastSubArray, i::Int)
@inline
@boundscheck checkbounds(V, i)
@inbounds r = V.parent[V.offset1 + V.stride1*i]
@inbounds r = V.parent[_reindexlinear(V, i)]
r
end
# We can avoid a multiplication if the first parent index is a Colon or AbstractUnitRange,
# or if all the indices are scalars, i.e. the view is for a single value only
FastContiguousSubArray{T,N,P,I<:Union{Tuple{Union{Slice, AbstractUnitRange}, Vararg{Any}},
Tuple{Vararg{ScalarIndex}}}} = SubArray{T,N,P,I,true}
function getindex(V::FastContiguousSubArray, i::Int)

# For vector views with linear indexing, we disambiguate to favor the stride/offset
# computation as that'll generally be faster than (or just as fast as) re-indexing into a range.
function getindex(V::FastSubArray{<:Any, 1}, i::Int)
@inline
@boundscheck checkbounds(V, i)
@inbounds r = V.parent[V.offset1 + i]
@inbounds r = V.parent[_reindexlinear(V, i)]
r
end

# We can avoid a multiplication if the first parent index is a Colon or AbstractUnitRange,
# or if all the indices are scalars, i.e. the view is for a single value only
FastContiguousSubArray{T,N,P,I<:Union{Tuple{Union{Slice, AbstractUnitRange}, Vararg{Any}},
Tuple{Vararg{ScalarIndex}}}} = SubArray{T,N,P,I,true}

@inline _reindexlinear(V::FastContiguousSubArray, i::Int) = V.offset1 + i
@inline _reindexlinear(V::FastContiguousSubArray, i::AbstractUnitRange{Int}) = V.offset1 .+ i

# parents of FastContiguousSubArrays may support fast indexing with AbstractUnitRanges,
# so we may just forward the indexing to the parent
# This may only be done for non-offset ranges, as the result would otherwise have offset axes
const OneBasedRanges = Union{OneTo{Int}, UnitRange{Int}, Slice{OneTo{Int}}, IdentityUnitRange{OneTo{Int}}}
function getindex(V::FastContiguousSubArray, i::OneBasedRanges)
@inline
@boundscheck checkbounds(V, i)
@inbounds r = V.parent[V.offset1 .+ i]
@inbounds r = V.parent[_reindexlinear(V, i)]
r
end

# For vector views with linear indexing, we disambiguate to favor the stride/offset
# computation as that'll generally be faster than (or just as fast as) re-indexing into a range.
function getindex(V::FastSubArray{<:Any, 1}, i::Int)
@inline
@boundscheck checkbounds(V, i)
@inbounds r = V.parent[V.offset1 + V.stride1*i]
r
end
function getindex(V::FastContiguousSubArray{<:Any, 1}, i::Int)
@inline
@boundscheck checkbounds(V, i)
@inbounds r = V.parent[V.offset1 + i]
r
end
@inline getindex(V::FastContiguousSubArray, i::Colon) = getindex(V, to_indices(V, (:,))...)

# Indexed assignment follows the same pattern as `getindex` above
Expand All @@ -373,40 +372,23 @@ end
function setindex!(V::FastSubArray, x, i::Int)
@inline
@boundscheck checkbounds(V, i)
@inbounds V.parent[V.offset1 + V.stride1*i] = x
@inbounds V.parent[_reindexlinear(V, i)] = x
V
end
function setindex!(V::FastContiguousSubArray, x, i::Int)
function setindex!(V::FastSubArray{<:Any, 1}, x, i::Int)
@inline
@boundscheck checkbounds(V, i)
@inbounds V.parent[V.offset1 + i] = x
@inbounds V.parent[_reindexlinear(V, i)] = x
V
end

function setindex!(V::FastSubArray, x, i::AbstractUnitRange{Int})
@inline
@boundscheck checkbounds(V, i)
@inbounds V.parent[V.offset1 .+ V.stride1 .* i] = x
V
end
function setindex!(V::FastContiguousSubArray, x, i::AbstractUnitRange{Int})
@inline
@boundscheck checkbounds(V, i)
@inbounds V.parent[V.offset1 .+ i] = x
@inbounds V.parent[_reindexlinear(V, i)] = x
V
end

function setindex!(V::FastSubArray{<:Any, 1}, x, i::Int)
@inline
@boundscheck checkbounds(V, i)
@inbounds V.parent[V.offset1 + V.stride1*i] = x
V
end
function setindex!(V::FastContiguousSubArray{<:Any, 1}, x, i::Int)
@inline
@boundscheck checkbounds(V, i)
@inbounds V.parent[V.offset1 + i] = x
V
end
@inline setindex!(V::FastSubArray, x, i::Colon) = setindex!(V, x, to_indices(V, (i,))...)

function isassigned(V::SubArray{T,N}, I::Vararg{Int,N}) where {T,N}
Expand All @@ -418,25 +400,13 @@ end
function isassigned(V::FastSubArray, i::Int)
@inline
@boundscheck checkbounds(Bool, V, i) || return false
@inbounds r = isassigned(V.parent, V.offset1 + V.stride1*i)
r
end
function isassigned(V::FastContiguousSubArray, i::Int)
@inline
@boundscheck checkbounds(Bool, V, i) || return false
@inbounds r = isassigned(V.parent, V.offset1 + i)
@inbounds r = isassigned(V.parent, _reindexlinear(V, i))
r
end
function isassigned(V::FastSubArray{<:Any, 1}, i::Int)
@inline
@boundscheck checkbounds(Bool, V, i) || return false
@inbounds r = isassigned(V.parent, V.offset1 + V.stride1*i)
r
end
function isassigned(V::FastContiguousSubArray{<:Any, 1}, i::Int)
@inline
@boundscheck checkbounds(Bool, V, i) || return false
@inbounds r = isassigned(V.parent, V.offset1 + i)
@inbounds r = isassigned(V.parent, _reindexlinear(V, i))
r
end

Expand Down

2 comments on commit 16871e7

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Executing the daily package evaluation, I will reply here when finished:

@nanosoldier runtests(isdaily = true)

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The package evaluation job you requested has completed - possible new issues were detected.
The full report is available.

Please sign in to comment.