From cd54957c1f9b29001b714eb05b718e32b9cbc9a6 Mon Sep 17 00:00:00 2001 From: Gigon Bae Date: Mon, 30 Oct 2023 13:29:18 -0700 Subject: [PATCH] Update packages (pybind11 and catch2) and do not use nvidia-docker command (#618) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ### Update Catch2 to v3.4.0 Without upgrading Catch2, The following error occurs when building on Ubuntu 22.04 due to glibc: ``` cucim/build-debug/_deps/deps-catch2-src/single_include/catch2/catch.hpp:10830:58: error: call to non-‘constexpr’ function ‘long int sysconf(int)’ 10830 | static constexpr std::size_t sigStackSize = 32768 >= MINSIGSTKSZ ? 32768 : MINSIGSTKSZ; ``` ### Update pybind11 to v2.11.1 Even with the latest version of pybind11, we still have an issue with `pybind11::array_t` when cuCIM is used in multithread without importing numpy in the main thread. See https://github.com/pybind/pybind11/pull/4877 Will need to wait for the next release of pybind11. ### Use runtime option instead of using nvidia-docker command nvidia-docker binary is not available if user doesn't install nvidia-docker2 package. This change uses runtime option instead of using nvidia-docker command. ### Apply pybind11 patch to avoid deadlock (until new release is available) This applies the following patches to pybind11: - https://github.com/pybind/pybind11/pull/4857 - https://github.com/pybind/pybind11/pull/4877 to avoid deadlock when using pybind11 without importing numpy in multi-threaded environment. Authors: - Gigon Bae (https://github.com/gigony) - Gregory Lee (https://github.com/grlee77) Approvers: - Gregory Lee (https://github.com/grlee77) - https://github.com/jakirkham URL: https://github.com/rapidsai/cucim/pull/618 --- cpp/cmake/deps/catch2.cmake | 7 +- cpp/cmake/deps/pybind11.cmake | 3 +- cpp/cmake/deps/pybind11_pr4857_4877.patch | 247 ++++++++++++++++++ .../cucim.kit.cumed/cmake/deps/catch2.cmake | 7 +- .../cucim.kit.cumed/tests/CMakeLists.txt | 6 +- cpp/plugins/cucim.kit.cumed/tests/main.cpp | 7 +- .../cucim.kit.cumed/tests/test_basic.cpp | 2 +- .../cucim.kit.cuslide/cmake/deps/catch2.cmake | 7 +- .../cucim.kit.cuslide/tests/CMakeLists.txt | 6 +- cpp/plugins/cucim.kit.cuslide/tests/main.cpp | 7 +- .../tests/test_philips_tiff.cpp | 2 +- .../tests/test_read_rawtiff.cpp | 2 +- .../tests/test_read_region.cpp | 3 +- cpp/tests/CMakeLists.txt | 6 +- cpp/tests/main.cpp | 7 +- cpp/tests/test_cufile.cpp | 2 +- cpp/tests/test_metadata.cpp | 2 +- cpp/tests/test_read_region.cpp | 2 +- dockcross-manylinux2014-x64 | 2 +- .../File-access_Experiments_on_TIFF.ipynb | 2 +- python/cmake/deps/pybind11.cmake | 3 +- python/cmake/deps/pybind11_pr4857_4877.patch | 247 ++++++++++++++++++ python/cucim/docs/getting_started/index.md | 2 +- scripts/run-dist | 4 +- 24 files changed, 544 insertions(+), 41 deletions(-) create mode 100644 cpp/cmake/deps/pybind11_pr4857_4877.patch create mode 100644 python/cmake/deps/pybind11_pr4857_4877.patch diff --git a/cpp/cmake/deps/catch2.cmake b/cpp/cmake/deps/catch2.cmake index f5e9ca616..4ee4bfabd 100644 --- a/cpp/cmake/deps/catch2.cmake +++ b/cpp/cmake/deps/catch2.cmake @@ -17,7 +17,7 @@ if (NOT TARGET deps::catch2) FetchContent_Declare( deps-catch2 GIT_REPOSITORY https://github.com/catchorg/Catch2.git - GIT_TAG v2.13.1 + GIT_TAG v3.4.0 GIT_SHALLOW TRUE ) FetchContent_GetProperties(deps-catch2) @@ -29,8 +29,9 @@ if (NOT TARGET deps::catch2) add_subdirectory(${deps-catch2_SOURCE_DIR} ${deps-catch2_BINARY_DIR} EXCLUDE_FROM_ALL) - # Include Append catch2's cmake module path so that we can use `include(ParseAndAddCatchTests)`. - list(APPEND CMAKE_MODULE_PATH "${deps-catch2_SOURCE_DIR}/contrib") + # Include Append catch2's cmake module path so that we can use `include(Catch)`. + # https://github.com/catchorg/Catch2/blob/devel/docs/cmake-integration.md#catchcmake-and-catchaddtestscmake + list(APPEND CMAKE_MODULE_PATH "${deps-catch2_SOURCE_DIR}/extras") set(CMAKE_MODULE_PATH ${CMAKE_MODULE_PATH} PARENT_SCOPE) add_library(deps::catch2 INTERFACE IMPORTED GLOBAL) diff --git a/cpp/cmake/deps/pybind11.cmake b/cpp/cmake/deps/pybind11.cmake index 1fbba6c9d..e4ea6f583 100644 --- a/cpp/cmake/deps/pybind11.cmake +++ b/cpp/cmake/deps/pybind11.cmake @@ -17,8 +17,9 @@ if (NOT TARGET deps::pybind11) FetchContent_Declare( deps-pybind11 GIT_REPOSITORY https://github.com/pybind/pybind11.git - GIT_TAG v2.10.3 + GIT_TAG v2.11.1 GIT_SHALLOW TRUE + PATCH_COMMAND git apply "${CMAKE_CURRENT_LIST_DIR}/pybind11_pr4857_4877.patch" || true ) FetchContent_GetProperties(deps-pybind11) if (NOT deps-pybind11_POPULATED) diff --git a/cpp/cmake/deps/pybind11_pr4857_4877.patch b/cpp/cmake/deps/pybind11_pr4857_4877.patch new file mode 100644 index 000000000..d6abec392 --- /dev/null +++ b/cpp/cmake/deps/pybind11_pr4857_4877.patch @@ -0,0 +1,247 @@ +diff --git a/CMakeLists.txt b/CMakeLists.txt +index 87ec103..f2f1d19 100644 +--- a/CMakeLists.txt ++++ b/CMakeLists.txt +@@ -132,6 +132,7 @@ set(PYBIND11_HEADERS + include/pybind11/embed.h + include/pybind11/eval.h + include/pybind11/gil.h ++ include/pybind11/gil_safe_call_once.h + include/pybind11/iostream.h + include/pybind11/functional.h + include/pybind11/numpy.h +diff --git a/include/pybind11/detail/common.h b/include/pybind11/detail/common.h +index 31a54c7..8906c1f 100644 +--- a/include/pybind11/detail/common.h ++++ b/include/pybind11/detail/common.h +@@ -118,6 +118,14 @@ + # endif + #endif + ++#if defined(PYBIND11_CPP20) ++# define PYBIND11_CONSTINIT constinit ++# define PYBIND11_DTOR_CONSTEXPR constexpr ++#else ++# define PYBIND11_CONSTINIT ++# define PYBIND11_DTOR_CONSTEXPR ++#endif ++ + // Compiler version assertions + #if defined(__INTEL_COMPILER) + # if __INTEL_COMPILER < 1800 +diff --git a/include/pybind11/gil.h b/include/pybind11/gil.h +index 570a558..da22f48 100644 +--- a/include/pybind11/gil.h ++++ b/include/pybind11/gil.h +@@ -11,6 +11,8 @@ + + #include "detail/common.h" + ++#include ++ + #if defined(WITH_THREAD) && !defined(PYBIND11_SIMPLE_GIL_MANAGEMENT) + # include "detail/internals.h" + #endif +@@ -137,7 +139,9 @@ private: + + class gil_scoped_release { + public: ++ // PRECONDITION: The GIL must be held when this constructor is called. + explicit gil_scoped_release(bool disassoc = false) : disassoc(disassoc) { ++ assert(PyGILState_Check()); + // `get_internals()` must be called here unconditionally in order to initialize + // `internals.tstate` for subsequent `gil_scoped_acquire` calls. Otherwise, an + // initialization race could occur as multiple threads try `gil_scoped_acquire`. +@@ -201,7 +205,11 @@ class gil_scoped_release { + PyThreadState *state; + + public: +- gil_scoped_release() : state{PyEval_SaveThread()} {} ++ // PRECONDITION: The GIL must be held when this constructor is called. ++ gil_scoped_release() { ++ assert(PyGILState_Check()); ++ state = PyEval_SaveThread(); ++ } + gil_scoped_release(const gil_scoped_release &) = delete; + gil_scoped_release &operator=(const gil_scoped_release &) = delete; + ~gil_scoped_release() { PyEval_RestoreThread(state); } +diff --git a/include/pybind11/gil_safe_call_once.h b/include/pybind11/gil_safe_call_once.h +new file mode 100644 +index 0000000..58b90b8 +--- /dev/null ++++ b/include/pybind11/gil_safe_call_once.h +@@ -0,0 +1,90 @@ ++// Copyright (c) 2023 The pybind Community. ++ ++ ++#include "detail/common.h" ++#include "gil.h" ++ ++#include ++#include ++ ++PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE) ++ ++// Use the `gil_safe_call_once_and_store` class below instead of the naive ++// ++// static auto imported_obj = py::module_::import("module_name"); // BAD, DO NOT USE! ++// ++// which has two serious issues: ++// ++// 1. Py_DECREF() calls potentially after the Python interpreter was finalized already, and ++// 2. deadlocks in multi-threaded processes (because of missing lock ordering). ++// ++// The following alternative avoids both problems: ++// ++// PYBIND11_CONSTINIT static py::gil_safe_call_once_and_store storage; ++// auto &imported_obj = storage // Do NOT make this `static`! ++// .call_once_and_store_result([]() { ++// return py::module_::import("module_name"); ++// }) ++// .get_stored(); ++// ++// The parameter of `call_once_and_store_result()` must be callable. It can make ++// CPython API calls, and in particular, it can temporarily release the GIL. ++// ++// `T` can be any C++ type, it does not have to involve CPython API types. ++// ++// The behavior with regard to signals, e.g. `SIGINT` (`KeyboardInterrupt`), ++// is not ideal. If the main thread is the one to actually run the `Callable`, ++// then a `KeyboardInterrupt` will interrupt it if it is running normal Python ++// code. The situation is different if a non-main thread runs the ++// `Callable`, and then the main thread starts waiting for it to complete: ++// a `KeyboardInterrupt` will not interrupt the non-main thread, but it will ++// get processed only when it is the main thread's turn again and it is running ++// normal Python code. However, this will be unnoticeable for quick call-once ++// functions, which is usually the case. ++template ++class gil_safe_call_once_and_store { ++public: ++ // PRECONDITION: The GIL must be held when `call_once_and_store_result()` is called. ++ template ++ gil_safe_call_once_and_store &call_once_and_store_result(Callable &&fn) { ++ if (!is_initialized_) { // This read is guarded by the GIL. ++ // Multiple threads may enter here, because the GIL is released in the next line and ++ // CPython API calls in the `fn()` call below may release and reacquire the GIL. ++ gil_scoped_release gil_rel; // Needed to establish lock ordering. ++ std::call_once(once_flag_, [&] { ++ // Only one thread will ever enter here. ++ gil_scoped_acquire gil_acq; ++ ::new (storage_) T(fn()); // fn may release, but will reacquire, the GIL. ++ is_initialized_ = true; // This write is guarded by the GIL. ++ }); ++ // All threads will observe `is_initialized_` as true here. ++ } ++ // Intentionally not returning `T &` to ensure the calling code is self-documenting. ++ return *this; ++ } ++ ++ // This must only be called after `call_once_and_store_result()` was called. ++ T &get_stored() { ++ assert(is_initialized_); ++ PYBIND11_WARNING_PUSH ++#if !defined(__clang__) && defined(__GNUC__) && __GNUC__ < 5 ++ // Needed for gcc 4.8.5 ++ PYBIND11_WARNING_DISABLE_GCC("-Wstrict-aliasing") ++#endif ++ return *reinterpret_cast(storage_); ++ PYBIND11_WARNING_POP ++ } ++ ++ constexpr gil_safe_call_once_and_store() = default; ++ PYBIND11_DTOR_CONSTEXPR ~gil_safe_call_once_and_store() = default; ++ ++private: ++ alignas(T) char storage_[sizeof(T)] = {}; ++ std::once_flag once_flag_ = {}; ++ bool is_initialized_ = false; ++ // The `is_initialized_`-`storage_` pair is very similar to `std::optional`, ++ // but the latter does not have the triviality properties of former, ++ // therefore `std::optional` is not a viable alternative here. ++}; ++ ++PYBIND11_NAMESPACE_END(PYBIND11_NAMESPACE) +diff --git a/include/pybind11/numpy.h b/include/pybind11/numpy.h +index 36077ec..1217c8d 100644 +--- a/include/pybind11/numpy.h ++++ b/include/pybind11/numpy.h +@@ -10,7 +10,10 @@ + #pragma once + + #include "pybind11.h" ++#include "detail/common.h" + #include "complex.h" ++#include "gil_safe_call_once.h" ++#include "pytypes.h" + + #include + #include +@@ -120,6 +123,20 @@ inline numpy_internals &get_numpy_internals() { + return *ptr; + } + ++PYBIND11_NOINLINE module_ import_numpy_core_submodule(const char *submodule_name) { ++ module_ numpy = module_::import("numpy"); ++ str version_string = numpy.attr("__version__"); ++ ++ module_ numpy_lib = module_::import("numpy.lib"); ++ object numpy_version = numpy_lib.attr("NumpyVersion")(version_string); ++ int major_version = numpy_version.attr("major").cast(); ++ ++ /* `numpy.core` was renamed to `numpy._core` in NumPy 2.0 as it officially ++ became a private module. */ ++ std::string numpy_core_path = major_version >= 2 ? "numpy._core" : "numpy.core"; ++ return module_::import((numpy_core_path + "." + submodule_name).c_str()); ++} ++ + template + struct same_size { + template +@@ -192,8 +209,8 @@ struct npy_api { + }; + + static npy_api &get() { +- static npy_api api = lookup(); +- return api; ++ PYBIND11_CONSTINIT static gil_safe_call_once_and_store storage; ++ return storage.call_once_and_store_result(lookup).get_stored(); + } + + bool PyArray_Check_(PyObject *obj) const { +@@ -263,9 +280,13 @@ private: + }; + + static npy_api lookup() { +- module_ m = module_::import("numpy.core.multiarray"); ++ module_ m = detail::import_numpy_core_submodule("multiarray"); + auto c = m.attr("_ARRAY_API"); + void **api_ptr = (void **) PyCapsule_GetPointer(c.ptr(), nullptr); ++ if (api_ptr == nullptr) { ++ raise_from(PyExc_SystemError, "FAILURE obtaining numpy _ARRAY_API pointer."); ++ throw error_already_set(); ++ } + npy_api api; + #define DECL_NPY_API(Func) api.Func##_ = (decltype(api.Func##_)) api_ptr[API_##Func]; + DECL_NPY_API(PyArray_GetNDArrayCFeatureVersion); +@@ -625,13 +646,14 @@ public: + char flags() const { return detail::array_descriptor_proxy(m_ptr)->flags; } + + private: +- static object _dtype_from_pep3118() { +- static PyObject *obj = module_::import("numpy.core._internal") +- .attr("_dtype_from_pep3118") +- .cast() +- .release() +- .ptr(); +- return reinterpret_borrow(obj); ++ static object &_dtype_from_pep3118() { ++ PYBIND11_CONSTINIT static gil_safe_call_once_and_store storage; ++ return storage ++ .call_once_and_store_result([]() { ++ return detail::import_numpy_core_submodule("_internal") ++ .attr("_dtype_from_pep3118"); ++ }) ++ .get_stored(); + } + + dtype strip_padding(ssize_t itemsize) { diff --git a/cpp/plugins/cucim.kit.cumed/cmake/deps/catch2.cmake b/cpp/plugins/cucim.kit.cumed/cmake/deps/catch2.cmake index 0872d67bb..6a35a9c0b 100644 --- a/cpp/plugins/cucim.kit.cumed/cmake/deps/catch2.cmake +++ b/cpp/plugins/cucim.kit.cumed/cmake/deps/catch2.cmake @@ -17,7 +17,7 @@ if (NOT TARGET deps::catch2) FetchContent_Declare( deps-catch2 GIT_REPOSITORY https://github.com/catchorg/Catch2.git - GIT_TAG v2.13.1 + GIT_TAG v3.4.0 GIT_SHALLOW TRUE ) FetchContent_GetProperties(deps-catch2) @@ -29,8 +29,9 @@ if (NOT TARGET deps::catch2) add_subdirectory(${deps-catch2_SOURCE_DIR} ${deps-catch2_BINARY_DIR} EXCLUDE_FROM_ALL) - # Include Append catch2's cmake module path so that we can use `include(ParseAndAddCatchTests)`. - list(APPEND CMAKE_MODULE_PATH "${deps-catch2_SOURCE_DIR}/contrib") + # Include Append catch2's cmake module path so that we can use `include(Catch)`. + # https://github.com/catchorg/Catch2/blob/devel/docs/cmake-integration.md#catchcmake-and-catchaddtestscmake + list(APPEND CMAKE_MODULE_PATH "${deps-catch2_SOURCE_DIR}/extras") set(CMAKE_MODULE_PATH ${CMAKE_MODULE_PATH} PARENT_SCOPE) add_library(deps::catch2 INTERFACE IMPORTED GLOBAL) diff --git a/cpp/plugins/cucim.kit.cumed/tests/CMakeLists.txt b/cpp/plugins/cucim.kit.cumed/tests/CMakeLists.txt index 95df56068..262c02499 100644 --- a/cpp/plugins/cucim.kit.cumed/tests/CMakeLists.txt +++ b/cpp/plugins/cucim.kit.cumed/tests/CMakeLists.txt @@ -57,6 +57,6 @@ target_include_directories(cumed_tests $ ) -include(ParseAndAddCatchTests) -# See https://github.com/catchorg/Catch2/blob/master/docs/cmake-integration.md#parseandaddcatchtestscmake for other options -ParseAndAddCatchTests(cumed_tests) +include(Catch) +# See https://github.com/catchorg/Catch2/blob/devel/docs/cmake-integration.md#catchcmake-and-catchaddtestscmake for other options +catch_discover_tests(cumed_tests) diff --git a/cpp/plugins/cucim.kit.cumed/tests/main.cpp b/cpp/plugins/cucim.kit.cumed/tests/main.cpp index aa02e0309..1a24e09b5 100644 --- a/cpp/plugins/cucim.kit.cumed/tests/main.cpp +++ b/cpp/plugins/cucim.kit.cumed/tests/main.cpp @@ -14,15 +14,16 @@ * limitations under the License. */ -//#define CATCH_CONFIG_MAIN -//#include +// #define CATCH_CONFIG_MAIN +// #include // Implement main explicitly to handle additional parameters. #define CATCH_CONFIG_RUNNER #include "config.h" #include "cucim/core/framework.h" -#include +#include +#include #include #include diff --git a/cpp/plugins/cucim.kit.cumed/tests/test_basic.cpp b/cpp/plugins/cucim.kit.cumed/tests/test_basic.cpp index 1191db58e..bb7178e3f 100644 --- a/cpp/plugins/cucim.kit.cumed/tests/test_basic.cpp +++ b/cpp/plugins/cucim.kit.cumed/tests/test_basic.cpp @@ -16,7 +16,7 @@ #include "config.h" -#include +#include #include TEST_CASE("Verify file", "[test_basic.cpp]") diff --git a/cpp/plugins/cucim.kit.cuslide/cmake/deps/catch2.cmake b/cpp/plugins/cucim.kit.cuslide/cmake/deps/catch2.cmake index f5e9ca616..4ee4bfabd 100644 --- a/cpp/plugins/cucim.kit.cuslide/cmake/deps/catch2.cmake +++ b/cpp/plugins/cucim.kit.cuslide/cmake/deps/catch2.cmake @@ -17,7 +17,7 @@ if (NOT TARGET deps::catch2) FetchContent_Declare( deps-catch2 GIT_REPOSITORY https://github.com/catchorg/Catch2.git - GIT_TAG v2.13.1 + GIT_TAG v3.4.0 GIT_SHALLOW TRUE ) FetchContent_GetProperties(deps-catch2) @@ -29,8 +29,9 @@ if (NOT TARGET deps::catch2) add_subdirectory(${deps-catch2_SOURCE_DIR} ${deps-catch2_BINARY_DIR} EXCLUDE_FROM_ALL) - # Include Append catch2's cmake module path so that we can use `include(ParseAndAddCatchTests)`. - list(APPEND CMAKE_MODULE_PATH "${deps-catch2_SOURCE_DIR}/contrib") + # Include Append catch2's cmake module path so that we can use `include(Catch)`. + # https://github.com/catchorg/Catch2/blob/devel/docs/cmake-integration.md#catchcmake-and-catchaddtestscmake + list(APPEND CMAKE_MODULE_PATH "${deps-catch2_SOURCE_DIR}/extras") set(CMAKE_MODULE_PATH ${CMAKE_MODULE_PATH} PARENT_SCOPE) add_library(deps::catch2 INTERFACE IMPORTED GLOBAL) diff --git a/cpp/plugins/cucim.kit.cuslide/tests/CMakeLists.txt b/cpp/plugins/cucim.kit.cuslide/tests/CMakeLists.txt index c779d63b8..3b9bde713 100644 --- a/cpp/plugins/cucim.kit.cuslide/tests/CMakeLists.txt +++ b/cpp/plugins/cucim.kit.cuslide/tests/CMakeLists.txt @@ -60,6 +60,6 @@ target_include_directories(cuslide_tests $ ) -include(ParseAndAddCatchTests) -# See https://github.com/catchorg/Catch2/blob/master/docs/cmake-integration.md#parseandaddcatchtestscmake for other options -ParseAndAddCatchTests(cuslide_tests) +include(Catch) +# See https://github.com/catchorg/Catch2/blob/devel/docs/cmake-integration.md#catchcmake-and-catchaddtestscmake for other options +catch_discover_tests(cuslide_tests) diff --git a/cpp/plugins/cucim.kit.cuslide/tests/main.cpp b/cpp/plugins/cucim.kit.cuslide/tests/main.cpp index 0f4e3f544..ad3b4f883 100644 --- a/cpp/plugins/cucim.kit.cuslide/tests/main.cpp +++ b/cpp/plugins/cucim.kit.cuslide/tests/main.cpp @@ -14,15 +14,16 @@ * limitations under the License. */ -//#define CATCH_CONFIG_MAIN -//#include +// #define CATCH_CONFIG_MAIN +// #include // Implement main explicitly to handle additional parameters. #define CATCH_CONFIG_RUNNER #include "config.h" #include "cucim/core/framework.h" -#include +#include +#include #include #include diff --git a/cpp/plugins/cucim.kit.cuslide/tests/test_philips_tiff.cpp b/cpp/plugins/cucim.kit.cuslide/tests/test_philips_tiff.cpp index 974ed85ea..82fb5ddb7 100644 --- a/cpp/plugins/cucim.kit.cuslide/tests/test_philips_tiff.cpp +++ b/cpp/plugins/cucim.kit.cuslide/tests/test_philips_tiff.cpp @@ -19,7 +19,7 @@ #include "cuslide/tiff/tiff.h" #include "config.h" -#include +#include #include TEST_CASE("Verify philips tiff file", "[test_philips_tiff.cpp]") diff --git a/cpp/plugins/cucim.kit.cuslide/tests/test_read_rawtiff.cpp b/cpp/plugins/cucim.kit.cuslide/tests/test_read_rawtiff.cpp index 54f4801ac..819e801c5 100644 --- a/cpp/plugins/cucim.kit.cuslide/tests/test_read_rawtiff.cpp +++ b/cpp/plugins/cucim.kit.cuslide/tests/test_read_rawtiff.cpp @@ -19,7 +19,7 @@ #include "config.h" #include -#include +#include #include #include #include diff --git a/cpp/plugins/cucim.kit.cuslide/tests/test_read_region.cpp b/cpp/plugins/cucim.kit.cuslide/tests/test_read_region.cpp index b28c6c78b..af808fd12 100644 --- a/cpp/plugins/cucim.kit.cuslide/tests/test_read_region.cpp +++ b/cpp/plugins/cucim.kit.cuslide/tests/test_read_region.cpp @@ -16,7 +16,8 @@ #include -#include +#include +#include #include #include diff --git a/cpp/tests/CMakeLists.txt b/cpp/tests/CMakeLists.txt index 2e892875b..dfd619b24 100644 --- a/cpp/tests/CMakeLists.txt +++ b/cpp/tests/CMakeLists.txt @@ -60,6 +60,6 @@ target_link_libraries(cucim_tests Threads::Threads # -lpthread ) -include(ParseAndAddCatchTests) -# See https://github.com/catchorg/Catch2/blob/master/docs/cmake-integration.md#parseandaddcatchtestscmake for other options -ParseAndAddCatchTests(cucim_tests) +include(Catch) +# See https://github.com/catchorg/Catch2/blob/devel/docs/cmake-integration.md#catchcmake-and-catchaddtestscmake for other options +catch_discover_tests(cucim_tests) diff --git a/cpp/tests/main.cpp b/cpp/tests/main.cpp index bdcdf1f43..07705522b 100644 --- a/cpp/tests/main.cpp +++ b/cpp/tests/main.cpp @@ -14,15 +14,16 @@ * limitations under the License. */ -//#define CATCH_CONFIG_MAIN -//#include +// #define CATCH_CONFIG_MAIN +// #include // Implement main explicitly to handle additional parameters. #define CATCH_CONFIG_RUNNER #include "config.h" #include "cucim/core/framework.h" -#include +#include +#include #include #include diff --git a/cpp/tests/test_cufile.cpp b/cpp/tests/test_cufile.cpp index a1330d52b..35c0c31a3 100644 --- a/cpp/tests/test_cufile.cpp +++ b/cpp/tests/test_cufile.cpp @@ -17,7 +17,7 @@ #include "cucim/logger/timer.h" #include "config.h" -#include +#include #include #include #include diff --git a/cpp/tests/test_metadata.cpp b/cpp/tests/test_metadata.cpp index 59a7057f6..537094ece 100644 --- a/cpp/tests/test_metadata.cpp +++ b/cpp/tests/test_metadata.cpp @@ -20,7 +20,7 @@ #include "cucim/core/framework.h" #include "cucim/io/format/image_format.h" -#include +#include #include #include #include diff --git a/cpp/tests/test_read_region.cpp b/cpp/tests/test_read_region.cpp index 95fa43b85..7b8fa80f9 100644 --- a/cpp/tests/test_read_region.cpp +++ b/cpp/tests/test_read_region.cpp @@ -17,7 +17,7 @@ #include #include -#include +#include #include #include diff --git a/dockcross-manylinux2014-x64 b/dockcross-manylinux2014-x64 index 18e7f50a9..e6edf726e 100755 --- a/dockcross-manylinux2014-x64 +++ b/dockcross-manylinux2014-x64 @@ -233,7 +233,7 @@ fi TTY_ARGS= tty -s && [ -z "$MSYS" ] && TTY_ARGS=-ti CONTAINER_NAME=dockcross_$RANDOM -nvidia-docker run $TTY_ARGS --name $CONTAINER_NAME \ +docker run --runtime nvidia $TTY_ARGS --name $CONTAINER_NAME \ -v "$HOST_PWD":/work \ $HOST_VOLUMES \ "${USER_IDS[@]}" \ diff --git a/notebooks/File-access_Experiments_on_TIFF.ipynb b/notebooks/File-access_Experiments_on_TIFF.ipynb index 9ebbf59cd..8f11c9f72 100644 --- a/notebooks/File-access_Experiments_on_TIFF.ipynb +++ b/notebooks/File-access_Experiments_on_TIFF.ipynb @@ -199,7 +199,7 @@ "#include \"cuslide/tiff/tiff.h\"\n", "#include \"config.h\"\n", "\n", - "#include \n", + "#include \n", "#include \n", "#include \n", "#include \n", diff --git a/python/cmake/deps/pybind11.cmake b/python/cmake/deps/pybind11.cmake index de4172935..ab0eab7f8 100644 --- a/python/cmake/deps/pybind11.cmake +++ b/python/cmake/deps/pybind11.cmake @@ -17,8 +17,9 @@ if (NOT TARGET deps::pybind11) FetchContent_Declare( deps-pybind11 GIT_REPOSITORY https://github.com/pybind/pybind11.git - GIT_TAG v2.10.3 + GIT_TAG v2.11.1 GIT_SHALLOW TRUE + PATCH_COMMAND git apply "${CMAKE_CURRENT_LIST_DIR}/pybind11_pr4857_4877.patch" || true ) FetchContent_GetProperties(deps-pybind11) if (NOT deps-pybind11_POPULATED) diff --git a/python/cmake/deps/pybind11_pr4857_4877.patch b/python/cmake/deps/pybind11_pr4857_4877.patch new file mode 100644 index 000000000..d6abec392 --- /dev/null +++ b/python/cmake/deps/pybind11_pr4857_4877.patch @@ -0,0 +1,247 @@ +diff --git a/CMakeLists.txt b/CMakeLists.txt +index 87ec103..f2f1d19 100644 +--- a/CMakeLists.txt ++++ b/CMakeLists.txt +@@ -132,6 +132,7 @@ set(PYBIND11_HEADERS + include/pybind11/embed.h + include/pybind11/eval.h + include/pybind11/gil.h ++ include/pybind11/gil_safe_call_once.h + include/pybind11/iostream.h + include/pybind11/functional.h + include/pybind11/numpy.h +diff --git a/include/pybind11/detail/common.h b/include/pybind11/detail/common.h +index 31a54c7..8906c1f 100644 +--- a/include/pybind11/detail/common.h ++++ b/include/pybind11/detail/common.h +@@ -118,6 +118,14 @@ + # endif + #endif + ++#if defined(PYBIND11_CPP20) ++# define PYBIND11_CONSTINIT constinit ++# define PYBIND11_DTOR_CONSTEXPR constexpr ++#else ++# define PYBIND11_CONSTINIT ++# define PYBIND11_DTOR_CONSTEXPR ++#endif ++ + // Compiler version assertions + #if defined(__INTEL_COMPILER) + # if __INTEL_COMPILER < 1800 +diff --git a/include/pybind11/gil.h b/include/pybind11/gil.h +index 570a558..da22f48 100644 +--- a/include/pybind11/gil.h ++++ b/include/pybind11/gil.h +@@ -11,6 +11,8 @@ + + #include "detail/common.h" + ++#include ++ + #if defined(WITH_THREAD) && !defined(PYBIND11_SIMPLE_GIL_MANAGEMENT) + # include "detail/internals.h" + #endif +@@ -137,7 +139,9 @@ private: + + class gil_scoped_release { + public: ++ // PRECONDITION: The GIL must be held when this constructor is called. + explicit gil_scoped_release(bool disassoc = false) : disassoc(disassoc) { ++ assert(PyGILState_Check()); + // `get_internals()` must be called here unconditionally in order to initialize + // `internals.tstate` for subsequent `gil_scoped_acquire` calls. Otherwise, an + // initialization race could occur as multiple threads try `gil_scoped_acquire`. +@@ -201,7 +205,11 @@ class gil_scoped_release { + PyThreadState *state; + + public: +- gil_scoped_release() : state{PyEval_SaveThread()} {} ++ // PRECONDITION: The GIL must be held when this constructor is called. ++ gil_scoped_release() { ++ assert(PyGILState_Check()); ++ state = PyEval_SaveThread(); ++ } + gil_scoped_release(const gil_scoped_release &) = delete; + gil_scoped_release &operator=(const gil_scoped_release &) = delete; + ~gil_scoped_release() { PyEval_RestoreThread(state); } +diff --git a/include/pybind11/gil_safe_call_once.h b/include/pybind11/gil_safe_call_once.h +new file mode 100644 +index 0000000..58b90b8 +--- /dev/null ++++ b/include/pybind11/gil_safe_call_once.h +@@ -0,0 +1,90 @@ ++// Copyright (c) 2023 The pybind Community. ++ ++ ++#include "detail/common.h" ++#include "gil.h" ++ ++#include ++#include ++ ++PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE) ++ ++// Use the `gil_safe_call_once_and_store` class below instead of the naive ++// ++// static auto imported_obj = py::module_::import("module_name"); // BAD, DO NOT USE! ++// ++// which has two serious issues: ++// ++// 1. Py_DECREF() calls potentially after the Python interpreter was finalized already, and ++// 2. deadlocks in multi-threaded processes (because of missing lock ordering). ++// ++// The following alternative avoids both problems: ++// ++// PYBIND11_CONSTINIT static py::gil_safe_call_once_and_store storage; ++// auto &imported_obj = storage // Do NOT make this `static`! ++// .call_once_and_store_result([]() { ++// return py::module_::import("module_name"); ++// }) ++// .get_stored(); ++// ++// The parameter of `call_once_and_store_result()` must be callable. It can make ++// CPython API calls, and in particular, it can temporarily release the GIL. ++// ++// `T` can be any C++ type, it does not have to involve CPython API types. ++// ++// The behavior with regard to signals, e.g. `SIGINT` (`KeyboardInterrupt`), ++// is not ideal. If the main thread is the one to actually run the `Callable`, ++// then a `KeyboardInterrupt` will interrupt it if it is running normal Python ++// code. The situation is different if a non-main thread runs the ++// `Callable`, and then the main thread starts waiting for it to complete: ++// a `KeyboardInterrupt` will not interrupt the non-main thread, but it will ++// get processed only when it is the main thread's turn again and it is running ++// normal Python code. However, this will be unnoticeable for quick call-once ++// functions, which is usually the case. ++template ++class gil_safe_call_once_and_store { ++public: ++ // PRECONDITION: The GIL must be held when `call_once_and_store_result()` is called. ++ template ++ gil_safe_call_once_and_store &call_once_and_store_result(Callable &&fn) { ++ if (!is_initialized_) { // This read is guarded by the GIL. ++ // Multiple threads may enter here, because the GIL is released in the next line and ++ // CPython API calls in the `fn()` call below may release and reacquire the GIL. ++ gil_scoped_release gil_rel; // Needed to establish lock ordering. ++ std::call_once(once_flag_, [&] { ++ // Only one thread will ever enter here. ++ gil_scoped_acquire gil_acq; ++ ::new (storage_) T(fn()); // fn may release, but will reacquire, the GIL. ++ is_initialized_ = true; // This write is guarded by the GIL. ++ }); ++ // All threads will observe `is_initialized_` as true here. ++ } ++ // Intentionally not returning `T &` to ensure the calling code is self-documenting. ++ return *this; ++ } ++ ++ // This must only be called after `call_once_and_store_result()` was called. ++ T &get_stored() { ++ assert(is_initialized_); ++ PYBIND11_WARNING_PUSH ++#if !defined(__clang__) && defined(__GNUC__) && __GNUC__ < 5 ++ // Needed for gcc 4.8.5 ++ PYBIND11_WARNING_DISABLE_GCC("-Wstrict-aliasing") ++#endif ++ return *reinterpret_cast(storage_); ++ PYBIND11_WARNING_POP ++ } ++ ++ constexpr gil_safe_call_once_and_store() = default; ++ PYBIND11_DTOR_CONSTEXPR ~gil_safe_call_once_and_store() = default; ++ ++private: ++ alignas(T) char storage_[sizeof(T)] = {}; ++ std::once_flag once_flag_ = {}; ++ bool is_initialized_ = false; ++ // The `is_initialized_`-`storage_` pair is very similar to `std::optional`, ++ // but the latter does not have the triviality properties of former, ++ // therefore `std::optional` is not a viable alternative here. ++}; ++ ++PYBIND11_NAMESPACE_END(PYBIND11_NAMESPACE) +diff --git a/include/pybind11/numpy.h b/include/pybind11/numpy.h +index 36077ec..1217c8d 100644 +--- a/include/pybind11/numpy.h ++++ b/include/pybind11/numpy.h +@@ -10,7 +10,10 @@ + #pragma once + + #include "pybind11.h" ++#include "detail/common.h" + #include "complex.h" ++#include "gil_safe_call_once.h" ++#include "pytypes.h" + + #include + #include +@@ -120,6 +123,20 @@ inline numpy_internals &get_numpy_internals() { + return *ptr; + } + ++PYBIND11_NOINLINE module_ import_numpy_core_submodule(const char *submodule_name) { ++ module_ numpy = module_::import("numpy"); ++ str version_string = numpy.attr("__version__"); ++ ++ module_ numpy_lib = module_::import("numpy.lib"); ++ object numpy_version = numpy_lib.attr("NumpyVersion")(version_string); ++ int major_version = numpy_version.attr("major").cast(); ++ ++ /* `numpy.core` was renamed to `numpy._core` in NumPy 2.0 as it officially ++ became a private module. */ ++ std::string numpy_core_path = major_version >= 2 ? "numpy._core" : "numpy.core"; ++ return module_::import((numpy_core_path + "." + submodule_name).c_str()); ++} ++ + template + struct same_size { + template +@@ -192,8 +209,8 @@ struct npy_api { + }; + + static npy_api &get() { +- static npy_api api = lookup(); +- return api; ++ PYBIND11_CONSTINIT static gil_safe_call_once_and_store storage; ++ return storage.call_once_and_store_result(lookup).get_stored(); + } + + bool PyArray_Check_(PyObject *obj) const { +@@ -263,9 +280,13 @@ private: + }; + + static npy_api lookup() { +- module_ m = module_::import("numpy.core.multiarray"); ++ module_ m = detail::import_numpy_core_submodule("multiarray"); + auto c = m.attr("_ARRAY_API"); + void **api_ptr = (void **) PyCapsule_GetPointer(c.ptr(), nullptr); ++ if (api_ptr == nullptr) { ++ raise_from(PyExc_SystemError, "FAILURE obtaining numpy _ARRAY_API pointer."); ++ throw error_already_set(); ++ } + npy_api api; + #define DECL_NPY_API(Func) api.Func##_ = (decltype(api.Func##_)) api_ptr[API_##Func]; + DECL_NPY_API(PyArray_GetNDArrayCFeatureVersion); +@@ -625,13 +646,14 @@ public: + char flags() const { return detail::array_descriptor_proxy(m_ptr)->flags; } + + private: +- static object _dtype_from_pep3118() { +- static PyObject *obj = module_::import("numpy.core._internal") +- .attr("_dtype_from_pep3118") +- .cast() +- .release() +- .ptr(); +- return reinterpret_borrow(obj); ++ static object &_dtype_from_pep3118() { ++ PYBIND11_CONSTINIT static gil_safe_call_once_and_store storage; ++ return storage ++ .call_once_and_store_result([]() { ++ return detail::import_numpy_core_submodule("_internal") ++ .attr("_dtype_from_pep3118"); ++ }) ++ .get_stored(); + } + + dtype strip_padding(ssize_t itemsize) { diff --git a/python/cucim/docs/getting_started/index.md b/python/cucim/docs/getting_started/index.md index 5b20d080a..5eff46f3f 100644 --- a/python/cucim/docs/getting_started/index.md +++ b/python/cucim/docs/getting_started/index.md @@ -99,7 +99,7 @@ After executing the command, type a password for the instance and open a web bro ```bash ... Port 10001 would be used...(http://172.26.120.129:10001) -2021-02-13 01:12:44 $ nvidia-docker run --gpus all -it --rm -v /home/repo/cucim/notebooks:/notebooks -p 10001:10001 cucim-jupyter -c echo -n 'Enter New Password: '; jupyter lab --ServerApp.password="$(python3 -u -c "from jupyter_server.auth import passwd;pw=input();print(passwd(pw));" | egrep 'sha|argon')" --ServerApp.root_dir=/notebooks --allow-root --port=10001 --ip=0.0.0.0 --no-browser +2021-02-13 01:12:44 $ docker run --runtime nvidia --gpus all -it --rm -v /home/repo/cucim/notebooks:/notebooks -p 10001:10001 cucim-jupyter -c echo -n 'Enter New Password: '; jupyter lab --ServerApp.password="$(python3 -u -c "from jupyter_server.auth import passwd;pw=input();print(passwd(pw));" | egrep 'sha|argon')" --ServerApp.root_dir=/notebooks --allow-root --port=10001 --ip=0.0.0.0 --no-browser Enter New Password: [I 2021-02-13 01:12:47.981 ServerApp] dask_labextension | extension was successfully linked. [I 2021-02-13 01:12:47.981 ServerApp] jupyter_server_proxy | extension was successfully linked. diff --git a/scripts/run-dist b/scripts/run-dist index af26406f4..60ed771e0 100755 --- a/scripts/run-dist +++ b/scripts/run-dist @@ -307,14 +307,14 @@ launch_notebooks() { run_command cp ${TOP}/*.whl ${TOP}/notebooks - run_command nvidia-docker build -t cucim-jupyter${gds_postfix} -f ${TOP}/docker/Dockerfile-jupyter${gds_postfix} ${TOP} + run_command docker build --runtime nvidia -t cucim-jupyter${gds_postfix} -f ${TOP}/docker/Dockerfile-jupyter${gds_postfix} ${TOP} [ $? -ne 0 ] && return 1 c_echo W "Port " G "$port" W " would be used...(" B "http://$(hostname -I | cut -d' ' -f 1):${port}" W ")" if [ -z "${gds_postfix}" ]; then - run_command nvidia-docker run --gpus all -it --rm \ + run_command docker run --runtime nvidia --gpus all -it --rm \ -v ${TOP}/notebooks:/notebooks \ -p ${port}:${port} \ cucim-jupyter \