-
-
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
Improve linear indexing performance for FastSubArrays #45371
Conversation
There are a lot of lines changes in 4232be0, but that's mainly to get the |
I believe it's better to open a separate PR for this because people usually do squash-and-merge. We can merge the separated PR quickly since it's only a test dependency. I left the version note here so we need to update it as well
|
Sounds good, I'll roll this back and add the |
8230203
to
2313165
Compare
I'm suppressing that LLVM fails to vectorlize the copy part of general |
Gentle bump. This performance issue exists on the current master ( |
Seems nice to get in. |
2313165
to
fcde993
Compare
This will be an offset array in general, so we need to restrict the method to 1-based ranges. This restores the behavior on `v"1.10"`. The method was added in #45371
) It's sad that compiler can't do this automatically. Some benchmark with `setindex!`: ```julia julia> a = zeros(Int, 100, 100); julia> @Btime $a[:,:] = $(1:10000); 1.340 μs (0 allocations: 0 bytes) #master: 3.350 μs (0 allocations: 0 bytes) julia> @Btime $a[:,:] = $(view(LinearIndices(a), 1:100, 1:100)); 10.000 μs (0 allocations: 0 bytes) #master: 11.000 μs (0 allocations: 0 bytes) ``` BTW optimization for `FastSubArray` introduced in #45371 still work after this change as the parent array might have their own `copyto!` optimization.
This PR forwards
AbstractUnitRange
indices forFastSubArrays
to the parent, making use of the fact that the parent might have efficient vector indexing methods defined.Some benchmarks:
getindex benchmarking script
on master
this PR:
setindex! benchmarking script
master
this PR: