Skip to content

Commit 0518487

Browse files
fschlimbfschlimb
authored
fixing GIL (#93)
* fixing GIL handlding when returning non-NDArray objects * fixing MPI and libz deps * adding relative RPATH so that libidtr and tbb etc are found - independent of python install-dir * fixing SHARPY_IDTR_SO in CI
1 parent 34c9572 commit 0518487

File tree

12 files changed

+129
-65
lines changed

12 files changed

+129
-65
lines changed

‎.github/workflows/ci.yml‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -119,9 +119,8 @@ jobs:
119119
. $GITHUB_WORKSPACE/third_party/install/miniconda/etc/profile.d/conda.sh
120120
. $GITHUB_WORKSPACE/third_party/install/miniconda/bin/activate sharpy
121121
MLIRROOT=$GITHUB_WORKSPACE/third_party/install/llvm-mlir IMEXROOT=$GITHUB_WORKSPACE/third_party/install/imex python -m pip install . -v
122-
# SHARPY_ROOT=`pip show -f sharpy | grep Location | awk '{print $2}'`
123-
# for some reason we cannot use the idtr from the install, not even a copy works
124-
libidtr=`ls ${PWD}/build/lib.linux-x86_64-cpython-*/sharpy/libidtr.so`
122+
libidtr=$(pip show -f sharpy | grep Location | awk '{print $2}' | tr -d '\n')
123+
libidtr=${libidtr}/$(pip show -f sharpy | grep libidtr | awk '{print $1}' | tr -d '\n')
125124
echo "SHARPY_IDTR_SO=${libidtr}" >> $GITHUB_ENV
126125
echo $SHARPY_IDTR_SO
127126
- name: Test SHARPY (pytest)

‎CMakeLists.txt‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,11 +28,11 @@ if(DEFINED ENV{IMEXROOT})
2828
else()
2929
set(IMEX_ROOT ${CONDA_PREFIX})
3030
endif()
31-
message("ll ${IMEX_ROOT}")
3231
if(MKLROOT STREQUAL UNSET OR TBBROOT STREQUAL UNSET)
3332
message(FATAL_ERROR "MKLROOT and TBBROOT not set (nor CONDA_PREFIX)")
3433
endif()
3534

35+
set(ZLIB_LIBRARY /usr/lib/x86_64-linux-gnu/libz.a)
3636
# C++ standard
3737
set(CMAKE_CXX_STANDARD 17)
3838
set(CMAKE_C_EXTENSIONS OFF)
@@ -50,9 +50,13 @@ message(STATUS "Expected IMEX sha: \"${EXPECTED_IMEX_SHA}\"")
5050
# Deps
5151
# ===============
5252
# Find dependent packages like IMEX, Python3 and NumPy
53+
find_package(ZLIB)
5354
find_package(Python3 COMPONENTS Interpreter Development.Module NumPy REQUIRED)
5455
find_package(pybind11 CONFIG)
55-
find_package(MPI REQUIRED)
56+
# FindMPI does not work with iMPI conda packages
57+
set(MPI_INCLUDE_PATH $ENV{I_MPI_ROOT}/include)
58+
set(MPI_CXX_LIBRARIES $ENV{I_MPI_ROOT}/lib/libmpi.so $ENV{I_MPI_ROOT}/lib/libmpicxx.so)
59+
# find_package(MPI REQUIRED CXX)
5660
find_package(LLVM REQUIRED CONFIG NO_SYSTEM_ENVIRONMENT_PATH NO_CMAKE_SYSTEM_PATH)
5761
message(STATUS "Using LLVMConfig.cmake in: ${LLVM_DIR}")
5862
find_package(MLIR REQUIRED CONFIG NO_SYSTEM_ENVIRONMENT_PATH NO_CMAKE_SYSTEM_PATH)
@@ -81,6 +85,12 @@ add_compile_options(
8185
)
8286
add_link_options("-pie" "$<$<CONFIG:RELEASE>:-Wl,-z,relro,-z,now>")
8387

88+
SET(CMAKE_SKIP_BUILD_RPATH FALSE)
89+
SET(CMAKE_BUILD_WITH_INSTALL_RPATH TRUE)
90+
SET(CMAKE_INSTALL_RPATH_USE_LINK_PATH FALSE)
91+
# we assume the typical python site-packages layout
92+
SET(CMAKE_INSTALL_RPATH "$ORIGIN;$ORIGIN/../../..")
93+
8494
#find_package(OpenMP)
8595

8696
set(MKL_LIBRARIES -L${MKLROOT}/lib -lmkl_intel_lp64 -lmkl_tbb_thread -lmkl_core -ltbb -lpthread -lrt -ldl -lm)
@@ -182,6 +192,7 @@ target_link_libraries(_sharpy PRIVATE
182192
LLVMX86AsmParser
183193
idtr
184194
tbb
195+
${ZLIB_LIBRARIES}
185196
)
186197
target_link_libraries(idtr PRIVATE
187198
${MPI_CXX_LIBRARIES}

‎setup.py‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,11 +26,10 @@ def build_cmake(self, ext):
2626
build_temp.mkdir(parents=True, exist_ok=True)
2727
extdir = pathlib.Path(self.get_ext_fullpath(ext.name))
2828
extdir.parent.mkdir(parents=True, exist_ok=True)
29-
3029
# example of cmake args
3130
config = "Debug" # if self.debug else 'RelWithDebInfo' #'Release'
3231
cmake_args = [
33-
"-DCMAKE_LIBRARY_OUTPUT_DIRECTORY=" + str(extdir.parent.absolute()),
32+
f"-DCMAKE_LIBRARY_OUTPUT_DIRECTORY={extdir.parent.absolute()}",
3433
"-DCMAKE_BUILD_TYPE=" + config,
3534
"-DCMAKE_VERBOSE_MAKEFILE=ON",
3635
"-G=Ninja",

‎src/IO.cpp‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,11 @@
77
#include "sharpy/IO.hpp"
88
#include "sharpy/Factory.hpp"
99
#include "sharpy/NDArray.hpp"
10+
#include "sharpy/PyTypes.hpp"
1011
#include "sharpy/SetGetItem.hpp"
1112
#include "sharpy/Transceiver.hpp"
1213
#include "sharpy/TypeDispatch.hpp"
1314

14-
#include <pybind11/numpy.h>
15-
#include <pybind11/pybind11.h>
16-
namespace py = pybind11;
17-
1815
namespace SHARPY {
1916

2017
// ***************************************************************************

‎src/NDArray.cpp‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,10 @@
1212
#include <algorithm>
1313
#include <iostream>
1414

15+
#include <pybind11/pybind11.h>
16+
#include <pybind11/stl.h>
17+
namespace py = pybind11;
18+
1519
namespace SHARPY {
1620

1721
NDArray::NDArray(id_type guid_, DTypeId dtype_, shape_type gShape,
@@ -132,8 +136,14 @@ void NDArray::NDADeleter::operator()(NDArray *a) const {
132136
}
133137

134138
NDArray::~NDArray() {
135-
if (_base)
136-
delete _base;
139+
if (_base) {
140+
if (_base->needGIL()) {
141+
py::gil_scoped_acquire acquire;
142+
delete _base;
143+
} else {
144+
delete _base;
145+
}
146+
}
137147
}
138148

139149
// **************************************************************************

‎src/SetGetItem.cpp‎

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -32,22 +32,23 @@ namespace py = pybind11;
3232
namespace SHARPY {
3333

3434
template <typename T> struct mk_array {
35-
template <typename C> static py::object op(C &&shp, void *&outPtr) {
35+
template <typename C> static py::handle op(C &&shp, void *&outPtr) {
3636
auto ary = py::array_t<T>(std::forward<C>(shp));
3737
outPtr = ary.mutable_data();
38-
return ary;
38+
return ary.release();
3939
}
4040
};
4141

4242
template <typename T> struct wrap_array {
4343
template <typename C, typename S>
44-
static py::object op(C &&shp, S &&str, void *data, const py::handle &handle) {
44+
static py::handle op(C &&shp, S &&str, void *data, const py::handle &handle) {
4545
return py::array(std::forward<C>(shp), std::forward<S>(str),
46-
reinterpret_cast<T *>(data), handle);
46+
reinterpret_cast<T *>(data), handle)
47+
.release();
4748
}
4849
};
4950

50-
py::object wrap(NDArray::ptr_type tnsr, const py::handle &handle) {
51+
py::handle wrap(NDArray::ptr_type tnsr, const py::handle &handle) {
5152
auto tmp_shp = tnsr->local_shape();
5253
auto tmp_str = tnsr->local_strides();
5354
auto nd = tnsr->ndims();
@@ -71,14 +72,22 @@ struct DeferredGetLocals
7172

7273
DeferredGetLocals() = default;
7374
DeferredGetLocals(const array_i::future_type &a, py::handle &handle)
74-
: _a(a.guid()), _handle(handle) {}
75+
: _a(a.guid()), _handle(handle) {
76+
py::gil_scoped_acquire acquire;
77+
_handle.inc_ref();
78+
}
79+
~DeferredGetLocals() {
80+
py::gil_scoped_acquire acquire;
81+
_handle.dec_ref();
82+
}
7583

7684
void run() override {
7785
auto aa = std::move(Registry::get(_a).get());
7886
auto a_ptr = std::dynamic_pointer_cast<NDArray>(aa);
7987
assert(a_ptr);
8088
auto res = wrap(a_ptr, _handle);
81-
set_value(py::make_tuple(res));
89+
auto tpl = py::make_tuple(py::reinterpret_steal<py::object>(res));
90+
set_value(tpl.release());
8291
}
8392

8493
bool generate_mlir(::mlir::OpBuilder &builder, const ::mlir::Location &loc,
@@ -116,7 +125,7 @@ struct DeferredGather
116125
bool sendonly = _root != REPLICATED && _root != myrank;
117126

118127
void *outPtr = nullptr;
119-
py::object res;
128+
py::handle res;
120129
if (!sendonly || !trscvr) {
121130
auto tmp = a_ptr->shape();
122131
std::vector<ssize_t> tmpv(tmp, &tmp[a_ptr->ndims()]);

‎src/_sharpy.cpp‎

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -119,17 +119,22 @@ void init(bool cw) {
119119

120120
// #########################################################################
121121

122-
/// trigger compile&run and return future value
122+
/// trigger compile&run and return python object
123123
#define PY_SYNC_RETURN(_f) \
124-
int vtWaitSym, vtSHARPYClass; \
125-
VT(VT_classdef, "sharpy", &vtSHARPYClass); \
126-
VT(VT_funcdef, "wait", vtSHARPYClass, &vtWaitSym); \
127-
VT(VT_begin, vtWaitSym); \
128-
py::gil_scoped_release release; \
129-
Service::run(); \
130-
auto r = (_f).get(); \
131-
VT(VT_end, vtWaitSym); \
132-
return r
124+
{ \
125+
int vtWaitSym, vtSHARPYClass; \
126+
VT(VT_classdef, "sharpy", &vtSHARPYClass); \
127+
VT(VT_funcdef, "wait", vtSHARPYClass, &vtWaitSym); \
128+
VT(VT_begin, vtWaitSym); \
129+
py::handle res; \
130+
{ \
131+
py::gil_scoped_release release; \
132+
Service::run(); \
133+
res = (_f).get(); \
134+
} \
135+
VT(VT_end, vtWaitSym); \
136+
return py::reinterpret_steal<py::object>(res); \
137+
}
133138

134139
/// trigger compile&run and return given attribute _x
135140
#define SYNC_RETURN(_f, _a) \
@@ -139,7 +144,7 @@ void init(bool cw) {
139144
VT(VT_begin, vtWaitSym); \
140145
py::gil_scoped_release release; \
141146
Service::run(); \
142-
auto r = (_f).get().get()->_a(); \
147+
auto r = (_f).get().get() -> _a(); \
143148
VT(VT_end, vtWaitSym); \
144149
return r
145150

‎src/idtr.cpp‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -352,13 +352,16 @@ using MRIdx1d = SHARPY::Unranked1DMemRefType<int64_t>;
352352
// FIXME hard-coded for contiguous layout
353353
template <typename T>
354354
void _idtr_reduce_all(int64_t dataRank, void *dataDescr, int op) {
355+
auto tc = SHARPY::getTransceiver();
356+
if (!tc)
357+
return;
355358
SHARPY::UnrankedMemRefType<T> data(dataRank, dataDescr);
356359
assert(dataRank == 0 || (dataRank == 1 && data.strides()[0] == 1));
357360
auto d = data.data();
358361
auto t = SHARPY::DTYPE<T>::value;
359362
auto r = dataRank ? data.sizes()[0] : 1;
360363
auto o = mlir2sharpy(static_cast<imex::ndarray::ReduceOpId>(op));
361-
SHARPY::getTransceiver()->reduce_all(d, t, r, o);
364+
tc->reduce_all(d, t, r, o);
362365
}
363366

364367
extern "C" {

‎src/include/sharpy/CppTypes.hpp‎

Lines changed: 54 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,27 @@ using InputAdapter = bitsery::InputBufferAdapter<Buffer>;
2828
using Serializer = bitsery::Serializer<OutputAdapter>;
2929
using Deserializer = bitsery::Deserializer<InputAdapter>;
3030

31+
/// @brief use this to provide a base object to the array
32+
// such a base object can own shared data
33+
// you might need to implement reference counting
34+
struct BaseObj {
35+
virtual ~BaseObj() {}
36+
virtual bool needGIL() const = 0;
37+
};
38+
39+
/// @brief Simple implementation of BaseObj for ref-counting types
40+
/// @tparam T ref-counting type, such as py::object of std::shared_Ptr
41+
/// we keep an object of the ref-counting type. Normal ref-counting/destructors
42+
/// will take care of the rest.
43+
template <typename T> struct SharedBaseObject : public BaseObj {
44+
SharedBaseObject(const SharedBaseObject &) = default;
45+
SharedBaseObject(SharedBaseObject &&) = default;
46+
SharedBaseObject(const T &o) : _base(o) {}
47+
SharedBaseObject(T &&o) : _base(std::forward<T>(o)) {}
48+
virtual bool needGIL() const { return false; }
49+
T _base;
50+
};
51+
3152
union PyScalar {
3253
int64_t _int;
3354
double _float;
@@ -42,17 +63,39 @@ enum _RANKS : rank_type {
4263
};
4364

4465
template <typename T> struct DTYPE {};
45-
template <> struct DTYPE<double> { constexpr static DTypeId value = FLOAT64; };
46-
template <> struct DTYPE<float> { constexpr static DTypeId value = FLOAT32; };
47-
template <> struct DTYPE<int64_t> { constexpr static DTypeId value = INT64; };
48-
template <> struct DTYPE<int32_t> { constexpr static DTypeId value = INT32; };
49-
template <> struct DTYPE<int16_t> { constexpr static DTypeId value = INT16; };
50-
template <> struct DTYPE<int8_t> { constexpr static DTypeId value = INT8; };
51-
template <> struct DTYPE<uint64_t> { constexpr static DTypeId value = UINT64; };
52-
template <> struct DTYPE<uint32_t> { constexpr static DTypeId value = UINT32; };
53-
template <> struct DTYPE<uint16_t> { constexpr static DTypeId value = UINT16; };
54-
template <> struct DTYPE<uint8_t> { constexpr static DTypeId value = UINT8; };
55-
template <> struct DTYPE<bool> { constexpr static DTypeId value = BOOL; };
66+
template <> struct DTYPE<double> {
67+
constexpr static DTypeId value = FLOAT64;
68+
};
69+
template <> struct DTYPE<float> {
70+
constexpr static DTypeId value = FLOAT32;
71+
};
72+
template <> struct DTYPE<int64_t> {
73+
constexpr static DTypeId value = INT64;
74+
};
75+
template <> struct DTYPE<int32_t> {
76+
constexpr static DTypeId value = INT32;
77+
};
78+
template <> struct DTYPE<int16_t> {
79+
constexpr static DTypeId value = INT16;
80+
};
81+
template <> struct DTYPE<int8_t> {
82+
constexpr static DTypeId value = INT8;
83+
};
84+
template <> struct DTYPE<uint64_t> {
85+
constexpr static DTypeId value = UINT64;
86+
};
87+
template <> struct DTYPE<uint32_t> {
88+
constexpr static DTypeId value = UINT32;
89+
};
90+
template <> struct DTYPE<uint16_t> {
91+
constexpr static DTypeId value = UINT16;
92+
};
93+
template <> struct DTYPE<uint8_t> {
94+
constexpr static DTypeId value = UINT8;
95+
};
96+
template <> struct DTYPE<bool> {
97+
constexpr static DTypeId value = BOOL;
98+
};
5699

57100
template <DTypeId DT> struct TYPE {};
58101
template <> struct TYPE<FLOAT64> {

‎src/include/sharpy/NDArray.hpp‎

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -19,25 +19,6 @@
1919
namespace SHARPY {
2020
class Transceiver;
2121

22-
/// @brief use this to provide a base object to the array
23-
// such a base object can own shared data
24-
// you might need to implement reference counting
25-
struct BaseObj {
26-
virtual ~BaseObj() {}
27-
};
28-
29-
/// @brief Simple implementation of BaseObj for ref-counting types
30-
/// @tparam T ref-counting type, such as py::object of std::shared_Ptr
31-
/// we keep an object of the ref-counting type. Normal ref-counting/destructors
32-
/// will take care of the rest.
33-
template <typename T> struct SharedBaseObject : public BaseObj {
34-
SharedBaseObject(const SharedBaseObject &) = default;
35-
SharedBaseObject(SharedBaseObject &&) = default;
36-
SharedBaseObject(const T &o) : _base(o) {}
37-
SharedBaseObject(T &&o) : _base(std::forward<T>(o)) {}
38-
T _base;
39-
};
40-
4122
/// The actual implementation of the SHARPYensor, implementing the array_i
4223
/// interface. It holds the array data and some meta information. The member
4324
/// attributes are mostly inspired by the needs of interacting with MLIR. It

0 commit comments

Comments
 (0)