From fd81a03ec91c4e729bb2c4f0456300015972bad0 Mon Sep 17 00:00:00 2001 From: Jason Rhinelander Date: Mon, 31 Jul 2017 23:32:34 -0400 Subject: [PATCH] Lazy instance value pointer allocation We currently allocate instance values when creating the instance itself (except when constructing the instance for a `cast()`), but there is no particular reason to do so: the instance itself and the internals (for a non-simple layout) are allocated via Python, with no reason to expect better locality from the invoked `operator new`. Moreover, it makes implementation of factory function constructors trickier and slightly less efficient: they don't use the pre-eallocate the memory, which means there is a pointless allocation and free. This commit makes the allocation lazy: instead of preallocating when creating the instance, the allocation happens when the instance is first loaded (if null at that time). In addition to making it more efficient to deal with cases that don't need preallocation, this also allows for a very slight performance increase by not needing to look up the instances types during allocation. (There is a lookup during the eventual load, of course, but that is happening already). --- include/pybind11/cast.h | 14 ++++++++++---- include/pybind11/detail/class.h | 16 ++++------------ include/pybind11/detail/common.h | 2 +- 3 files changed, 15 insertions(+), 17 deletions(-) diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index e1823bd56..4b5a4bfc5 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -560,7 +560,7 @@ inline PyThreadState *get_thread_state_unchecked() { // Forward declarations inline void keep_alive_impl(handle nurse, handle patient); -inline PyObject *make_new_instance(PyTypeObject *type, bool allocate_value = true); +inline PyObject *make_new_instance(PyTypeObject *type); class type_caster_generic { public: @@ -591,7 +591,7 @@ public: } } - auto inst = reinterpret_steal(make_new_instance(tinfo->type, false /* don't allocate value */)); + auto inst = reinterpret_steal(make_new_instance(tinfo->type)); auto wrapper = reinterpret_cast(inst.ptr()); wrapper->owned = false; void *&valueptr = values_and_holders(wrapper).begin()->value_ptr(); @@ -647,8 +647,14 @@ public: protected: // Base methods for generic caster; there are overridden in copyable_holder_caster - void load_value(const value_and_holder &v_h) { - value = v_h.value_ptr(); + void load_value(value_and_holder &&v_h) { + auto *&vptr = v_h.value_ptr(); + // Lazy allocation for unallocated values: + if (vptr == nullptr) { + auto *type = v_h.type ? v_h.type : typeinfo; + vptr = type->operator_new(type->type_size); + } + value = vptr; } bool try_implicit_casts(handle src, bool convert) { for (auto &cast : typeinfo->implicit_casts) { diff --git a/include/pybind11/detail/class.h b/include/pybind11/detail/class.h index 6f9667e05..40efabcc1 100644 --- a/include/pybind11/detail/class.h +++ b/include/pybind11/detail/class.h @@ -232,11 +232,10 @@ inline bool deregister_instance(instance *self, void *valptr, const type_info *t return ret; } -/// Instance creation function for all pybind11 types. It only allocates space for the C++ object -/// (or multiple objects, for Python-side inheritance from multiple pybind11 types), but doesn't -/// call the constructor -- an `__init__` function must do that (followed by an `init_instance` -/// to set up the holder and register the instance). -inline PyObject *make_new_instance(PyTypeObject *type, bool allocate_value /*= true (in cast.h)*/) { +/// Instance creation function for all pybind11 types. It allocates the internal instance layout for +/// holding C++ objects and holders. Allocation is done lazily (the first time the instance is cast +/// to a reference or pointer), and initialization is done by an `__init__` function. +inline PyObject *make_new_instance(PyTypeObject *type) { #if defined(PYPY_VERSION) // PyPy gets tp_basicsize wrong (issue 2482) under multiple inheritance when the first inherited // object is a a plain Python type (i.e. not derived from an extension type). Fix it. @@ -251,13 +250,6 @@ inline PyObject *make_new_instance(PyTypeObject *type, bool allocate_value /*= t inst->allocate_layout(); inst->owned = true; - // Allocate (if requested) the value pointers; otherwise leave them as nullptr - if (allocate_value) { - for (auto &v_h : values_and_holders(inst)) { - void *&vptr = v_h.value_ptr(); - vptr = v_h.type->operator_new(v_h.type->type_size); - } - } return self; } diff --git a/include/pybind11/detail/common.h b/include/pybind11/detail/common.h index 2201d2b2c..c7b69f58b 100644 --- a/include/pybind11/detail/common.h +++ b/include/pybind11/detail/common.h @@ -434,7 +434,7 @@ struct instance { /// If true, get_internals().patients has an entry for this object bool has_patients : 1; - /// Initializes all of the above type/values/holders data + /// Initializes all of the above type/values/holders data (but not the instance values themselves) void allocate_layout(); /// Destroys/deallocates all of the above