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

annotate the signature type of the first argument of kwcall method #49439

Merged
merged 1 commit into from
Apr 22, 2023

Conversation

aviatesk
Copy link
Member

This commit annotates the signature type of the first argument of kwcall method as kwargs::NamedTuple.
Previously it's annotated as ::Any but only NamedTuple object is supported (and synthesized by the frontend) as the first argument to kwcall method.

This commit annotates the signature type of the first argument of
`kwcall` method as `kwargs::NamedTuple`.
Previously it's annotated as `::Any` but only `NamedTuple` object is
supported (and synthesized by the frontend) as the first argument to
`kwcall` method.
@aviatesk aviatesk requested a review from vtjnash April 20, 2023 08:01
@aviatesk aviatesk merged commit 357bcdd into master Apr 22, 2023
@aviatesk aviatesk deleted the avi/kwcall-namedtuple branch April 22, 2023 05:09
@vtjnash
Copy link
Member

vtjnash commented Apr 22, 2023

Did you check performance and memory use of this? There are optimized dispatch lookup tables for Any that do not exist for NamedTuples.

@aviatesk
Copy link
Member Author

I used this script:

global inline_checker = x -> x + 1

function call_kwfunc_itr(kwfunc, arg, itr)
    local r = 0
    r += kwfunc(arg; kw=itr[1])
    r += kwfunc(arg; kw=itr[2])
    r += kwfunc(arg; kw=itr[3])
    r += kwfunc(arg; kw=itr[4])
    r += kwfunc(arg; kw=itr[5])
    r
end

function simple_kwfunc(a; kw=nothing)
    c = isa(kw, Function)
    inline_checker(c) # cause dynamic dispatch to prevent inlining
end

function simple_kwfunc_nospec(a; @nospecialize kw=nothing)
    c = isa(kw, Function)
    inline_checker(c) # cause dynamic dispatch to prevent inlining
end

function simple_kwfunc_kws(a; kws...)
    c = isa(kws[1], Function)
    inline_checker(c) # cause dynamic dispatch to prevent inlining
end

dynamic_itr = Any[sin, muladd, "foo", nothing, missing]   # untyped container can cause excessive runtime dispatch

using BenchmarkTools

call_kwfunc_itr(simple_kwfunc, 1, dynamic_itr)
call_kwfunc_itr(simple_kwfunc_nospec, 1, dynamic_itr)
call_kwfunc_itr(simple_kwfunc_kws, 1, dynamic_itr)

@benchmark call_kwfunc_itr(simple_kwfunc, 1, dynamic_itr)
@benchmark call_kwfunc_itr(simple_kwfunc_nospec, 1, dynamic_itr)
@benchmark call_kwfunc_itr(simple_kwfunc_kws, 1, dynamic_itr)

And there does not seem to be no big difference?

the previous commit (57b68f2)

julia> @benchmark call_kwfunc_itr(simple_kwfunc, 1, dynamic_itr)
BenchmarkTools.Trial: 10000 samples with 187 evaluations.
 Range (min  max):  555.706 ns   3.315 μs  ┊ GC (min  max): 0.00%  78.52%
 Time  (median):     582.219 ns              ┊ GC (median):    0.00%
 Time  (mean ± σ):   591.227 ns ± 35.978 ns  ┊ GC (mean ± σ):  0.04% ±  0.79%

           ▁▆█▄                                                 
  ▁▁▁▁▂▁▂▄▇████▆▅▅▄▄▃▃▃▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁ ▂
  556 ns          Histogram: frequency by time          686 ns <

 Memory estimate: 32 bytes, allocs estimate: 2.

julia> @benchmark call_kwfunc_itr(simple_kwfunc_nospec, 1, dynamic_itr)
BenchmarkTools.Trial: 10000 samples with 379 evaluations.
 Range (min  max):  248.459 ns  537.158 ns  ┊ GC (min  max): 0.00%  0.00%
 Time  (median):     257.037 ns               ┊ GC (median):    0.00%
 Time  (mean ± σ):   258.401 ns ±  11.020 ns  ┊ GC (mean ± σ):  0.00% ± 0.00%

  ▁▇▆      ▇█▄▂       ▂▁       ▁▁▁▁▁▁                           ▂
  ████▃▃▅▅▄█████▇▇▇▇▇▇███▇██▇▆▆███████▇▇▇▆▇█▇▇▇▇▇▇▆▆▆▆▆▇▅▅▄▆▅▄▅ █
  248 ns        Histogram: log(frequency) by time        300 ns <

 Memory estimate: 0 bytes, allocs estimate: 0.

