What is the best way to re-use a temporary vector

General Usage Performance
,
11

You can fix that via

julia> @eval LegendrePolynomials function doublefactorial(::Type{T}, n) where T
           p = convert(T, one(n))
           for i in n:-2:1
               p *= convert(T, i)
           end
           convert(T, p)
       end

in order to replace the original

function doublefactorial(T, n)
           p = convert(T, one(n))
           for i in n:-2:1
               p *= convert(T, i)
           end
           convert(T, p)
       end

Somebody might need to do a PR on that for LegendrePolynomials.jl.

However, I would much more interested in why this goes wrong here and getting it fixed in the compiler!

This is clearly a failure of constant-propagation / inlining / specialization / type-inference. You don’t see it by benchmarking dnPl in isolation because this has different state of inlining / const-prop heuristics.

The bad pattern – using T as an argument when you really mean ::Type{T} – is very commonly used. It would be better if we could make it reliably do the right thing.

PS. I looked at

julia> import Profile
julia> with_cache(30); Profile.Allocs.clear();Profile.Allocs.@profile with_cache(30); p = Profile.Allocs.fetch(); Profile.print(p)
Overhead ╎ [+additional indent] Count File:Line; Function
=========================================================
   ╎432 @Base/client.jl:541; _start()
   ╎ 432 @Base/client.jl:567; repl_main
   ╎  432 @Base/client.jl:430; run_main_repl(interactive::Bool, quiet::Bool, banner::Symbol, history_file::Bool, color_set::Bool)
   ╎   432 @Base/essentials.jl:1052; invokelatest
   ╎    432 @Base/essentials.jl:1055; #invokelatest#2
   ╎     432 @Base/client.jl:446; (::Base.var"#1139#1141"{Bool, Symbol, Bool})(REPL::Module)
   ╎    ╎ 432 @REPL/src/REPL.jl:469; run_repl(repl::REPL.AbstractREPL, consumer::Any)
   ╎    ╎  432 @REPL/src/REPL.jl:483; run_repl(repl::REPL.AbstractREPL, consumer::Any; backend_on_current_task::Bool, backend::Any)
   ╎    ╎   432 @REPL/src/REPL.jl:324; kwcall(::NamedTuple, ::typeof(REPL.start_repl_backend), backend::REPL.REPLBackend, consumer::Any)
   ╎    ╎    432 @REPL/src/REPL.jl:327; start_repl_backend(backend::REPL.REPLBackend, consumer::Any; get_module::Function)
   ╎    ╎     432 @REPL/src/REPL.jl:342; repl_backend_loop(backend::REPL.REPLBackend, get_module::Function)
   ╎    ╎    ╎ 432 @REPL/src/REPL.jl:245; eval_user_input(ast::Any, backend::REPL.REPLBackend, mod::Module)
   ╎    ╎    ╎  432 @Base/boot.jl:430; eval
   ╎    ╎    ╎   432 REPL[3]:6; with_cache(n::Int64)
   ╎    ╎    ╎    432 @LegendrePolynomials/src/LegendrePolynomials.jl:352; dnPl
   ╎    ╎    ╎     432 @LegendrePolynomials/src/LegendrePolynomials.jl:284; _unsafednPl!
 64╎    ╎    ╎    ╎ 64  @LegendrePolynomials/src/LegendrePolynomials.jl:236; doublefactorial(T::Type, n::Int64)
368╎    ╎    ╎    ╎ 368 @LegendrePolynomials/src/LegendrePolynomials.jl:238; doublefactorial(T::Type, n::Int64)
Total snapshots: 27
Total bytes: 432

From there you can take a look at the relevant source code and you immediately see the code-smell (proper spelling has ::Type{T} as argument, also that should be @inline).

On the other hand, seeing the code in isolation, I would not have expected that to actually create trouble! Especially given that it works fine when you benchmark dnPl in isolation.

3 Likes