Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refcounting Smart Pointers #702

Open
wants to merge 18 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
d7ac59e
Add smart pointer classes for managing pyobject pointers.
insertinterestingnamehere May 24, 2016
844d532
Update dynd-python codebase to use new py_ref class.
insertinterestingnamehere May 24, 2016
3658354
Fix MSVC compilation error.
insertinterestingnamehere Jun 3, 2016
acca45b
Rewrite smart pointers that encapsulate Python references.
insertinterestingnamehere Jun 4, 2016
ecc96d3
Add RAII class to release the GIL.
insertinterestingnamehere Jun 6, 2016
fb7d0eb
Add explicit syntax for borrowing a reference from a wrapped PyObject*.
insertinterestingnamehere Jun 9, 2016
791696a
Add convenience function for null-checking a smart pointer wrapping a
insertinterestingnamehere Jun 9, 2016
24d4a32
Add a convenience function to aid in casting from a possibly null
insertinterestingnamehere Jun 9, 2016
4d0d38e
Add a series of assert statements to ensure that wrapped references
insertinterestingnamehere Jun 9, 2016
ebeb883
Make release method for py_ref return an owned or borrowed reference
insertinterestingnamehere Jun 9, 2016
059e012
Move the release method of pydynd::py_ref_tmpl to be a function that …
insertinterestingnamehere Jun 9, 2016
7f2eaba
Rename py_ref_tmpl to py_ref_t.
insertinterestingnamehere Jun 9, 2016
495faf4
Add a method to get an owned reference from a given reference.
insertinterestingnamehere Jun 9, 2016
a1aa405
Add method to py_ref_t to create a new owned reference from a given
insertinterestingnamehere Jun 9, 2016
7962d97
Add method to py_ref_t to copy the current reference.
insertinterestingnamehere Jun 9, 2016
fc522f4
Fix typo in disallow_null where release was not correctly used.
insertinterestingnamehere Jun 10, 2016
9e8bf42
Close a reference leak in smart pointers for managing PyObject pointers.
insertinterestingnamehere Jun 22, 2016
4d9ec89
Add more commenting to the new py_ref classes.
insertinterestingnamehere Aug 3, 2016
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 29 additions & 29 deletions dynd/include/callables/apply_pyobject_callable.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,35 +24,35 @@ namespace nd {
return dst_tp;
}

/*
void instantiate(dynd::nd::call_node *&node, char *DYND_UNUSED(data), dynd::nd::kernel_builder *ckb,
const dynd::ndt::type &dst_tp, const char *dst_arrmeta, intptr_t nsrc,
const dynd::ndt::type *src_tp, const char *const *src_arrmeta, dynd::kernel_request_t kernreq,
intptr_t nkwd, const dynd::nd::array *kwds,
const std::map<std::string, dynd::ndt::type> &tp_vars)
{
pydynd::PyGILState_RAII pgs;

std::vector<dynd::ndt::type> src_tp_copy(nsrc);
for (int i = 0; i < nsrc; ++i) {
src_tp_copy[i] = src_tp[i];
}

intptr_t ckb_offset = ckb->size();
ckb->emplace_back<apply_pyobject_kernel>(kernreq);
apply_pyobject_kernel *self = ckb->get_at<apply_pyobject_kernel>(ckb_offset);
self->m_proto = dynd::ndt::callable_type::make(dst_tp, src_tp_copy);
self->m_pyfunc = func;
Py_XINCREF(self->m_pyfunc);
self->m_dst_arrmeta = dst_arrmeta;
self->m_src_arrmeta.resize(nsrc);
copy(src_arrmeta, src_arrmeta + nsrc, self->m_src_arrmeta.begin());

dynd::ndt::type child_src_tp = dynd::ndt::make_type<pyobject_type>();
dynd::nd::assign->instantiate(node, nullptr, ckb, dst_tp, dst_arrmeta, 1, &child_src_tp, nullptr,
dynd::kernel_request_single, 0, nullptr, tp_vars);
}
*/
/*
void instantiate(dynd::nd::call_node *&node, char *DYND_UNUSED(data), dynd::nd::kernel_builder *ckb,
const dynd::ndt::type &dst_tp, const char *dst_arrmeta, intptr_t nsrc,
const dynd::ndt::type *src_tp, const char *const *src_arrmeta, dynd::kernel_request_t kernreq,
intptr_t nkwd, const dynd::nd::array *kwds,
const std::map<std::string, dynd::ndt::type> &tp_vars)
{
pydynd::with_gil pgs;

std::vector<dynd::ndt::type> src_tp_copy(nsrc);
for (int i = 0; i < nsrc; ++i) {
src_tp_copy[i] = src_tp[i];
}

intptr_t ckb_offset = ckb->size();
ckb->emplace_back<apply_pyobject_kernel>(kernreq);
apply_pyobject_kernel *self = ckb->get_at<apply_pyobject_kernel>(ckb_offset);
self->m_proto = dynd::ndt::callable_type::make(dst_tp, src_tp_copy);
self->m_pyfunc = func;
Py_XINCREF(self->m_pyfunc);
self->m_dst_arrmeta = dst_arrmeta;
self->m_src_arrmeta.resize(nsrc);
copy(src_arrmeta, src_arrmeta + nsrc, self->m_src_arrmeta.begin());

dynd::ndt::type child_src_tp = dynd::ndt::make_type<pyobject_type>();
dynd::nd::assign->instantiate(node, nullptr, ckb, dst_tp, dst_arrmeta, 1, &child_src_tp, nullptr,
dynd::kernel_request_single, 0, nullptr, tp_vars);
}
*/
};

} // namespace pydynd::nd::functional
Expand Down
219 changes: 110 additions & 109 deletions dynd/include/callables/assign_to_pyarrayobject_callable.hpp
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#pragma once

