Added kwargs to invokelatest. #22646

rofinn · 2017-07-01T03:19:19Z

rofinn · 2017-07-01T21:01:03Z

Not sure what's causing the xcode build issue.

martinholters · 2017-07-04T11:16:26Z

The first Travis OSX error is

/Users/travis/build/JuliaLang/julia/src/codegen.cpp:1:1: error: unknown type name 'wnloadQueue'
wnloadQueue</string>

Maybe the codegen.cpp file got corrupted somehow? Also, for the other errors, the reported line numbers seem to be off, so maybe the beginning of that file was garbled? I'll restart the build, let's see whether that helps...

stevengj · 2017-07-06T18:16:22Z

We should double-check performance here, as @JeffBezanson noted in #21543 that invokelatest is performance-critical for remote calls. (I don't know whether nanosoldier has benchmark coverage of this?)

rofinn · 2017-07-06T20:42:10Z

I'm not sure if these benchmark tests are cheating but:

Regular Args

Base performance:

# Test Function
julia> issue19774(x::Int)::Int = 0
issue19774 (generic function with 1 method)

# 0.6.0-rc3.0
julia> @benchmark issue19774(0)
BenchmarkTools.Trial:
  memory estimate:  0 bytes
  allocs estimate:  0
  --------------
  minimum time:     1.503 ns (0.00% GC)
  median time:      1.507 ns (0.00% GC)
  mean time:        1.666 ns (0.00% GC)
  maximum time:     149.130 ns (0.00% GC)
  --------------
  samples:          10000
  evals/sample:     1000

# PR
julia> @benchmark issue19774(0)
BenchmarkTools.Trial:
  memory estimate:  0 bytes
  allocs estimate:  0
  --------------
  minimum time:     1.495 ns (0.00% GC)
  median time:      1.505 ns (0.00% GC)
  mean time:        1.601 ns (0.00% GC)
  maximum time:     51.402 ns (0.00% GC)
  --------------
  samples:          10000
  evals/sample:     1000

Okay, almost identical.

With invokelatest

function foo()
    eval(:(issue19774(x::Int)::Int = 2))
    return Base.invokelatest(issue19774, 0)
end

# 0.6.0-rc3.0
julia> @benchmark foo()
BenchmarkTools.Trial:
  memory estimate:  17.85 KiB
  allocs estimate:  285
  --------------
  minimum time:     2.261 ms (0.00% GC)
  median time:      2.649 ms (0.00% GC)
  mean time:        2.787 ms (0.09% GC)
  maximum time:     7.857 ms (59.68% GC)
  --------------
  samples:          1783
  evals/sample:     1

# PR
julia> @benchmark foo()
BenchmarkTools.Trial:
  memory estimate:  18.95 KiB
  allocs estimate:  312
  --------------
  minimum time:     2.362 ms (0.00% GC)
  median time:      2.855 ms (0.00% GC)
  mean time:        2.962 ms (0.25% GC)
  maximum time:     15.707 ms (78.52% GC)
  --------------
  samples:          1678
  evals/sample:     1

Okay, almost identical again, so it looks like the PR doesn't hurt invokelatest performance with regular args.

Kwargs.

# Test Function
julia> kwargs19774(x::Int, y::Int; z::Int=0)::Int = 1
kwargs19774 (generic function with 1 method)

# Base performance.
julia> @benchmark kwargs19774(2, 3; z=1)
BenchmarkTools.Trial:
  memory estimate:  96 bytes
  allocs estimate:  1
  --------------
  minimum time:     39.195 ns (0.00% GC)
  median time:      41.929 ns (0.00% GC)
  mean time:        56.237 ns (20.11% GC)
  maximum time:     38.655 μs (99.86% GC)
  --------------
  samples:          10000
  evals/sample:     992

# Overwriting kwargs19774
julia> function foo_kwargs()
          eval(:(kwargs19774(x::Int, y::Int; z::Int=3)::Int = z))
          return kwargs19774(2, 3; z=1)
       end

