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

Poor performance of vcat/hcat/hvcat on mixed input types #48850

Closed
mikmoore opened this issue Mar 1, 2023 · 1 comment · Fixed by #48933
Closed

Poor performance of vcat/hcat/hvcat on mixed input types #48850

mikmoore opened this issue Mar 1, 2023 · 1 comment · Fixed by #48933
Labels
performance Must go faster

Comments

@mikmoore
Copy link
Contributor

mikmoore commented Mar 1, 2023

From this Discourse thread.

On version 1.8.0 (sorry it's what i have handy, but the offending method appears unchanged on current master):

julia> using BenchmarkTools

julia> x = 1; y = 2:2;

julia> @btime vcat($x, $x);
  26.506 ns (1 allocation: 80 bytes)

julia> @btime vcat($y, $y);
  28.744 ns (1 allocation: 80 bytes)

julia> @btime vcat($x, $y);
  2.667 μs (35 allocations: 1.05 KiB)

with similar results for hcat and hvcat.

An initial inspection suggests that the offending methods for vcat/hcat are LinearAlgebra._vcat(A::Union{Number, LinearAlgebra.UniformScaling, AbstractVecOrMat}...; array_type) and the matching _hcat, which are (as of this writing) defined at uniformscaling.jl#L426. A loop over the input vararg tuple appears to cause dynamic dispatch.

hvcat has the same problem, likely arising from a similar loop in uniformscaling.jl#446.

@mikmoore mikmoore changed the title Poor performance of vcat/hcat on mixed input types Poor performance of vcat/hcat/hvcat on mixed input types Mar 1, 2023
@inkydragon inkydragon added the performance Must go faster label Mar 4, 2023
@dkarrasch
Copy link
Member

I guess I'm and #41394 are to blame. GitHub says that will only released as v1.9, but I do find the corresponding code in v1.8.x. I think I have a solution to this, will make a PR soon.

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

Successfully merging a pull request may close this issue.

3 participants