From 701340825661ee44787d940fa934ace526f8e6a2 Mon Sep 17 00:00:00 2001
From: Joel Mason <jobba1@hotmail.com>
Date: Wed, 28 Mar 2018 20:44:23 +1100
Subject: [PATCH 1/6] Ideas for speeding up tuple access

with benchmarks
---
 benchmarks/tupleperf.jl | 45 +++++++++++++++++++++++++++++++++
 src/PyCall.jl           |  1 +
 src/fasttuples.jl       | 56 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 102 insertions(+)
 create mode 100644 benchmarks/tupleperf.jl
 create mode 100644 src/fasttuples.jl

diff --git a/benchmarks/tupleperf.jl b/benchmarks/tupleperf.jl
new file mode 100644
index 00000000..e8f4e8cf
--- /dev/null
+++ b/benchmarks/tupleperf.jl
@@ -0,0 +1,45 @@
+using PyCall, BenchmarkTools, DataStructures
+
+# This would be put in __init__
+PyCall.pytuple_type_ptr[] = cglobal(@pysym :PyTuple_Type)
+
+results = OrderedDict{String,Any}()
+let
+    np = pyimport("numpy")
+    nprandint = np["random"]["randint"]
+    nprand = np["random"]["rand"]
+    res = PyNULL()
+
+    tuplen = 16
+    tpl = convert(PyObject, (1:tuplen...))
+
+    tplidx = rand(0:(tuplen-1))
+    results["standard get"] = @benchmark get($tpl, PyObject, $tplidx)
+    println("standard get:\n"); display(results["standard get"])
+    println("--------------------------------------------------")
+
+    tplidx = rand(0:(tuplen-1))
+    results["get!"] = @benchmark get!($res, $tpl, PyObject, $tplidx)
+    println("get!:\n"); display(results["get!"])
+    println("--------------------------------------------------")
+
+    tplidx = rand(0:(tuplen-1))
+    results["getwpyisinst!"] = @benchmark getwpyisinst!($res, $tpl, PyObject, $tplidx)
+    println("getwpyisinst!:\n"); display(results["getwpyisinst!"])
+    println("--------------------------------------------------")
+
+    tplidx = rand(0:(tuplen-1))
+    results["getwtuple_ptr!"] = @benchmark getwtuple_ptr!($res, $tpl, PyObject, $tplidx)
+    println("getwtuple_ptr!:\n"); display(results["getwtuple_ptr!"])
+    println("--------------------------------------------------")
+
+    tplidx = rand(0:(tuplen-1))
+    results["unsafe_gettpl!"] = @benchmark unsafe_gettpl!($res, $tpl, PyObject, $tplidx)
+    println("unsafe_gettpl!:\n"); display(results["unsafe_gettpl!"])
+    println("--------------------------------------------------")
+end
+
+println("")
+println("Mean times")
+println("----------")
+foreach((r)->println(rpad(r[1],20), ": ", mean(r[2])), results)
diff --git a/src/PyCall.jl b/src/PyCall.jl
index 37cd10c2..12997828 100644
--- a/src/PyCall.jl
+++ b/src/PyCall.jl
@@ -170,6 +170,7 @@ include("pyiterator.jl")
 include("pyclass.jl")
 include("callback.jl")
 include("io.jl")
+include("fasttuples.jl")
 
 #########################################################################
 
diff --git a/src/fasttuples.jl b/src/fasttuples.jl
new file mode 100644
index 00000000..ef6dc710
--- /dev/null
+++ b/src/fasttuples.jl
@@ -0,0 +1,56 @@
+export gettplidx, gettplidx!, getwtuple_ptr!, getwpyisinst!, unsafe_gettpl!
+
+const MAX_PYINT = 31
+const pyints = Ref{Vector{PyObject}}()
+const pytuple_type_ptr = Ref{PyPtr}()
+
+import Base: get!
+function get!(ret::PyObject, o::PyObject, returntype::TypeTuple, k)
+    ret.o = @pycheckn ccall((@pysym :PyObject_GetItem),
+                                 PyPtr, (PyPtr,PyPtr), o, PyObject(k))
+    convert(returntype, ret)
+end
+
+function getwpyisinst!(ret::PyObject, o::PyObject, returntype::TypeTuple, idx)
+    if pyisinstance(o, @pyglobalobj :PyTuple_Type)
+        ret.o = (@pycheckn ccall(@pysym(:PyTuple_GetItem), PyPtr, (PyPtr, Cint), o, idx))
+        pyincref_(ret.o)
+    else
+        ret.o = @pycheckn ccall((@pysym :PyObject_GetItem),
+                                     PyPtr, (PyPtr,PyPtr), o, PyObject(idx))
+    end
+    return convert(returntype, ret)
+end
+
+function getwtuple_ptr!(ret::PyObject, o::PyObject, returntype::TypeTuple, idx)
+    pytype_ptr = unsafe_load(o.o).ob_type
+    if pytype_ptr == pytuple_type_ptr[]
+        ret.o = (@pycheckn ccall(@pysym(:PyTuple_GetItem), PyPtr, (PyPtr, Cint), o, idx))
+        pyincref_(ret.o)
+    else
+        # pyerr_clear()
+        ret.o = @pycheckn ccall((@pysym :PyObject_GetItem),
+                                     PyPtr, (PyPtr,PyPtr), o, PyObject(idx))
+    end
+    return convert(returntype, ret)
+end
+
+# struct PyTuple_struct
+# refs:
+#   https://github.com/python/cpython/blob/da1734c58d2f97387ccc9676074717d38b044128/Include/object.h#L106-L115
+#   https://github.com/python/cpython/blob/da1734c58d2f97387ccc9676074717d38b044128/Include/tupleobject.h#L25-L33
+struct PyVar_struct
+    ob_refcnt::Int
+    ob_type::Ptr{Cvoid}
+    ob_size::Int
+    # ob_item::Ptr{PyPtr}
+end
+
+function unsafe_gettpl!(ret::PyObject, o::PyObject, returntype::TypeTuple, idx::Int)
+    pytype_ptr = unsafe_load(o.o).ob_type
+    # get address of ob_item (just after the end of the struct)
+    itemsptr = Base.reinterpret(Ptr{PyPtr}, o.o + sizeof(PyVar_struct))
+    ret.o = unsafe_load(itemsptr, idx)
+    pyincref_(ret.o)
+    return convert(returntype, ret)
+end

From 9ae0a18697842ab4c3e6709a05817159f984c53a Mon Sep 17 00:00:00 2001
From: Joel Mason <jobba1@hotmail.com>
Date: Tue, 10 Apr 2018 04:40:25 +1000
Subject: [PATCH 2/6] Simplify, add fast list access w int keys too

---
 benchmarks/tupleperf.jl | 13 -------------
 src/PyCall.jl           | 11 +++++++++--
 src/fasttuples.jl       | 42 ++++++++++-------------------------------
 3 files changed, 19 insertions(+), 47 deletions(-)

diff --git a/benchmarks/tupleperf.jl b/benchmarks/tupleperf.jl
index e8f4e8cf..e98fcea0 100644
--- a/benchmarks/tupleperf.jl
+++ b/benchmarks/tupleperf.jl
@@ -1,8 +1,5 @@
 using PyCall, BenchmarkTools, DataStructures
 
-# This would be put in __init__
-PyCall.pytuple_type_ptr[] = cglobal(@pysym :PyTuple_Type)
-
 results = OrderedDict{String,Any}()
 let
     np = pyimport("numpy")
@@ -23,16 +20,6 @@ let
     println("get!:\n"); display(results["get!"])
     println("--------------------------------------------------")
 
-    tplidx = rand(0:(tuplen-1))
-    results["getwpyisinst!"] = @benchmark getwpyisinst!($res, $tpl, PyObject, $tplidx)
-    println("getwpyisinst!:\n"); display(results["getwpyisinst!"])
-    println("--------------------------------------------------")
-
-    tplidx = rand(0:(tuplen-1))
-    results["getwtuple_ptr!"] = @benchmark getwtuple_ptr!($res, $tpl, PyObject, $tplidx)
-    println("getwtuple_ptr!:\n"); display(results["getwtuple_ptr!"])
-    println("--------------------------------------------------")
-
     tplidx = rand(0:(tuplen-1))
     results["unsafe_gettpl!"] = @benchmark unsafe_gettpl!($res, $tpl, PyObject, $tplidx)
     println("unsafe_gettpl!:\n"); display(results["unsafe_gettpl!"])
diff --git a/src/PyCall.jl b/src/PyCall.jl
index 12997828..09ba3d73 100644
--- a/src/PyCall.jl
+++ b/src/PyCall.jl
@@ -10,11 +10,12 @@ export pycall, pyimport, pyimport_e, pybuiltin, PyObject, PyReverseDims,
        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,
