The `cfunction` 1.0 deserves #26486

vtjnash · 2018-03-16T04:51:42Z

This PR turns cfunction into a macro, and has it work more similarly to ccall (no runtime compiler required, defined by the containing method parameters, supports any type object including closures). In most cases, v0.6 code should just need to add an @. But it also has new advanced capabilities to efficiently create gc-managed runtime closures pointers! This will fix #12256, fix #26070 and close #1096 (I feel slightly cheated to not get to close a 3-digit issue on this one 😜).

Implements https://github.com/vtjnash/julep/wiki/0004:-Static-compilation-for-cfunction

The best place to see the net effect of this change is perhaps the stdlib, but also the docs (where I have an example of qsort), or the Julep (which has a more complete exploration of qsort usages), or lastly perhaps the tests (which try to have examples of weird uses of almost everything).

~~Syntax questions:~~

Creating a closure over the value of a local variable requires annotating with $: i.e. ptr = @cfunction $cl RT (A, B). However, I've also considered instead using local for this purpose instead: ptr = @cfunction local cl RT (A, B). Thoughts on which one reads better? As can be seen from there being zero usages in this repo (outside of tests), this feature isn't a particularly common need / the other form is more preferred and easier to use (when possible), so I don't really care either way.
Should I continue to use basically the same syntax as before (i.e. just add @), or use the following alternate syntax rearrangement, or just try to provide both: ptr = @cfunction cl(A, B)::RT (which, for the variant that creates a closure over the value of the local variable cl, would be ptr = @cfunction $cl(A, B)::RT or ptr = @cfunction local cl(A, B)::RT)

maleadt · 2018-03-16T06:59:16Z

stdlib/LibGit2/src/tree.jl

    # NOTE: don't use @check/GitError directly, because the code can be arbitrary
+    payload = Any[ tree, f ]
+    cbf = @cfunction(function treewalk_callback(root_cstr::Cstring, entry_ptr::Ptr{Cvoid}, payload::Vector{Any})::Cint


Isn't it possible to capture tree and f instead of using the payload mechanism?

Yes, it's just slower that way than to just use the C API correctly

I presume it's slower because closure support requires a stub that sets the locals and jumps to the "real" function? Whereas without any closed-over variables, it can just call directly.

Yes, and in addition to the indirect function call, it also requires allocation to be able to track the lifetime, and runtime work to manage and handle that, etc.

JeffBezanson · 2018-03-16T15:52:53Z

base/c.jl

+    @cfunction(callable, ReturnType, (ArgumentTypes...,)) -> Ptr{Cvoid}
+    @cfunction(\$callable, ReturnType, (ArgumentTypes...,)) -> CFunction
+
+Generate a C-callable function pointer from the Julia function `f`


There is no f :)

JeffBezanson · 2018-03-16T15:56:10Z

base/exports.jl

@@ -925,6 +925,7 @@ export

 # C interface
    cfunction,


Remove this?

JeffBezanson · 2018-03-16T16:00:23Z

doc/src/manual/calling-c-and-fortran-code.md

+## Closure cfunctions
+
+The first argument to [`@cfunction`](@ref) can be marked with a `$`, in which case
+the return value will instead be a `struct CFunction` which closes over the argument.


Out of curiosity: what happens if I don't use a $, but I give a global name that's bound to a non-singleton function object?

Like ccall, it's resolved by the macro to close over the value (not the binding), since that's almost always what you want anyways, and makes it easier to avoid accidentally shooting your self in the (gc) foot.

JeffBezanson · 2018-03-16T16:12:21Z

I think the $ syntax is good. It looks like the first argument can also be a function expression, which also gets $ treatment?

vtjnash · 2018-03-16T16:30:24Z

which also gets $ treatment?

Not really. Because I felt that it read nicely to keep the function definition and use together this way, I'm (ab)using the knowledge that it got evaluated in global scope early, and then treated as a constant. In either case, it can be an arbitrary expression, the $ keyword just indicates when to evaluate it.

stevengj · 2018-03-16T17:07:29Z

+1 for ptr = @cfunction $cl(A, B)::RT

nalimilan · 2018-03-16T19:06:31Z

doc/src/manual/calling-c-and-fortran-code.md

