From 6bcdde9d406ffb0666a8dffabb8ddc7cee3cce8e Mon Sep 17 00:00:00 2001 From: Howard Butler Date: Thu, 4 Jan 2024 16:53:16 -0600 Subject: [PATCH 1/9] executeStream already does GIL release, we don't need to do it. pybind11 does GIL acquire with every method call so we don't need to be doing that either --- pdal/libpdalpython.cpp | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-) diff --git a/pdal/libpdalpython.cpp b/pdal/libpdalpython.cpp index a7b5147a..437dce63 100644 --- a/pdal/libpdalpython.cpp +++ b/pdal/libpdalpython.cpp @@ -30,7 +30,6 @@ namespace pdal { }; std::vector getDrivers() { - py::gil_scoped_acquire acquire; std::vector drivers; pdal::StageFactory f(false); @@ -59,7 +58,6 @@ namespace pdal { }; py::object getOptions() { - py::gil_scoped_acquire acquire; py::object json = py::module_::import("json"); py::dict stageOptions; @@ -94,7 +92,6 @@ namespace pdal { }; std::vector getDimensions() { - py::gil_scoped_acquire acquire; py::object np = py::module_::import("numpy"); py::object dtype = np.attr("dtype"); std::vector dims; @@ -112,13 +109,11 @@ namespace pdal { std::string getReaderDriver(std::filesystem::path const& p) { - py::gil_scoped_acquire acquire; return StageFactory::inferReaderDriver(p.string()); } std::string getWriterDriver(std::filesystem::path const& p) { - py::gil_scoped_acquire acquire; return StageFactory::inferWriterDriver(p.string()); } @@ -142,7 +137,6 @@ namespace pdal { } py::object getMetadata() { - py::gil_scoped_acquire acquire; py::object json = py::module_::import("json"); std::stringstream strm; @@ -175,12 +169,7 @@ namespace pdal { } point_count_t executeStream(point_count_t streamLimit) { - point_count_t response(0); - { - py::gil_scoped_release release; - response = getExecutor()->executeStream(streamLimit); - } - return response; + return getExecutor()->executeStream(streamLimit); } std::unique_ptr iterator(int chunk_size, int prefetch) { @@ -190,7 +179,6 @@ namespace pdal { } void setInputs(std::vector ndarrays) { - py::gil_scoped_acquire acquire; _inputs.clear(); for (const auto& ndarray: ndarrays) { PyArrayObject* ndarray_ptr = (PyArrayObject*)ndarray.ptr(); @@ -209,7 +197,6 @@ namespace pdal { std::string getSrsWKT2() { return getExecutor()->getSrsWKT2(); } py::object getQuickInfo() { - py::gil_scoped_acquire acquire; py::object json = py::module_::import("json"); std::string response; @@ -275,7 +262,6 @@ namespace pdal { void delExecutor() { _executor.reset(); } PipelineExecutor* getExecutor() { - py::gil_scoped_acquire acquire; if (!_executor) _executor.reset(new PipelineExecutor(getJson(), _inputs, _loglevel)); return _executor.get(); From 86ee4e27bf1f0981fd760446b7503f0ab3bfbcfa Mon Sep 17 00:00:00 2001 From: Howard Butler Date: Thu, 4 Jan 2024 16:58:24 -0600 Subject: [PATCH 2/9] wip --- pdal/libpdalpython.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/pdal/libpdalpython.cpp b/pdal/libpdalpython.cpp index 437dce63..500e46d5 100644 --- a/pdal/libpdalpython.cpp +++ b/pdal/libpdalpython.cpp @@ -169,7 +169,12 @@ namespace pdal { } point_count_t executeStream(point_count_t streamLimit) { - return getExecutor()->executeStream(streamLimit); + point_count_t response(0); + { + py::gil_scoped_release release; + response = getExecutor()->executeStream(streamLimit); + } + return response; } std::unique_ptr iterator(int chunk_size, int prefetch) { From d37647b27acaed15752db3730ce7330cd2134938 Mon Sep 17 00:00:00 2001 From: Howard Butler Date: Thu, 4 Jan 2024 17:02:34 -0600 Subject: [PATCH 3/9] put back acq --- pdal/libpdalpython.cpp | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/pdal/libpdalpython.cpp b/pdal/libpdalpython.cpp index 500e46d5..a7b5147a 100644 --- a/pdal/libpdalpython.cpp +++ b/pdal/libpdalpython.cpp @@ -30,6 +30,7 @@ namespace pdal { }; std::vector getDrivers() { + py::gil_scoped_acquire acquire; std::vector drivers; pdal::StageFactory f(false); @@ -58,6 +59,7 @@ namespace pdal { }; py::object getOptions() { + py::gil_scoped_acquire acquire; py::object json = py::module_::import("json"); py::dict stageOptions; @@ -92,6 +94,7 @@ namespace pdal { }; std::vector getDimensions() { + py::gil_scoped_acquire acquire; py::object np = py::module_::import("numpy"); py::object dtype = np.attr("dtype"); std::vector dims; @@ -109,11 +112,13 @@ namespace pdal { std::string getReaderDriver(std::filesystem::path const& p) { + py::gil_scoped_acquire acquire; return StageFactory::inferReaderDriver(p.string()); } std::string getWriterDriver(std::filesystem::path const& p) { + py::gil_scoped_acquire acquire; return StageFactory::inferWriterDriver(p.string()); } @@ -137,6 +142,7 @@ namespace pdal { } py::object getMetadata() { + py::gil_scoped_acquire acquire; py::object json = py::module_::import("json"); std::stringstream strm; @@ -184,6 +190,7 @@ namespace pdal { } void setInputs(std::vector ndarrays) { + py::gil_scoped_acquire acquire; _inputs.clear(); for (const auto& ndarray: ndarrays) { PyArrayObject* ndarray_ptr = (PyArrayObject*)ndarray.ptr(); @@ -202,6 +209,7 @@ namespace pdal { std::string getSrsWKT2() { return getExecutor()->getSrsWKT2(); } py::object getQuickInfo() { + py::gil_scoped_acquire acquire; py::object json = py::module_::import("json"); std::string response; @@ -267,6 +275,7 @@ namespace pdal { void delExecutor() { _executor.reset(); } PipelineExecutor* getExecutor() { + py::gil_scoped_acquire acquire; if (!_executor) _executor.reset(new PipelineExecutor(getJson(), _inputs, _loglevel)); return _executor.get(); From 55374d79a59317ddefde26e55fadcdd2272c4dd8 Mon Sep 17 00:00:00 2001 From: Howard Butler Date: Thu, 4 Jan 2024 17:05:58 -0600 Subject: [PATCH 4/9] wip --- pdal/libpdalpython.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/pdal/libpdalpython.cpp b/pdal/libpdalpython.cpp index a7b5147a..cd12c4e1 100644 --- a/pdal/libpdalpython.cpp +++ b/pdal/libpdalpython.cpp @@ -142,7 +142,6 @@ namespace pdal { } py::object getMetadata() { - py::gil_scoped_acquire acquire; py::object json = py::module_::import("json"); std::stringstream strm; From 5f30171aad4ee510599aea2c823ee5b8028e2fd3 Mon Sep 17 00:00:00 2001 From: Howard Butler Date: Thu, 4 Jan 2024 17:17:32 -0600 Subject: [PATCH 5/9] wip2 --- pdal/libpdalpython.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/pdal/libpdalpython.cpp b/pdal/libpdalpython.cpp index cd12c4e1..4bcb8001 100644 --- a/pdal/libpdalpython.cpp +++ b/pdal/libpdalpython.cpp @@ -189,7 +189,6 @@ namespace pdal { } void setInputs(std::vector ndarrays) { - py::gil_scoped_acquire acquire; _inputs.clear(); for (const auto& ndarray: ndarrays) { PyArrayObject* ndarray_ptr = (PyArrayObject*)ndarray.ptr(); From e28d84c024e4868c26e0f500a0234593f49e87ca Mon Sep 17 00:00:00 2001 From: Howard Butler Date: Thu, 4 Jan 2024 17:20:26 -0600 Subject: [PATCH 6/9] wip3 --- pdal/libpdalpython.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/pdal/libpdalpython.cpp b/pdal/libpdalpython.cpp index 4bcb8001..0f70f8d5 100644 --- a/pdal/libpdalpython.cpp +++ b/pdal/libpdalpython.cpp @@ -207,7 +207,6 @@ namespace pdal { std::string getSrsWKT2() { return getExecutor()->getSrsWKT2(); } py::object getQuickInfo() { - py::gil_scoped_acquire acquire; py::object json = py::module_::import("json"); std::string response; From caf64fffb81eb19932bba65d0c8ce85af9a9d58e Mon Sep 17 00:00:00 2001 From: Howard Butler Date: Thu, 4 Jan 2024 17:26:54 -0600 Subject: [PATCH 7/9] wip4 --- pdal/libpdalpython.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/pdal/libpdalpython.cpp b/pdal/libpdalpython.cpp index 0f70f8d5..094f27ec 100644 --- a/pdal/libpdalpython.cpp +++ b/pdal/libpdalpython.cpp @@ -272,7 +272,6 @@ namespace pdal { void delExecutor() { _executor.reset(); } PipelineExecutor* getExecutor() { - py::gil_scoped_acquire acquire; if (!_executor) _executor.reset(new PipelineExecutor(getJson(), _inputs, _loglevel)); return _executor.get(); From 60bd6bc70cabb2471ebebc87a2186a6b5ed01c66 Mon Sep 17 00:00:00 2001 From: Howard Butler Date: Thu, 4 Jan 2024 17:29:55 -0600 Subject: [PATCH 8/9] wip4 --- pdal/libpdalpython.cpp | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/pdal/libpdalpython.cpp b/pdal/libpdalpython.cpp index 094f27ec..60171cbb 100644 --- a/pdal/libpdalpython.cpp +++ b/pdal/libpdalpython.cpp @@ -30,7 +30,6 @@ namespace pdal { }; std::vector getDrivers() { - py::gil_scoped_acquire acquire; std::vector drivers; pdal::StageFactory f(false); @@ -59,7 +58,6 @@ namespace pdal { }; py::object getOptions() { - py::gil_scoped_acquire acquire; py::object json = py::module_::import("json"); py::dict stageOptions; @@ -94,7 +92,6 @@ namespace pdal { }; std::vector getDimensions() { - py::gil_scoped_acquire acquire; py::object np = py::module_::import("numpy"); py::object dtype = np.attr("dtype"); std::vector dims; @@ -112,13 +109,11 @@ namespace pdal { std::string getReaderDriver(std::filesystem::path const& p) { - py::gil_scoped_acquire acquire; return StageFactory::inferReaderDriver(p.string()); } std::string getWriterDriver(std::filesystem::path const& p) { - py::gil_scoped_acquire acquire; return StageFactory::inferWriterDriver(p.string()); } @@ -272,6 +267,7 @@ namespace pdal { void delExecutor() { _executor.reset(); } PipelineExecutor* getExecutor() { + py::gil_scoped_acquire acquire; if (!_executor) _executor.reset(new PipelineExecutor(getJson(), _inputs, _loglevel)); return _executor.get(); From ad69d185fb92c85152a9d2ea75fa059fe69ccc07 Mon Sep 17 00:00:00 2001 From: Howard Butler Date: Fri, 5 Jan 2024 07:14:51 -0600 Subject: [PATCH 9/9] comment why we need gil_scope_acquire for getExecutor --- pdal/libpdalpython.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pdal/libpdalpython.cpp b/pdal/libpdalpython.cpp index 60171cbb..7bd526a0 100644 --- a/pdal/libpdalpython.cpp +++ b/pdal/libpdalpython.cpp @@ -267,6 +267,10 @@ namespace pdal { void delExecutor() { _executor.reset(); } PipelineExecutor* getExecutor() { + // We need to acquire the GIL before we create the executor + // because this method does Python init stuff but pybind11 doesn't + // automatically encapsulate it with a gil_scoped_acquire like it + // does for all of the other methods it knows about py::gil_scoped_acquire acquire; if (!_executor) _executor.reset(new PipelineExecutor(getJson(), _inputs, _loglevel));