#include <dynd/callables/base_callable.hpp>
#include "kernels/assign_to_pyarrayobject_kernel.hpp"
#include <dynd/callables/base_callable.hpp>

/**
* This sets up a ckernel to copy from a dynd array
Expand All @@ -22,127 +22,128 @@ class assign_to_pyarrayobject_callable : public dynd::nd::base_callable {
return dst_tp;
}

/*
void instantiate(char *data, dynd::nd::kernel_builder *ckb, const dynd::ndt::type &dst_tp, const char *dst_arrmeta,
intptr_t nsrc, const dynd::ndt::type *src_tp, const char *const *src_arrmeta,
dynd::kernel_request_t kernreq, intptr_t nkwd, const dynd::nd::array *kwds,
const std::map<std::string, dynd::ndt::type> &tp_vars)
{
PyObject *dst_obj = *reinterpret_cast<PyObject *const *>(dst_arrmeta);
uintptr_t dst_alignment = reinterpret_cast<const uintptr_t *>(dst_arrmeta)[1];
/*
void instantiate(char *data, dynd::nd::kernel_builder *ckb, const dynd::ndt::type &dst_tp, const char *dst_arrmeta,
intptr_t nsrc, const dynd::ndt::type *src_tp, const char *const *src_arrmeta,
dynd::kernel_request_t kernreq, intptr_t nkwd, const dynd::nd::array *kwds,
const std::map<std::string, dynd::ndt::type> &tp_vars)
{
PyObject *dst_obj = *reinterpret_cast<PyObject *const *>(dst_arrmeta);
uintptr_t dst_alignment = reinterpret_cast<const uintptr_t *>(dst_arrmeta)[1];

PyArray_Descr *dtype = reinterpret_cast<PyArray_Descr *>(dst_obj);

// If there is no object type in the numpy type, get the dynd equivalent
// type and use it to do the copying
if (!PyDataType_FLAGCHK(dtype, NPY_ITEM_HASOBJECT)) {
dynd::ndt::type dst_view_tp = pydynd::_type_from_numpy_dtype(dtype, dst_alignment);
nd::array error_mode = assign_error_fractional;
nd::assign->instantiate(node, NULL, ckb, dst_view_tp, NULL, 1, src_tp, src_arrmeta, kernreq, 1, &error_mode,
std::map<std::string, ndt::type>());
return;
}
PyArray_Descr *dtype = reinterpret_cast<PyArray_Descr *>(dst_obj);

if (PyDataType_ISOBJECT(dtype)) {
dynd::nd::assign->instantiate(node, NULL, ckb, dynd::ndt::make_type<pyobject_type>(), NULL, nsrc, src_tp,
src_arrmeta, kernreq, 0, NULL, tp_vars);
return;
}

if (PyDataType_HASFIELDS(dtype)) {
if (src_tp[0].get_id() != dynd::struct_id && src_tp[0].get_id() != dynd::tuple_id) {
std::stringstream ss;
pydynd::pyobject_ownref dtype_str(PyObject_Str((PyObject *)dtype));
ss << "Cannot assign from source dynd type " << src_tp[0] << " to numpy type "
<< pydynd::pystring_as_string(dtype_str.get());
throw std::invalid_argument(ss.str());
// If there is no object type in the numpy type, get the dynd equivalent
// type and use it to do the copying
if (!PyDataType_FLAGCHK(dtype, NPY_ITEM_HASOBJECT)) {
dynd::ndt::type dst_view_tp = pydynd::_type_from_numpy_dtype(dtype, dst_alignment);
nd::array error_mode = assign_error_fractional;
nd::assign->instantiate(node, NULL, ckb, dst_view_tp, NULL, 1, src_tp, src_arrmeta, kernreq, 1, &error_mode,
std::map<std::string, ndt::type>());
return;
}

// Get the fields out of the numpy dtype
std::vector<PyArray_Descr *> field_dtypes_orig;
std::vector<std::string> field_names_orig;
std::vector<size_t> field_offsets_orig;
pydynd::extract_fields_from_numpy_struct(dtype, field_dtypes_orig, field_names_orig, field_offsets_orig);
intptr_t field_count = field_dtypes_orig.size();
if (field_count != src_tp[0].extended<dynd::ndt::tuple_type>()->get_field_count()) {
std::stringstream ss;
pydynd::pyobject_ownref dtype_str(PyObject_Str((PyObject *)dtype));
ss << "Cannot assign from source dynd type " << src_tp[0] << " to numpy type "
<< pydynd::pystring_as_string(dtype_str.get());
throw std::invalid_argument(ss.str());
if (PyDataType_ISOBJECT(dtype)) {
dynd::nd::assign->instantiate(node, NULL, ckb, dynd::ndt::make_type<pyobject_type>(), NULL, nsrc, src_tp,
src_arrmeta, kernreq, 0, NULL, tp_vars);
return;
}

// Permute the numpy fields to match with the dynd fields
std::vector<PyArray_Descr *> field_dtypes;
std::vector<size_t> field_offsets;
if (src_tp[0].get_id() == dynd::struct_id) {
field_dtypes.resize(field_count);
field_offsets.resize(field_count);
for (intptr_t i = 0; i < field_count; ++i) {
intptr_t src_i = src_tp[0].extended<dynd::ndt::struct_type>()->get_field_index(field_names_orig[i]);
if (src_i >= 0) {
field_dtypes[src_i] = field_dtypes_orig[i];
field_offsets[src_i] = field_offsets_orig[i];
}
else {
std::stringstream ss;
pydynd::pyobject_ownref dtype_str(PyObject_Str((PyObject *)dtype));
ss << "Cannot assign from source dynd type " << src_tp[0] << " to numpy type "
<< pydynd::pystring_as_string(dtype_str.get());
throw std::invalid_argument(ss.str());
if (PyDataType_HASFIELDS(dtype)) {
if (src_tp[0].get_id() != dynd::struct_id && src_tp[0].get_id() != dynd::tuple_id) {
std::stringstream ss;
pydynd::py_ref dtype_str = capture_if_not_null(PyObject_Str((PyObject *)dtype));
ss << "Cannot assign from source dynd type " << src_tp[0] << " to numpy type "
<< pydynd::pystring_as_string(dtype_str.get());
throw std::invalid_argument(ss.str());
}

// Get the fields out of the numpy dtype
std::vector<PyArray_Descr *> field_dtypes_orig;
std::vector<std::string> field_names_orig;
std::vector<size_t> field_offsets_orig;
pydynd::extract_fields_from_numpy_struct(dtype, field_dtypes_orig, field_names_orig, field_offsets_orig);
intptr_t field_count = field_dtypes_orig.size();
if (field_count != src_tp[0].extended<dynd::ndt::tuple_type>()->get_field_count()) {
std::stringstream ss;
pydynd::py_ref dtype_str = capture_if_not_null(PyObject_Str((PyObject *)dtype));
ss << "Cannot assign from source dynd type " << src_tp[0] << " to numpy type "
<< pydynd::pystring_as_string(dtype_str.get());
throw std::invalid_argument(ss.str());
}

// Permute the numpy fields to match with the dynd fields
std::vector<PyArray_Descr *> field_dtypes;
std::vector<size_t> field_offsets;
if (src_tp[0].get_id() == dynd::struct_id) {
field_dtypes.resize(field_count);
field_offsets.resize(field_count);
for (intptr_t i = 0; i < field_count; ++i) {
intptr_t src_i = src_tp[0].extended<dynd::ndt::struct_type>()->get_field_index(field_names_orig[i]);
if (src_i >= 0) {
field_dtypes[src_i] = field_dtypes_orig[i];
field_offsets[src_i] = field_offsets_orig[i];
}
else {
std::stringstream ss;
pydynd::py_ref dtype_str = capture_if_not_null(PyObject_Str((PyObject *)dtype));
ss << "Cannot assign from source dynd type " << src_tp[0] << " to numpy type "
<< pydynd::pystring_as_string(dtype_str.get());
throw std::invalid_argument(ss.str());
}
}
}
}
else {
// In the tuple case, use position instead of name
field_dtypes.swap(field_dtypes_orig);
field_offsets.swap(field_offsets_orig);
}
else {
// In the tuple case, use position instead of name
field_dtypes.swap(field_dtypes_orig);
field_offsets.swap(field_offsets_orig);
}

std::vector<dynd::ndt::type> dst_fields_tp(field_count, dynd::ndt::make_type<void>());
std::vector<copy_to_numpy_arrmeta> dst_arrmeta_values(field_count);
std::vector<const char *> dst_fields_arrmeta(field_count);
for (intptr_t i = 0; i < field_count; ++i) {
dst_arrmeta_values[i].dst_dtype = field_dtypes[i];
dst_arrmeta_values[i].dst_alignment = dst_alignment | field_offsets[i];
dst_fields_arrmeta[i] = reinterpret_cast<const char *>(&dst_arrmeta_values[i]);
}
std::vector<dynd::ndt::type> dst_fields_tp(field_count, dynd::ndt::make_type<void>());
std::vector<copy_to_numpy_arrmeta> dst_arrmeta_values(field_count);
std::vector<const char *> dst_fields_arrmeta(field_count);
for (intptr_t i = 0; i < field_count; ++i) {
dst_arrmeta_values[i].dst_dtype = field_dtypes[i];
dst_arrmeta_values[i].dst_alignment = dst_alignment | field_offsets[i];
dst_fields_arrmeta[i] = reinterpret_cast<const char *>(&dst_arrmeta_values[i]);
}

const uintptr_t *src_arrmeta_offsets = src_tp[0].extended<dynd::ndt::tuple_type>()->get_arrmeta_offsets_raw();
dynd::shortvector<const char *> src_fields_arrmeta(field_count);
for (intptr_t i = 0; i != field_count; ++i) {
src_fields_arrmeta[i] = src_arrmeta[0] + src_arrmeta_offsets[i];
}
const uintptr_t *src_arrmeta_offsets = src_tp[0].extended<dynd::ndt::tuple_type>()->get_arrmeta_offsets_raw();
dynd::shortvector<const char *> src_fields_arrmeta(field_count);
for (intptr_t i = 0; i != field_count; ++i) {
src_fields_arrmeta[i] = src_arrmeta[0] + src_arrmeta_offsets[i];
}

// Todo: Remove this
dynd::nd::callable af = dynd::nd::make_callable<assign_to_pyarrayobject_callable>();
// Todo: Remove this
dynd::nd::callable af = dynd::nd::make_callable<assign_to_pyarrayobject_callable>();

const std::vector<ndt::type> &src_field_tp = src_tp[0].extended<dynd::ndt::tuple_type>()->get_field_types();
const uintptr_t *src_data_offsets = src_tp[0].extended<dynd::ndt::tuple_type>()->get_data_offsets(src_arrmeta[0]);
const std::vector<ndt::type> &src_field_tp = src_tp[0].extended<dynd::ndt::tuple_type>()->get_field_types();
const uintptr_t *src_data_offsets =
src_tp[0].extended<dynd::ndt::tuple_type>()->get_data_offsets(src_arrmeta[0]);

intptr_t self_offset = ckb->size();
ckb->emplace_back<nd::tuple_unary_op_ck>(kernreq);
nd::tuple_unary_op_ck *self = ckb->get_at<nd::tuple_unary_op_ck>(self_offset);
self->m_fields.resize(field_count);
for (intptr_t i = 0; i < field_count; ++i) {
self = ckb->get_at<nd::tuple_unary_op_ck>(self_offset);
nd::tuple_unary_op_item &field = self->m_fields[i];
field.child_kernel_offset = ckb->size() - self_offset;
field.dst_data_offset = field_offsets[i];
field.src_data_offset = src_data_offsets[i];
nd::array error_mode = ndt::traits<assign_error_mode>::na();
af->instantiate(node, NULL, ckb, dst_fields_tp[i], dst_fields_arrmeta[i], 1, &src_field_tp[i],
&src_fields_arrmeta[i], kernel_request_single, 1, &error_mode,
std::map<std::string, ndt::type>());
intptr_t self_offset = ckb->size();
ckb->emplace_back<nd::tuple_unary_op_ck>(kernreq);
nd::tuple_unary_op_ck *self = ckb->get_at<nd::tuple_unary_op_ck>(self_offset);
self->m_fields.resize(field_count);
for (intptr_t i = 0; i < field_count; ++i) {
self = ckb->get_at<nd::tuple_unary_op_ck>(self_offset);
nd::tuple_unary_op_item &field = self->m_fields[i];
field.child_kernel_offset = ckb->size() - self_offset;
field.dst_data_offset = field_offsets[i];
field.src_data_offset = src_data_offsets[i];
nd::array error_mode = ndt::traits<assign_error_mode>::na();
af->instantiate(node, NULL, ckb, dst_fields_tp[i], dst_fields_arrmeta[i], 1, &src_field_tp[i],
&src_fields_arrmeta[i], kernel_request_single, 1, &error_mode,
std::map<std::string, ndt::type>());
}
return;
}
else {
std::stringstream ss;
ss << "TODO: implement assign from source dynd type " << src_tp[0] << " to numpy type "
<< pydynd::pyobject_repr((PyObject *)dtype);
throw std::invalid_argument(ss.str());
}
return;
}
else {
std::stringstream ss;
ss << "TODO: implement assign from source dynd type " << src_tp[0] << " to numpy type "
<< pydynd::pyobject_repr((PyObject *)dtype);
throw std::invalid_argument(ss.str());
}
}
*/
*/
};
14 changes: 8 additions & 6 deletions dynd/include/callables/assign_to_pyobject_callable.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -289,11 +289,12 @@ namespace nd {
ckb_offset = kb.size();
self_ck->m_src_tp = src0_tp;
self_ck->m_src_arrmeta = src_arrmeta[0];
self_ck->m_field_names.reset(PyTuple_New(field_count));
self_ck->m_field_names = capture_if_not_null(PyTuple_New(field_count));
for (intptr_t i = 0; i < field_count; ++i) {
const dynd::string &rawname = src0_tp.extended<dynd::ndt::struct_type>()->get_field_name(i);
pydynd::pyobject_ownref name(PyUnicode_DecodeUTF8(rawname.begin(), rawname.end() - rawname.begin(), NULL));
PyTuple_SET_ITEM(self_ck->m_field_names.get(), i, name.release());
pydynd::py_ref name =
capture_if_not_null(PyUnicode_DecodeUTF8(rawname.begin(), rawname.end() - rawname.begin(), NULL));
PyTuple_SET_ITEM(self_ck->m_field_names.get(), i, pydynd::release(std::move(name)));
}
self_ck->m_copy_el_offsets.resize(field_count);

Expand Down Expand Up @@ -333,11 +334,12 @@ namespace nd {
intptr_t field_count = src_tp[0].extended<dynd::ndt::struct_type>()->get_field_count();
const dynd::ndt::type *field_types = src_tp[0].extended<dynd::ndt::struct_type>()->get_field_types_raw();
const uintptr_t *arrmeta_offsets = src_tp[0].extended<dynd::ndt::struct_type>()->get_arrmeta_offsets_raw();
self_ck->m_field_names.reset(PyTuple_New(field_count));
self_ck->m_field_names = capture_if_not_null(PyTuple_New(field_count));
for (intptr_t i = 0; i < field_count; ++i) {
const dynd::string &rawname = src_tp[0].extended<dynd::ndt::struct_type>()->get_field_name(i);
pydynd::pyobject_ownref name(PyUnicode_DecodeUTF8(rawname.begin(), rawname.end() - rawname.begin(), NULL));
PyTuple_SET_ITEM(self_ck->m_field_names.get(), i, name.release());
pydynd::py_ref name =
capture_if_not_null(PyUnicode_DecodeUTF8(rawname.begin(), rawname.end() - rawname.begin(), NULL));
PyTuple_SET_ITEM(self_ck->m_field_names.get(), i, pydynd::release(std::move(name)));
}
self_ck->m_copy_el_offsets.resize(field_count);
for (intptr_t i = 0; i < field_count; ++i) {
Expand Down
Loading