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

Regression in memory allocs when forming NTuple #41512

Closed
jw3126 opened this issue Jul 8, 2021 · 6 comments
Closed

Regression in memory allocs when forming NTuple #41512

jw3126 opened this issue Jul 8, 2021 · 6 comments
Labels
performance Must go faster regression Regression in behavior compared to a previous version

Comments

@jw3126
Copy link
Contributor

jw3126 commented Jul 8, 2021

The following code allocates on master and 1.7beta2, but does not allocate on 1.6:

Tuple3(itr) = NTuple{3, Float64}(itr)::NTuple{3,Float64}

function doit(itr)
    for x in itr
        Tuple3(x)
    end
end

itr = [randn(3) for _ in 1:10000]
doit(itr)
@time doit(itr)
master:  0.021418 seconds (120.00 k allocations: 2.899 MiB)
julia 1.6:  0.000032 seconds

This lead to regressions in two packages for me:
https://github.com/jw3126/VoxelRayTracers.jl/blob/master/src/core.jl#L71
https://github.com/jw3126/LinearInterpolations.jl/blob/master/src/LinearInterpolations.jl#L407
https://github.com/jw3126/LinearInterpolations.jl/blob/master/test/test_LinearInterpolations.jl#L259

@simeonschaub
Copy link
Member

Fixed in #41074, but looks like we forgot to mark that for backporting.

@jw3126
Copy link
Contributor Author

jw3126 commented Jul 8, 2021

@simeonschaub I could reproduce with today's master. Note that type inference works, but still it allocates for me. Could you run my example just to double check?

@jw3126
Copy link
Contributor Author

jw3126 commented Jul 8, 2021

@simeonschaub this is how it looks on today's master for me:

   _       _ _(_)_     |  Documentation: https://docs.julialang.org
  (_)     | (_) (_)    |
   _ _   _| |_  __ _   |  Type "?" for help, "]?" for Pkg help.
  | | | | | | |/ _` |  |
  | | |_| | | | (_| |  |  Version 1.8.0-DEV.155 (2021-07-07)
 _/ |\__'_|_|_|\__'_|  |  Commit 97f817a379* (0 days old master)
|__/                   |

julia> Tuple3(itr) = NTuple{3, Float64}(itr)::NTuple{3,Float64}
Tuple3 (generic function with 1 method)

julia> function doit(itr);
           for x in itr
               Tuple3(x)
           end
       end
doit (generic function with 1 method)

julia> itr = [randn(3) for _ in 1:10000];doit(itr);

julia> @time doit(itr)
  0.023238 seconds (120.00 k allocations: 2.899 MiB)

@simeonschaub simeonschaub reopened this Jul 8, 2021
@simeonschaub
Copy link
Member

Ah sorry, I misunderstood. Would be good to fix this.

@simeonschaub simeonschaub added the regression Regression in behavior compared to a previous version label Jul 8, 2021
@KristofferC KristofferC added the performance Must go faster label Jul 8, 2021
@JeffBezanson
Copy link
Sponsor Member

Base._totuple was inlined in 1.6, but isn't anymore. The cost is fine, and the method is even marked inline, so I guess it might be due to recursion detection?

@jw3126
Copy link
Contributor Author

jw3126 commented Jan 31, 2023

Seems to be fixed for me on 1.8.5.

@jw3126 jw3126 closed this as completed Jan 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Must go faster regression Regression in behavior compared to a previous version
Projects
None yet
Development

No branches or pull requests

4 participants