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

Regression calling cfunction #23020

Closed
musm opened this issue Jul 29, 2017 · 12 comments · Fixed by #23066
Closed

Regression calling cfunction #23020

musm opened this issue Jul 29, 2017 · 12 comments · Fixed by #23066
Labels
deprecation This change introduces or involves a deprecation docs This change adds or pertains to documentation

Comments

@musm
Copy link
Contributor

musm commented Jul 29, 2017

using BenchmarkTools

foo(x) = x + 1
f() = cfunction(foo, Int, (Int,))

@btime f();
  815.538 ns (3 allocations: 96 bytes)

# version
# Julia Version 0.7.0-DEV.1159
# Commit 8d752a07f4* (2017-07-29 02:32 UTC)
# OS: Windows (x86_64-w64-mingw32)

Julia 0.6

using BenchmarkTools

foo(x) = x + 1
f() = cfunction(foo, Int, (Int,))

julia> @btime f();
  1.578 ns (0 allocations: 0 bytes)
@yuyichao
Copy link
Contributor

Use cfunction(foo, Int, Tuple{Int}) instead.

@yuyichao yuyichao added deprecation This change introduces or involves a deprecation docs This change adds or pertains to documentation labels Jul 29, 2017
@tkelman
Copy link
Contributor

tkelman commented Jul 29, 2017

should the (Int,) form be deprecated if it's a performance trap?

@yuyichao
Copy link
Contributor

That's why I also added the deprecation label....

@musm
Copy link
Contributor Author

musm commented Jul 30, 2017

@yuyichao Is there reference for the updated syntax ?

@yuyichao
Copy link
Contributor

New tuple type syntax? #10380 ?

@musm
Copy link
Contributor Author

musm commented Jul 31, 2017

Thanks. Does it not seem a little inconsistent that, e.g., ccall (and some other functions) takes a tuple of types but cfunction and invoke take a tuple type, or is that something else?

@tkelman
Copy link
Contributor

tkelman commented Jul 31, 2017

It should probably eventually be deprecated in ccall too - we should make a decision on whether we're going to do that soon

@musm
Copy link
Contributor Author

musm commented Jul 31, 2017

Of the top of my head code_llvm, code_typed, code_warntype, ccall all use a tuple of types instead of a tuple type.

@yuyichao
Copy link
Contributor

ccall is special since it's a syntax and not a function.
Other functions also doesn't matter since they are only for reflection.

@quinnj
Copy link
Member

quinnj commented Aug 26, 2017

I'm not a big fan of the inconsistency here, the fact that we now have
cfunction(foo, Cint, Tuple{Ptr{Void}})
vs.
ccall((:foo, lib), Void, (Ptr{Void},), ptr)
As a user/library author, how am I supposed to remember/guess whether you want a Tuple{...} or a (...). Let's pick one and use it everywhere, ccall, reflection, etc.

@yuyichao
Copy link
Contributor

Tuple of types is only kept for convinience. If people don't like that, we can certainly just drop all of them.

@musm
Copy link
Contributor Author

musm commented Aug 26, 2017

@quinnj I thought so too initially, but I'm pretty satisfied with @yuyichao comment above.

  • invoke, cfunction operatore on julia function and thus the tuple type. (consistent)
  • ccall is syntax
  • reflection methods doesn't matter

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deprecation This change introduces or involves a deprecation docs This change adds or pertains to documentation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants