Skip to content

Commit

Permalink
Python gc in python thread (#54)
Browse files Browse the repository at this point in the history 
* some simplification/speed improvements

* This aint workin' yet

* all tests pass with cooperative reference cleanup

* editing comments

* simplification after reading more of concurrent-deque api

* This aint workin' yet

* all tests pass with cooperative reference cleanup

* editing comments

* simplification after reading more of concurrent-deque api

* updated tech.resource
  • Loading branch information
@cnuernber
cnuernber committed Jan 21, 2020
1 parent c17c7a0 commit 3ee5f86
Show file tree
Hide file tree
Showing 6 changed files with 150 additions and 86 deletions.
2 changes: 1 addition & 1 deletion project.clj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
:url "https://www.eclipse.org/legal/epl-2.0/"}
:dependencies [[org.clojure/clojure "1.10.1"]
[camel-snake-kebab "0.4.0"]
[techascent/tech.datatype "4.69"]
[techascent/tech.datatype "4.70"]
[org.clojure/data.json "0.2.7"]]
:repl-options {:init-ns user}
:java-source-paths ["java"])
12 changes: 6 additions & 6 deletions src/libpython_clj/python/bridge.clj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,24 +40,25 @@
[expose-bridge-to-python!
pybridge->bridge
create-bridge-from-att-map]]
[libpython-clj.python.gc :as pygc]
[clojure.stacktrace :as st]
[tech.jna :as jna]
[tech.v2.tensor :as dtt]
[tech.v2.datatype.casting :as casting]
[tech.v2.datatype.functional :as dtype-fn]
[tech.v2.datatype :as dtype]
[tech.resource :as resource]
[clojure.set :as c-set]
[clojure.tools.logging :as log])
(:import [java.util Map RandomAccess List Map$Entry Iterator UUID]
[java.util.concurrent ConcurrentHashMap]
[java.util.concurrent ConcurrentHashMap ConcurrentLinkedQueue]
[clojure.lang IFn Symbol Keyword Seqable
Fn MapEntry Range LongRange]
[tech.v2.datatype ObjectReader ObjectWriter ObjectMutable
ObjectIter MutableRemove]
[tech.v2.datatype.typed_buffer TypedBuffer]
[tech.v2.tensor.protocols PTensor]
[com.sun.jna Pointer]
[tech.resource GCReference]
[java.io Writer]
[libpython_clj.jna JVMBridge
CFunction$KeyWordFunction
Expand Down Expand Up @@ -539,8 +540,7 @@
long-addr (get-attr ctypes "data")
hash-ary {:ctypes-map ctypes}
ptr-val (-> (Pointer. long-addr)
(resource/track #(get hash-ary :ctypes-map)
pyobj/*pyobject-tracking-flags*))]
(pygc/track #(get hash-ary :ctypes-map)))]
{:ptr ptr-val
:datatype np-dtype
:shape shape
Expand Down Expand Up @@ -782,7 +782,7 @@
[& body]
`(delay
(with-gil
(with-bindings {#'pyobj/*pyobject-tracking-flags* [:gc]}
(with-bindings {#'pygc/*stack-gc-context* nil}
~@body))))


Expand Down Expand Up @@ -1094,7 +1094,7 @@
initial-buffer
(->py-tuple shape)
(->py-tuple strides))]
(resource/track retval #(get buffer-desc :ptr) pyobj/*pyobject-tracking-flags*))))
(pygc/track retval #(get buffer-desc :ptr)))))


(extend-type Object
Expand Down
67 changes: 67 additions & 0 deletions src/libpython_clj/python/gc.clj
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
(ns libpython-clj.python.gc
"Binding of various sort of gc semantics optimized specifically for
libpython-clj. For general bindings, see tech.resource"
(:import [java.util.concurrent ConcurrentHashMap ConcurrentLinkedDeque]
[java.lang.ref ReferenceQueue]
[tech.resource GCReference]))


(set! *warn-on-reflection* true)


(defonce ^:dynamic *stack-gc-context* nil)
(defn stack-context
^ConcurrentLinkedDeque []
*stack-gc-context*)


(defonce reference-queue-var (ReferenceQueue.))
(defn reference-queue
^ReferenceQueue []
reference-queue-var)


(defonce ptr-set-var (ConcurrentHashMap/newKeySet))
(defn ptr-set
^java.util.Set []
ptr-set-var)


(defn track
[item dispose-fn]
(let [ptr-val (GCReference. item (reference-queue) (fn [ptr-val]
(.remove (ptr-set) ptr-val)
(dispose-fn)))
^ConcurrentLinkedDeque stack-context (stack-context)]
;;We have to keep track of the pointer. If we do not the pointer gets gc'd then
;;it will not be put on the reference queue when the object itself is gc'd.
;;Nice little gotcha there.
(if stack-context
(.add stack-context ptr-val)
;;Ensure we don't lose track of the weak reference. If it gets cleaned up
;;the gc system will fail.
(.add (ptr-set) ptr-val))
item))


(defn clear-reference-queue
[]
(when-let [next-ref (.poll (reference-queue))]
(.run ^Runnable next-ref)
(recur)))


(defn clear-stack-context
[]
(when-let [next-ref (.pollLast (stack-context))]
(.run ^Runnable next-ref)
(recur)))


(defmacro with-stack-context
[& body]
`(with-bindings {#'*stack-gc-context* (ConcurrentLinkedDeque.)}
(try
~@body
(finally
(clear-stack-context)))))
5 changes: 3 additions & 2 deletions src/libpython_clj/python/interpreter.clj
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
(ns libpython-clj.python.interpreter
(:require [libpython-clj.jna :as libpy ]
[libpython-clj.jna.base :as libpy-base]
[tech.resource :as resource]
[libpython-clj.python.gc :as pygc]
[libpython-clj.python.logging
:refer [log-error log-warn log-info]]
[tech.jna :as jna]
Expand Down Expand Up @@ -351,6 +351,7 @@ print(json.dumps(
(try
~@body
(finally
(pygc/clear-reference-queue)
(when new-binding?#
(release-gil! interp#)))))))))

Expand Down Expand Up @@ -437,7 +438,7 @@ print(json.dumps(
(recur library-names))))
;;Set program name
(when-let [program-name (or program-name *program-name* "")]
(resource/stack-resource-context
(pygc/with-stack-context
(libpy/PySys_SetArgv 0 (-> program-name
(jna/string->wide-ptr)))))
(let [type-symbols (libpy/lookup-type-symbols)
Expand Down
146 changes: 73 additions & 73 deletions src/libpython_clj/python/object.clj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,13 @@
:as py-proto]
[libpython-clj.jna.base :as libpy-base]
[libpython-clj.jna :as libpy]
[libpython-clj.python.gc :as pygc]
[clojure.stacktrace :as st]
[tech.jna :as jna]
[tech.resource :as resource]
[tech.v2.datatype :as dtype]
[tech.v2.datatype.protocols :as dtype-proto]
[tech.v2.datatype.casting :as casting]
[tech.resource :as resource]
[tech.v2.tensor]
[clojure.tools.logging :as log])
(:import [com.sun.jna Pointer CallbackReference]
Expand Down Expand Up @@ -96,13 +97,10 @@
(py-proto/->jvm item options)))


(def ^:dynamic *object-reference-logging* false)

(def object-reference-logging nil)

(def ^:dynamic *object-reference-tracker* nil)


(def ^:dynamic *pyobject-tracking-flags* [:gc])
(def object-reference-tracker nil)


(defn incref
Expand Down Expand Up @@ -130,41 +128,44 @@
(if (and pyobj
(not= (Pointer/nativeValue (libpy/as-pyobj pyobj))
(Pointer/nativeValue (libpy/as-pyobj (libpy/Py_None)))))
(let [interpreter (ensure-bound-interpreter)
pyobj-value (Pointer/nativeValue (libpy/as-pyobj pyobj))
py-type-name (name (python-type pyobj))]
(when *object-reference-logging*
(let [obj-data (PyObject. (Pointer. pyobj-value))]
(println (format "tracking object - 0x%x:%4d:%s"
pyobj-value
(.ob_refcnt obj-data)
py-type-name))))
(when *object-reference-tracker*
(swap! *object-reference-tracker*
update pyobj-value #(inc (or % 0))))
;;We ask the garbage collector to track the python object and notify
;;us when it is released. We then decref on that event.
(resource/track pyobj
#(with-gil
(try
(let [refcount (refcount pyobj)
obj-data (PyObject. (Pointer. pyobj-value))]
(if (< refcount 1)
(log/errorf "Fatal error -- releasing object - 0x%x:%4d:%s
(do
(ensure-bound-interpreter)
(let [pyobj-value (Pointer/nativeValue (libpy/as-pyobj pyobj))
py-type-name (name (python-type pyobj))]
(when object-reference-logging
(let [obj-data (PyObject. (Pointer. pyobj-value))]
(println (format "tracking object - 0x%x:%4d:%s"
pyobj-value
(.ob_refcnt obj-data)
py-type-name))))
(when object-reference-tracker
(swap! object-reference-tracker
update pyobj-value #(inc (or % 0))))
;;We ask the garbage collector to track the python object and notify
;;us when it is released. We then decref on that event.
(pygc/track pyobj
;;No longer with-gil. Because cleanup is cooperative, the gil is
;;guaranteed to be captured here already.
#(try
;;Intentionally overshadow pyobj. We cannot access it here.
(let [pyobj (Pointer. pyobj-value)
refcount (refcount pyobj)
obj-data (PyObject. pyobj)]
(if (< refcount 1)
(log/errorf "Fatal error -- releasing object - 0x%x:%4d:%s
Object's refcount is bad. Crash is imminent" pyobj-value refcount py-type-name)
(when *object-reference-logging*
(println (format "releasing object - 0x%x:%4d:%s"
pyobj-value
(.ob_refcnt obj-data)
py-type-name))))
(when *object-reference-tracker*
(swap! *object-reference-tracker*
update pyobj-value (fn [arg]
(dec (or arg 0))))))
(libpy/Py_DecRef (Pointer. pyobj-value))
(catch Throwable e
(log/error e "Exception while releasing object"))))
*pyobject-tracking-flags*))
(when object-reference-logging
(println (format "releasing object - 0x%x:%4d:%s"
pyobj-value
(.ob_refcnt obj-data)
py-type-name))))
(when object-reference-tracker
(swap! object-reference-tracker
update pyobj-value (fn [arg]
(dec (or arg 0))))))
(libpy/Py_DecRef (Pointer. pyobj-value))
(catch Throwable e
(log/error e "Exception while releasing object"))))))
(do
;;Special handling for PyNone types
(libpy/Py_DecRef pyobj)
Expand All @@ -173,9 +174,8 @@ Object's refcount is bad. Crash is imminent" pyobj-value refcount py-type-name)

(defmacro stack-resource-context
[& body]
`(with-bindings {#'*pyobject-tracking-flags* [:stack :gc]}
(resource/stack-resource-context
~@body)))
`(pygc/with-stack-context
~@body))


(defn incref-wrap-pyobject
Expand Down Expand Up @@ -391,37 +391,37 @@ Object's refcount is bad. Crash is imminent" pyobj-value refcount py-type-name)
doc
function]
:as method-data}]
(resource/stack-resource-context
(when-not (cfunc-instance? function)
(throw (Exception.
(format "Callbacks must implement one of the CFunction interfaces:
;;Here we really do need a resource stack context
(when-not (cfunc-instance? function)
(throw (Exception.
(format "Callbacks must implement one of the CFunction interfaces:
%s" (type function)))))
(let [meth-flags (long (cond
(instance? CFunction$NoArgFunction function)
@libpy/METH_NOARGS

(instance? CFunction$TupleFunction function)
@libpy/METH_VARARGS

(instance? CFunction$KeyWordFunction function)
(bit-or @libpy/METH_KEYWORDS @libpy/METH_VARARGS)
:else
(throw (ex-info (format "Failed due to type: %s"
(type function))
{}))))
name-ptr (jna/string->ptr name)
doc-ptr (jna/string->ptr doc)]
(set! (.ml_name method-def) name-ptr)
(set! (.ml_meth method-def) (CallbackReference/getFunctionPointer function))
(set! (.ml_flags method-def) (int meth-flags))
(set! (.ml_doc method-def) doc-ptr)
(.write method-def)
(pyinterp/conj-forever! (assoc method-data
:name-ptr name-ptr
:doc-ptr doc-ptr
:callback-object function
:method-definition method-def))
method-def)))
(let [meth-flags (long (cond
(instance? CFunction$NoArgFunction function)
@libpy/METH_NOARGS

(instance? CFunction$TupleFunction function)
@libpy/METH_VARARGS

(instance? CFunction$KeyWordFunction function)
(bit-or @libpy/METH_KEYWORDS @libpy/METH_VARARGS)
:else
(throw (ex-info (format "Failed due to type: %s"
(type function))
{}))))
name-ptr (jna/string->ptr-untracked name)
doc-ptr (jna/string->ptr-untracked doc)]
(set! (.ml_name method-def) name-ptr)
(set! (.ml_meth method-def) (CallbackReference/getFunctionPointer function))
(set! (.ml_flags method-def) (int meth-flags))
(set! (.ml_doc method-def) doc-ptr)
(.write method-def)
(pyinterp/conj-forever! (assoc method-data
:name-ptr name-ptr
:doc-ptr doc-ptr
:callback-object function
:method-definition method-def))
method-def))


(defn method-def-data->method-def
Expand Down
4 changes: 0 additions & 4 deletions test/libpython_clj/stress_test.clj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,6 @@ def getmultidata():
(gd-fn)))


;;If you want to see how the sausage is made...
(alter-var-root #'libpython-clj.python.object/*object-reference-logging*
(constantly false))

;;Ensure that failure to open resource context before tracking for stack
;;related things causes immediate failure. Unless you feel like being pedantic,
;;this isn't necessary. The python library automatically switches to normal gc-only
Expand Down

0 comments on commit 3ee5f86

Please sign in to comment.