+       unsafe_gettpl!
 
 import Base: size, ndims, similar, copy, getindex, setindex!, stride,
        convert, pointer, summary, convert, show, haskey, keys, values,
-       eltype, get, delete!, empty!, length, isempty, start, done,
+       eltype, get, get!, delete!, empty!, length, isempty, start, done,
        next, filter!, hash, splice!, pop!, ==, isequal, push!,
        append!, insert!, prepend!, unsafe_convert
 import Compat: pushfirst!, popfirst!, firstindex, lastindex
@@ -136,6 +137,12 @@ function Base.copy!(dest::PyObject, src::PyObject)
     return pyincref(dest)
 end
 
+function Base.copy!(dest::PyObject, src::PyPtr)
+    pydecref(dest)
+    dest.o = src
+    return pyincref(dest)
+end
+
 pyisinstance(o::PyObject, t::PyObject) =
   !ispynull(t) && ccall((@pysym :PyObject_IsInstance), Cint, (PyPtr,PyPtr), o, t.o) == 1
 
diff --git a/src/fasttuples.jl b/src/fasttuples.jl
index ef6dc710..0e16aa7f 100644
--- a/src/fasttuples.jl
+++ b/src/fasttuples.jl
@@ -1,38 +1,15 @@
-export gettplidx, gettplidx!, getwtuple_ptr!, getwpyisinst!, unsafe_gettpl!
 
-const MAX_PYINT = 31
-const pyints = Ref{Vector{PyObject}}()
-const pytuple_type_ptr = Ref{PyPtr}()
-
-import Base: get!
-function get!(ret::PyObject, o::PyObject, returntype::TypeTuple, k)
-    ret.o = @pycheckn ccall((@pysym :PyObject_GetItem),
-                                 PyPtr, (PyPtr,PyPtr), o, PyObject(k))
-    convert(returntype, ret)
-end
-
-function getwpyisinst!(ret::PyObject, o::PyObject, returntype::TypeTuple, idx)
+function get!(ret::PyObject, o::PyObject, returntype::TypeTuple, k::Integer)
     if pyisinstance(o, @pyglobalobj :PyTuple_Type)
-        ret.o = (@pycheckn ccall(@pysym(:PyTuple_GetItem), PyPtr, (PyPtr, Cint), o, idx))
-        pyincref_(ret.o)
+        copy!(ret, @pycheckn ccall(@pysym(:PyTuple_GetItem), PyPtr, (PyPtr, Cint), o, k))
+    elseif pyisinstance(o, @pyglobalobj :PyList_Type)
+        copy!(ret, @pycheckn ccall(@pysym( :PyList_GetItem), PyPtr, (PyPtr, Cint), o, k))
     else
+        pydecref(ret)
         ret.o = @pycheckn ccall((@pysym :PyObject_GetItem),
-                                     PyPtr, (PyPtr,PyPtr), o, PyObject(idx))
+                                     PyPtr, (PyPtr,PyPtr), o, PyObject(k))
     end
-    return convert(returntype, ret)
-end
-
-function getwtuple_ptr!(ret::PyObject, o::PyObject, returntype::TypeTuple, idx)
-    pytype_ptr = unsafe_load(o.o).ob_type
-    if pytype_ptr == pytuple_type_ptr[]
-        ret.o = (@pycheckn ccall(@pysym(:PyTuple_GetItem), PyPtr, (PyPtr, Cint), o, idx))
-        pyincref_(ret.o)
-    else
-        # pyerr_clear()
-        ret.o = @pycheckn ccall((@pysym :PyObject_GetItem),
-                                     PyPtr, (PyPtr,PyPtr), o, PyObject(idx))
-    end
-    return convert(returntype, ret)
+    convert(returntype, ret)
 end
 
 # struct PyTuple_struct
@@ -46,11 +23,12 @@ struct PyVar_struct
     # ob_item::Ptr{PyPtr}
 end
 
-function unsafe_gettpl!(ret::PyObject, o::PyObject, returntype::TypeTuple, idx::Int)
+function unsafe_gettpl!(ret::PyObject, o::PyObject, returntype::TypeTuple, k::Int)
     pytype_ptr = unsafe_load(o.o).ob_type
     # get address of ob_item (just after the end of the struct)
     itemsptr = Base.reinterpret(Ptr{PyPtr}, o.o + sizeof(PyVar_struct))
-    ret.o = unsafe_load(itemsptr, idx)
+    pydecref(ret)
+    ret.o = unsafe_load(itemsptr, k)
     pyincref_(ret.o)
     return convert(returntype, ret)
 end

