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

Faster pycall. Adds pycall! #492

Merged
merged 22 commits into from
Jul 11, 2018
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 55 additions & 0 deletions benchmarks/callperf.jl
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
using PyCall, BenchmarkTools, DataStructures
using PyCall: _pycall!, pycall_legacy

results = OrderedDict{String,Any}()

let
np = pyimport("numpy")
nprand = np["random"]["rand"]
ret = PyNULL()
args_lens = (0,1,2,3,7,12,17)
# args_lens = (7,3,1)
# args_lens = (3,)
arr_sizes = (ntuple(i->1, len) for len in args_lens)

for (i, arr_size) in enumerate(arr_sizes)
nprand_pywrapfn = pywrapfn(nprand, length(arr_size))

pyargsptr = ccall((@pysym :PyTuple_New), PyPtr, (Int,), length(arr_size))
arr_size_str = args_lens[i] < 5 ? "$arr_size" : "$(args_lens[i])*(1,1,...)"

results["pycall_legacy $arr_size_str"] = @benchmark pycall_legacy($nprand, PyObject, $arr_size...)
println("pycall_legacy $arr_size_str:\n"); display(results["pycall_legacy $arr_size_str"])
println("--------------------------------------------------")

results["pycall $arr_size_str"] = @benchmark pycall($nprand, PyObject, $arr_size...)
println("pycall $arr_size_str:\n"); display(results["pycall $arr_size_str"])
println("--------------------------------------------------")

results["pycall! $arr_size_str"] = @benchmark pycall!($ret, $nprand, PyObject, $arr_size...)
println("pycall! $arr_size_str:\n"); display(results["pycall! $arr_size_str"])
println("--------------------------------------------------")

results["_pycall! $arr_size_str"] = @benchmark $_pycall!($ret, $pyargsptr, $nprand, $arr_size)
println("_pycall! $arr_size_str:\n"); display(results["_pycall! $arr_size_str"])
println("--------------------------------------------------")

results["nprand_pywrapfn $arr_size_str"] = @benchmark $nprand_pywrapfn($arr_size...)
println("nprand_pywrapfn $arr_size_str:\n"); display(results["nprand_pywrapfn $arr_size_str"])
println("--------------------------------------------------")

# args already set by nprand_pywrapfn calls above
results["nprand_pywrapfn_noargs $arr_size_str"] = @benchmark $nprand_pywrapfn()
println("nprand_pywrapfn_noargs $arr_size_str:\n"); display(results["nprand_pywrapfn_noargs $arr_size_str"])
println("--------------------------------------------------")
end
end
#
println("")
println("Mean times")
println("----------")
foreach((r)->println(rpad(r[1],33), "\t", mean(r[2])), results)
println("")
println("Median times")
println("----------")
foreach((r)->println(rpad(r[1],33), "\t", median(r[2])), results)
45 changes: 15 additions & 30 deletions src/PyCall.jl
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,14 @@ module PyCall

using Compat, VersionParsing

export pycall, pyimport, pyimport_e, pybuiltin, PyObject, PyReverseDims,
export pycall, pycall!, pyimport, pyimport_e, pybuiltin, PyObject, PyReverseDims,
PyPtr, pyincref, pydecref, pyversion, PyArray, PyArray_Info,
pyerr_check, pyerr_clear, pytype_query, PyAny, @pyimport, PyDict,
pyisinstance, pywrap, pytypeof, pyeval, PyVector, pystring, pystr, pyrepr,
pyraise, pytype_mapping, pygui, pygui_start, pygui_stop,
pygui_stop_all, @pylab, set!, PyTextIO, @pysym, PyNULL, ispynull, @pydef,
pyimport_conda, @py_str, @pywith, @pycall, pybytes, pyfunction, pyfunctionret
pyimport_conda, @py_str, @pywith, @pycall, pybytes, pyfunction, pyfunctionret,
pywrapfn, pysetarg!, pysetargs!

import Base: size, ndims, similar, copy, getindex, setindex!, stride,
convert, pointer, summary, convert, show, haskey, keys, values,
Expand Down Expand Up @@ -97,8 +98,13 @@ it is equivalent to a `PyNULL()` object.
"""
ispynull(o::PyObject) = o.o == PyPtr_NULL

function pydecref_(o::PyPtr)
ccall(@pysym(:Py_DecRef), Cvoid, (PyPtr,), o)
return o
end

function pydecref(o::PyObject)
ccall(@pysym(:Py_DecRef), Cvoid, (PyPtr,), o.o)
pydecref_(o.o)
o.o = PyPtr_NULL
o
end
Expand Down Expand Up @@ -689,11 +695,12 @@ function pybuiltin(name)
end

#########################################################################
include("pyfncall.jl")

"""
Low-level version of `pycall(o, ...)` that always returns `PyObject`.
"""
function _pycall(o::Union{PyObject,PyPtr}, args...; kwargs...)
function _pycall_legacy(o::Union{PyObject,PyPtr}, args...; kwargs...)
Copy link
Contributor Author

@JobJob JobJob Apr 13, 2018

Choose a reason for hiding this comment

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

This is just here for the benchmark comparison, the whole function, and the pycall_legacy methods below, need to be removed before merging.

oargs = map(PyObject, args)
nargs = length(args)
sigatomic_begin()
Expand Down Expand Up @@ -725,33 +732,11 @@ end

Call the given Python function (typically looked up from a module) with the given args... (of standard Julia types which are converted automatically to the corresponding Python types if possible), converting the return value to returntype (use a returntype of PyObject to return the unconverted Python object reference, or of PyAny to request an automated conversion)
"""
pycall(o::Union{PyObject,PyPtr}, returntype::TypeTuple, args...; kwargs...) =
return convert(returntype, _pycall(o, args...; kwargs...))::returntype

pycall(o::Union{PyObject,PyPtr}, ::Type{PyAny}, args...; kwargs...) =
return convert(PyAny, _pycall(o, args...; kwargs...))

(o::PyObject)(args...; kws...) = pycall(o, PyAny, args...; kws...)
PyAny(o::PyObject) = convert(PyAny, o)


"""
@pycall func(args...)::T
pycall_legacy(o::Union{PyObject,PyPtr}, returntype::TypeTuple, args...; kwargs...) =
return convert(returntype, _pycall_legacy(o, args...; kwargs...)) #::returntype

Convenience macro which turns `func(args...)::T` into pycall(func, T, args...)
"""
macro pycall(ex)
if !(isexpr(ex,:(::)) && isexpr(ex.args[1],:call))
throw(ArgumentError("Usage: @pycall func(args...)::T"))
end
func = ex.args[1].args[1]
args, kwargs = ex.args[1].args[2:end], []
if isexpr(args[1],:parameters)
kwargs, args = args[1], args[2:end]
end
T = ex.args[2]
:(pycall($(map(esc,[kwargs; func; T; args])...)))
end
pycall_legacy(o::Union{PyObject,PyPtr}, ::Type{PyAny}, args...; kwargs...) =
return convert(PyAny, _pycall_legacy(o, args...; kwargs...))

#########################################################################
# Once Julia lets us overload ".", we will use [] to access items, but
Expand Down
168 changes: 168 additions & 0 deletions src/pyfncall.jl
Original file line number Diff line number Diff line change
@@ -0,0 +1,168 @@

# pyarg_tuples[i] is a pointer to a python tuple of length i-1
# const pyarg_tuples = Vector{PyPtr}(32)
const pyarg_tuples = PyPtr[]

"""
Low-level version of `pycall!(ret, o, ...; kwargs...)`
Sets `ret.o` to the result of the call, and returns `ret::PyObject`
"""
function _pycall!(ret::PyObject, o::Union{PyObject,PyPtr}, args, kwargs)
if isempty(kwargs)
kw = C_NULL
else
kw = PyObject(Dict{AbstractString, Any}([Pair(string(k), v) for (k, v) in kwargs]))
end
_pycall!(ret, o, args, length(args), kw)
end

