From 71d5851701c1ac762204e31a0620aeeb8551cc50 Mon Sep 17 00:00:00 2001 From: Ichthyostega Date: Sat, 25 May 2024 05:14:36 +0200 Subject: [PATCH] Library: implement optional invocation of destructors This is the first draft, implementing the invocation explicitly through a trampoline function. While it seems to work, the formulation can probably be simplified.... --- src/lib/allocation-cluster.cpp | 76 +++++++++++++++++--- src/lib/allocation-cluster.hpp | 87 +++++++++++++++++++++-- src/lib/linked-elements.hpp | 20 ++++-- tests/library/allocation-cluster-test.cpp | 49 +++++++------ wiki/thinkPad.ichthyo.mm | 33 +++++++-- 5 files changed, 221 insertions(+), 44 deletions(-) diff --git a/src/lib/allocation-cluster.cpp b/src/lib/allocation-cluster.cpp index 78e7a969e..bb905a921 100644 --- a/src/lib/allocation-cluster.cpp +++ b/src/lib/allocation-cluster.cpp @@ -70,8 +70,32 @@ namespace lib { { Alloc::destroy (heap, static_cast (storage)); } + + /** + * Special allocator-policy for lib::LinkedElements + * - does not allow to allocate new elements + * - can hook up elements allocated elsewhere + * - ensure the destructor of all elements is invoked + */ + struct PolicyInvokeDtor + : lib::linked_elements::NoOwnership + { + /** + * while this policy doesn't take ownership, + * it ensures the destructor is invoked + */ + template + void + destroy (X* elm) + { + REQUIRE (elm); + elm->~X(); + } + }; + } + /** * An _overlay view_ for the AllocationCluster to add functionality * for adding / clearing extents and registering optional deleter functions. @@ -91,9 +115,15 @@ namespace lib { struct Destructor { Destructor* next; - ///////////////////////////////////////////////OOO store deleter function here + DtorInvoker dtor; + + ~Destructor() + { + if (dtor) + (*dtor) (this); + } }; - using Destructors = lib::LinkedElements; + using Destructors = lib::LinkedElements; struct Extent : util::NonCopyable @@ -112,7 +142,7 @@ namespace lib { ManagementView view_; - StorageManager() = delete; ///< @warning used as _overlay view_ only, never created + StorageManager() = delete; ///< @warning used as _overlay view_ only, never created public: static StorageManager& @@ -135,6 +165,20 @@ namespace lib { view_.extents.clear(); } + void + addDestructor (void* dtor) + { + auto& destructor = * static_cast (dtor); + getCurrentBlockStart()->dtors.push (destructor); + } + + void + discardLastDestructor() + { + getCurrentBlockStart()->dtors.pop(); + } + + size_t determineExtentCnt() ///< @warning finalises current block { @@ -152,12 +196,13 @@ namespace lib { static auto determineStorageSize (AllocationCluster const&); private: - byte* + Extent* getCurrentBlockStart() const { - return static_cast(view_.storage.pos) - + view_.storage.rest - - EXTENT_SIZ; + void* pos = static_cast(view_.storage.pos) + + view_.storage.rest + - EXTENT_SIZ; + return static_cast (pos); } void @@ -221,7 +266,22 @@ namespace lib { ENSURE (allocRequest + OVERHEAD <= EXTENT_SIZ); StorageManager::access(*this).addBlock(); } - + + + void + AllocationCluster::registerDestructor (void* dtor) + { + StorageManager::access(*this).addDestructor (dtor); + } + + + void + AllocationCluster::discardLastDestructor() + { + StorageManager::access(*this).discardLastDestructor(); + } + + /* === diagnostics helpers === */ diff --git a/src/lib/allocation-cluster.hpp b/src/lib/allocation-cluster.hpp index 2eda39b41..fbe595cc0 100644 --- a/src/lib/allocation-cluster.hpp +++ b/src/lib/allocation-cluster.hpp @@ -45,6 +45,8 @@ #include "lib/error.hpp" #include "lib/nocopy.hpp" +#include +#include #include @@ -125,11 +127,10 @@ namespace lib { template - TY& - create (ARGS&& ...args) - { - return * new(allot()) TY (std::forward (args)...); - } + TY& create (ARGS&& ...); + + template + TY& createDisposable (ARGS&& ...); template Allocator @@ -167,7 +168,14 @@ namespace lib { return static_cast (allotMemory (cnt * sizeof(X), alignof(X))); } + typedef void (*DtorInvoker) (void*); + + template + void* allotWithDeleter(); + void expandStorage (size_t); + void registerDestructor (void*); + void discardLastDestructor(); bool _is_within_limits (size_t,size_t); friend class test::AllocationCluster_test; @@ -179,6 +187,75 @@ namespace lib { //-----implementation-details------------------------ + template + TY& + AllocationCluster::createDisposable (ARGS&& ...args) + { + return * new(allot()) TY (std::forward (args)...); + } + + template + TY& + AllocationCluster::create (ARGS&& ...args) + { + if constexpr (std::is_trivial_v) + return createDisposable (std::forward (args)...); + + void* storage = allotWithDeleter(); + try { + return * new(storage) TY (std::forward (args)...); + } + catch(...) + { + discardLastDestructor(); + throw; + } + } + + /** + * Establish a storage arrangement with a callback to invoke the destructor. + * @remark the purpose of AllocationCluster is to avoid deallocation of individual objects; + * thus the position and type of allocated payload objects is discarded. However, + * sometimes it is desirable to ensure invocation of object destructors; in this case, + * a linked list of destructor callbacks is hooked up in the storage extent. These + * callback records are always allocated directly before the actual payload object, + * and use a special per-type trampoline function to invoke the destructor, passing + * a properly adjusted self-pointer. + */ + template + void* + AllocationCluster::allotWithDeleter() + { + /** + * Memory layout frame to place a payload object + * and store a destructor callback as intrusive linked list. + * @note this object is never constructed, but it is used to + * reinterpret the StorageManager::Destructor record, + * causing invocation of the destructor of the payload object, + * which is always placed immediately behind. + */ + struct TypedDtorInvoker + { + void* next; + DtorInvoker dtor; + X payload; + + /** trampoline function: invoke the destructor of the payload type */ + static void + invokePayloadDtor (void* self) + { + REQUIRE (self); + TypedDtorInvoker* instance = static_cast (self); + instance->payload.~X(); + } + }; + + TypedDtorInvoker* allocation = allot(); + allocation->dtor = &TypedDtorInvoker::invokePayloadDtor; + registerDestructor (allocation); + return & allocation->payload; + } + } // namespace lib #endif /*LIB_ALLOCATION_CLUSTER_H*/ diff --git a/src/lib/linked-elements.hpp b/src/lib/linked-elements.hpp index 8420b4b56..8f5459782 100644 --- a/src/lib/linked-elements.hpp +++ b/src/lib/linked-elements.hpp @@ -323,12 +323,20 @@ namespace lib { } -// template< class TY =N> -// TY& //_________________________________________ -// emplace () ///< add object of type TY, using 0-arg ctor -// { -// return push (ALO::template create()); -// } + /** extract the top-most element, if any + * @warning gives up ownership; if this list manages ownership, + * then the caller is responsible for deallocating the removed entry + * @return pointer to the removed element + */ + N* + pop() + { + N* elm = head_; + if (head_) + head_ = head_->next; + return elm; + } + /** prepend object of type TY, forwarding ctor args */ template diff --git a/tests/library/allocation-cluster-test.cpp b/tests/library/allocation-cluster-test.cpp index 8f623a538..3565c2dbc 100644 --- a/tests/library/allocation-cluster-test.cpp +++ b/tests/library/allocation-cluster-test.cpp @@ -201,6 +201,7 @@ namespace test { verifyInternals() { CHECK (0==checksum); + long markSum; { AllocationCluster clu; // no allocation happened yet @@ -265,34 +266,17 @@ namespace test { CHECK (slot(0) == 0); // deliberately fill up the first extent completely -SHOW_EXPR(size_t(currBlock())) -SHOW_EXPR(size_t(clu.storage_.pos)) -SHOW_EXPR(clu.storage_.rest) -SHOW_EXPR(posOffset()) for (uint i=clu.storage_.rest; i>0; --i) clu.create (i); -SHOW_EXPR(size_t(currBlock())) -SHOW_EXPR(size_t(clu.storage_.pos)) -SHOW_EXPR(clu.storage_.rest) -SHOW_EXPR(posOffset()) -SHOW_EXPR(slot(0)) - CHECK (clu.storage_.rest == 0); + CHECK (clu.storage_.rest == 0); // no space left in current extent CHECK (posOffset() == BLOCKSIZ); -SHOW_EXPR(clu.numBytes()) - CHECK (clu.numBytes() == BLOCKSIZ - 2*sizeof(void*)); + CHECK (clu.numBytes() == BLOCKSIZ - 2*sizeof(void*)); // now using all the rest behind the admin »slots« CHECK (clu.numExtents() == 1); CHECK (slot(0) == 0); CHECK (blk == currBlock()); // but still in the initial extent // trigger overflow and allocation of second extent char& c2 = clu.create ('U'); -SHOW_EXPR(size_t(currBlock())) -SHOW_EXPR(size_t(clu.storage_.pos)) -SHOW_EXPR(clu.storage_.rest) -SHOW_EXPR(posOffset()) -SHOW_EXPR(slot(0)) -SHOW_EXPR(clu.numBytes()) -SHOW_EXPR(clu.numExtents()) CHECK (blk != currBlock()); // allocation moved to a new extent CHECK (getAddr(c2) == currBlock() + 2*sizeof(void*)); // c2 resides immediately after the two administrative »slots« CHECK (clu.storage_.rest == BLOCKSIZ - posOffset()); @@ -304,8 +288,33 @@ SHOW_EXPR(clu.numExtents()) CHECK (c1 == 'X'); CHECK (c2 == 'U'); CHECK (i3 == 42); + + // allocate a "disposable" object (dtor will not be called) +SHOW_EXPR(clu.numBytes()) +SHOW_EXPR(posOffset()) + size_t pp = posOffset(); + auto& o1 = clu.createDisposable> (4); + CHECK (o1.getID() == 4); +SHOW_EXPR(clu.numBytes()) +SHOW_EXPR(posOffset()) +SHOW_EXPR(checksum) + markSum = checksum; + CHECK (checksum == 4+4); + CHECK (alignof(Dummy<2>) == alignof(char)); + CHECK (posOffset() - pp == sizeof(Dummy<2>)); + + // allocate a similar object, + // but this time also enrolling the destructor + pp = posOffset(); + auto& o2 = clu.create> (8); + CHECK (o2.getID() == 8); + CHECK (checksum == markSum + 8+8); +SHOW_EXPR(clu.numBytes()) +SHOW_EXPR(posOffset()) +SHOW_EXPR(checksum) } - CHECK (0==checksum); +SHOW_EXPR(checksum) + CHECK (checksum == markSum); } diff --git a/wiki/thinkPad.ichthyo.mm b/wiki/thinkPad.ichthyo.mm index 681586616..877a9cd9e 100644 --- a/wiki/thinkPad.ichthyo.mm +++ b/wiki/thinkPad.ichthyo.mm @@ -81768,8 +81768,8 @@ Date:   Thu Apr 20 18:53:17 2023 +0200
- - + + @@ -81815,12 +81815,25 @@ Date:   Thu Apr 20 18:53:17 2023 +0200
 - + + + + + + + + + + + + + + @@ -81902,8 +81915,8 @@ Date:   Thu Apr 20 18:53:17 2023 +0200
- - + + @@ -81926,6 +81939,16 @@ Date:   Thu Apr 20 18:53:17 2023 +0200
+ + + + + + + + + +