From 8b25815f72ae4f5743cde3d4a45067737980df88 Mon Sep 17 00:00:00 2001 From: Twan van Laarhoven Date: Thu, 7 May 2020 22:25:02 +0200 Subject: [PATCH] Re-enabled intrusive_ptr --- src/data/action/symbol_part.cpp | 2 +- src/data/card.hpp | 2 +- src/gui/set/console_panel.cpp | 1 + src/script/script_manager.cpp | 16 +---- src/script/value.cpp | 14 ++-- src/util/io/package.hpp | 2 +- src/util/smart_ptr.hpp | 111 +++++++++++++++++++++++++------- 7 files changed, 99 insertions(+), 49 deletions(-) diff --git a/src/data/action/symbol_part.cpp b/src/data/action/symbol_part.cpp index 89684f9e..543f883d 100644 --- a/src/data/action/symbol_part.cpp +++ b/src/data/action/symbol_part.cpp @@ -252,7 +252,7 @@ void CurveDragAction::move(const Vector2D& delta, double t) { ControlPointAddAction::ControlPointAddAction(const SymbolShapeP& shape, UInt insert_after, double t) : shape(shape) - , new_point(make_shared()) + , new_point(make_intrusive()) , insert_after(insert_after) , point1(shape->getPoint(insert_after)) , point2(shape->getPoint(insert_after + 1)) diff --git a/src/data/card.hpp b/src/data/card.hpp index f1222757..7ab98821 100644 --- a/src/data/card.hpp +++ b/src/data/card.hpp @@ -25,7 +25,7 @@ DECLARE_POINTER_TYPE(StyleSheet); // ----------------------------------------------------------------------------- : Card /// A card from a card Set -class Card : public IntrusivePtrVirtualBase { +class Card : public IntrusivePtrVirtualBase, public IntrusiveFromThis { public: /// Default constructor, uses game_for_new_cards to make the game Card(); diff --git a/src/gui/set/console_panel.cpp b/src/gui/set/console_panel.cpp index f34e232a..8ba2030e 100644 --- a/src/gui/set/console_panel.cpp +++ b/src/gui/set/console_panel.cpp @@ -13,6 +13,7 @@ #include #include #include +#include #include #include #include diff --git a/src/script/script_manager.cpp b/src/script/script_manager.cpp index dbd1d9c4..085d8546 100644 --- a/src/script/script_manager.cpp +++ b/src/script/script_manager.cpp @@ -134,20 +134,8 @@ void SetScriptManager::initDependencies(Context& ctx, StyleSheet& stylesheet) { void SetScriptManager::onAction(const Action& action, bool undone) { TYPE_CASE(action, ValueAction) { if (action.card) { - #ifdef USE_INTRUSIVE_PTR - // we can just turn the Card* into a CardP - updateValue(*action.valueP, CardP(const_cast(action.card))); - return; - #else - // find the affected card - FOR_EACH(card, set.cards) { - if (card->data.contains(action.valueP)) { - updateValue(*action.valueP, card); - return; - } - } - assert(false); - #endif + updateValue(*action.valueP, const_cast(action.card)->intrusive_from_this()); + return; } else { // is it a keyword's fake value? KeywordTextValue* value = dynamic_cast(action.valueP.get()); diff --git a/src/script/value.cpp b/src/script/value.cpp index 86225dd2..00a08167 100644 --- a/src/script/value.cpp +++ b/src/script/value.cpp @@ -210,8 +210,8 @@ ScriptValueP rangeIterator(int start, int end) { // ----------------------------------------------------------------------------- : Integers -#ifdef USE_INTRUSIVE_PTR - #define USE_POOL_ALLOCATOR +#if defined(USE_INTRUSIVE_PTR) && !defined(USE_POOL_ALLOCATOR) + #define USE_POOL_ALLOCATOR 0 // disabled by default #endif // Integer values @@ -224,8 +224,8 @@ public: double toDouble() const override { return value; } int toInt() const override { return value; } protected: -#ifdef USE_POOL_ALLOCATOR - virtual void destroy() { +#if USE_POOL_ALLOCATOR + void destroy() const override { boost::singleton_pool::free(this); } #endif @@ -233,7 +233,7 @@ private: int value; }; -#if defined(USE_POOL_ALLOCATOR) && !defined(USE_INTRUSIVE_PTR) +#if USE_POOL_ALLOCATOR && !USE_INTRUSIVE_PTR // deallocation function for pool allocated integers void destroy_value(ScriptInt* v) { boost::singleton_pool::free(v); @@ -241,8 +241,8 @@ private: #endif ScriptValueP to_script(int v) { -#ifdef USE_POOL_ALLOCATOR - #ifdef USE_INTRUSIVE_PTR +#if USE_POOL_ALLOCATOR + #if USE_INTRUSIVE_PTR return ScriptValueP( new(boost::singleton_pool::malloc()) ScriptInt(v)); diff --git a/src/util/io/package.hpp b/src/util/io/package.hpp index e52e0771..2036af83 100644 --- a/src/util/io/package.hpp +++ b/src/util/io/package.hpp @@ -183,7 +183,7 @@ class Package : public IntrusivePtrVirtualBase { protected: // TODO: I dislike putting this here very much. There ought to be a better way. - virtual VCSP getVCS() { return make_shared(); } + virtual VCSP getVCS() { return make_intrusive(); } /// true if this is a zip file, false if a directory bool isZipfile() const { return !wxDirExists(filename); } diff --git a/src/util/smart_ptr.hpp b/src/util/smart_ptr.hpp index 66f94872..80d9e115 100644 --- a/src/util/smart_ptr.hpp +++ b/src/util/smart_ptr.hpp @@ -8,7 +8,7 @@ /** @file util/smart_ptr.hpp * - * @brief Utilities related to boost smart pointers + * @brief Smart pointers and related utility functions */ // ----------------------------------------------------------------------------- : Includes @@ -22,21 +22,96 @@ using std::dynamic_pointer_cast; using std::make_shared; using std::make_unique; +#ifndef USE_INTRUSIVE_PTR + #define USE_INTRUSIVE_PTR 1 +#endif + +// ----------------------------------------------------------------------------- : Intrusive pointers + +#if USE_INTRUSIVE_PTR + +#include +#include +using boost::intrusive_ptr; + +/// Base class for types that can be pointed to +template class IntrusivePtrBase { +public: + IntrusivePtrBase() {} + // don't copy or assign ref count + IntrusivePtrBase(IntrusivePtrBase const&) {} + void operator = (IntrusivePtrBase const&) {} +protected: + inline void destroy() const { + delete static_cast(this); + } +private: + mutable std::atomic ref_count = 0; + template friend void intrusive_ptr_add_ref(const IntrusivePtrBase* ptr); + template friend void intrusive_ptr_release(const IntrusivePtrBase* ptr); +}; + +template void intrusive_ptr_add_ref(const IntrusivePtrBase* ptr) { + ++(ptr->ref_count); +} + +template void intrusive_ptr_release(const IntrusivePtrBase* ptr) { + if (--(ptr->ref_count) == 0) { + static_cast(ptr)->destroy(); + } +} + +template +class IntrusiveFromThis { +public: + inline intrusive_ptr intrusive_from_this() noexcept { + return intrusive_ptr(static_cast(this)); + } +}; + +/// Allocate an object of type T and store it in a new intrusive_ptr, similar to std::make_shared +template +inline intrusive_ptr make_intrusive(Args&&... args) { + return intrusive_ptr(new T(std::forward(args)...)); +} + +// ----------------------------------------------------------------------------- : Shared pointers +#else + +template using intrusive_ptr = shared_ptr; + +/// Base class for types that can be pointed to +template class IntrusivePtrBase {}; + +template +class IntrusiveFromThis : public std::enable_shared_from_this { +public: + inline intrusive_ptr intrusive_from_this() { + return shared_from_this(); + } +}; + +/// Allocate an object of type T and store it in a new intrusive_ptr, similar to std::make_shared +template +inline intrusive_ptr make_intrusive(Args&&... args) { + return std::make_shared(std::forward(args)...); +} + +#endif + // ----------------------------------------------------------------------------- : Declaring +/// Declares the type TypeP as a intrusive_ptr +#define DECLARE_POINTER_TYPE(Type) \ + class Type; \ + typedef intrusive_ptr Type##P; + /// Declares the type TypeP as a shared_ptr #define DECLARE_SHARED_POINTER_TYPE(Type) \ class Type; \ typedef shared_ptr Type##P; -// ----------------------------------------------------------------------------- : Shared pointers - -#define DECLARE_POINTER_TYPE DECLARE_SHARED_POINTER_TYPE - -template using intrusive_ptr = shared_ptr; - -/// Base class for types that can be pointed to -template class IntrusivePtrBase {}; +// ----------------------------------------------------------------------------- : Utility /// IntrusivePtrBase with a virtual destructor class IntrusivePtrVirtualBase : public IntrusivePtrBase { @@ -49,27 +124,13 @@ public: virtual ~IntrusivePtrBaseWithDelete() {} protected: /// Delete this object - virtual void destroy() { + virtual void destroy() const { delete this; } + template friend void intrusive_ptr_release(const IntrusivePtrBase* ptr); }; -template -class IntrusiveFromThis : public std::enable_shared_from_this { -protected: - inline intrusive_ptr intrusive_from_this() { - return shared_from_this(); - } -}; - -/// Allocate an object of type T and store it in a new intrusive_ptr, similar to std::make_shared -template -inline intrusive_ptr make_intrusive(Args&&... args) { - return std::make_shared(std::forward(args)...); -} - /// Pointer to 'anything' typedef intrusive_ptr VoidP; -// ----------------------------------------------------------------------------- : Intrusive pointers