julia> @benchmark foo_kwargs()
BenchmarkTools.Trial:
  memory estimate:  18.17 KiB
  allocs estimate:  258
  --------------
  minimum time:     1.189 ms (0.00% GC)
  median time:      1.273 ms (0.00% GC)
  mean time:        1.395 ms (0.35% GC)
  maximum time:     18.418 ms (92.97% GC)
  --------------
  samples:          3533
  evals/sample:     1

# With invokelatest
julia> function foo_kwargs()
          eval(:(kwargs19774(x::Int, y::Int; z::Int=3)::Int = z))
          return Base.invokelatest(kwargs19774, 2, 3; z=1)
       end
foo_kwargs (generic function with 1 method)

julia> @benchmark foo_kwargs()
BenchmarkTools.Trial:
  memory estimate:  139.40 KiB
  allocs estimate:  2544
  --------------
  minimum time:     14.964 ms (0.00% GC)
  median time:      15.985 ms (0.00% GC)
  mean time:        16.381 ms (0.26% GC)
  maximum time:     28.723 ms (44.39% GC)
  --------------
  samples:          305
  evals/sample:     1

So, keyword performance isn't great, but it doesn't seem to hurt regular arg performance.

* Replaced the `eval` with a module to help with readability. * Added a test case to demonstrate the failure condition that invokelatest fixes.

vchuravy

You are benchmarking the eval overhead more than the actual cost of invokelatest

issue19774(x::Int)::Int = 0
kwargs19774(x::Int, y::Int; z::Int=0)::Int = 1

function newinvokelatest(f, args...; kwargs...)
    inner(f, args...) = f(args...; kwargs...)
    Core._apply_latest(inner, (f, args...))
end

function foo()
    return Base.invokelatest(issue19774, 0)
end
function foo_new()
    return newinvokelatest(issue19774, 0)
end
function foo_kwargs()
    return newinvokelatest(kwargs19774, 2, 3; z=1)
end

Results

# Current implementation
julia> @benchmark foo()
BenchmarkTools.Trial: 
  memory estimate:  0 bytes
  allocs estimate:  0
  --------------
  minimum time:     54.827 ns (0.00% GC)
  median time:      55.091 ns (0.00% GC)
  mean time:        55.673 ns (0.00% GC)
  maximum time:     130.690 ns (0.00% GC)
  --------------
  samples:          10000
  evals/sample:     985

# Using inner func
julia> @benchmark foo_new()
BenchmarkTools.Trial: 
  memory estimate:  112 bytes
  allocs estimate:  3
  --------------
  minimum time:     830.962 ns (0.00% GC)
  median time:      846.474 ns (0.00% GC)
  mean time:        877.734 ns (0.82% GC)
  maximum time:     21.935 μs (91.31% GC)
  --------------
  samples:          10000
  evals/sample:     78

# Kwargs in the mix
julia> @benchmark foo_kwargs()
BenchmarkTools.Trial: 
  memory estimate:  592 bytes
  allocs estimate:  14
  --------------
  minimum time:     4.265 μs (0.00% GC)
  median time:      4.325 μs (0.00% GC)
  mean time:        4.510 μs (2.55% GC)
  maximum time:     599.276 μs (96.54% GC)
  --------------
  samples:          10000
  evals/sample:     7

The following implementation performs better at least in my tests

function newinvokelatest2(f, args...; kwargs...)
    inner() = f(args...; kwargs...)
    Core._apply_latest(inner)
end

# foo_new with newinvokelatest2
@benchmark foo_new()
BenchmarkTools.Trial: 
  memory estimate:  112 bytes
  allocs estimate:  2
  --------------
  minimum time:     61.380 ns (0.00% GC)
  median time:      64.960 ns (0.00% GC)
  mean time:        77.797 ns (12.66% GC)
  maximum time:     2.548 μs (94.48% GC)
  --------------
  samples:          10000
  evals/sample:     982