@@ -182,26 +185,26 @@ an integer less/greater than zero if `a` should appear before/after `b` (or zero
 is permitted). Now, suppose that we have a 1d array `A` of values in Julia that we want to sort
 using the `qsort` function (rather than Julia's built-in `sort` function). Before we worry about
 calling `qsort` and passing arguments, we need to write a comparison function that works for some
-arbitrary type T:
+arbitrary type T (which defines `<`):


T isn't used in the example anymore.

nalimilan · 2018-03-16T19:06:54Z

doc/src/manual/calling-c-and-fortran-code.md

       end
 mycompare (generic function with 1 method)
 ```

 Notice that we have to be careful about the return type: `qsort` expects a function returning
-a C `int`, so we must be sure to return `Cint` via a call to `convert` and a `typeassert`.
+a C `int`, so we annotate the return type of the function be sure it returns a `Cint`.


Missing "to"?

tknopp · 2018-03-17T07:49:56Z

@vtjnash: Any chance that this will fix https://discourse.julialang.org/t/performance-issue-with-gtk-jl/6171. That inference on callbacks requires up to minutes is a major issue when using Gtk currently.

macro head uses this to resolve symbols, which we want to allow even in pure context (generated functions)

Provide static support for handling dynamic calls and closures

giordano · 2018-06-01T17:07:28Z

Did I miss it or this change isn't mentioned in NEWS.md?

stevengj · 2018-06-08T14:26:37Z

base/c.jl

+    f::Any
+    _1::Ptr{Cvoid}
+    _2::Ptr{Cvoid}
+    let construtor = false end


s/construtor/constructor/

(not that it matters, since this variable is not visible) … a comment explaining the purpose of this line would be helpful

It disables creation of the default constructors (ensuring that this type is not accidentally constructible from user code).

maleadt reviewed Mar 16, 2018

View reviewed changes

thofma mentioned this pull request Mar 16, 2018

Enh/acb integrate Nemocas/Nemo.jl#382

Closed

2 tasks

JeffBezanson reviewed Mar 16, 2018

View reviewed changes

base/exports.jl Outdated

@@ -925,6 +925,7 @@ export

# C interface

cfunction,

Copy link

Member

JeffBezanson Mar 16, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove this?

JeffBezanson reviewed Mar 16, 2018

View reviewed changes

vtjnash force-pushed the jn/cfunction-closure branch from de8ea4a to e0d3951 Compare March 16, 2018 16:19

nalimilan reviewed Mar 16, 2018

View reviewed changes

vtjnash force-pushed the jn/cfunction-closure branch 2 times, most recently from 4380acd to dffb8cc Compare March 28, 2018 18:20

vtjnash force-pushed the jn/cfunction-closure branch from dffb8cc to bd386ff Compare April 3, 2018 22:04

vtjnash added 2 commits April 4, 2018 17:56

eval: add fast-path for . expr

2ba69e8

macro head uses this to resolve symbols, which we want to allow even in pure context (generated functions)

cfunction macro: extend cfunction capabilities

81f6524

Provide static support for handling dynamic calls and closures

vtjnash force-pushed the jn/cfunction-closure branch from bd386ff to 81f6524 Compare April 4, 2018 22:00

vtjnash merged commit 28e7956 into master Apr 5, 2018

vtjnash deleted the jn/cfunction-closure branch April 5, 2018 16:20

Keno mentioned this pull request Apr 7, 2018

dotted getindex has wrong error in the REPL #26739

Closed

mlubin mentioned this pull request Apr 24, 2018

cfunction deprecation JuliaLang/Compat.jl#535

Closed

fredrikekre added the needs news A NEWS entry is required for this change label Jun 1, 2018

stevengj mentioned this pull request Jun 8, 2018

Base.CFunction is user-visible but undocumented #27490

Closed

stevengj reviewed Jun 8, 2018

View reviewed changes

vtjnash removed the needs news A NEWS entry is required for this change label Jun 8, 2018

adamryczkowski mentioned this pull request Aug 24, 2018

Migration towards Julia 1.0: Problem with Schur decomposition malmaud/TensorFlow.jl#421

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

The `cfunction` 1.0 deserves #26486

The `cfunction` 1.0 deserves #26486

vtjnash commented Mar 16, 2018 •

edited

Loading

maleadt Mar 16, 2018

vtjnash Mar 16, 2018

StefanKarpinski Mar 16, 2018

vtjnash Mar 16, 2018

JeffBezanson Mar 16, 2018

JeffBezanson Mar 16, 2018

JeffBezanson Mar 16, 2018

vtjnash Mar 16, 2018

JeffBezanson commented Mar 16, 2018

vtjnash commented Mar 16, 2018

stevengj commented Mar 16, 2018

nalimilan Mar 16, 2018

nalimilan Mar 16, 2018

tknopp commented Mar 17, 2018

giordano commented Jun 1, 2018

stevengj Jun 8, 2018

vtjnash Jun 8, 2018

The cfunction 1.0 deserves #26486

The cfunction 1.0 deserves #26486

Conversation

vtjnash commented Mar 16, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JeffBezanson commented Mar 16, 2018

vtjnash commented Mar 16, 2018

stevengj commented Mar 16, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tknopp commented Mar 17, 2018

giordano commented Jun 1, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

The `cfunction` 1.0 deserves #26486

The `cfunction` 1.0 deserves #26486

vtjnash commented Mar 16, 2018 •

edited

Loading