-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
iterate
on SubArray
uses slow default fallback
#43295
Comments
Use a function Base.iterate(x::SubArray, state = (Iterators.product(x.indices...),))
Base.@_propagate_inbounds_meta
y = iterate(state...)
y === nothing && return nothing
x.parent[y[1]...], (state[1], Base.tail(y)...)
end Before: @btime f($(view(v, 1:2^12))); # 2.189 μs (0 allocations: 0 bytes) After: @btime f($(view(v, 1:2^12))); #1.040 μs (0 allocations: 0 bytes) Edit: There's no performance difference if we change eltype to |
Does the following work for you? Seems to fix it for me. function iterate(A::SubArray, state=(eachindex(A),))
y = iterate(state...)
y === nothing && return nothing
# indices of subarray are assumed valid
@inbounds(A[y[1]]), (state[1], tail(y)...)
end edit: On second thought, even though |
The 1-based I'm not sure should we close this issue. The MWE has been fixed, but the whole problem has not been solved. |
Right, let's keep the issue open. The fix in #48720 (thanks again, @matthias314) will probably fix the issue for most users, but far from all. |
I've started a discussion in #48773 to gauge the support for an approach based on |
It turns out the default fallback is actually rather slow for
SubArray
:I think this is rather bad as
SubArray
is a fairly fundamental type, and people tend to use it specifically when they are looking for performance.It appears to be the boundschecking that is particularly expensive for
SubArray
. #42030 is related to this, but even if it is decided that default implementations should not use@inbounds
, this should still be solved by creating a specialized method forSubArray
.This occurs on Julia 1.6.3, 1.7.0 and master as of 2021-12-02.
The text was updated successfully, but these errors were encountered: