simplify pycall! code to eliminate argument-tuple caching … the logic was just way too complicated and hard to get right, for questionable gain

Author: stevengj

src/pyfncall.jl

@@ -1,8 +1,3 @@
-
-# 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`
@@ -21,90 +16,22 @@ 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{Cvoid}, PyObject}=C_NULL)
-    # 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]
-    # We temporarily set pyarg_tuples[nargs+1] to NULL to ensure that nested
-    # calls to pycall don't try to use the same tuple object (issue #533).
-    pyarg_tuples[nargs+1] = C_NULL
+function _pycall!(ret::PyObject, o::Union{PyObject,PyPtr}, args, nargs::Int=length(args),
+                  kw::Union{Ptr{Cvoid}, PyObject}=C_NULL)
+    pyargsptr = @pycheckn ccall((@pysym :PyTuple_New), PyPtr, (Int,), nargs)
     try
-        pysetargs!(pyargsptr, args, nargs)
+        for i = 1:nargs
+            pyarg = PyObject(args[i])
+            pyincref(pyarg) # PyTuple_SetItem steals the reference
+            @pycheckz ccall((@pysym :PyTuple_SetItem), Cint,
+                                (PyPtr,Int,PyPtr), pyargsptr, i-1, pyarg)
+        end
         return __pycall!(ret, pyargsptr, o, kw) #::PyObject
     finally
-        if nargs > 0 && unsafe_load(pyargsptr).ob_refcnt > 1
-            # we are no longer the only reference to the tuple,
-            # so PyTuple_SetItem will fail (see check_pyargsptr)
-            pydecref_(pyargsptr)
-        else
-            pyunsetargs!(pyargsptr, nargs)   # garbage-collect args
-            pydecref_(pyarg_tuples[nargs+1]) # no-op if it's still NULL
-            pyarg_tuples[nargs+1] = pyargsptr # restore tuple cache
-        end
-    end
-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
-        p = pyarg_tuples[nargs+1]
-        if p == C_NULL || unsafe_load(p).ob_refcnt > 1
-            pydecref_(p) # no-op for NULL p
-            pyarg_tuples[nargs+1] =
-                @pycheckn ccall((@pysym :PyTuple_New), PyPtr, (Int,), nargs)
-        end
-    end
-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
-
-"""
-Decref the `N`` elements of the Python tuple `pyargsptr` by setting the
-elements to NULL, to ensure that they are garbage-collected.
-"""
-function pyunsetargs!(pyargsptr::PyPtr, N::Int)
-    for i = 1:N
-        @pycheckz ccall((@pysym :PyTuple_SetItem), Cint, (PyPtr,Int,PyPtr), pyargsptr, i-1, C_NULL)
+        pydecref_(pyargsptr)
     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`.
@@ -116,9 +43,8 @@ function __pycall!(ret::PyObject, pyargsptr::PyPtr, o::Union{PyObject,PyPtr},
     try
         retptr = @pycheckn ccall((@pysym :PyObject_Call), PyPtr, (PyPtr,PyPtr,PyPtr), o,
                         pyargsptr, kw)
-        pyincref_(retptr)
-        pydecref(ret)
-        ret.o = retptr
+        pydecref_(ret.o)
+        ret.o = pyincref_(retptr)
     finally
         sigatomic_end()
     end