rofinn · 2017-07-17T22:45:02Z

@vchuravy Yeah, those tests seem a lot more reasonable, thanks! I've also updated the function to avoid passing args to the closure as per your suggestion. @yuyichao Would you be able to review when you get the chance?

Call `Core._applu_latest` to support `open` mode in `APIResponder` on Julia 0.6. We can call `Compat.invoke_latest` instead after JuliaLang/julia#22646 is merged and assimilated in Compat.jl suitably.

Call `Core._apply_latest` to support `open` mode in `APIResponder` on Julia 0.6. We can call `Compat.invokelatest` instead after JuliaLang/julia#22646 is merged and assimilated in Compat.jl suitably.

stevengj · 2017-07-24T12:03:32Z

base/essentials.jl

+    # We use a closure (`inner`) to handle kwargs.
+    inner() = f(args...; kwargs...)
+    Core._apply_latest(inner)
+end


It seems weird to me to construct a new closure for each call. Wouldn't it be better to do:

_invoker(f, args, kwargs) = f(args..., kwargs...) invokelatest(f, args...; kwargs...) = Core._apply_latest(_invoker, args, kwargs)

?

Wouldn't you need to do Core._apply_latest(_invoker, (f, args, kwargs))? In that case, the no-kwargs conditions would be slower.

julia> using BenchmarkTools julia> issue19774(x::Int)::Int = 0 issue19774 (generic function with 1 method) julia> kwargs19774(x::Int, y::Int; z::Int=0)::Int = 1 kwargs19774 (generic function with 1 method) julia> function newinvokelatest(f, args...; kwargs...) inner() = f(args...; kwargs...) Core._apply_latest(inner) end newinvokelatest (generic function with 1 method) julia> newinvokelatest2(f, args...; kwargs...) = Core._apply_latest(_newinvoker, (f, args, kwargs)) newinvokelatest2 (generic function with 1 method) julia> _newinvoker(f, args, kwargs) = f(args...; kwargs...) _newinvoker (generic function with 1 method) julia> function foo() return Base.invokelatest(issue19774, 0) end foo (generic function with 1 method) julia> function foo_new() return newinvokelatest(issue19774, 0) end foo_new (generic function with 1 method) julia> function foo_new2() return newinvokelatest2(issue19774, 0) end foo_new2 (generic function with 1 method) julia> function foo_kwargs() return newinvokelatest(kwargs19774, 2, 3; z=1) end foo_kwargs (generic function with 1 method) julia> function foo_kwargs2() return newinvokelatest2(kwargs19774, 2, 3; z=1) end foo_kwargs2 (generic function with 1 method) julia> @benchmark foo() BenchmarkTools.Trial: memory estimate: 0 bytes allocs estimate: 0 -------------- minimum time: 55.914 ns (0.00% GC) median time: 56.209 ns (0.00% GC) mean time: 59.357 ns (0.00% GC) maximum time: 193.226 ns (0.00% GC) -------------- samples: 10000 evals/sample: 984 julia> @benchmark foo_new() BenchmarkTools.Trial: memory estimate: 112 bytes allocs estimate: 2 -------------- minimum time: 69.090 ns (0.00% GC) median time: 71.658 ns (0.00% GC) mean time: 84.819 ns (9.79% GC) maximum time: 2.234 μs (93.51% GC) -------------- samples: 10000 evals/sample: 976 julia> @benchmark foo_new2() BenchmarkTools.Trial: memory estimate: 128 bytes allocs estimate: 3 -------------- minimum time: 231.818 ns (0.00% GC) median time: 235.975 ns (0.00% GC) mean time: 266.367 ns (4.67% GC) maximum time: 9.024 μs (92.26% GC) -------------- samples: 10000 evals/sample: 451 julia> @benchmark foo_kwargs() BenchmarkTools.Trial: memory estimate: 1008 bytes allocs estimate: 20 -------------- minimum time: 3.734 μs (0.00% GC) median time: 3.801 μs (0.00% GC) mean time: 4.224 μs (3.51% GC) maximum time: 638.504 μs (0.00% GC) -------------- samples: 10000 evals/sample: 8 julia> @benchmark foo_kwargs2() BenchmarkTools.Trial: memory estimate: 560 bytes allocs estimate: 12 -------------- minimum time: 3.399 μs (0.00% GC) median time: 3.485 μs (0.00% GC) mean time: 3.902 μs (2.53% GC) maximum time: 517.470 μs (95.49% GC) -------------- samples: 10000 evals/sample: 8