From 4ff9f68e6e61302fec761006e5e06be505185f57 Mon Sep 17 00:00:00 2001
From: Joel Mason <jobba1@hotmail.com>
Date: Tue, 10 Apr 2018 04:45:10 +1000
Subject: [PATCH 3/6] Rename file to get.jl, move other get methods into it

---
 benchmarks/tupleperf.jl       |  7 ++++++-
 src/PyCall.jl                 | 23 +----------------------
 src/{fasttuples.jl => get.jl} | 22 ++++++++++++++++++++++
 3 files changed, 29 insertions(+), 23 deletions(-)
 rename src/{fasttuples.jl => get.jl} (61%)

diff --git a/benchmarks/tupleperf.jl b/benchmarks/tupleperf.jl
index e98fcea0..c46407cf 100644
--- a/benchmarks/tupleperf.jl
+++ b/benchmarks/tupleperf.jl
@@ -11,10 +11,15 @@ let
     tpl = convert(PyObject, (1:tuplen...))
 
     tplidx = rand(0:(tuplen-1))
-    results["standard get"] = @benchmark get($tpl, PyObject, $tplidx)
+    results["standard get"] = @benchmark get($tpl, PyObject, PyObject($tplidx))
     println("standard get:\n"); display(results["standard get"])
     println("--------------------------------------------------")
 
+    tplidx = rand(0:(tuplen-1))
+    results["faster get"] = @benchmark get($tpl, PyObject, $tplidx)
+    println("faster get:\n"); display(results["faster get"])
+    println("--------------------------------------------------")
+
     tplidx = rand(0:(tuplen-1))
     results["get!"] = @benchmark get!($res, $tpl, PyObject, $tplidx)
     println("get!:\n"); display(results["get!"])
diff --git a/src/PyCall.jl b/src/PyCall.jl
index 09ba3d73..f2f303e3 100644
--- a/src/PyCall.jl
+++ b/src/PyCall.jl
@@ -177,7 +177,7 @@ include("pyiterator.jl")
 include("pyclass.jl")
 include("callback.jl")
 include("io.jl")
-include("fasttuples.jl")
+include("get.jl")
 
 #########################################################################
 
@@ -761,27 +761,6 @@ macro pycall(ex)
     :(pycall($(map(esc,[kwargs; func; T; args])...)))
 end
 
-#########################################################################
-# Once Julia lets us overload ".", we will use [] to access items, but
-# for now we can define "get".
-
-function get(o::PyObject, returntype::TypeTuple, k, default)
-    r = ccall((@pysym :PyObject_GetItem), PyPtr, (PyPtr,PyPtr), o,PyObject(k))
-    if r == C_NULL
-        pyerr_clear()
-        default
-    else
-        convert(returntype, PyObject(r))
-    end
-end
-
-get(o::PyObject, returntype::TypeTuple, k) =
-    convert(returntype, PyObject(@pycheckn ccall((@pysym :PyObject_GetItem),
-                                 PyPtr, (PyPtr,PyPtr), o, PyObject(k))))
-
-get(o::PyObject, k, default) = get(o, PyAny, k, default)
-get(o::PyObject, k) = get(o, PyAny, k)
-
 function delete!(o::PyObject, k)
     e = ccall((@pysym :PyObject_DelItem), Cint, (PyPtr, PyPtr), o, PyObject(k))
     if e == -1
diff --git a/src/fasttuples.jl b/src/get.jl
similarity index 61%
rename from src/fasttuples.jl
rename to src/get.jl
index 0e16aa7f..fd0df6f9 100644
--- a/src/fasttuples.jl
+++ b/src/get.jl
@@ -1,3 +1,23 @@
+#########################################################################
+# Once Julia lets us overload ".", we will use [] to access items, but
+# for now we can define "get".
+
+function get(o::PyObject, returntype::TypeTuple, k, default)
+    r = ccall((@pysym :PyObject_GetItem), PyPtr, (PyPtr,PyPtr), o,PyObject(k))
+    if r == C_NULL
+        pyerr_clear()
+        default
+    else
+        convert(returntype, PyObject(r))
+    end
+end
+
+get(o::PyObject, returntype::TypeTuple, k) =
+    convert(returntype, PyObject(@pycheckn ccall((@pysym :PyObject_GetItem),
+                                 PyPtr, (PyPtr,PyPtr), o, PyObject(k))))
+
+get(o::PyObject, k, default) = get(o, PyAny, k, default)
+get(o::PyObject, k) = get(o, PyAny, k)
 
 function get!(ret::PyObject, o::PyObject, returntype::TypeTuple, k::Integer)
     if pyisinstance(o, @pyglobalobj :PyTuple_Type)