julia> @benchmark call_kwfunc_itr(simple_kwfunc_kws, 1, dynamic_itr)
BenchmarkTools.Trial: 10000 samples with 185 evaluations.
 Range (min  max):  556.081 ns   4.121 μs  ┊ GC (min  max): 0.00%  78.95%
 Time  (median):     583.335 ns              ┊ GC (median):    0.00%
 Time  (mean ± σ):   591.977 ns ± 41.955 ns  ┊ GC (mean ± σ):  0.05% ±  0.79%

           ▅▇█▇▅▂                                               
  ▁▁▁▁▁▁▁▃▇███████▇▆▅▄▃▃▃▃▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▁▁▂▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁ ▂
  556 ns          Histogram: frequency by time          677 ns <

 Memory estimate: 32 bytes, allocs estimate: 2.

this PR (357bcdd)

julia> @benchmark call_kwfunc_itr(simple_kwfunc, 1, dynamic_itr)
BenchmarkTools.Trial: 10000 samples with 187 evaluations.
 Range (min  max):  550.134 ns   3.294 μs  ┊ GC (min  max): 0.00%  78.85%
 Time  (median):     572.861 ns              ┊ GC (median):    0.00%
 Time  (mean ± σ):   585.157 ns ± 38.791 ns  ┊ GC (mean ± σ):  0.04% ±  0.79%

   ▁▁  ▁▅██▇▅▅▅▄▄▄▄▃▂▁▂▁▂▂▂▂▁▁▁▁▂▂▂▁▁▁▁ ▁                      ▂
  ▇███▆█████████████████████████████████████▇▇▇█▇█▆▇▇▇▅▆▆▆▅▆▆▅ █
  550 ns        Histogram: log(frequency) by time       694 ns <

 Memory estimate: 32 bytes, allocs estimate: 2.

julia> @benchmark call_kwfunc_itr(simple_kwfunc_nospec, 1, dynamic_itr)
BenchmarkTools.Trial: 10000 samples with 355 evaluations.
 Range (min  max):  256.456 ns   3.609 μs  ┊ GC (min  max): 0.00%  0.00%
 Time  (median):     259.507 ns              ┊ GC (median):    0.00%
 Time  (mean ± σ):   268.259 ns ± 38.205 ns  ┊ GC (mean ± σ):  0.00% ± 0.00%

   ▇█▃     ▅▆▃▂▂    ▁▃▃▃▂▂▁▁ ▁▁▁▁▂▁▁                           ▂
  ▇███▆▇▇▆████████▇█████████████████▇█████▇█▇▆▆▆▆▆▇▆▆▆▄▆▅▅▅▄▅▅ █
  256 ns        Histogram: log(frequency) by time       318 ns <

 Memory estimate: 0 bytes, allocs estimate: 0.

julia> @benchmark call_kwfunc_itr(simple_kwfunc_kws, 1, dynamic_itr)
BenchmarkTools.Trial: 10000 samples with 187 evaluations.
 Range (min  max):  551.021 ns   3.627 μs  ┊ GC (min  max): 0.00%  80.38%
 Time  (median):     580.658 ns              ┊ GC (median):    0.00%
 Time  (mean ± σ):   592.102 ns ± 40.868 ns  ┊ GC (mean ± σ):  0.05% ±  0.80%

     ▁▂▂ ▂▅▇██▇▆▅▅▅▅▄▄▄▃▂▂▃▃▃▃▂▂▂▁▂▂▂▂▁▁▁▁▁▁ ▁                 ▂
  ▄▇████████████████████████████████████████████▇▇▇█▇▇▇▆█▆▆▆▆▇ █
  551 ns        Histogram: log(frequency) by time       695 ns <

 Memory estimate: 32 bytes, allocs estimate: 2.

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

Successfully merging this pull request may close these issues.

2 participants