Skip to content

Commit

Permalink
Reduce binary size overhead of new-style constructors
Browse files Browse the repository at this point in the history 
The lookup of the `self` type and value pointer are moved out of
template code and into `dispatcher`. This brings down the binary
size of constructors back to the level of the old placement-new
approach. (It also avoids a second lookup for `init_instance`.)

With this implementation, mixing old- and new-style constructors
in the same overload set may result in some runtime overhead for
temporary allocations/deallocations, but this should be fine as
old style constructors are phased out.
  • Loading branch information
@dean0x7d
dean0x7d committed Aug 21, 2017
1 parent 9f6a636 commit 0daf608
Show file tree
Hide file tree
Showing 4 changed files with 107 additions and 102 deletions.
14 changes: 12 additions & 2 deletions include/pybind11/attr.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -133,8 +133,8 @@ struct argument_record {
/// Internal data structure which holds metadata about a bound function (signature, overloads, etc.)
struct function_record {
function_record()
: is_constructor(false), is_stateless(false), is_operator(false),
has_args(false), has_kwargs(false), is_method(false) { }
: is_constructor(false), is_new_style_constructor(false), is_stateless(false),
is_operator(false), has_args(false), has_kwargs(false), is_method(false) { }

/// Function name
char *name = nullptr; /* why no C++ strings? They generate heavier code.. */
Expand Down Expand Up @@ -163,6 +163,9 @@ struct function_record {
/// True if name == '__init__'
bool is_constructor : 1;

/// True if this is a new-style `__init__` defined in `detail/init.h`
bool is_new_style_constructor : 1;

/// True if this is a stateless function pointer
bool is_stateless : 1;

Expand Down Expand Up @@ -281,6 +284,9 @@ inline function_call::function_call(function_record &f, handle p) :
args_convert.reserve(f.nargs);
}

/// Tag for a new-style `__init__` defined in `detail/init.h`
struct is_new_style_constructor { };

/**
* Partial template specializations to process custom attributes provided to
* cpp_function_ and class_. These are either used to initialize the respective
Expand Down Expand Up @@ -339,6 +345,10 @@ template <> struct process_attribute<is_operator> : process_attribute_default<is
static void init(const is_operator &, function_record *r) { r->is_operator = true; }
};

template <> struct process_attribute<is_new_style_constructor> : process_attribute_default<is_new_style_constructor> {
static void init(const is_new_style_constructor &, function_record *r) { r->is_new_style_constructor = true; }
};

/// Process a keyword argument attribute (*without* a default value)
template <> struct process_attribute<arg> : process_attribute_default<arg> {
static void init(const arg &a, function_record *r) {
Expand Down
138 changes: 54 additions & 84 deletions include/pybind11/detail/init.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,33 +13,29 @@

NAMESPACE_BEGIN(PYBIND11_NAMESPACE)
NAMESPACE_BEGIN(detail)
NAMESPACE_BEGIN(initimpl)

inline void no_nullptr(void *ptr) {
if (!ptr) throw type_error("pybind11::init(): factory function returned nullptr");
}
template <>
class type_caster<value_and_holder> {
public:
bool load(handle h, bool) {
value = reinterpret_cast<value_and_holder *>(h.ptr());
return true;
}

// Makes sure the `value` for the given value_and_holder is not preallocated (e.g. by a previous
// old-style placement new `__init__` that requires a preallocated, uninitialized value). If
// preallocated, deallocate. Returns the (null) value pointer reference ready for allocation.
inline void *&deallocate(value_and_holder &v_h) {
if (v_h) v_h.type->dealloc(v_h);
return v_h.value_ptr();
}
template <typename> using cast_op_type = value_and_holder &;
operator value_and_holder &() { return *value; }
static PYBIND11_DESCR name() { return type_descr(_<value_and_holder>()); }

PYBIND11_NOINLINE inline value_and_holder load_v_h(handle self_, type_info *tinfo) {
if (!self_ || !tinfo)
throw type_error("__init__(self, ...) called with invalid `self` argument");
private:
value_and_holder *value = nullptr;
};

auto *inst = reinterpret_cast<instance *>(self_.ptr());
auto result = inst->get_value_and_holder(tinfo, false);
if (!result.inst)
throw type_error("__init__(self, ...) called with invalid `self` argument");
NAMESPACE_BEGIN(initimpl)

return result;
inline void no_nullptr(void *ptr) {
if (!ptr) throw type_error("pybind11::init(): factory function returned nullptr");
}


// Implementing functions for all forms of py::init<...> and py::init(...)
template <typename Class> using Cpp = typename Class::type;
template <typename Class> using Alias = typename Class::type_alias;
Expand All @@ -64,7 +60,7 @@ constexpr bool is_alias(void *) { return false; }
template <typename Class>
void construct_alias_from_cpp(std::true_type /*is_alias_constructible*/,
value_and_holder &v_h, Cpp<Class> &&base) {
deallocate(v_h) = new Alias<Class>(std::move(base));
v_h.value_ptr() = new Alias<Class>(std::move(base));
}
template <typename Class>
[[noreturn]] void construct_alias_from_cpp(std::false_type /*!is_alias_constructible*/,
Expand Down Expand Up @@ -98,19 +94,17 @@ void construct(value_and_holder &v_h, Cpp<Class> *ptr, bool need_alias) {
// it was a normal instance, then steal the holder away into a local variable; thus
// the holder and destruction happens when we leave the C++ scope, and the holder
// class gets to handle the destruction however it likes.
deallocate(v_h) = ptr;
v_h.value_ptr() = ptr;
v_h.set_instance_registered(true); // To prevent init_instance from registering it
v_h.type->init_instance(v_h.inst, nullptr); // Set up the holder
Holder<Class> temp_holder(std::move(v_h.holder<Holder<Class>>())); // Steal the holder
v_h.type->dealloc(v_h); // Destroys the moved-out holder remains, resets value ptr to null
v_h.set_instance_registered(false);

construct_alias_from_cpp<Class>(is_alias_constructible<Class>{}, v_h, std::move(*ptr));
}
else {
// Otherwise the type isn't inherited, so we don't need an Alias and can just store the Cpp
// pointer directory:
deallocate(v_h) = ptr;
} else {
// Otherwise the type isn't inherited, so we don't need an Alias
v_h.value_ptr() = ptr;
}
}

Expand All @@ -119,7 +113,7 @@ void construct(value_and_holder &v_h, Cpp<Class> *ptr, bool need_alias) {
template <typename Class, enable_if_t<Class::has_alias, int> = 0>
void construct(value_and_holder &v_h, Alias<Class> *alias_ptr, bool) {
no_nullptr(alias_ptr);
deallocate(v_h) = static_cast<Cpp<Class> *>(alias_ptr);
v_h.value_ptr() = static_cast<Cpp<Class> *>(alias_ptr);
}

// Holder return: copy its pointer, and move or copy the returned holder into the new instance's
Expand All @@ -133,7 +127,7 @@ void construct(value_and_holder &v_h, Holder<Class> holder, bool need_alias) {
throw type_error("pybind11::init(): construction failed: returned holder-wrapped instance "
"is not an alias instance");

deallocate(v_h) = ptr;
v_h.value_ptr() = ptr;
v_h.type->init_instance(v_h.inst, &holder);
}

Expand All @@ -148,7 +142,7 @@ void construct(value_and_holder &v_h, Cpp<Class> &&result, bool need_alias) {
if (Class::has_alias && need_alias)
construct_alias_from_cpp<Class>(is_alias_constructible<Class>{}, v_h, std::move(result));
else
deallocate(v_h) = new Cpp<Class>(std::move(result));
v_h.value_ptr() = new Cpp<Class>(std::move(result));
}

// return-by-value version 2: returning a value of the alias type itself. We move-construct an
Expand All @@ -158,50 +152,38 @@ template <typename Class>
void construct(value_and_holder &v_h, Alias<Class> &&result, bool) {
static_assert(std::is_move_constructible<Alias<Class>>::value,
"pybind11::init() return-by-alias-value factory function requires a movable alias class");
deallocate(v_h) = new Alias<Class>(std::move(result));
v_h.value_ptr() = new Alias<Class>(std::move(result));
}

// Implementing class for py::init<...>()
template <typename... Args> struct constructor {
template <typename... Args>
struct constructor {
template <typename Class, typename... Extra, enable_if_t<!Class::has_alias, int> = 0>
static void execute(Class &cl, const Extra&... extra) {
auto *cl_type = get_type_info(typeid(Cpp<Class>));
cl.def("__init__", [cl_type](handle self_, Args... args) {
auto v_h = load_v_h(self_, cl_type);
// If this value is already registered it must mean __init__ is invoked multiple times;
// we really can't support that in C++, so just ignore the second __init__.
if (v_h.instance_registered()) return;

construct<Class>(v_h, new Cpp<Class>(std::forward<Args>(args)...), false);
}, extra...);
cl.def("__init__", [](value_and_holder &v_h, Args... args) {
v_h.value_ptr() = new Cpp<Class>(std::forward<Args>(args)...);
}, is_new_style_constructor(), extra...);
}

template <typename Class, typename... Extra,
enable_if_t<Class::has_alias &&
std::is_constructible<Cpp<Class>, Args...>::value, int> = 0>
static void execute(Class &cl, const Extra&... extra) {
auto *cl_type = get_type_info(typeid(Cpp<Class>));
cl.def("__init__", [cl_type](handle self_, Args... args) {
auto v_h = load_v_h(self_, cl_type);
if (v_h.instance_registered()) return; // Ignore duplicate __init__ calls (see above)

if (Py_TYPE(v_h.inst) == cl_type->type)
construct<Class>(v_h, new Cpp<Class>(std::forward<Args>(args)...), false);
cl.def("__init__", [](value_and_holder &v_h, Args... args) {
if (Py_TYPE(v_h.inst) == v_h.type->type)
v_h.value_ptr() = new Cpp<Class>(std::forward<Args>(args)...);
else
construct<Class>(v_h, new Alias<Class>(std::forward<Args>(args)...), true);
}, extra...);
v_h.value_ptr() = new Alias<Class>(std::forward<Args>(args)...);
}, is_new_style_constructor(), extra...);
}

template <typename Class, typename... Extra,
enable_if_t<Class::has_alias &&
!std::is_constructible<Cpp<Class>, Args...>::value, int> = 0>
static void execute(Class &cl, const Extra&... extra) {
auto *cl_type = get_type_info(typeid(Cpp<Class>));
cl.def("__init__", [cl_type](handle self_, Args... args) {
auto v_h = load_v_h(self_, cl_type);
if (v_h.instance_registered()) return; // Ignore duplicate __init__ calls (see above)
construct<Class>(v_h, new Alias<Class>(std::forward<Args>(args)...), true);
}, extra...);
cl.def("__init__", [](value_and_holder &v_h, Args... args) {
v_h.value_ptr() = new Alias<Class>(std::forward<Args>(args)...);
}, is_new_style_constructor(), extra...);
}
};

Expand All @@ -210,17 +192,15 @@ template <typename... Args> struct alias_constructor {
template <typename Class, typename... Extra,
enable_if_t<Class::has_alias && std::is_constructible<Alias<Class>, Args...>::value, int> = 0>
static void execute(Class &cl, const Extra&... extra) {
auto *cl_type = get_type_info(typeid(Cpp<Class>));
cl.def("__init__", [cl_type](handle self_, Args... args) {
auto v_h = load_v_h(self_, cl_type);
if (v_h.instance_registered()) return; // Ignore duplicate __init__ calls (see above)
construct<Class>(v_h, new Alias<Class>(std::forward<Args>(args)...), true);
}, extra...);
cl.def("__init__", [](value_and_holder &v_h, Args... args) {
v_h.value_ptr() = new Alias<Class>(std::forward<Args>(args)...);
}, is_new_style_constructor(), extra...);
}
};

// Implementation class for py::init(Func) and py::init(Func, AliasFunc)
template <typename CFunc, typename AFuncIn, typename... Args> struct factory {
template <typename CFunc, typename AFuncIn, typename... Args>
struct factory {
private:
using CFuncType = typename std::remove_reference<CFunc>::type;
using AFunc = conditional_t<std::is_void<AFuncIn>::value, void_type, AFuncIn>;
Expand Down Expand Up @@ -248,21 +228,16 @@ template <typename CFunc, typename AFuncIn, typename... Args> struct factory {
template <typename Class, typename... Extra,
enable_if_t<!Class::has_alias || std::is_void<AFuncIn>::value, int> = 0>
void execute(Class &cl, const Extra&... extra) && {
auto *cl_type = get_type_info(typeid(Cpp<Class>));
#if defined(PYBIND11_CPP14)
cl.def("__init__", [cl_type, func = std::move(class_factory)]
cl.def("__init__", [func = std::move(class_factory)]
#else
CFuncType &func = class_factory;
cl.def("__init__", [cl_type, func]
cl.def("__init__", [func]
#endif
(handle self_, Args... args) {
auto v_h = load_v_h(self_, cl_type);
// If this value is already registered it must mean __init__ is invoked multiple times;
// we really can't support that in C++, so just ignore the second __init__.
if (v_h.instance_registered()) return;

construct<Class>(v_h, func(std::forward<Args>(args)...), Py_TYPE(v_h.inst) != cl_type->type);
}, extra...);
(value_and_holder &v_h, Args... args) {
construct<Class>(v_h, func(std::forward<Args>(args)...),
Py_TYPE(v_h.inst) != v_h.type->type);
}, is_new_style_constructor(), extra...);
}

// Add __init__ definition for a class with an alias *and* distinct alias factory; the former is
Expand All @@ -271,26 +246,21 @@ template <typename CFunc, typename AFuncIn, typename... Args> struct factory {
template <typename Class, typename... Extra,
enable_if_t<Class::has_alias && !std::is_void<AFuncIn>::value, int> = 0>
void execute(Class &cl, const Extra&... extra) && {
auto *cl_type = get_type_info(typeid(Cpp<Class>));

#if defined(PYBIND11_CPP14)
cl.def("__init__", [cl_type, class_func = std::move(class_factory), alias_func = std::move(alias_factory)]
cl.def("__init__", [class_func = std::move(class_factory), alias_func = std::move(alias_factory)]
#else
CFuncType &class_func = class_factory;
AFuncType &alias_func = alias_factory;
cl.def("__init__", [cl_type, class_func, alias_func]
cl.def("__init__", [class_func, alias_func]
#endif
(handle self_, Args... args) {
auto v_h = load_v_h(self_, cl_type);
if (v_h.instance_registered()) return; // (see comment above)

if (Py_TYPE(v_h.inst) == cl_type->type)
(value_and_holder &v_h, Args... args) {
if (Py_TYPE(v_h.inst) == v_h.type->type)
// If the instance type equals the registered type we don't have inheritance, so
// don't need the alias and can construct using the class function:
construct<Class>(v_h, class_func(std::forward<Args>(args)...), false);
else
construct<Class>(v_h, alias_func(std::forward<Args>(args)...), true);
}, extra...);
}, is_new_style_constructor(), extra...);
}
};

Expand Down
42 changes: 34 additions & 8 deletions include/pybind11/pybind11.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -242,8 +242,9 @@ class cpp_function : public function {
.cast<std::string>() + ".";
#endif
signature += tinfo->type->tp_name;
} else if (rec->is_constructor && arg_index == 0 && detail::same_type(typeid(handle), *t) && rec->scope) {
// A py::init(...) constructor takes `self` as a `handle`; rewrite it to the type
} else if (rec->is_new_style_constructor && arg_index == 0) {
// A new-style `__init__` takes `self` as `value_and_holder`.
// Rewrite it to the proper class type.
#if defined(PYPY_VERSION)
signature += rec->scope.attr("__module__").cast<std::string>() + ".";
#endif
Expand Down Expand Up @@ -425,6 +426,23 @@ class cpp_function : public function {
handle parent = n_args_in > 0 ? PyTuple_GET_ITEM(args_in, 0) : nullptr,
result = PYBIND11_TRY_NEXT_OVERLOAD;

auto self_value_and_holder = value_and_holder();
if (overloads->is_constructor) {
const auto tinfo = get_type_info((PyTypeObject *) overloads->scope.ptr());
const auto pi = reinterpret_cast<instance *>(parent.ptr());
self_value_and_holder = pi->get_value_and_holder(tinfo, false);

if (!self_value_and_holder.type || !self_value_and_holder.inst) {
PyErr_SetString(PyExc_TypeError, "__init__(self, ...) called with invalid `self` argument");
return nullptr;
}

// If this value is already registered it must mean __init__ is invoked multiple times;
// we really can't support that in C++, so just ignore the second __init__.
if (self_value_and_holder.instance_registered())
return none().release().ptr();
}

try {
// We do this in two passes: in the first pass, we load arguments with `convert=false`;
// in the second, we allow conversion (except for arguments with an explicit
Expand Down Expand Up @@ -472,6 +490,18 @@ class cpp_function : public function {
size_t args_to_copy = std::min(pos_args, n_args_in);
size_t args_copied = 0;

// 0. Inject new-style `self` argument
if (it->is_new_style_constructor) {
// The `value` may have been preallocated by an old-style `__init__`
// if it was a preceding candidate for overload resolution.
if (self_value_and_holder)
self_value_and_holder.type->dealloc(self_value_and_holder);

call.args.push_back(reinterpret_cast<PyObject *>(&self_value_and_holder));
call.args_convert.push_back(false);
++args_copied;
}

// 1. Copy any position arguments given.
bool bad_arg = false;
for (; args_copied < args_to_copy; ++args_copied) {
Expand Down Expand Up @@ -715,13 +745,9 @@ class cpp_function : public function {
PyErr_SetString(PyExc_TypeError, msg.c_str());
return nullptr;
} else {
if (overloads->is_constructor) {
auto tinfo = get_type_info((PyTypeObject *) overloads->scope.ptr());
if (overloads->is_constructor && !self_value_and_holder.holder_constructed()) {
auto *pi = reinterpret_cast<instance *>(parent.ptr());
auto v_h = pi->get_value_and_holder(tinfo);
if (!v_h.holder_constructed()) {
tinfo->init_instance(pi, nullptr);
}
self_value_and_holder.type->init_instance(pi, nullptr);
}
return result.ptr();
}
Expand Down
Loading

0 comments on commit 0daf608

Please sign in to comment.