From 856d8a3b519e4fc99e7b6749fbed67df33128741 Mon Sep 17 00:00:00 2001 From: Ichthyostega Date: Thu, 20 Apr 2023 18:53:17 +0200 Subject: [PATCH] Library: allow to reverse intrusive single linked list Looks like we'll actually retain and use this low-level solution in cases where we just can not afford heap allocations but need to keep polymorphic objects close to one another in memory. Since single linked lists are filled by prepending, it is rather common to need the reversed order of elements for traversal, which can be achieved in linear time. And while we're here, we can modernise the templated emplacement functions --- src/lib/allocation-cluster.cpp | 11 +- src/lib/allocation-cluster.hpp | 55 +--- src/lib/linked-elements.hpp | 259 +++++------------- ...ispatcher-mock.hpp => mock-dispatcher.hpp} | 17 +- tests/library/linked-elements-test.cpp | 112 ++++++-- 5 files changed, 193 insertions(+), 261 deletions(-) rename tests/core/steam/engine/{dispatcher-mock.hpp => mock-dispatcher.hpp} (90%) diff --git a/src/lib/allocation-cluster.cpp b/src/lib/allocation-cluster.cpp index 6d37bd96f..5007e5c47 100644 --- a/src/lib/allocation-cluster.cpp +++ b/src/lib/allocation-cluster.cpp @@ -73,7 +73,7 @@ namespace lib { public: MemoryManager(TypeInfo info) : top_(0) { reset(info); } - ~MemoryManager() { purge(); } + ~MemoryManager() { purge(); } size_t size() const; @@ -224,8 +224,8 @@ namespace lib { void* AllocationCluster::initiateAlloc (size_t& slot) { - if (!slot || slot > typeHandlers_.size() || !handler(slot) ) - return 0; // Memory manager not yet initialised + if (!slot or slot > typeHandlers_.size() or !handler(slot) ) + return nullptr; // Memory manager not yet initialised else return handler(slot)->allocate(); } @@ -241,7 +241,7 @@ namespace lib { if (slot > typeHandlers_.size()) typeHandlers_.resize(slot); - if (!handler(slot)) + if (not handler(slot)) handler(slot).reset (new MemoryManager (type)); } @@ -290,7 +290,4 @@ namespace lib { - - - } // namespace lib diff --git a/src/lib/allocation-cluster.hpp b/src/lib/allocation-cluster.hpp index bf9fb1872..6095fc85e 100644 --- a/src/lib/allocation-cluster.hpp +++ b/src/lib/allocation-cluster.hpp @@ -53,6 +53,7 @@ #include "lib/scoped-holder.hpp" #include "lib/scoped-holder-transfer.hpp" +#include #include @@ -93,46 +94,14 @@ namespace lib { public: AllocationCluster (); - ~AllocationCluster () noexcept; + ~AllocationCluster () noexcept; - template + template TY& - create () + create (ARGS&& ...args) { - TY* obj = new(allocation()) TY(); - return commit(obj); - } - - template - TY& - create (P0& p0) - { - TY* obj = new(allocation()) TY (p0); - return commit(obj); - } - - template - TY& - create (P0& p0, P1& p1) - { - TY* obj = new(allocation()) TY (p0,p1); - return commit(obj); - } - - template - TY& - create (P0& p0, P1& p1, P2& p2) - { - TY* obj = new(allocation()) TY (p0,p1,p2); - return commit(obj); - } - - template - TY& - create (P0& p0, P1& p1, P2& p2, P3& p3) - { - TY* obj = new(allocation()) TY (p0,p1,p2,p3); + TY* obj = new(allocation()) TY (std::forward (args)...); return commit(obj); } @@ -178,9 +147,9 @@ namespace lib { static size_t maxTypeIDs; - typedef ScopedPtrHolder HMemManager; - typedef Allocator_TransferNoncopyable Allo; - typedef std::vector ManagerTable; + using HMemManager = ScopedPtrHolder; + using Allo = Allocator_TransferNoncopyable; + using ManagerTable = std::vector; ManagerTable typeHandlers_; ///< table of active MemoryManager instances @@ -253,7 +222,7 @@ namespace lib { setup() { ClassLock guard; - if (!id_) + if (0 == id_) id_= ++maxTypeIDs; return TypeInfo ((TY*) 0 ); @@ -282,7 +251,7 @@ namespace lib { AllocationCluster::allocation() { void *mem = initiateAlloc (TypeSlot::get()); - if (!mem) + if (not mem) mem = initiateAlloc (TypeSlot::setup(),TypeSlot::get()); ENSURE (mem); return mem; @@ -311,7 +280,5 @@ namespace lib { - - } // namespace lib -#endif +#endif /*LIB_ALLOCATION_CLUSTER_H*/ diff --git a/src/lib/linked-elements.hpp b/src/lib/linked-elements.hpp index e11da68e4..f0c1d1f93 100644 --- a/src/lib/linked-elements.hpp +++ b/src/lib/linked-elements.hpp @@ -24,7 +24,7 @@ ** Intrusive single linked list with optional ownership. ** This helper template allows to attach a number of tightly integrated ** elements with low overhead. Typically, these elements are to be attached - ** once and never changed. Optionally, elements can be created using a + ** once and never changed. Optionally, elements can be created using a ** custom allocation scheme; the holder might also take ownership. These ** variations in functionality are controlled by policy templates. ** @@ -37,8 +37,8 @@ ** - convenient access through Lumiera Forward Iterators ** - the need to integrate tightly with a custom allocator ** - ** @note this linked list container is \em intrusive and thus needs the help - ** of the element type, which must provide a pointer member \c next. + ** @note this linked list container is _intrusive_ and thus needs the help + ** of the element type, which must *provide a pointer member* `next`. ** Consequently, each such node element can't be member in ** multiple collections at the same time ** @note as usual, any iterator relies on the continued existence and @@ -65,8 +65,7 @@ #include "lib/iter-adapter.hpp" #include "lib/util.hpp" -#include - +#include namespace lib { @@ -81,8 +80,11 @@ namespace lib { /** * Policy for LinkedElements: taking ownership * and possibly creating heap allocated Nodes + * @note is util::MoveOnly to enforce ownership + * on behalf of LinkedElements */ struct OwningHeapAllocated + : util::MoveOnly { typedef void* CustomAllocator; @@ -99,69 +101,11 @@ namespace lib { /** this policy creates new elements * simply by heap allocation */ - template + template TY& - create () + create (ARGS&& ...args) { - return *new TY(); - } - - - template - TY& //____________________________________________ - create (A1 a1) ///< create object of type TY, using 1-arg ctor - { - return *new TY(a1); - } - - - template< class TY - , typename A1 - , typename A2 - > - TY& //____________________________________________ - create (A1 a1, A2 a2) ///< create object of type TY, using 2-arg ctor - { - return *new TY(a1,a2); - } - - - template< class TY - , typename A1 - , typename A2 - , typename A3 - > - TY& //____________________________________________ - create (A1 a1, A2 a2, A3 a3) ///< create object of type TY, using 3-arg ctor - { - return *new TY(a1,a2,a3); - } - - - template< class TY - , typename A1 - , typename A2 - , typename A3 - , typename A4 - > - TY& //____________________________________________ - create (A1 a1, A2 a2, A3 a3, A4 a4) ///< create object of type TY, using 4-arg ctor - { - return *new TY(a1,a2,a3,a4); - } - - - template< class TY - , typename A1 - , typename A2 - , typename A3 - , typename A4 - , typename A5 - > - TY& //____________________________________________ - create (A1 a1, A2 a2, A3 a3, A4 a4, A5 a5) ///< create object of type TY, using 5-arg ctor - { - return *new TY(a1,a2,a3,a4,a5); + return *new TY (std::forward (args)...); } }; @@ -191,6 +135,13 @@ namespace lib { { /* does nothing */ } + + template + TY& + create (ARGS&&...) + { + static_assert ("NoOwnership allocation strategy can not allocate elements"); + } // ... you'll have to do that yourself (that's the whole point of using this strategy) }; @@ -234,55 +185,11 @@ namespace lib { } - template - TY& //__________________________________________ - create () ///< place object of type TY, by default ctor + template + TY& + create (ARGS&& ...args) { - return cluster_.create(); - } - - - template - TY& //___________________________________________ - create (A1 a1) ///< place object of type TY, using 1-arg ctor - { - return cluster_.create (a1); - } - - - template< class TY - , typename A1 - , typename A2 - > - TY& //___________________________________________ - create (A1 a1, A2 a2) ///< place object of type TY, using 2-arg ctor - { - return cluster_.create (a1,a2); - } - - - template< class TY - , typename A1 - , typename A2 - , typename A3 - > - TY& //___________________________________________ - create (A1 a1, A2 a2, A3 a3) ///< place object of type TY, using 3-arg ctor - { - return cluster_.create (a1,a2,a3); - } - - - template< class TY - , typename A1 - , typename A2 - , typename A3 - , typename A4 - > - TY& //___________________________________________ - create (A1 a1, A2 a2, A3 a3, A4 a4) ///< place object of type TY, using 4-arg ctor - { - return cluster_.create (a1,a2,a3,a4); + return cluster_.create (std::forward (args)...); } }; @@ -315,8 +222,7 @@ namespace lib { , class ALO = linked_elements::OwningHeapAllocated > class LinkedElements - : util::NonCopyable - , ALO + : ALO { N* head_; @@ -332,6 +238,13 @@ namespace lib { : head_{nullptr} { } + LinkedElements (LinkedElements const&) = default; + LinkedElements (LinkedElements&& rr) + : head_{rr.head_} + { // possibly transfer ownership + rr.head_ = nullptr; + } + /** @param allo custom allocator or memory manager * to be used by the policy for creating and * discarding of node elements @@ -349,11 +262,11 @@ namespace lib { * causes already created elements to be destroyed */ template - LinkedElements (IT elements) + LinkedElements (IT&& elements) : head_{nullptr} { try { - pushAll (elements); + pushAll (std::forward (elements)); } catch(...) { @@ -410,75 +323,46 @@ namespace lib { } - template< class TY > - TY& //_________________________________________ - pushNew () ///< add object of type TY, using 0-arg ctor +// template< class TY =N> +// TY& //_________________________________________ +// emplace () ///< add object of type TY, using 0-arg ctor +// { +// return push (ALO::template create()); +// } + + /** prepend object of type TY, forwarding ctor args */ + template + TY& + emplace (ARGS&& ...args) { - return push (ALO::template create()); + return push (ALO::template create (std::forward (args)...)); } - template< class TY - , typename A1 - > - TY& //_________________________________________ - pushNew (A1 a1) ///< add object of type TY, using 1-arg ctor + + /** + * Mutate the complete list to change the order of elements. + * @remark since pushing prepends, elements are initially in reverse order + * @warning this operation invalidates iterators and has O(n) cost; + * ownership and elements themselves are not affected. + */ + LinkedElements& + reverse() { - return push (ALO::template create(a1)); + if (not empty()) + { + N* p = head_->next; + head_->next = nullptr; + while (p) + { + N& n = *p; + p = p->next; + push (n); + } } + return *this; } - template< class TY - , typename A1 - , typename A2 - > - TY& //_________________________________________ - pushNew (A1 a1, A2 a2) ///< add object of type TY, using 2-arg ctor - { - return push (ALO::template create(a1,a2)); - } - - - template< class TY - , typename A1 - , typename A2 - , typename A3 - > - TY& //_________________________________________ - pushNew (A1 a1, A2 a2, A3 a3) ///< add object of type TY, using 3-arg ctor - { - return push (ALO::template create(a1,a2,a3)); - } - - - template< class TY - , typename A1 - , typename A2 - , typename A3 - , typename A4 - > - TY& //_________________________________________ - pushNew (A1 a1, A2 a2, A3 a3, A4 a4) ///< add object of type TY, using 4-arg ctor - { - return push (ALO::template create(a1,a2,a3,a4)); - } - - - template< class TY - , typename A1 - , typename A2 - , typename A3 - , typename A4 - , typename A5 - > - TY& //_________________________________________ - pushNew (A1 a1, A2 a2, A3 a3, A4 a4, A5 a5) ///< add object of type TY, using 5-arg ctor - { - return push (ALO::template create(a1,a2,a3,a4,a5)); - } - - - /* === Element access and iteration === */ @@ -487,13 +371,13 @@ namespace lib { operator[] (size_t index) const { N* p = head_; - while (p && index) + while (p and index) { p = p->next; --index; } - if (!p || index) + if (!p or index) throw error::Logic ("Attempt to access element beyond the end of LinkedElements list" , LERR_(INDEX_BOUNDS)); else @@ -501,9 +385,18 @@ namespace lib { } + /** @warning unchecked */ + N& + top() const + { + REQUIRE (head_, "NIL"); + return *(unConst(this)->head_); + } + + /** @warning traverses to count the elements */ size_t - size () const + size() const { uint count=0; N* p = head_; @@ -517,7 +410,7 @@ namespace lib { bool - empty () const + empty() const { return !head_; } diff --git a/tests/core/steam/engine/dispatcher-mock.hpp b/tests/core/steam/engine/mock-dispatcher.hpp similarity index 90% rename from tests/core/steam/engine/dispatcher-mock.hpp rename to tests/core/steam/engine/mock-dispatcher.hpp index 72dcc89e6..036bf48be 100644 --- a/tests/core/steam/engine/dispatcher-mock.hpp +++ b/tests/core/steam/engine/mock-dispatcher.hpp @@ -39,7 +39,7 @@ ////#include "lib/time/timequant.hpp" ////#include "lib/format-cout.hpp" #include "lib/depend.hpp" -//#include "lib/itertools.hpp" +#include "lib/itertools.hpp" //#include "lib/util-coll.hpp" #include "lib/test/test-helper.hpp" //#include "lib/util.hpp" @@ -115,8 +115,19 @@ namespace test { } }; + /// @deprecated this setup is confusing and dangerous (instance identity is ambiguous) lib::Depend mockDispatcher; + + + inline auto + defineBottomProvision() + { +// return lib::singleValIterator( +// JobTicket::buildProvision ( +// )); + } + }//(End)internal test helpers.... #if false /////////////////////////////////////////////////////////////////////////////////////////////////////////////UNIMPLEMENTED :: TICKET #1221 @@ -134,7 +145,9 @@ namespace test { { public: - MockJobTicket() { }; +// MockJobTicket() +// : JobTicket{JobTicket::Provisions{defineBottomProvision()}} +// { }; private: }; diff --git a/tests/library/linked-elements-test.cpp b/tests/library/linked-elements-test.cpp index 15820c0aa..caaf471b4 100644 --- a/tests/library/linked-elements-test.cpp +++ b/tests/library/linked-elements-test.cpp @@ -69,15 +69,15 @@ namespace test{ { Nummy* next; - Nummy() + Nummy() : Dummy() - , next(0) + , next{0} { } explicit Nummy (int i) - : Dummy(i) - , next(0) + : Dummy{i} + , next{0} { if (i == exception_trigger) throw error::Fatal("simulated error", LERR_(PROVOKED_FAILURE)); @@ -102,7 +102,7 @@ namespace test{ /** - * Helper to produce a pre-determined series + * Helper to produce a pre-determined series * of objects to populate a LinkedElements list. * @note just happily heap allocating new instances * and handing them out. The LinkedElements list @@ -140,7 +140,6 @@ namespace test{ class Populator : public NummyGenerator::iterator { - public: explicit Populator (uint numElms) @@ -165,13 +164,13 @@ namespace test{ /// default case: ownership for heap allocated nodes - typedef LinkedElements List; + using List = LinkedElements; /// managing existing node elements without taking ownership - typedef LinkedElements ListNotOwner; + using ListNotOwner = LinkedElements; /// creating nodes in-place, using a custom allocator for creation and disposal - typedef LinkedElements ListCustomAllocated; + using ListCustomAllocated = LinkedElements; @@ -188,6 +187,7 @@ namespace test{ { simpleUsage(); iterating(); + reverseList(); verify_nonOwnership(); verify_ExceptionSafety(); @@ -207,11 +207,11 @@ namespace test{ CHECK (0 == elements.size()); CHECK (0 == Dummy::checksum()); - elements.pushNew(1); - elements.pushNew(2); - elements.pushNew(3); - elements.pushNew(4); - elements.pushNew(5); + elements.emplace(1); + elements.emplace(2); + elements.emplace(3); + elements.emplace(4); + elements.emplace(5); CHECK (!isnil (elements)); CHECK (5 == elements.size()); CHECK (0 != Dummy::checksum()); @@ -227,9 +227,9 @@ namespace test{ CHECK (0 == elements.size()); CHECK (0 == Dummy::checksum()); - elements.pushNew(); - elements.pushNew(); - elements.pushNew(); + elements.emplace(); + elements.emplace(); + elements.emplace(); CHECK (3 == elements.size()); CHECK (0 != Dummy::checksum()); @@ -245,9 +245,9 @@ namespace test{ { List elements; for (uint i=1; i<=NUM_ELEMENTS; ++i) - elements.pushNew(i); + elements.emplace(i); - // since elements where pushed, + // since elements where pushed, // they should appear in reversed order int check=NUM_ELEMENTS; List::iterator ii = elements.begin(); @@ -293,6 +293,68 @@ namespace test{ } + void + reverseList() + { + CHECK (0 == Dummy::checksum()); + { + List list; + CHECK (isnil (list)); + list.reverse(); + CHECK (isnil (list)); + CHECK (0 == Dummy::checksum()); + + list.emplace(1); + CHECK (not isnil (list)); + CHECK (1 == list[0].getVal()); + CHECK (1 == Dummy::checksum()); + list.reverse(); + CHECK (1 == Dummy::checksum()); + CHECK (1 == list[0].getVal()); + CHECK (not isnil (list)); + + list.emplace(2); + CHECK (not isnil (list)); + CHECK (2 == list.size()); + CHECK (2 == list[0].getVal()); + CHECK (2+1 == Dummy::checksum()); + list.reverse(); + CHECK (1+2 == Dummy::checksum()); + CHECK (1 == list[0].getVal()); + CHECK (2 == list.size()); + + list.emplace(3); + CHECK (3 == list.size()); + CHECK (3 == list.top().getVal()); + CHECK (3+1+2 == Dummy::checksum()); + list.reverse(); + CHECK (2 == list[0].getVal()); + CHECK (1 == list[1].getVal()); + CHECK (3 == list[2].getVal()); + List::iterator ii = list.begin(); + CHECK (2 == ii->getVal()); + ++ii; + CHECK (1 == ii->getVal()); + ++ii; + CHECK (3 == ii->getVal()); + ++ii; + CHECK (isnil (ii)); + CHECK (2+1+3 == Dummy::checksum()); + + list.emplace(4); + CHECK (4 == list.top().getVal()); + CHECK (3 == list[3].getVal()); + list.reverse(); + CHECK (3 == list[0].getVal()); + CHECK (1 == list[1].getVal()); + CHECK (2 == list[2].getVal()); + CHECK (4 == list[3].getVal()); + CHECK (3+1+2+4 == Dummy::checksum()); + } + CHECK (0 == Dummy::checksum()); + } + + /** @test add some node elements to the LinkedElements list * but without taking ownership or performing any * memory management. This usage pattern is helpful @@ -346,11 +408,11 @@ namespace test{ __triggerErrorAt(3); - elements.pushNew(1); - elements.pushNew(2); + elements.emplace(1); + elements.emplace(2); CHECK (1+2 == Dummy::checksum()); - VERIFY_ERROR (PROVOKED_FAILURE, elements.pushNew(3) ); + VERIFY_ERROR (PROVOKED_FAILURE, elements.emplace(3) ); CHECK (1+2 == Dummy::checksum()); CHECK (2 == elements.size()); @@ -420,9 +482,9 @@ namespace test{ ListCustomAllocated elements(allocator); - elements.pushNew> (2); - elements.pushNew> (4,5); - elements.pushNew> (7,8,9); + elements.emplace> (2); + elements.emplace> (4,5); + elements.emplace> (7,8,9); CHECK (sum(9) == Dummy::checksum()); CHECK (3 == allocator.size());