Skip to content

Commit 9aa2e68

Browse files
thchrmbauman
andauthored
skip unnecessary alias-check in collect(::AbstractArray) from copyto! (#55748)
As discussed on Slack with @MasonProtter & @jakobnissen, `collect` currently does a usually cheap - but sometimes expensive - aliasing check (via `unalias`->`mightalias`->`dataid` -> `objectid`) before copying contents over; this check is unnecessary, however, since the source array is newly created and cannot possibly alias the input. This PR fixes that by swapping from `copyto!` to `copyto_unaliased!` in the `_collect_indices` implementations where the swap is straightforward (e.g., it is not so straightforward for the fallback `_collect_indices(indsA, A)`, so I skipped it there). This improves the following example substantially: ```jl struct GarbageVector{N} <: AbstractVector{Int} v :: Vector{Int} garbage :: NTuple{N, Int} end GarbageVector{N}(v::Vector{Int}) where N = GarbageVector{N}(v, ntuple(identity, Val(N))) Base.getindex(gv::GarbageVector, i::Int) = gv.v[i] Base.size(gv::GarbageVector) = size(gv.v) using BenchmarkTools v = rand(Int, 10) gv = GarbageVector{100}(v) @Btime collect($v); # 30 ns (v1.10.4) -> 30 ns (PR) @Btime collect($gv); # 179 ns (v1.10.4) -> 30 ns (PR) ``` Relatedly, it seems the fact that `mightalias` is comparing immutable contents as well - and hence slowing down the `unalias` check for the above `GarbageVector` via a slow `objectid` on tuples - is suboptimal. I don't know how to fix that though, so I'd like to leave that outside this PR. (Probably related to #26237) Co-authored-by: Matt Bauman <[email protected]>
1 parent 96b5b2c commit 9aa2e68

File tree

1 file changed

+10
-3
lines changed

1 file changed

+10
-3
lines changed

base/array.jl

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -748,9 +748,16 @@ function _collect(cont, itr, ::HasEltype, isz::SizeUnknown)
748748
return a
749749
end
750750

751-
_collect_indices(::Tuple{}, A) = copyto!(Array{eltype(A),0}(undef), A)
752-
_collect_indices(indsA::Tuple{Vararg{OneTo}}, A) =
753-
copyto!(Array{eltype(A)}(undef, length.(indsA)), A)
751+
function _collect_indices(::Tuple{}, A)
752+
dest = Array{eltype(A),0}(undef)
753+
isempty(A) && return dest
754+
return copyto_unaliased!(IndexStyle(dest), dest, IndexStyle(A), A)
755+
end
756+
function _collect_indices(indsA::Tuple{Vararg{OneTo}}, A)
757+
dest = Array{eltype(A)}(undef, length.(indsA))
758+
isempty(A) && return dest
759+
return copyto_unaliased!(IndexStyle(dest), dest, IndexStyle(A), A)
760+
end
754761
function _collect_indices(indsA, A)
755762
B = Array{eltype(A)}(undef, length.(indsA))
756763
copyto!(B, CartesianIndices(axes(B)), A, CartesianIndices(indsA))

0 commit comments

Comments
 (0)