"""
Low-level version of `pycall!(ret, o, ...)` for when `kw` is already in python
friendly format but you don't have the python tuple to hold the arguments
(`pyargsptr`). Sets `ret.o` to the result of the call, and returns `ret::PyObject`.
"""
function _pycall!(ret::PyObject, o::Union{PyObject,PyPtr}, args,
nargs::Int=length(args), kw::Union{Ptr{Nothing}, PyObject}=C_NULL)
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this be a PyPtr rather than Ptr{Nothing}?

Copy link
Contributor Author

@JobJob JobJob Jul 3, 2018

Choose a reason for hiding this comment

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

No that's for the C_NULL when there aren't any kwargs. But good - that nudged me to add some tests for the kwargs case.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ptr{Nothing} is an implementation detail; it's best to use Ptr{Cvoid} (Base in 0.7, available from Compat in 0.6)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ta

# pyarg_tuples[i] is a pointer to a python tuple of length i-1
for n in length(pyarg_tuples):nargs
push!(pyarg_tuples, @pycheckn ccall((@pysym :PyTuple_New), PyPtr, (Int,), n))
end
check_pyargsptr(nargs)
pyargsptr = pyarg_tuples[nargs+1]
return _pycall!(ret, pyargsptr, o, args, nargs, kw) #::PyObject
end

"""
Handle the situation where a callee had previously called incref on the
arguments tuple that was passed to it. We need to hold the only reference to the
arguments tuple, since setting a tuple item is only allowed when there is just
one reference to the tuple (tuples are supposed to be immutable in Python in all
other cases). Note that this actually happens when creating new builtin
exceptions, ref: https://github.com/python/cpython/blob/480ab05d5fee2b8fa161f799af33086a4e68c7dd/Objects/exceptions.c#L48
OTOH this py"def foo(*args): global z; z=args" doesn't trigger this.
Fortunately, this check for ob_refcnt is fast - only a few cpu clock cycles.
"""
function check_pyargsptr(nargs::Int)
if nargs > 0 && unsafe_load(pyarg_tuples[nargs+1]).ob_refcnt > 1
pydecref_(pyarg_tuples[nargs+1])
pyarg_tuples[nargs+1] =
@pycheckn ccall((@pysym :PyTuple_New), PyPtr, (Int,), nargs)
end
end

"""
Low-level version of `pycall!(ret, o, ...)` for when `kw` is already in python
friendly format and you have the python tuple to hold the arguments (`pyargsptr`).
Sets the tuple's values to the python version of your arguments, and calls the
function. Sets `ret.o` to the result of the call, and returns `ret::PyObject`.
"""
function _pycall!(ret::PyObject, pyargsptr::PyPtr, o::Union{PyObject,PyPtr},
args, nargs::Int=length(args), kw::Union{Ptr{Nothing}, PyObject}=C_NULL)
pysetargs!(pyargsptr, args, nargs)
return __pycall!(ret, pyargsptr, o, kw) #::PyObject
end

"""
```
pysetargs!(pyargsptr::PyPtr, args...)
```
Convert `args` to `PyObject`s, and set them as the elements of the Python tuple
pointed to by `pyargsptr`
"""
function pysetargs!(pyargsptr::PyPtr, args, N::Int)
for i = 1:N
pysetarg!(pyargsptr, args[i], i)
end
end

"""
```
pysetarg!(pyargsptr::PyPtr, arg, i::Integer=1)
```
Convert `arg` to a `PyObject`, and set it as the `i-1`th element of the Python
tuple pointed to by `pyargsptr`
"""
function pysetarg!(pyargsptr::PyPtr, arg, i::Integer=1)
pyarg = PyObject(arg)
pyincref(pyarg) # PyTuple_SetItem steals the reference
@pycheckz ccall((@pysym :PyTuple_SetItem), Cint,
(PyPtr,Int,PyPtr), pyargsptr, i-1, pyarg)
end

