From 4f6191264697872a3711eff99fa8c1c8e87c00b3 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Wed, 16 Jun 2021 17:47:22 -0700 Subject: [PATCH] `py::smart_holder` `std::shared_ptr` deleter simplification & optimization. (#3041) --- include/pybind11/detail/smart_holder_poc.h | 109 +++++++++++---------- 1 file changed, 57 insertions(+), 52 deletions(-) diff --git a/include/pybind11/detail/smart_holder_poc.h b/include/pybind11/detail/smart_holder_poc.h index 2e03bc5e1..47d368b8b 100644 --- a/include/pybind11/detail/smart_holder_poc.h +++ b/include/pybind11/detail/smart_holder_poc.h @@ -56,36 +56,43 @@ Details: namespace pybindit { namespace memory { -template -struct guarded_builtin_delete { - std::shared_ptr flag_ptr; - explicit guarded_builtin_delete(std::shared_ptr armed_flag_ptr) - : flag_ptr{armed_flag_ptr} {} - template ::value, int>::type = 0> - void operator()(T *raw_ptr) { - if (*flag_ptr) - delete raw_ptr; - } - template ::value, int>::type = 0> - void operator()(T *) { - // This noop operator is needed to avoid a compilation error (for `delete raw_ptr;`), but - // throwing an exception from here could std::terminate the process. Therefore the runtime - // check for lifetime-management correctness is implemented elsewhere (in - // ensure_pointee_is_destructible()). +struct guarded_delete { + void (*del_ptr)(void *); + bool armed_flag; + guarded_delete(void (*del_ptr)(void *), bool armed_flag) + : del_ptr{del_ptr}, armed_flag{armed_flag} {} + void operator()(void *raw_ptr) const { + if (armed_flag) + (*del_ptr)(raw_ptr); } }; +template ::value, int>::type = 0> +inline void builtin_delete_if_destructible(void *raw_ptr) { + delete (T *) raw_ptr; +} + +template ::value, int>::type = 0> +inline void builtin_delete_if_destructible(void *) { + // This noop operator is needed to avoid a compilation error (for `delete raw_ptr;`), but + // throwing an exception from a destructor will std::terminate the process. Therefore the + // runtime check for lifetime-management correctness is implemented elsewhere (in + // ensure_pointee_is_destructible()). +} + +template +guarded_delete make_guarded_builtin_delete(bool armed_flag) { + return guarded_delete(builtin_delete_if_destructible, armed_flag); +} + template -struct guarded_custom_deleter { - std::shared_ptr flag_ptr; - explicit guarded_custom_deleter(std::shared_ptr armed_flag_ptr) - : flag_ptr{armed_flag_ptr} {} - void operator()(T *raw_ptr) { - if (*flag_ptr) - D()(raw_ptr); - } +inline void custom_delete(void *raw_ptr) { + D()((T *) raw_ptr); +} + +template +guarded_delete make_guarded_custom_deleter(bool armed_flag) { + return guarded_delete(custom_delete, armed_flag); }; template @@ -96,7 +103,6 @@ inline bool is_std_default_delete(const std::type_info &rtti_deleter) { struct smart_holder { const std::type_info *rtti_uqp_del = nullptr; - std::shared_ptr vptr_deleter_armed_flag_ptr; std::shared_ptr vptr; bool vptr_is_using_noop_deleter : 1; bool vptr_is_using_builtin_delete : 1; @@ -108,7 +114,7 @@ struct smart_holder { // Design choice: smart_holder is movable but not copyable. smart_holder(smart_holder &&) = default; smart_holder(const smart_holder &) = delete; - smart_holder &operator=(smart_holder &&) = default; + smart_holder &operator=(smart_holder &&) = delete; smart_holder &operator=(const smart_holder &) = delete; smart_holder() @@ -116,12 +122,6 @@ struct smart_holder { vptr_is_external_shared_ptr{false}, is_populated{false}, is_disowned{false}, pointee_depends_on_holder_owner{false} {} - explicit smart_holder(bool vptr_deleter_armed_flag) - : vptr_deleter_armed_flag_ptr{new bool{vptr_deleter_armed_flag}}, - vptr_is_using_noop_deleter{false}, vptr_is_using_builtin_delete{false}, - vptr_is_external_shared_ptr{false}, is_populated{false}, is_disowned{false}, - pointee_depends_on_holder_owner{false} {} - bool has_pointee() const { return vptr.get() != nullptr; } template @@ -194,10 +194,20 @@ struct smart_holder { } } + void reset_vptr_deleter_armed_flag(bool armed_flag) { + // The const_cast is only for certain compilers (Ubuntu 20 GCC 6.3.0 being one). + auto vptr_del_ptr = const_cast(std::get_deleter(vptr)); + if (vptr_del_ptr == nullptr) { + throw std::runtime_error( + "smart_holder::reset_vptr_deleter_armed_flag() called in an invalid context."); + } + vptr_del_ptr->armed_flag = armed_flag; + } + template static smart_holder from_raw_ptr_unowned(T *raw_ptr) { - smart_holder hld(false); - hld.vptr.reset(raw_ptr, guarded_builtin_delete(hld.vptr_deleter_armed_flag_ptr)); + smart_holder hld; + hld.vptr.reset(raw_ptr, [](void *) {}); hld.vptr_is_using_noop_deleter = true; hld.is_populated = true; return hld; @@ -227,8 +237,8 @@ struct smart_holder { template static smart_holder from_raw_ptr_take_ownership(T *raw_ptr) { ensure_pointee_is_destructible("from_raw_ptr_take_ownership"); - smart_holder hld(true); - hld.vptr.reset(raw_ptr, guarded_builtin_delete(hld.vptr_deleter_armed_flag_ptr)); + smart_holder hld; + hld.vptr.reset(raw_ptr, make_guarded_builtin_delete(true)); hld.vptr_is_using_builtin_delete = true; hld.is_populated = true; return hld; @@ -236,21 +246,18 @@ struct smart_holder { // Caller is responsible for ensuring preconditions (SMART_HOLDER_WIP: details). void disown() { - *vptr_deleter_armed_flag_ptr = false; - is_disowned = true; + reset_vptr_deleter_armed_flag(false); + is_disowned = true; } // Caller is responsible for ensuring preconditions (SMART_HOLDER_WIP: details). void reclaim_disowned() { - *vptr_deleter_armed_flag_ptr = true; - is_disowned = false; + reset_vptr_deleter_armed_flag(true); + is_disowned = false; } // Caller is responsible for ensuring preconditions (SMART_HOLDER_WIP: details). - void release_disowned() { - vptr.reset(); - vptr_deleter_armed_flag_ptr.reset(); - } + void release_disowned() { vptr.reset(); } // SMART_HOLDER_WIP: review this function. void ensure_can_release_ownership(const char *context = "ensure_can_release_ownership") { @@ -261,7 +268,7 @@ struct smart_holder { // Caller is responsible for ensuring preconditions (SMART_HOLDER_WIP: details). void release_ownership() { - *vptr_deleter_armed_flag_ptr = false; + reset_vptr_deleter_armed_flag(false); release_disowned(); } @@ -275,15 +282,13 @@ struct smart_holder { template static smart_holder from_unique_ptr(std::unique_ptr &&unq_ptr) { - smart_holder hld(true); + smart_holder hld; hld.rtti_uqp_del = &typeid(D); hld.vptr_is_using_builtin_delete = is_std_default_delete(*hld.rtti_uqp_del); if (hld.vptr_is_using_builtin_delete) { - hld.vptr.reset(unq_ptr.get(), - guarded_builtin_delete(hld.vptr_deleter_armed_flag_ptr)); + hld.vptr.reset(unq_ptr.get(), make_guarded_builtin_delete(true)); } else { - hld.vptr.reset(unq_ptr.get(), - guarded_custom_deleter(hld.vptr_deleter_armed_flag_ptr)); + hld.vptr.reset(unq_ptr.get(), make_guarded_custom_deleter(true)); } unq_ptr.release(); hld.is_populated = true;