Skip to content
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

Showing a user defined abstract array takes a very long time #20832

Closed
KristofferC opened this issue Feb 27, 2017 · 6 comments
Closed

Showing a user defined abstract array takes a very long time #20832

KristofferC opened this issue Feb 27, 2017 · 6 comments
Labels
compiler:inference Type inference performance Must go faster

Comments

@KristofferC
Copy link
Member

KristofferC commented Feb 27, 2017

The following:

immutable Tensor{order, dim, T <: Real, M} <: AbstractArray{T, order}
   data::NTuple{M, T}
end


Base.size{dim}(::Tensor{4, dim}) = (dim, dim, dim, dim)

@inline function Base.getindex(S::Tensor, i::Int)
    checkbounds(S, i)
    @inbounds v = S.data[i]
    return v
end

Base.IndexStyle(::Type{<: Tensor}) = IndexLinear()

@time show(Tensor{4, 3, Float64, 1}((rand(1)...)))

takes the following time on 0.6:

25.599718 seconds (90.54 M allocations: 4.054 GiB, 5.15% gc time)

The equivalent code on 0.5 takes:

0.864693 seconds (698.92 k allocations: 28.239 MB, 16.58% gc time)

#20792 made no difference

@stevengj stevengj added io Involving the I/O subsystem: libuv, read, write, etc. performance Must go faster labels Feb 27, 2017
@mbauman
Copy link
Member

mbauman commented Feb 27, 2017

I'd bet that this is effectively the same as #20334 (comment) — we seem to be sitting right on the edge of some catastrophically bad inference behavior. Once it's inferred, the second call takes less than 10 ms.

@mbauman mbauman added compiler:inference Type inference and removed io Involving the I/O subsystem: libuv, read, write, etc. labels Feb 27, 2017
@KristofferC
Copy link
Member Author

KristofferC commented Feb 27, 2017

Doing something like:

Base.size{dim}(::Tensor{1, dim}) = (dim,)
Base.size{dim}(::Tensor{2, dim}) = (dim, dim)
Base.size{dim}(::Tensor{3, dim}) = (dim, dim, dim)
Base.size{dim}(::Tensor{4, dim}) = (dim, dim, dim, dim)

for i in 1:4
    @time show(DevNull, Tensor{i, 3, Float64, 1}((rand(1)...)));
end

gives:

  0.329113 seconds (172.88 k allocations: 7.538 MiB)
  0.384870 seconds (179.73 k allocations: 7.736 MiB, 28.97% gc time)
 24.560876 seconds (90.18 M allocations: 4.039 GiB, 4.27% gc time)
  1.823787 seconds (1.87 M allocations: 90.867 MiB, 2.73% gc time)

Some new path when rank = 3?

@vtjnash
Copy link
Member

vtjnash commented Feb 27, 2017

Yes, there's a codepath in show_nd (specifically view(a, indices(a,1), indices(a,2), idxs...), that is only active for ndims >= 3)

@mbauman
Copy link
Member

mbauman commented Feb 27, 2017

Minimal reproduction:

julia> struct T <: AbstractArray{Int,1}; end
       Base.size(::T) = (1,)

julia> f1(I...) = Base.index_ndims(I...)
       @time f1(1)
  0.005126 seconds (1.41 k allocations: 81.339 KiB)
(true,)

julia> f2(A, n) = Base._maybe_reshape_parent(A, n);
       @time f2(T(), (true,));
  0.001032 seconds (634 allocations: 38.453 KiB)

julia> f3(A, I...) = Base._maybe_reshape_parent(A, Base.index_ndims(I...))
       @time f3(T(), 1);
 32.339036 seconds (55.48 M allocations: 2.480 GiB, 2.98% gc time)

@fredrikekre
Copy link
Member

Fixed by #20874? Or is there more that could be done for this issue?

On master I get for the example in OP:
1.062920 seconds (1.05 M allocations: 49.059 MiB, 12.69% gc time) master
0.647839 seconds (699.03 k allocations: 28.136 MB, 6.55% gc time) on 0.5

and the example from #20832 (comment)

julia> @time f3(T(), 1);
  0.120183 seconds (184.32 k allocations: 8.861 MiB)

@KristofferC
Copy link
Member Author

Can probably close then. 1 second is still quite long but it is at least in the ball park of 0.5 now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler:inference Type inference performance Must go faster
Projects
None yet
Development

No branches or pull requests

5 participants