"""
Lowest level version of `pycall!(ret, o, ...)`, assumes `pyargsptr` and `kw`
have all their args set to Python values, so we can just call the function `o`.
Sets `ret.o` to the result of the call, and returns `ret::PyObject`.
"""
function __pycall!(ret::PyObject, pyargsptr::PyPtr, o::Union{PyObject,PyPtr},
kw::Union{Ptr{Nothing}, PyObject})
sigatomic_begin()
try
retptr = @pycheckn ccall((@pysym :PyObject_Call), PyPtr, (PyPtr,PyPtr,PyPtr), o,
pyargsptr, kw)
pyincref_(retptr)
pydecref(ret)
ret.o = retptr
finally
sigatomic_end()
end
return ret #::PyObject
end

"""
```
pycall!(ret::PyObject, o::Union{PyObject,PyPtr}, returntype::Type, args...; kwargs...)
```
Set `ret` to the result of `pycall(o, returntype, args...; kwargs)` and return
`convert(returntype, ret)`.
Avoids allocating an extra PyObject for `ret`. See `pycall` for other details.
"""
pycall!(ret::PyObject, o::Union{PyObject,PyPtr}, returntype::TypeTuple, args...; kwargs...) =
return convert(returntype, _pycall!(ret, o, args, kwargs))

function pycall!(ret::PyObject, o::T, returntype::Type{PyObject}, args...; kwargs...) where
T<:Union{PyObject,PyPtr}
return _pycall!(ret, o, args, kwargs)
end

pycall!(ret::PyObject, o::Union{PyObject,PyPtr}, ::Type{PyAny}, args...; kwargs...) =
return convert(PyAny, _pycall!(ret, o, args, kwargs))

"""
```
pycall(o::Union{PyObject,PyPtr}, returntype::TypeTuple, args...; kwargs...)
```
Call the given Python function (typically looked up from a module) with the
given args... (of standard Julia types which are converted automatically to the
corresponding Python types if possible), converting the return value to
returntype (use a returntype of PyObject to return the unconverted Python object
reference, or of PyAny to request an automated conversion)
"""
pycall(o::Union{PyObject,PyPtr}, returntype::TypeTuple, args...; kwargs...) =
return convert(returntype, _pycall!(PyNULL(), o, args, kwargs))::returntype

pycall(o::Union{PyObject,PyPtr}, ::Type{PyAny}, args...; kwargs...) =
return convert(PyAny, _pycall!(PyNULL(), o, args, kwargs))

(o::PyObject)(args...; kwargs...) =
return convert(PyAny, _pycall!(PyNULL(), o, args, kwargs))

PyAny(o::PyObject) = convert(PyAny, o)

"""
@pycall func(args...)::T

Convenience macro which turns `func(args...)::T` into pycall(func, T, args...)
"""
macro pycall(ex)
if !(isexpr(ex,:(::)) && isexpr(ex.args[1],:call))
throw(ArgumentError("Usage: @pycall func(args...)::T"))
end
func = ex.args[1].args[1]
args, kwargs = ex.args[1].args[2:end], []
if isexpr(args[1],:parameters)
kwargs, args = args[1], args[2:end]
end
T = ex.args[2]
:(pycall($(map(esc,[kwargs; func; T; args])...)))
end
2 changes: 2 additions & 0 deletions test/runtests.jl
Original file line number Diff line number Diff line change
Expand Up @@ -560,3 +560,5 @@ end
@test_throws ErrorException @pywith IgnoreError(false) error()
@test (@pywith IgnoreError(true) error(); true)
end

include("test_pyfncall.jl")
26 changes: 26 additions & 0 deletions test/test_pyfncall.jl
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
using Compat.Test, PyCall

py"""
def mklist(*args):
return list(args)
"""
@testset "pycall!" begin
pymklist = py"mklist"
res = PyNULL()

function pycall_checks(res, pyf, RetType, args...)
pycall_res = pycall(pyf, RetType, args...)
res_converted = pycall!(res, pyf, RetType, args...)
@test convert(RetType, res) == res_converted
@test res_converted == pycall_res
RetType != PyAny && @test res_converted isa RetType
end

@testset "basics" begin
args = ("a", 2, 4.5)
for RetType in (PyObject, PyAny, Tuple)
pycall_checks(res, pymklist, RetType, args...)
GC.gc()
end
end
end