8000 Refcounting Smart Pointers by insertinterestingnamehere · Pull Request #702 · libdynd/dynd-python · GitHub
[go: up one dir, main page]
Skip to content

Refcounting Smart Pointers #702

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.

Sign up for GitHub

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

Open
wants to merge 18 commits into
base: master
Choose a base branch
from
Open

Refcounting Smart Pointers #702

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
d7ac59e
Add smart pointer classes for mana 8000 ging 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
Prev Previous commit
Next Next commit
Update dynd-python codebase to use new py_ref class.
  • Loading branch information
@insertinterestingnamehere
insertinterestingnamehere committed Jul 1, 2016
commit 844d532121ef28f82154f7c95f17db63fe159d36
219 changes: 110 additions & 109 deletions dynd/include/callables/assign_to_pyarrayobject_callable.hpp
View file
Open in desktop
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());
}
}
*/
*/
};
12 changes: 7 additions & 5 deletions dynd/include/callables/assign_to_pyobject_callable.hpp
View file
Open in desktop
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, name.get());
}
self_ck->m_copy_el_offsets.resize(field_count);

Expand Down Expand Up @@ -333,10 +334,11 @@ 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));
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, name.release());
}
self_ck->m_copy_el_offsets.resize(field_count);
Expand Down
38 changes: 22 additions & 16 deletions dynd/include/kernels/apply_pyobject_kernel.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ struct apply_pyobject_kernel : dynd::nd::base_strided_kernel<apply_pyobject_kern
if (Py_REFCNT(item) != 1 || pydynd::array_to_cpp_ref(item)->get_use_count() != 1) {
std::stringstream ss;
ss << "Python callback function ";
pydynd::pyobject_ownref pyfunc_repr(PyObject_Repr(m_pyfunc));
pydynd::py_ref pyfunc_repr = pydynd::capture_if_not_null(PyObject_Repr(m_pyfunc));
ss << pydynd::pystring_as_string(pyfunc_repr.get());
ss << ", called by dynd, held a reference to parameter ";
ss << (i + 1) << " which contained temporary memory.";
Expand Down Expand Up @@ -70,7 +70,7 @@ struct apply_pyobject_kernel : dynd::nd::base_strided_kernel<apply_pyobject_kern
const dynd::ndt::type &dst_tp = fpt->get_return_type();
const std::vector<dynd::ndt::type> &src_tp = fpt->get_argument_types();
// First set up the parameters in a tuple
pydynd::pyobject_ownref args(PyTuple_New(nsrc));
pydynd::py_ref args = pydynd::capture_if_not_null(PyTuple_New(nsrc));
for (intptr_t i = 0; i != nsrc; ++i) {
dynd::ndt::type tp = src_tp[i];
dynd::nd::array n = dynd::nd::make_array(tp, const_cast<char *>(src[i]), dynd::nd::read_access_flag);
Expand All @@ -80,12 +80,15 @@ struct apply_pyobject_kernel : dynd::nd::base_strided_kernel<apply_pyobject_kern
PyTuple_SET_ITEM(args.get(), i, pydynd::array_from_cpp(std::move(n)));
}
// Now call the function
pydynd::pyobject_ownref res(PyObject_Call(m_pyfunc, args.get(), NULL));
// Copy the result into the destination memory
PyObject *child_obj = res.get();
char *child_src = reinterpret_cast<char *>(&child_obj);
get_child()->single(dst, &child_src);
res.clear();
PyObject *child_obj;
char *child_src;
{ // This scope exists to limit the lifetime of py_ref res.
pydynd::py_ref res = pydynd::capture_if_not_null(PyObject_Call(m_pyfunc, args.get(), NULL));
// Copy the result into the destination memory
child_obj = res.get();
child_src = reinterpret_cast<char *>(&child_obj);
get_child()->single(dst, &child_src);
}
// Validate that the call didn't hang onto the ephemeral data
// pointers we used. This is done after the dst assignment, because
// the function result may have contained a reference to an argument.
Expand All @@ -99,7 +102,7 @@ struct apply_pyobject_kernel : dynd::nd::base_strided_kernel<apply_pyobject_kern
const dynd::ndt::type &dst_tp = fpt->get_return_type();
const std::vector<dynd::ndt::type> &src_tp = fpt->get_argument_types();
// First set up the parameters in a tuple
pydynd::pyobject_ownref args(PyTuple_New(nsrc));
pydynd::py_ref args = pydynd::capture_if_not_null(PyTuple_New(nsrc));
for (intptr_t i = 0; i != nsrc; ++i) {
dynd::ndt::type tp = src_tp[i];
dynd::nd::array n = dynd::nd::make_array(tp, const_cast<char *>(src[i]), dynd::nd::read_access_flag);
Expand All @@ -111,12 +114,15 @@ struct apply_pyobject_kernel : dynd::nd::base_strided_kernel<apply_pyobject_kern
// Do the loop, reusing the args we created
for (size_t j = 0; j != count; ++j) {
// Call the function
pydynd::pyobject_ownref res(PyObject_Call(m_pyfunc, args.get(), NULL));
// Copy the result into the destination memory
PyObject *child_obj = res.get();
char *child_src = reinterpret_cast<char *>(&child_obj);
get_child()->single(dst, &child_src);
res.clear();
PyObject *child_obj;
char *child_src;
{ // Scope to hold lifetime of py_ref res.
pydynd::py_ref res = pydynd::capture_if_not_null(PyObject_Call(m_pyfunc, args.get(), NULL));
// Copy the result into the destination memory
child_obj = res.get();
child_src = reinterpret_cast<char *>(&child_obj);
get_child()->single(dst, &child_src);
}
// Validate that the call didn't hang onto the ephemeral data
// pointers we used. This is done after the dst assignment, because
// the function result may have contained a reference to an argument.
Expand All @@ -125,7 +131,7 @@ struct apply_pyobject_kernel : dynd::nd::base_strided_kernel<apply_pyobject_kern
dst += dst_stride;
for (intptr_t i = 0; i != nsrc; ++i) {
const dynd::nd::array &n = pydynd::array_to_cpp_ref(PyTuple_GET_ITEM(args.get(), i));
// n->set_data(n->get_data() + src_stride[i]);
// n->set_data(n->get_data() + src_stride[i]);
}
}
}
Expand Down
Loading
0