Call `Core._apply_latest` to support `open` mode in `APIResponder` on Julia 0.6. We can call `Compat.invokelatest` instead after JuliaLang/julia#22646 is merged and assimilated in Compat.jl suitably.

omus · 2017-07-26T23:24:05Z

I'll merge this tomorrow unless there are objections.

Added kwargs to invokelatest.

2775cd6

ararslan requested a review from yuyichao July 1, 2017 04:05

Cleaned up invokelatest tests.

af5e541

* Replaced the `eval` with a module to help with readability. * Added a test case to demonstrate the failure condition that invokelatest fixes.

vchuravy requested changes Jul 7, 2017

View reviewed changes

Moved f and args inside closure rather than passing them in.

8efd4f7

tkelman referenced this pull request in JuliaEarth/GeoStats.jl Jul 13, 2017

Refactor examples function

91287d2

ihnorton mentioned this pull request Jul 22, 2017

RFC: Use invokelatest in jl_Function_call JuliaPy/PyCall.jl#416

Merged

vchuravy approved these changes Jul 24, 2017

View reviewed changes

tanmaykm mentioned this pull request Jul 24, 2017

dynamic reevaluation of function JuliaWeb/JuliaWebAPI.jl#57

Merged

stevengj reviewed Jul 24, 2017

View reviewed changes

stevengj approved these changes Jul 26, 2017

View reviewed changes

omus merged commit d6e1e86 into JuliaLang:master Jul 27, 2017

omus deleted the invokelatest-keywords branch July 27, 2017 14:53

BoPeng mentioned this pull request Nov 30, 2017

Slow start of Julia kernel. vatlab/sos-julia#3

Closed

omus mentioned this pull request Dec 4, 2017

Support keywords in invokelatest JuliaLang/Compat.jl#424

Merged

omus mentioned this pull request Feb 6, 2018

Added support for kwargs in @mock calls. JuliaTesting/Mocking.jl#21

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added kwargs to invokelatest. #22646

Added kwargs to invokelatest. #22646

rofinn commented Jul 1, 2017 •

edited

Loading

rofinn commented Jul 1, 2017

martinholters commented Jul 4, 2017

stevengj commented Jul 6, 2017

rofinn commented Jul 6, 2017

vchuravy left a comment •

edited

Loading

rofinn commented Jul 17, 2017

stevengj Jul 24, 2017

rofinn Jul 26, 2017

stevengj Jul 26, 2017

omus commented Jul 26, 2017

Added kwargs to invokelatest. #22646

Added kwargs to invokelatest. #22646

Conversation

rofinn commented Jul 1, 2017 • edited Loading

rofinn commented Jul 1, 2017

martinholters commented Jul 4, 2017

stevengj commented Jul 6, 2017

rofinn commented Jul 6, 2017

Regular Args

Base performance:

With invokelatest

Kwargs.

vchuravy left a comment • edited Loading

Choose a reason for hiding this comment

Results

rofinn commented Jul 17, 2017

stevengj Jul 24, 2017

Choose a reason for hiding this comment

rofinn Jul 26, 2017

Choose a reason for hiding this comment

stevengj Jul 26, 2017

Choose a reason for hiding this comment

omus commented Jul 26, 2017

rofinn commented Jul 1, 2017 •

edited

Loading

vchuravy left a comment •

edited

Loading