@@ -12,6 +32,8 @@ function get!(ret::PyObject, o::PyObject, returntype::TypeTuple, k::Integer)
     convert(returntype, ret)
 end
 
+get(o::PyObject, returntype::TypeTuple, k::Integer) = get!(PyNULL(), o, returntype, k)
+
 # struct PyTuple_struct
 # refs:
 #   https://github.com/python/cpython/blob/da1734c58d2f97387ccc9676074717d38b044128/Include/object.h#L106-L115

From 3355de4c9c8c23a5c65b3424ae994368154acd2e Mon Sep 17 00:00:00 2001
From: Joel Mason <jobba1@hotmail.com>
Date: Tue, 10 Apr 2018 16:47:11 +1000
Subject: [PATCH 4/6] Add pydecref_, more tests and more `get!` methods

pydecref_ avoids the `o.o = PyPtr_NULL`, which isn't needed in `copy!(dest::PyObject, src::PyPtr)`
---
 benchmarks/seqperf.jl   | 41 +++++++++++++++++++++++++++++
 benchmarks/tupleperf.jl | 37 --------------------------
 src/PyCall.jl           | 10 ++++---
 src/get.jl              | 58 ++++++++++++++++++++++++++++++++---------
 test/runtests.jl        | 52 +++++++++++++++++++++++++++++++++---
 5 files changed, 142 insertions(+), 56 deletions(-)
 create mode 100644 benchmarks/seqperf.jl
 delete mode 100644 benchmarks/tupleperf.jl

diff --git a/benchmarks/seqperf.jl b/benchmarks/seqperf.jl
new file mode 100644
index 00000000..7eb8c7fd
--- /dev/null
+++ b/benchmarks/seqperf.jl
@@ -0,0 +1,41 @@
+using PyCall, BenchmarkTools, DataStructures
+
+results = OrderedDict{String,Any}()
+let
+    np = pyimport("numpy")
+    nprandint = np["random"]["randint"]
+    nprand = np["random"]["rand"]
+    res = PyNULL()
+
+    tuplen = 16
+    tpl = convert(PyObject, (1:tuplen...))
+    lst = convert(PyObject, Any[1:tuplen...])
+    for (name, pycoll) in zip(("tpl", "lst"), (tpl, lst))
+        idx = rand(0:(tuplen-1))
+        results["standard get $name"] = @benchmark get($pycoll, PyObject, PyObject($idx))
+        println("standard get:\n"); display(results["standard get $name"])
+        println("--------------------------------------------------")
+
+        idx = rand(0:(tuplen-1))
+        results["faster get $name"] = @benchmark get($pycoll, PyObject, $idx)
+        println("faster get:\n"); display(results["faster get $name"])
+        println("--------------------------------------------------")
+
+        idx = rand(0:(tuplen-1))
+        results["get! $name"] = @benchmark get!($res, $pycoll, PyObject, $idx)
+        println("get!:\n"); display(results["get! $name"])
+        println("--------------------------------------------------")
+
+        if pycoll == tpl
+            idx = rand(0:(tuplen-1))
+            results["unsafe_gettpl!"] = @benchmark unsafe_gettpl!($res, $tpl, PyObject, $idx)
+            println("unsafe_gettpl!:\n"); display(results["unsafe_gettpl!"])
+            println("--------------------------------------------------")
+        end
+    end
+end
+
+println("")
+println("Mean times")
+println("----------")
+foreach((r)->println(rpad(r[1],20), ": ", mean(r[2])), results)
diff --git a/benchmarks/tupleperf.jl b/benchmarks/tupleperf.jl
deleted file mode 100644
index c46407cf..00000000
--- a/benchmarks/tupleperf.jl
+++ /dev/null
@@ -1,37 +0,0 @@
-using PyCall, BenchmarkTools, DataStructures
-
-results = OrderedDict{String,Any}()
-let
-    np = pyimport("numpy")
-    nprandint = np["random"]["randint"]
-    nprand = np["random"]["rand"]
-    res = PyNULL()
-
-    tuplen = 16
-    tpl = convert(PyObject, (1:tuplen...))
-
-    tplidx = rand(0:(tuplen-1))
-    results["standard get"] = @benchmark get($tpl, PyObject, PyObject($tplidx))
-    println("standard get:\n"); display(results["standard get"])
-    println("--------------------------------------------------")
-
-    tplidx = rand(0:(tuplen-1))
-    results["faster get"] = @benchmark get($tpl, PyObject, $tplidx)
-    println("faster get:\n"); display(results["faster get"])
-    println("--------------------------------------------------")
-
-    tplidx = rand(0:(tuplen-1))
-    results["get!"] = @benchmark get!($res, $tpl, PyObject, $tplidx)
-    println("get!:\n"); display(results["get!"])
-    println("--------------------------------------------------")
-
-    tplidx = rand(0:(tuplen-1))
-    results["unsafe_gettpl!"] = @benchmark unsafe_gettpl!($res, $tpl, PyObject, $tplidx)
-    println("unsafe_gettpl!:\n"); display(results["unsafe_gettpl!"])
-    println("--------------------------------------------------")
-end
-
-println("")
-println("Mean times")
-println("----------")
-foreach((r)->println(rpad(r[1],20), ": ", mean(r[2])), results)
diff --git a/src/PyCall.jl b/src/PyCall.jl
index f2f303e3..d45241d0 100644
--- a/src/PyCall.jl
+++ b/src/PyCall.jl
@@ -98,8 +98,12 @@ 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)
+end
+
 function pydecref(o::PyObject)
