Skip to content

Conversation

@jishnub
Copy link
Member

@jishnub jishnub commented Apr 21, 2024

SubArrays have a specialized aliasing check that uses both the parent and the indices to detect potential overlaps. However, wrappers of SubArrays use only the dataids, which is usually determined only by the parent, ignoring the indices. This PR extends the aliasing check to reshaped subarrays as well, by forwarding the check to the parent subarray. This makes the following not allocate:

On master:

julia> C = rand(2000,2000);

julia> @btime $C[2:end, :] .= permutedims(@view $C[1, :]);
  2.816 ms (3 allocations: 15.70 KiB)

This PR

julia> @btime $C[2:end, :] .= permutedims(@view $C[1, :]);
  2.882 ms (0 allocations: 0 bytes)

@jishnub jishnub added the arrays [a, r, r, a, y, s] label Apr 21, 2024
@LilithHafner LilithHafner added the performance Must go faster label Apr 21, 2024
Copy link
Member

@LilithHafner LilithHafner left a comment

Choose a reason for hiding this comment

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

That benchmark is not particularly convincing because in this case it is actually faster to make an unaliased copy of the RHS because that makes the copy faster. On my computer the PR is a 2x runtime regression in that case even though it eliminates the allocations.

This benchmark is a bit more convincing:

julia> C = rand(2000,2000);

julia> @b reshape(@view($C[:, 1:1000]), 2_000_000) .= reshape(@view($C[:, 1001:2000]), 2_000_000)
1.149 ms (3 allocs: 15.259 MiB)

julia> @eval Base begin
       mightalias(A::ReshapedArray, B::ReshapedArray) = mightalias(parent(A), parent(B))
       # special handling for reshaped SubArrays that dispatches to the subarray aliasing check
       mightalias(A::ReshapedArray, B::SubArray) = mightalias(parent(A), B)
       mightalias(A::SubArray, B::ReshapedArray) = mightalias(A, parent(B))
       end
mightalias (generic function with 6 methods)

julia> @b reshape(@view($C[:, 1:1000]), 2_000_000) .= reshape(@view($C[:, 1001:2000]), 2_000_000)
374.877 μs

My conclusion here is that on some architectures and use-cases it makes sense to copy even when unnecessary. This does not mean we should keep lower quality mightalias checking. LGTM.

@jishnub
Copy link
Member Author

jishnub commented Apr 21, 2024

I agree, and changing a non-copying version to a copying one is easy at the call site.

@LilithHafner LilithHafner merged commit e57a7e4 into master Apr 21, 2024
@LilithHafner LilithHafner deleted the jishnub/reshapesubarrayaliasing branch April 21, 2024 17:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

arrays [a, r, r, a, y, s] performance Must go faster

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants