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

Allocation regression when iterating over Adjoint of Matrix #47703

Closed
pablosanjose opened this issue Nov 25, 2022 · 4 comments · Fixed by #48120
Closed

Allocation regression when iterating over Adjoint of Matrix #47703

pablosanjose opened this issue Nov 25, 2022 · 4 comments · Fixed by #48120
Labels
linear algebra Linear algebra performance Must go faster regression Regression in behavior compared to a previous version

Comments

@pablosanjose
Copy link
Contributor

Ref: https://discourse.julialang.org/t/allocations-again/90780/13

António Araújo noted the following, which started happening sometime between v1.8.1 and v1.8.3

using BenchmarkTools

function test(x, Y)
    sum!(x, Y')
    return nothing
end

x = rand(Float64, 100)
Y = rand(Float64, 1, 100)
julia> @btime(test($x, $Y))
  301.619 ns (2 allocations: 96 bytes)

In v1.8.0 and v1.8.1 we have instead

julia> @btime(test($x, $Y))
  68.719 ns (0 allocations: 0 bytes)
@jishnub
Copy link
Contributor

jishnub commented Nov 25, 2022

Can confirm allocations on v1.8.2

julia> @btime test($x, $Y);
  490.247 ns (2 allocations: 96 bytes)

julia> VERSION
v"1.8.2"

@KristofferC
Copy link
Sponsor Member

v1.8.1...v1.8.2 are the commits between 1.8.1 and 1.8.2

@fredrikekre
Copy link
Member

fredrikekre commented Nov 25, 2022

#46605 then maybe?

@giordano giordano added performance Must go faster regression Regression in behavior compared to a previous version linear algebra Linear algebra labels Nov 25, 2022
@N5N3
Copy link
Member

N5N3 commented Nov 25, 2022

Some of regression comes from the the allocation caused by

switch_dim12(B::AbstractVector) = permutedims(B)

We can ignore it by define switch_dim12(B::AbstractVector{<:Number}) = transpose(B).
Then we'll find the root cause

f(x, Y) = Base.mapreducedim!(identity, Base.add_sum, x, Y')
f2(x, Y) = (g(x, y) = Base.add_sum(x, y); Base.mapreducedim!(identity, g, x, Y'))

@btime f2($x, $Y); # simulated 1.8.0: 27.008 ns (0 allocations: 0 bytes) 
@btime f($x, $Y);  #          master: 112.000 ns (0 allocations: 0 bytes)

This is not suppressing as our mapreducedim! is not optimized for this case (dim1 reduction with small size)
If we increase the size of Y to 100x100 than f is about 2.5x faster than f2. So It might be hard to make all cases faster.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
linear algebra Linear algebra performance Must go faster regression Regression in behavior compared to a previous version
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants