Specialized codegen for opaque closure calls

[Keno]

Benchmark:

using Base.Experimental: @opaque
f() = @opaque (x::Float64)->x+1.0
vec = [f() for i = 1:10_000];
g((x,f),) = f(Float64(x))

Before:

julia> @time mapreduce(g, +, enumerate(vec))
  0.001928 seconds (30.00 k allocations: 781.297 KiB)
5.0015e7

After:

julia> @time mapreduce(g, +, enumerate(vec))
  0.000085 seconds (3 allocations: 48 bytes)
5.0015e7

[aviatesk]

Nice improvements!
I tried out this pull request for a while and found this error:

julia> cls = x::Float64 -> x + 1.0
#3 (generic function with 1 method)

julia> opc = Base.Experimental.@opaque x::Float64 -> x + 1.0
(::Float64)::Any->◌

julia> callcls(cls, x) = cls(x)
callcls (generic function with 1 method)

julia> callcls(cls, 1.0)
2.0

julia> callcls(opc, 1.0)

[15898] signal (11.2): Segmentation fault: 11
in expression starting at REPL[5]:1
unknown function (ip: 0x0)
Allocations: 636559 (Pool: 635390; Big: 1169); GC: 1
[1]    15898 segmentation fault  ./usr/bin/julia

[chriselrod]

I can reproduce aviatesk's report above:

julia> using Base.Experimental: @opaque

julia> fo = @opaque (x::Float64)->(x+1.0)
(::Float64)::Any->◌

julia> f = (x::Float64)->(x+1.0)
#4 (generic function with 1 method)

julia> g(f,x) = f(x)
g (generic function with 1 method)

julia> g(f, 5.0)
6.0

julia> g(fo, 5.0)

[13851] signal (11.1): Segmentation fault
in expression starting at REPL[6]:1
unknown function (ip: (nil))
Allocations: 2000414 (Pool: 1998635; Big: 1779); GC: 4
segmentation fault (core dumped)

OpaqueClosures have long been touted as a potential FunctionWrappers replacement.
Two issues currently:

1. segmentation fault
2. what's the syntax for specifying the return type? Is that reasonable to infer from a concrete argument type?

EDIT:
I do see that

julia> f()
(::Float64)::Float64->◌

so, when @opaque is used inside f, it does infer the return type.

[Keno]

• segmentation fault

I did say WIP right in the pull request title ;)

• what's the syntax for specifying the return type? Is that reasonable to infer from a concrete argument type?

Currently, the macro infers the return type, but the underlying mechanism has the capability to assert it. We could add a function that restricts the return, but for the time being, if you need to assert it, I would use a return type declaration (which converts rather than asserts, but should be good enough).

[Keno]

Updated. I think this is complete now. Could maybe use another test or two for the tricky cases, but I think the implementation is pretty much there.

[aviatesk]

analyzegc is complaining

ANALYZE src/clang-tidy-opaque_closure
--
  | /cache/build/default-amdci4-5/julialang/julia-master/src/opaque_closure.c:61:19: error: Implicit Atomic seq_cst synchronization [concurrency-implicit-atomics,-warnings-as-errors]
  | specptr = ci->specptr.fptr;
  | ^
  | /cache/build/default-amdci4-5/julialang/julia-master/src/opaque_closure.c:98:19: error: Implicit Atomic seq_cst synchronization [concurrency-implicit-atomics,-warnings-as-errors]
  | specptr = ci->specptr.fptr;
  | ^
  | make: *** [Makefile:498: clang-tidy-opaque_closure] Error 1

[vtjnash]

Does this need support in all of the other places we declare "custom" calling conventions? (e.g. jl_invoke_api, staticdata.c) It is probably quite awkward currently to add new ones unfortunately.

[Keno]

I'm not sure. The only place this goes is the codeinstance invoke in the builtin mt, but that already has a codeinstance in it with this exact same ->invoke, so how does that work now?

[vtjnash]

Yeah, that does seem unclear. The magic strings in decls.functionObject do have weird handling in various places though, so I don't know if this needs it too.

[vtjnash]

This seems to assume that specptr is always specsig, but sometimes codegen may put a different token into invoke, in which case the object it stored into specptr might not be the requisite object. It seems like we might need 2 fields: one for the closure data given by invoke, and one for the guaranteed specsig (assuming specsig is valid), which might have been allocated by emit_cfunc_invalidate to be a reverse-trampoline

[Keno]

The code above is supposed to check for those special cases and rewrite them into something that works for OC. This does not have the exact same semantics as the corresponding codeinstance fields.

[vtjnash]

This commit message seems to be completely wrong after the squash merge. Please try to be accurate, as otherwise bisecting and backporting and blaming become confusing to discuss why we are working with a commit described as "WIP codegen for opaque closure calls", since WIP should not be on master.

[ChrisRackauckas]

What's the final performance here vs FunctionWrappers?

[oscardssmith]

I'm seeing ~15% faster (assuming this is a valid benchmark)

julia> using Base.Experimental: @opaque
julia> import FunctionWrappers: FunctionWrapper

julia> f() = @opaque (x::Float64)->x+1.0
f (generic function with 1 method)

julia> vec = [f() for i = 1:10_000];

julia> g((x,f),) = f(Float64(x))
g (generic function with 1 method)

julia> @time mapreduce(g, +, enumerate(vec))
  0.042754 seconds (70.03 k allocations: 4.816 MiB, 99.60% compilation time)
5.0015e7

julia> vec = [f() for i = 1:10_000_000];

julia> @time mapreduce(g, +, enumerate(vec))
  0.051242 seconds (3 allocations: 48 bytes)
5.0000015e13

julia> f() = FunctionWrapper{Float64, Tuple{Float64,}}(x->x+1.0)
f (generic function with 1 method)

julia> vec = [f() for i = 1:10_000_000];

julia> @time mapreduce(g, +, enumerate(vec))
  0.140326 seconds (95.18 k allocations: 6.408 MiB, 56.31% compilation time)
5.0000015e13

julia> @time mapreduce(g, +, enumerate(vec))
  0.060598 seconds (3 allocations: 48 bytes)
5.0000015e13