-    ccall(@pysym(:Py_DecRef), Cvoid, (PyPtr,), o.o)
+    pydecref_(o.o)
     o.o = PyPtr_NULL
     o
 end
@@ -138,9 +142,9 @@ function Base.copy!(dest::PyObject, src::PyObject)
 end
 
 function Base.copy!(dest::PyObject, src::PyPtr)
-    pydecref(dest)
+    pydecref_(dest.o)
     dest.o = src
-    return pyincref(dest)
+    return pyincref_(dest.o)
 end
 
 pyisinstance(o::PyObject, t::PyObject) =
diff --git a/src/get.jl b/src/get.jl
index fd0df6f9..1eaaa79f 100644
--- a/src/get.jl
+++ b/src/get.jl
@@ -2,8 +2,11 @@
 # Once Julia lets us overload ".", we will use [] to access items, but
 # for now we can define "get".
 
-function get(o::PyObject, returntype::TypeTuple, k, default)
-    r = ccall((@pysym :PyObject_GetItem), PyPtr, (PyPtr,PyPtr), o,PyObject(k))
+###############################
+# get with k<:Any and a default
+###############################
+function get!(ret::PyObject, o::PyObject, returntype::TypeTuple, k, default)
+    r = ccall((@pysym :PyObject_GetItem), PyPtr, (PyPtr,PyPtr), o, PyObject(k))
     if r == C_NULL
         pyerr_clear()
         default
@@ -12,27 +15,58 @@ function get(o::PyObject, returntype::TypeTuple, k, default)
     end
 end
 
-get(o::PyObject, returntype::TypeTuple, k) =
-    convert(returntype, PyObject(@pycheckn ccall((@pysym :PyObject_GetItem),
-                                 PyPtr, (PyPtr,PyPtr), o, PyObject(k))))
+get(o::PyObject, returntype::TypeTuple, k, default) =
+  get!(PyNULL(), o, returntype, k, default)
 
+# returntype defaults to PyAny
+get!(ret::PyObject, o::PyObject, k, default) = get!(ret, o, PyAny, k, default)
 get(o::PyObject, k, default) = get(o, PyAny, k, default)
+
+###############################
+# get with k<:Any
+###############################
+function get!(ret::PyObject, o::PyObject, returntype::TypeTuple, k)
+    pydecref(ret)
+    ret.o = @pycheckn ccall((@pysym :PyObject_GetItem),
+                                 PyPtr, (PyPtr,PyPtr), o, PyObject(k))
+    return convert(returntype, ret)
+end
+
+get(o::PyObject, returntype::TypeTuple, k) = get!(PyNULL(), o, returntype, k)
+
+# returntype defaults to PyAny
+get!(ret::PyObject, o::PyObject, k) = get!(ret, o, PyAny, k)
 get(o::PyObject, k) = get(o, PyAny, k)
 
+###############################
+# get with k<:Integer
+###############################
 function get!(ret::PyObject, o::PyObject, returntype::TypeTuple, k::Integer)
     if pyisinstance(o, @pyglobalobj :PyTuple_Type)
         copy!(ret, @pycheckn ccall(@pysym(:PyTuple_GetItem), PyPtr, (PyPtr, Cint), o, k))
     elseif pyisinstance(o, @pyglobalobj :PyList_Type)
         copy!(ret, @pycheckn ccall(@pysym( :PyList_GetItem), PyPtr, (PyPtr, Cint), o, k))
     else
