diff --git a/src/lib/allocation-cluster.cpp b/src/lib/allocation-cluster.cpp index bb905a921..59a879d0e 100644 --- a/src/lib/allocation-cluster.cpp +++ b/src/lib/allocation-cluster.cpp @@ -100,7 +100,7 @@ namespace lib { * An _overlay view_ for the AllocationCluster to add functionality * for adding / clearing extents and registering optional deleter functions. * @warning this is a tricky construct to operate each Allocation Cluster - * with the absolute necessary minimum of organisational overhead. + * with the absolute minimum of organisational overhead necessary. * The key point to note is that StorageManager is layout compatible * with AllocationCluster itself — achieved through use of the union * ManagementView, which holds a Storage descriptor member, but @@ -112,19 +112,10 @@ namespace lib { */ class AllocationCluster::StorageManager { - struct Destructor - { - Destructor* next; - DtorInvoker dtor; - - ~Destructor() - { - if (dtor) - (*dtor) (this); - } - }; + using Destructors = lib::LinkedElements; + /** Block of allocated storage */ struct Extent : util::NonCopyable { @@ -142,7 +133,7 @@ namespace lib { ManagementView view_; - StorageManager() = delete; ///< @warning used as _overlay view_ only, never created + StorageManager() = delete; ///< @note used as _overlay view_ only, never created public: static StorageManager& @@ -166,16 +157,9 @@ namespace lib { } void - addDestructor (void* dtor) + attach (Destructor& dtor) { - auto& destructor = * static_cast (dtor); - getCurrentBlockStart()->dtors.push (destructor); - } - - void - discardLastDestructor() - { - getCurrentBlockStart()->dtors.pop(); + getCurrentBlockStart()->dtors.push (dtor); } @@ -195,6 +179,7 @@ namespace lib { static auto determineStorageSize (AllocationCluster const&); + private: Extent* getCurrentBlockStart() const @@ -240,7 +225,7 @@ namespace lib { /** - * On shutdown of the AllocationCluster walks all extents and invokes all + * The shutdown of an AllocationCluster walks all extents and invokes all * registered deleter functions and then discards the complete storage. * @note it is possible to allocate objects as _disposable_ — meaning * that no destructors will be enrolled and called for such objects. @@ -254,6 +239,11 @@ namespace lib { ERROR_LOG_AND_IGNORE (progress, "discarding AllocationCluster") + /** virtual dtor to cause invocation of the payload's dtor on clean-up */ + AllocationCluster::Destructor::~Destructor() { }; + + + /** * Expand the alloted storage pool by a block, * suitable to accommodate at least the indicated request. @@ -269,16 +259,9 @@ namespace lib { void - AllocationCluster::registerDestructor (void* dtor) + AllocationCluster::registerDestructor (Destructor& dtor) { - StorageManager::access(*this).addDestructor (dtor); - } - - - void - AllocationCluster::discardLastDestructor() - { - StorageManager::access(*this).discardLastDestructor(); + StorageManager::access(*this).attach (dtor); } diff --git a/src/lib/allocation-cluster.hpp b/src/lib/allocation-cluster.hpp index fbe595cc0..43df8f2a3 100644 --- a/src/lib/allocation-cluster.hpp +++ b/src/lib/allocation-cluster.hpp @@ -24,8 +24,8 @@ ** Memory management for the low-level model (render nodes network). ** The model is organised into temporal segments, which are considered ** to be structurally constant and uniform. The objects within each - ** segment are strongly interconnected, and thus each segment is - ** being built in a single build process and is replaced or released + ** segment are strongly interconnected, and thus each segment is + ** created within a single build process and is replaced or released ** as a whole. AllocationCluster implements memory management to ** support this usage pattern. ** @@ -35,7 +35,7 @@ ** ** @see allocation-cluster-test.cpp ** @see builder::ToolFactory - ** @see frameid.hpp + ** @see linked-elements.hpp */ @@ -55,32 +55,17 @@ namespace lib { namespace test { class AllocationCluster_test; } // declared friend for low-level-checks - /** + /** * A pile of objects sharing common allocation and lifecycle. - * AllocationCluster owns a number of object families of various types. - * Each of those contains a initially undetermined (but rather large) - * number of individual objects, which can be expected to be allocated - * within a short timespan and which are to be released cleanly on - * destruction of the AllocationCluster. We provide a service creating - * individual objects with arbitrary ctor parameters. - * @warning make sure the objects dtors aren't called and object references - * aren't used after shutting down a given AllocationCluster. - * @todo implement a facility to control the oder in which - * the object families are to be discarded. Currently - * they are just purged in reverse order defined by - * the first request for allocating a certain type. - * @todo should we use an per-instance lock? We can't avoid - * the class-wide lock, unless also the type-ID registration - * is done on a per-instance base. AllocationCluster is intended - * to be used within the builder, which executes in a dedicated - * thread. Thus I doubt lock contention could be a problem and - * we can avoid using a mutex per instance. Re-evaluate this! - * @todo currently all AllocationCluster instances share the same type-IDs. - * When used within different usage contexts this leads to some slots - * remaining empty, because not every situation uses any type encountered. - * wouldn't it be desirable to have multiple distinct contexts, each with - * its own set of Type-IDs and maybe also separate locking? - * Is this issue worth the hassle? //////////////////////////////TICKET #169 + * AllocationCluster owns a heterogeneous collection of objects of various types. + * Typically, allocation happens during a short time span when building a new segment, + * and objects are used together until the segment is discarded. The primary leverage + * is to bulk-allocate memory, and to avoid invoking destructors (and thus to access + * a lot of _cache-cold memory pages_ on clean-up). A Stdlib compliant #Allocator + * is provided for use with STL containers. The actual allocation uses heap memory + * in _extents_ of hard-wired size, by the accompanying StorageManager. + * @warning use #createDisposable whenever possible, but be sure to understand + * the ramifications of _not invoking_ an object's destructor. */ class AllocationCluster : util::MoveOnly @@ -121,17 +106,11 @@ namespace lib { }; Storage storage_; + public: AllocationCluster (); ~AllocationCluster () noexcept; - - template - TY& create (ARGS&& ...); - - template - TY& createDisposable (ARGS&& ...); - template Allocator getAllocator() @@ -140,6 +119,13 @@ namespace lib { } + template + TY& create (ARGS&& ...); + + template + TY& createDisposable (ARGS&& ...); + + /* === diagnostics === */ size_t numExtents() const; @@ -168,14 +154,32 @@ namespace lib { return static_cast (allotMemory (cnt * sizeof(X), alignof(X))); } - typedef void (*DtorInvoker) (void*); + class Destructor + : util::NonCopyable + { + public: + virtual ~Destructor(); ///< this is an interface + Destructor* next{nullptr};// intrusive linked list... + }; + + /** @internal storage frame with the actual payload object, + * which can be attached to a list of destructors to invoke + */ template - void* allotWithDeleter(); + struct AllocationWithDestructor + : Destructor + { + X payload; + + template + AllocationWithDestructor (ARGS&& ...args) + : payload(std::forward (args)...) + { } + }; void expandStorage (size_t); - void registerDestructor (void*); - void discardLastDestructor(); + void registerDestructor (Destructor&); bool _is_within_limits (size_t,size_t); friend class test::AllocationCluster_test; @@ -187,13 +191,22 @@ namespace lib { //-----implementation-details------------------------ + /** + * Factory function: place a new instance into this AllocationCluster, + * but *without invoking its destructor* on clean-up (for performance reasons). + */ template TY& AllocationCluster::createDisposable (ARGS&& ...args) { return * new(allot()) TY (std::forward (args)...); } - + + /** + * Factory function: place a new instance into this AllocationCluster; + * the object will be properly destroyed when the cluster goes out of scope. + * @note whenever possible, the #createDisposable variant should be preferred + */ template TY& AllocationCluster::create (ARGS&& ...args) @@ -201,59 +214,10 @@ namespace lib { 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; + using Frame = AllocationWithDestructor; + auto& frame = createDisposable (std::forward (args)...); + registerDestructor (frame); + return frame.payload; } diff --git a/src/lib/linked-elements.hpp b/src/lib/linked-elements.hpp index 8f5459782..bae3e50d9 100644 --- a/src/lib/linked-elements.hpp +++ b/src/lib/linked-elements.hpp @@ -323,20 +323,6 @@ namespace lib { } - /** 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 3565c2dbc..921d00693 100644 --- a/tests/library/allocation-cluster-test.cpp +++ b/tests/library/allocation-cluster-test.cpp @@ -290,14 +290,9 @@ namespace test { 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)); @@ -309,11 +304,27 @@ SHOW_EXPR(checksum) auto& o2 = clu.create> (8); CHECK (o2.getID() == 8); CHECK (checksum == markSum + 8+8); -SHOW_EXPR(clu.numBytes()) + CHECK (posOffset() - pp > sizeof(Dummy<2>) + 2*sizeof(void*)); + CHECK (slot(1) > 0); + CHECK (size_t(&o2) - slot(1) == 2*sizeof(void*)); // Object resides in a Destructor frame, + using Dtor = AllocationCluster::Destructor; // ... which has been hooked up into admin-slot-1 of the current extent + auto dtor = (Dtor*)slot(1); + CHECK (dtor->next == nullptr); + + // any other object with non-trivial destructor.... + string rands = lib::test::randStr(9); + pp = posOffset(); + string& s1 = clu.create (rands); +SHOW_EXPR(pp) SHOW_EXPR(posOffset()) -SHOW_EXPR(checksum) +SHOW_EXPR(size_t(&s1)) + CHECK (posOffset() - pp >= sizeof(string) + 2*sizeof(void*)); + CHECK (size_t(&s1) - slot(1) == 2*sizeof(void*)); // again the Destructor frame is placed immediately before the object + auto dtor2 = (Dtor*)slot(1); // and it has been prepended to the destructors-list in current extent +SHOW_EXPR(size_t(dtor2->next)) + CHECK (dtor2->next == dtor); // with the destructor of o2 hooked up behind + CHECK (dtor->next == nullptr); } -SHOW_EXPR(checksum) CHECK (checksum == markSum); } diff --git a/wiki/thinkPad.ichthyo.mm b/wiki/thinkPad.ichthyo.mm index 877a9cd9e..e00fd6639 100644 --- a/wiki/thinkPad.ichthyo.mm +++ b/wiki/thinkPad.ichthyo.mm @@ -81716,7 +81716,7 @@ Date:   Thu Apr 20 18:53:17 2023 +0200
- + @@ -81754,8 +81754,9 @@ Date:   Thu Apr 20 18:53:17 2023 +0200
 - - + + + @@ -81768,8 +81769,8 @@ Date:   Thu Apr 20 18:53:17 2023 +0200
 - - + + @@ -81821,17 +81822,28 @@ Date:   Thu Apr 20 18:53:17 2023 +0200
- + - + - - + + + + + + + + + + + + + @@ -81839,14 +81851,16 @@ Date:   Thu Apr 20 18:53:17 2023 +0200
- + - - - + + - - + + + + + @@ -81936,7 +81950,8 @@ Date:   Thu Apr 20 18:53:17 2023 +0200
 - + + @@ -81945,8 +81960,8 @@ Date:   Thu Apr 20 18:53:17 2023 +0200
- - + +