Skip to content

Commit

Permalink
Revert "Avoid allocations in views of views (#53231)" (#57044)
Browse files Browse the repository at this point in the history
This reverts commit 2bd4cf8. (#53231)

The reason for this revert is that it caused exponential blowup of types
in iterated views causing some packages to simply freeze when doing
something that worked ok in 1.10.

In my opinion, the perf gain from the PR is not outweighed by the "risk"
of hitting this compilation blowup case.

Fixes #56760.

Co-authored-by: KristofferC <kristoffer.carlsson@juliacomputing.com>
(cherry picked from commit b23f557)
  • Loading branch information
KristofferC committed Jan 14, 2025
1 parent f66a1cf commit 343c6ee
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 28 deletions.
6 changes: 3 additions & 3 deletions base/subarray.jl
Original file line number Diff line number Diff line change
Expand Up @@ -295,18 +295,18 @@ reindex(idxs::Tuple{Slice, Vararg{Any}}, subidxs::Tuple{Any, Vararg{Any}}) =

# Re-index into parent vectors with one subindex
reindex(idxs::Tuple{AbstractVector, Vararg{Any}}, subidxs::Tuple{Any, Vararg{Any}}) =
(@_propagate_inbounds_meta; (maybeview(idxs[1], subidxs[1]), reindex(tail(idxs), tail(subidxs))...))
(@_propagate_inbounds_meta; (idxs[1][subidxs[1]], reindex(tail(idxs), tail(subidxs))...))

# Parent matrices are re-indexed with two sub-indices
reindex(idxs::Tuple{AbstractMatrix, Vararg{Any}}, subidxs::Tuple{Any, Any, Vararg{Any}}) =
(@_propagate_inbounds_meta; (maybeview(idxs[1], subidxs[1], subidxs[2]), reindex(tail(idxs), tail(tail(subidxs)))...))
(@_propagate_inbounds_meta; (idxs[1][subidxs[1], subidxs[2]], reindex(tail(idxs), tail(tail(subidxs)))...))

# In general, we index N-dimensional parent arrays with N indices
@generated function reindex(idxs::Tuple{AbstractArray{T,N}, Vararg{Any}}, subidxs::Tuple{Vararg{Any}}) where {T,N}
if length(subidxs.parameters) >= N
subs = [:(subidxs[$d]) for d in 1:N]
tail = [:(subidxs[$d]) for d in N+1:length(subidxs.parameters)]
:(@_propagate_inbounds_meta; (maybeview(idxs[1], $(subs...)), reindex(tail(idxs), ($(tail...),))...))
:(@_propagate_inbounds_meta; (idxs[1][$(subs...)], reindex(tail(idxs), ($(tail...),))...))
else
:(throw(ArgumentError("cannot re-index SubArray with fewer indices than dimensions\nThis should not occur; please submit a bug report.")))
end
Expand Down
25 changes: 0 additions & 25 deletions test/subarray.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1026,31 +1026,6 @@ catch err
err isa ErrorException && startswith(err.msg, "syntax:")
end


@testset "avoid allocating in reindex" begin
a = reshape(1:16, 4, 4)
inds = ([2,3], [3,4])
av = view(a, inds...)
av2 = view(av, 1, 1)
@test parentindices(av2) === (2,3)
av2 = view(av, 2:2, 2:2)
@test parentindices(av2) === (view(inds[1], 2:2), view(inds[2], 2:2))

inds = (reshape([eachindex(a);], size(a)),)
av = view(a, inds...)
av2 = view(av, 1, 1)
@test parentindices(av2) === (1,)
av2 = view(av, 2:2, 2:2)
@test parentindices(av2) === (view(inds[1], 2:2, 2:2),)

inds = (reshape([eachindex(a);], size(a)..., 1),)
av = view(a, inds...)
av2 = view(av, 1, 1, 1)
@test parentindices(av2) === (1,)
av2 = view(av, 2:2, 2:2, 1:1)
@test parentindices(av2) === (view(inds[1], 2:2, 2:2, 1:1),)
end

@testset "isassigned" begin
a = Vector{BigFloat}(undef, 5)
a[2] = 0
Expand Down

0 comments on commit 343c6ee

Please sign in to comment.