-        pydecref(ret)
-        ret.o = @pycheckn ccall((@pysym :PyObject_GetItem),
-                                     PyPtr, (PyPtr,PyPtr), o, PyObject(k))
+        return get!(ret, o, returntype, PyObject(k))
     end
-    convert(returntype, ret)
+    return convert(returntype, ret)
 end
 
-get(o::PyObject, returntype::TypeTuple, k::Integer) = get!(PyNULL(), o, returntype, k)
+get(o::PyObject, returntype::TypeTuple, k::Integer) =
+    get!(PyNULL(), o, returntype, k)
+
+# default to PyObject(k) methods for no returntype, and default variants
+get!(ret::PyObject, o::PyObject, returntype::TypeTuple, k::Integer, default) =
+    get!(ret, o, returntype, PyObject(k), default)
+
+get!(ret::PyObject, o::PyObject, k::Integer) = get!(ret, o, PyObject(k))
+
+get!(ret::PyObject, o::PyObject, k::Integer, default) =
+    get!(ret, o, PyObject(k), default)
+
+###############################
+# unsafe_gettpl!
+###############################
 
 # struct PyTuple_struct
 # refs:
@@ -49,8 +83,6 @@ function unsafe_gettpl!(ret::PyObject, o::PyObject, returntype::TypeTuple, k::In
     pytype_ptr = unsafe_load(o.o).ob_type
     # get address of ob_item (just after the end of the struct)
     itemsptr = Base.reinterpret(Ptr{PyPtr}, o.o + sizeof(PyVar_struct))
-    pydecref(ret)
-    ret.o = unsafe_load(itemsptr, k)
-    pyincref_(ret.o)
+    copy!(ret, unsafe_load(itemsptr, k+1)) # unsafe_load is 1-based
     return convert(returntype, ret)
 end
diff --git a/test/runtests.jl b/test/runtests.jl
index 3db84b83..cc56311e 100644
--- a/test/runtests.jl
+++ b/test/runtests.jl
@@ -284,9 +284,8 @@ pymodule_exists(s::AbstractString) = !ispynull(pyimport_e(s))
             @test o3[2,3,4] == 9
         end
     end
-
-    # list operations:
-    let o = PyObject(Any[8,3])
+    @testset "list operations" begin
+        lst = Any[8,3]; o = PyObject(lst)
         @test collect(push!(o, 5)) == [8,3,5]
         @test pop!(o) == 5 && collect(o) == [8,3]
         @test popfirst!(o) == 8 && collect(o) == [3]
@@ -294,6 +293,19 @@ pymodule_exists(s::AbstractString) = !ispynull(pyimport_e(s))
         @test collect(prepend!(o, [5,4,2])) == [5,4,2,9,3]
         @test collect(append!(o, [1,6,8])) == [5,4,2,9,3,1,6,8]
         @test isempty(empty!(o))
+
+        # get! with preallocated return object
+        res = PyNULL()
+        lst = Any[8,3,5]; o = PyObject(lst) # re-init since emptied
+        lpyo = map(PyObject, lst)
+        for i in eachindex(lst)
+            @test get!(res, o, PyObject, i-1) == lpyo[i]
+            @test get!(res, o, i-1) == lst[i] # PyAny default
+        end
+
+        # get! with default if key not found
+        @test get!(res, o, PyObject, 3, 12) == 12
+        @test get!(res, o, 3, 12) == 12
     end
     let o = PyObject(Any[8,3])
         @test collect(append!(o, o)) == [8,3,8,3]
@@ -301,6 +313,40 @@ pymodule_exists(s::AbstractString) = !ispynull(pyimport_e(s))
         @test collect(prepend!(o, o)) == [8,3,8,3,1,8,3,8,3,1]
     end
 
+    @testset "tuple get" begin
+        tpl = ("a","b","c"); o = PyObject(tpl); tpl_os = map(PyObject, tpl)
+
+        # get with PyObject key
+        @test ((get(o, PyObject, PyObject(i-1)) for i in eachindex(tpl))...,) == tpl_os
+        # get with PyObject key and default
+        @test get(o, PyObject, PyObject(3), "z") == PyObject("z")
+
+        # get with Integer key
+        @test ((get(o, PyObject, PyObject(i-1)) for i in eachindex(tpl))...,) == tpl_os
+        # get with Integer key and default
+        @test get(o, PyObject, 3, "z") == PyObject("z")
+
+        # get!
+        res = PyNULL()
+        for i in eachindex(tpl)
+            @test get!(res, o, PyObject, i-1) == tpl_os[i]
+            @test get!(res, o, PyObject, PyObject(i-1)) == tpl_os[i]
+            @test get!(res, o, i-1) == tpl[i]
+            @test get!(res, o, PyObject(i-1)) == tpl[i]
+        end
+
+        # get! with default if key not found
+        @test get!(res, o, PyObject, 3, "z") == PyObject("z")
+        @test get!(res, o, PyObject, PyObject(3), "z") == PyObject("z")
+        @test get!(res, o, 3, "z") == PyObject("z")
+        @test get!(res, o, PyObject(3), "z") == PyObject("z")
+
+        # fast unsafe_gettpl!
+        for i in eachindex(tpl)
+            @test unsafe_gettpl!(res, o, PyObject, i-1) == tpl_os[i]
+        end
+    end
+
     # issue #216:
     @test length(collect(pyimport("itertools")[:combinations]([1,2,3],2))) == 3
 

From 0aa811775ef47b81a37e9ca288b1cc9e5a40f0ce Mon Sep 17 00:00:00 2001
From: Joel Mason <jobba1@hotmail.com>
Date: Tue, 10 Apr 2018 16:55:16 +1000
Subject: [PATCH 5/6] Use pydecref_ for the other copy! method too

---
 src/PyCall.jl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/PyCall.jl b/src/PyCall.jl
index d45241d0..b907b080 100644
--- a/src/PyCall.jl
+++ b/src/PyCall.jl
@@ -136,7 +136,7 @@ function pystealref!(o::PyObject)
 end
 
 function Base.copy!(dest::PyObject, src::PyObject)
-    pydecref(dest)
+    pydecref_(dest.o)
     dest.o = src.o
     return pyincref(dest)
 end

From 770591b25f7fb7089c3cd10c6044359792e20b11 Mon Sep 17 00:00:00 2001
From: Joel Mason <jobba1@hotmail.com>
Date: Wed, 18 Apr 2018 21:37:54 +1000
Subject: [PATCH 6/6] Fix order of inc/decrefs and unify copy! methods

And add a couple of missing decrefs
---
 src/PyCall.jl | 15 +++++++--------
 src/get.jl    | 13 ++++++++++---
 2 files changed, 17 insertions(+), 11 deletions(-)

diff --git a/src/PyCall.jl b/src/PyCall.jl
index b907b080..5225b345 100644
--- a/src/PyCall.jl
+++ b/src/PyCall.jl
@@ -135,16 +135,15 @@ function pystealref!(o::PyObject)
     return optr
 end
 
-function Base.copy!(dest::PyObject, src::PyObject)
-    pydecref_(dest.o)
-    dest.o = src.o
-    return pyincref(dest)
-end
+Base.copy!(dest::PyObject, src::PyObject) = Base.copy!(dest, src.o)
 
 function Base.copy!(dest::PyObject, src::PyPtr)
-    pydecref_(dest.o)
-    dest.o = src
-    return pyincref_(dest.o)
+    if dest.o != src
+        pyincref_(src)
+        pydecref_(dest.o)
+        dest.o = src
+    end
+    return dest
 end
 
 pyisinstance(o::PyObject, t::PyObject) =
diff --git a/src/get.jl b/src/get.jl
index 1eaaa79f..0370faa0 100644
--- a/src/get.jl
+++ b/src/get.jl
@@ -11,7 +11,11 @@ function get!(ret::PyObject, o::PyObject, returntype::TypeTuple, k, default)
         pyerr_clear()
         default
     else
-        convert(returntype, PyObject(r))
+        if r != ret.o
+            pydecref_(ret.o)
+            ret.o = r
+        end
+        convert(returntype, ret)
     end
 end
 
@@ -26,9 +30,12 @@ get(o::PyObject, k, default) = get(o, PyAny, k, default)
 # get with k<:Any
 ###############################
 function get!(ret::PyObject, o::PyObject, returntype::TypeTuple, k)
-    pydecref(ret)
-    ret.o = @pycheckn ccall((@pysym :PyObject_GetItem),
+    r = @pycheckn ccall((@pysym :PyObject_GetItem),
                                  PyPtr, (PyPtr,PyPtr), o, PyObject(k))
+    if r != ret.o
+        pydecref_(ret.o)
+        ret.o = r
+    end
     return convert(returntype, ret)
 end