From d6be74be2e8e65cd2c117eea894586bdcdea6eaf Mon Sep 17 00:00:00 2001 From: jianfengmao Date: Mon, 25 Nov 2024 21:15:24 -0700 Subject: [PATCH 1/4] Fix a bug in Py Obj cleanup & reduce contention --- src/main/java/org/jpy/PyObject.java | 2 +- src/main/java/org/jpy/PyObjectReferences.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/jpy/PyObject.java b/src/main/java/org/jpy/PyObject.java index 44f0740..f9a063d 100644 --- a/src/main/java/org/jpy/PyObject.java +++ b/src/main/java/org/jpy/PyObject.java @@ -42,7 +42,7 @@ public class PyObject implements AutoCloseable { private static final AtomicReference CLEANUP_THREAD = new AtomicReference<>(); - private static final boolean CLEANUP_ON_INIT = Boolean.parseBoolean(System.getProperty("PyObject.cleanup_on_init", "true")); + private static final boolean CLEANUP_ON_INIT = Boolean.parseBoolean(System.getProperty("PyObject.cleanup_on_init", "false")); private static final boolean CLEANUP_ON_THREAD = Boolean.parseBoolean(System.getProperty("PyObject.cleanup_on_thread", "true")); diff --git a/src/main/java/org/jpy/PyObjectReferences.java b/src/main/java/org/jpy/PyObjectReferences.java index 4afea8a..f0d947a 100644 --- a/src/main/java/org/jpy/PyObjectReferences.java +++ b/src/main/java/org/jpy/PyObjectReferences.java @@ -80,7 +80,7 @@ public int threadSafeCleanup() { return threadSafeCleanup(buffer); } - private int threadSafeCleanup(long[] buffer) { + private synchronized int threadSafeCleanup(long[] buffer) { return PyLib.ensureGil(() -> { int index = 0; while (index < buffer.length) { From 159e78ecf2193ef915eb721427814175cdf0455e Mon Sep 17 00:00:00 2001 From: jianfengmao Date: Wed, 27 Nov 2024 14:03:26 -0700 Subject: [PATCH 2/4] Remove unneeded code --- src/main/java/org/jpy/PyObject.java | 5 ----- src/main/java/org/jpy/PyObjectReferences.java | 6 +----- 2 files changed, 1 insertion(+), 10 deletions(-) diff --git a/src/main/java/org/jpy/PyObject.java b/src/main/java/org/jpy/PyObject.java index f9a063d..13f36a1 100644 --- a/src/main/java/org/jpy/PyObject.java +++ b/src/main/java/org/jpy/PyObject.java @@ -42,8 +42,6 @@ public class PyObject implements AutoCloseable { private static final AtomicReference CLEANUP_THREAD = new AtomicReference<>(); - private static final boolean CLEANUP_ON_INIT = Boolean.parseBoolean(System.getProperty("PyObject.cleanup_on_init", "false")); - private static final boolean CLEANUP_ON_THREAD = Boolean.parseBoolean(System.getProperty("PyObject.cleanup_on_thread", "true")); private static void startCleanupThread() { @@ -71,9 +69,6 @@ public static int cleanup() { PyObject(long pointer, boolean fromJNI) { state = new PyObjectState(pointer); if (fromJNI) { - if (CLEANUP_ON_INIT) { - REFERENCES.threadSafeCleanup(); // only performs *one* cleanup - } if (CLEANUP_ON_THREAD) { // ensures that we've only started after python has been started, and we know there is something to cleanup startCleanupThread(); diff --git a/src/main/java/org/jpy/PyObjectReferences.java b/src/main/java/org/jpy/PyObjectReferences.java index f0d947a..c5cd2b8 100644 --- a/src/main/java/org/jpy/PyObjectReferences.java +++ b/src/main/java/org/jpy/PyObjectReferences.java @@ -76,11 +76,7 @@ private Reference asRef(PyObject pyObject) { /** * This should *only* be invoked through the proxy, or when we *know* we have the GIL. */ - public int threadSafeCleanup() { - return threadSafeCleanup(buffer); - } - - private synchronized int threadSafeCleanup(long[] buffer) { + public synchronized int threadSafeCleanup() { return PyLib.ensureGil(() -> { int index = 0; while (index < buffer.length) { From 5de1fe28f740455dacaf68b5e3f124e29061f6ce Mon Sep 17 00:00:00 2001 From: jianfengmao Date: Thu, 5 Dec 2024 10:06:42 -0700 Subject: [PATCH 3/4] enable the java-to-python tests --- setup.py | 8 +++++--- src/test/java/org/jpy/PyObjectTest.java | 11 ++++++----- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/setup.py b/setup.py index d17385a..f5d9f5c 100644 --- a/setup.py +++ b/setup.py @@ -222,7 +222,10 @@ def test_python_java_classes(): def test_maven(): jpy_config = os.path.join(_build_dir(), 'jpyconfig.properties') - mvn_args = '-DargLine=-Xmx512m -Djpy.config=' + jpy_config + ' -Djpy.debug=true' + mvn_args = ('-DargLine=-Xmx512m -Djpy.config=' + jpy_config + ' -Djpy.debug=true' + + # disable the PyObject cleanup thread because the asynchronous nature of it makes + # stopPython/startPython flakey. + ' -DPyObject.cleanup_on_thread=false') log.info("Executing Maven goal 'test' with arg line " + repr(mvn_args)) code = subprocess.call(['mvn', 'test', mvn_args], shell=platform.system() == 'Windows') return code == 0 @@ -282,8 +285,7 @@ def test_java(self): suite.addTest(test_python_with_java_runtime) suite.addTest(test_python_with_java_classes) - # comment out because the asynchronous nature of the PyObject GC in Java makes stopPython/startPython flakey. - # suite.addTest(test_java) + suite.addTest(test_java) return suite diff --git a/src/test/java/org/jpy/PyObjectTest.java b/src/test/java/org/jpy/PyObjectTest.java index 2253fae..7f07090 100644 --- a/src/test/java/org/jpy/PyObjectTest.java +++ b/src/test/java/org/jpy/PyObjectTest.java @@ -204,16 +204,17 @@ public void testCall() throws Exception { public void testGetSetAttributes() throws Exception { // Python equivalent: // - // >>> import imp - // >>> myobj = imp.new_module('myobj') + // >>> import impportlib.util + // >>> mod_spec = importlib.util.spec_from_loader('myobj', loader=None) + // >>> myobj = importlib.util.module_from_spec(mod_spec) // >>> myobj.a = 'Tut tut!' // >>> myobj.a // 'Tut tut!' // try ( - final PyModule imp = PyModule.importModule("imp"); - final PyObject myobj = imp.call("new_module", "myobj")) { - // Call imp.new_module('') module + final PyModule imp = PyModule.importModule("importlib.util"); + final PyObject moduleSpec = imp.call("spec_from_loader", "myobj", null); + final PyObject myobj = imp.call("module_from_spec", moduleSpec)) { myobj.setAttribute("a", "Tut tut!"); Assert.assertEquals("Tut tut!", myobj.getAttribute("a", String.class)); try (final PyObject a = myobj.getAttribute("a")) { From 4f7b61c6a56f3e91f09c3e26ab4de2b43c0117af Mon Sep 17 00:00:00 2001 From: jianfengmao Date: Thu, 5 Dec 2024 13:46:27 -0700 Subject: [PATCH 4/4] Revert "enable the java-to-python tests" This reverts commit 5de1fe28f740455dacaf68b5e3f124e29061f6ce. --- setup.py | 8 +++----- src/test/java/org/jpy/PyObjectTest.java | 11 +++++------ 2 files changed, 8 insertions(+), 11 deletions(-) diff --git a/setup.py b/setup.py index f5d9f5c..d17385a 100644 --- a/setup.py +++ b/setup.py @@ -222,10 +222,7 @@ def test_python_java_classes(): def test_maven(): jpy_config = os.path.join(_build_dir(), 'jpyconfig.properties') - mvn_args = ('-DargLine=-Xmx512m -Djpy.config=' + jpy_config + ' -Djpy.debug=true' + - # disable the PyObject cleanup thread because the asynchronous nature of it makes - # stopPython/startPython flakey. - ' -DPyObject.cleanup_on_thread=false') + mvn_args = '-DargLine=-Xmx512m -Djpy.config=' + jpy_config + ' -Djpy.debug=true' log.info("Executing Maven goal 'test' with arg line " + repr(mvn_args)) code = subprocess.call(['mvn', 'test', mvn_args], shell=platform.system() == 'Windows') return code == 0 @@ -285,7 +282,8 @@ def test_java(self): suite.addTest(test_python_with_java_runtime) suite.addTest(test_python_with_java_classes) - suite.addTest(test_java) + # comment out because the asynchronous nature of the PyObject GC in Java makes stopPython/startPython flakey. + # suite.addTest(test_java) return suite diff --git a/src/test/java/org/jpy/PyObjectTest.java b/src/test/java/org/jpy/PyObjectTest.java index 7f07090..2253fae 100644 --- a/src/test/java/org/jpy/PyObjectTest.java +++ b/src/test/java/org/jpy/PyObjectTest.java @@ -204,17 +204,16 @@ public void testCall() throws Exception { public void testGetSetAttributes() throws Exception { // Python equivalent: // - // >>> import impportlib.util - // >>> mod_spec = importlib.util.spec_from_loader('myobj', loader=None) - // >>> myobj = importlib.util.module_from_spec(mod_spec) + // >>> import imp + // >>> myobj = imp.new_module('myobj') // >>> myobj.a = 'Tut tut!' // >>> myobj.a // 'Tut tut!' // try ( - final PyModule imp = PyModule.importModule("importlib.util"); - final PyObject moduleSpec = imp.call("spec_from_loader", "myobj", null); - final PyObject myobj = imp.call("module_from_spec", moduleSpec)) { + final PyModule imp = PyModule.importModule("imp"); + final PyObject myobj = imp.call("new_module", "myobj")) { + // Call imp.new_module('') module myobj.setAttribute("a", "Tut tut!"); Assert.assertEquals("Tut tut!", myobj.getAttribute("a", String.class)); try (final PyObject a = myobj.getAttribute("a")) {