Skip to content

Commit 4bd7c82

Browse files
fschlimbfschlimb
committed
Revert "fixing GIL handlding when returning non-NDArray objects"
This reverts commit 3504f76.
1 parent 3504f76 commit 4bd7c82

File tree

8 files changed

+57
-109
lines changed

8 files changed

+57
-109
lines changed

‎src/IO.cpp‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,14 @@
77
#include "sharpy/IO.hpp"
88
#include "sharpy/Factory.hpp"
99
#include "sharpy/NDArray.hpp"
10-
#include "sharpy/PyTypes.hpp"
1110
#include "sharpy/SetGetItem.hpp"
1211
#include "sharpy/Transceiver.hpp"
1312
#include "sharpy/TypeDispatch.hpp"
1413

14+
#include <pybind11/numpy.h>
15+
#include <pybind11/pybind11.h>
16+
namespace py = pybind11;
17+
1518
namespace SHARPY {
1619

1720
// ***************************************************************************

‎src/NDArray.cpp‎

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

15-
#include <pybind11/pybind11.h>
16-
#include <pybind11/stl.h>
17-
namespace py = pybind11;
18-
1915
namespace SHARPY {
2016

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

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

149139
// **************************************************************************

‎src/SetGetItem.cpp‎

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

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

4242
template <typename T> struct wrap_array {
4343
template <typename C, typename S>
44-
static py::handle op(C &&shp, S &&str, void *data, const py::handle &handle) {
44+
static py::object 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)
47-
.release();
46+
reinterpret_cast<T *>(data), handle);
4847
}
4948
};
5049

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

7372
DeferredGetLocals() = default;
7473
DeferredGetLocals(const array_i::future_type &a, py::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-
}
74+
: _a(a.guid()), _handle(handle) {}
8375

8476
void run() override {
8577
auto aa = std::move(Registry::get(_a).get());
8678
auto a_ptr = std::dynamic_pointer_cast<NDArray>(aa);
8779
assert(a_ptr);
8880
auto res = wrap(a_ptr, _handle);
89-
auto tpl = py::make_tuple(py::reinterpret_steal<py::object>(res));
90-
set_value(tpl.release());
81+
set_value(py::make_tuple(res));
9182
}
9283

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

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

‎src/_sharpy.cpp‎

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

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

122-
/// trigger compile&run and return python object
122+
/// trigger compile&run and return future value
123123
#define PY_SYNC_RETURN(_f) \
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-
}
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
138133

139134
/// trigger compile&run and return given attribute _x
140135
#define SYNC_RETURN(_f, _a) \
@@ -144,7 +139,7 @@ void init(bool cw) {
144139
VT(VT_begin, vtWaitSym); \
145140
py::gil_scoped_release release; \
146141
Service::run(); \
147-
auto r = (_f).get().get() -> _a(); \
142+
auto r = (_f).get().get()->_a(); \
148143
VT(VT_end, vtWaitSym); \
149144
return r
150145

‎src/include/sharpy/CppTypes.hpp‎

Lines changed: 11 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -28,27 +28,6 @@ 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-
5231
union PyScalar {
5332
int64_t _int;
5433
double _float;
@@ -63,39 +42,17 @@ enum _RANKS : rank_type {
6342
};
6443

6544
template <typename T> struct DTYPE {};
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-
};
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; };
9956

10057
template <DTypeId DT> struct TYPE {};
10158
template <> struct TYPE<FLOAT64> {

‎src/include/sharpy/NDArray.hpp‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,25 @@
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+
2241
/// The actual implementation of the SHARPYensor, implementing the array_i
2342
/// interface. It holds the array data and some meta information. The member
2443
/// attributes are mostly inspired by the needs of interacting with MLIR. It

‎src/include/sharpy/PyTypes.hpp‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,6 @@ namespace py = pybind11;
1111

1212
namespace SHARPY {
1313

14-
template <> inline bool SharedBaseObject<py::object>::needGIL() const {
15-
return true;
16-
}
17-
template <> inline bool SharedBaseObject<py::handle>::needGIL() const {
18-
return true;
19-
}
20-
2114
template <typename T> py::object get_impl_dtype() {
2215
return get_impl_dtype(DTYPE<T>::value);
2316
};

‎src/include/sharpy/SetGetItem.hpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,8 @@
1414
namespace SHARPY {
1515

1616
struct GetItem {
17-
using py_promise_type = std::promise<py::handle>;
18-
using py_future_type = std::shared_future<py::handle>;
17+
using py_promise_type = std::promise<py::object>;
18+
using py_future_type = std::shared_future<py::object>;
1919

2020
static FutureArray *__getitem__(const FutureArray &a,
2121
const std::vector<py::slice> &v);

0 commit comments

Comments
 (0)