From 773325f1bc20e786ac26f9ea6c7f271ec644b32b Mon Sep 17 00:00:00 2001 From: Ichthyostega Date: Tue, 11 Jun 2024 02:48:23 +0200 Subject: [PATCH] Library: rearrange strategy code MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Parts of the decision logic for element handling was packaged as separate »strategy« class — but this turned out to be neither a real abstraction, nor configurable in any way. Thus it is better to simplify the structure and turn these type predicates into simple private member functions of the SeveralBuilder itself --- src/lib/several-builder.hpp | 262 ++++++++++++------------- tests/library/several-builder-test.cpp | 4 +- wiki/thinkPad.ichthyo.mm | 188 +++++++++++------- 3 files changed, 237 insertions(+), 217 deletions(-) diff --git a/src/lib/several-builder.hpp b/src/lib/several-builder.hpp index 84764396f..fa3ba6e55 100644 --- a/src/lib/several-builder.hpp +++ b/src/lib/several-builder.hpp @@ -89,7 +89,13 @@ namespace lib { using util::max; using util::min; using util::_Fmt; + using std::is_trivially_move_constructible_v; using std::is_trivially_destructible_v; + using std::has_virtual_destructor_v; + using std::is_trivially_copyable_v; + using std::is_same_v; + using lib::meta::is_Subclass; + /** * Helper to determine the »spread« required to hold @@ -187,7 +193,6 @@ namespace lib { }; }; - using std::is_trivially_copyable_v; template class ALO> struct AllocationPolicy @@ -217,51 +222,6 @@ namespace lib { catch(...) { newBucket->destroy(); } return newBucket; -/* - size_t buffSiz{data? data->buffSiz : 0}; - if (demand == buffSiz) - return data; - if (demand > buffSiz) - {// grow into exponentially expanded new allocation - size_t spread = data? data->spread : sizeof(I); - size_t safetyLim = LUMIERA_MAX_ORDINAL_NUMBER * spread; - size_t expandAlloc = min (safetyLim - ,max (2*buffSiz, demand)); - if (expandAlloc < demand) - throw err::State{_Fmt{"Storage expansion for Several-collection " - "exceeds safety limit of %d bytes"} % safetyLim - ,LERR_(SAFETY_LIMIT)}; - // allocate new storage block... - size_t newCnt = demand / spread; - if (newCnt * spread < demand) ++newCnt; - Bucket* newBucket = Fac::create (newCnt, spread); - // move (or copy) existing data... - size_t cnt = data? data->cnt : 0; - for (size_t idx=0; idx (newBucket, idx - ,std::move_if_noexcept (data->subscript(idx))); - ////////////////////////////////////////////////////////OOO schee... aba mia brauchn E, ned I !!!!! - // discard old storage - if (data) - Fac::template destroy (data); - return newBucket; - } - else - {// shrink into precisely fitting new allocation - Bucket* newBucket{nullptr}; - if (data) - { - size_t cnt{data->cnt}; - ASSERT (cnt > 0); - newBucket = Fac::create (cnt, data->spread); - for (size_t idx=0; idx (newBucket, idx - ,std::move_if_noexcept (data->subscript(idx))); ////////////OOO selbes Problem: E hier - Fac::template destroy (data); - } - return newBucket; - } - */ } void @@ -286,91 +246,8 @@ namespace lib { template using HeapOwn = AllocationPolicy; - - using std::is_trivially_move_constructible_v; - using std::is_trivially_destructible_v; - using std::has_virtual_destructor_v; - using std::is_trivially_copyable_v; - using std::is_same_v; - using lib::meta::is_Subclass; - - template - struct HandlingStrategy - { - enum DestructionMethod{ UNKNOWN - , TRIVIAL - , ELEMENT - , VIRTUAL - }; - static Literal - render (DestructionMethod m) - { - switch (m) - { - case TRIVIAL: return "trivial"; - case ELEMENT: return "fixed-element-type"; - case VIRTUAL: return "virtual-baseclass"; - default: - throw err::Logic{"unknown DestructionMethod"}; - } - } - - DestructionMethod destructor{UNKNOWN}; - bool lock_move{false}; - - - /** mark that we're about to accept an otherwise unknown type, - * which can not be trivially moved. This irrevocably disables - * relocations by low-level `memove` for this container instance */ - template - void - probeMoveCapability() - { - if (not (is_same_v or is_trivially_copyable_v)) - lock_move = true; - } - - bool - canWildMove() - { - return is_trivially_copyable_v and not lock_move; - } - - template - typename ArrayBucket::Deleter - selectDestructor (FAC const& factory) - { - if (is_Subclass() and has_virtual_destructor_v) - { - __ensureMark (VIRTUAL); - return [factory](ArrayBucket* bucket){ unConst(factory).template destroy (bucket); }; - } - if (is_trivially_destructible_v) - { - __ensureMark (TRIVIAL); - return [factory](ArrayBucket* bucket){ unConst(factory).template destroy (bucket); }; - } - if (is_same_v and is_Subclass()) - { - __ensureMark (ELEMENT); - return [factory](ArrayBucket* bucket){ unConst(factory).template destroy (bucket); }; - } - throw err::Invalid{_Fmt{"Unsupported kind of destructor for element type %s."} - % util::typeStr()}; - } - - template - void - __ensureMark (DestructionMethod expectedKind) - { - if (destructor != UNKNOWN and destructor != expectedKind) - throw err::Invalid{_Fmt{"Unable to handle destructor for element type %s, " - "since this container has been primed to use %s-deleters."} - % util::typeStr() % render(expectedKind)}; - destructor = expectedKind; - } - }; - } + }//(End)implementation details + /** * Wrap a vector holding objects of a subtype and @@ -387,7 +264,8 @@ namespace lib { { using Coll = Several; - HandlingStrategy handling_{}; + using Bucket = ArrayBucket; + using Deleter = typename Bucket::Deleter; public: SeveralBuilder() = default; @@ -400,6 +278,8 @@ namespace lib { { } + /* ===== Builder API ===== */ + SeveralBuilder&& reserve (size_t cntElm) { @@ -415,6 +295,12 @@ namespace lib { return move(*this); } + + /** + * Terminal Builder: complete and lock the collection contents. + * @note the SeveralBuilder is sliced away, effectively + * returning only the pointer to the ArrayBucket. + */ Several build() { @@ -435,10 +321,10 @@ namespace lib { emplaceElm (ARGS&& ...args) { // mark when target type is not trivially movable - handling_.template probeMoveCapability(); + probeMoveCapability(); // ensure sufficient element capacity or the ability to adapt element spread - if (Coll::spread() < reqSiz() and not (Coll::empty() or handling_.canWildMove())) + if (Coll::spread() < reqSiz() and not (Coll::empty() or canWildMove())) throw err::Invalid{_Fmt{"Unable to place element of type %s (size=%d)" "into container for element size %d."} % util::typeStr() % reqSiz() % Coll::spread()}; @@ -446,7 +332,7 @@ namespace lib { // ensure sufficient storage or verify the ability to re-allocate if (not (Coll::hasReserve(reqSiz()) or POL::canExpand(reqSiz()) - or not handling_.lock_move)) + or canDynGrow())) throw err::Invalid{_Fmt{"Unable to accommodate further element of type %s "} % util::typeStr()}; @@ -465,8 +351,7 @@ namespace lib { ensureDeleter() { // ensure clean-up can be handled properly - typename POL::Fac& factory(*this); - typename ArrayBucket::Deleter deleterFunctor = handling_.template selectDestructor (factory); + Deleter deleterFunctor = selectDestructor(); if (Coll::data_->deleter) return; Coll::data_->deleter = deleterFunctor; } @@ -493,15 +378,18 @@ namespace lib { Coll::data_ = POL::realloc (Coll::data_, newCnt,spread); } ENSURE (Coll::data_); - if (handling_.canWildMove() and spread != Coll::spread()) + if (canWildMove() and spread != Coll::spread()) adjustSpread (spread); } void fitStorage() { - if (handling_.lock_move or not Coll::data_) + if (not Coll::data) return; + if (not canDynGrow()) + throw err::Invalid{"Unable to shrink storage for Several-collection, " + "since at least one element can not be moved."}; Coll::data_ = POL::realloc (Coll::data_, Coll::size(), Coll::spread()); } @@ -537,6 +425,102 @@ namespace lib { newPos += idx * newSpread; std::memmove (newPos, oldPos, util::min (oldSpread,newSpread)); } + + + + /* ==== Logic do decide about possible element handling ==== */ + + enum DestructionMethod{ UNKNOWN + , TRIVIAL + , ELEMENT + , VIRTUAL + }; + static Literal + render (DestructionMethod m) + { + switch (m) + { + case TRIVIAL: return "trivial"; + case ELEMENT: return "fixed-element-type"; + case VIRTUAL: return "virtual-baseclass"; + default: + throw err::Logic{"unknown DestructionMethod"}; + } + } + + DestructionMethod destructor{UNKNOWN}; + bool lock_move{false}; + + + /** + * Select a suitable method for invoking the element destructors + * and build a λ-object to be stored as deleter function alongside + * with the data; this includes a _copy_ of the embedded allocator, + * which in many cases is a monostate empty base class. + * @note this collection is _primed_ by the first element added, + * causing to lock into one of the possible destructor schemes; + * the reason is, we do not retain the information of the individual + * element types and thus we must employ one coherent scheme for all. + */ + template + Deleter + selectDestructor () + { + typename POL::Fac& factory(*this); + + if (is_Subclass() and has_virtual_destructor_v) + { + __ensureMark (VIRTUAL); + return [factory](ArrayBucket* bucket){ unConst(factory).template destroy (bucket); }; + } + if (is_trivially_destructible_v) + { + __ensureMark (TRIVIAL); + return [factory](ArrayBucket* bucket){ unConst(factory).template destroy (bucket); }; + } + if (is_same_v and is_Subclass()) + { + __ensureMark (ELEMENT); + return [factory](ArrayBucket* bucket){ unConst(factory).template destroy (bucket); }; + } + throw err::Invalid{_Fmt{"Unsupported kind of destructor for element type %s."} + % util::typeStr()}; + } + + template + void + __ensureMark (DestructionMethod expectedKind) + { + if (destructor != UNKNOWN and destructor != expectedKind) + throw err::Invalid{_Fmt{"Unable to handle destructor for element type %s, " + "since this container has been primed to use %s-deleters."} + % util::typeStr() % render(expectedKind)}; + destructor = expectedKind; + } + + + /** mark that we're about to accept an otherwise unknown type, + * which can not be trivially moved. This irrevocably disables + * relocations by low-level `memove` for this container instance */ + template + void + probeMoveCapability() + { + if (not (is_same_v or is_trivially_copyable_v)) + lock_move = true; + } + + bool + canWildMove() + { + return is_trivially_copyable_v and not lock_move; + } + + bool + canDynGrow() + { + return not lock_move; + } }; diff --git a/tests/library/several-builder-test.cpp b/tests/library/several-builder-test.cpp index 71cf2600c..f4190681b 100644 --- a/tests/library/several-builder-test.cpp +++ b/tests/library/several-builder-test.cpp @@ -116,8 +116,8 @@ namespace test{ - /** @test TODO demonstrate basic behaviour - * @todo WIP 6/24 ✔ define ⟶ 🔁 implement + /** @test demonstrate basic behaviour + * @todo WIP 6/24 ✔ define ⟶ ✔ implement */ void simpleUsage() diff --git a/wiki/thinkPad.ichthyo.mm b/wiki/thinkPad.ichthyo.mm index ac6086d21..fe503f725 100644 --- a/wiki/thinkPad.ichthyo.mm +++ b/wiki/thinkPad.ichthyo.mm @@ -81662,8 +81662,8 @@ Date:   Thu Apr 20 18:53:17 2023 +0200
- - + + @@ -81707,10 +81707,10 @@ Date:   Thu Apr 20 18:53:17 2023 +0200
- - - - + + + + @@ -81728,7 +81728,7 @@ Date:   Thu Apr 20 18:53:17 2023 +0200
- + @@ -81776,8 +81776,8 @@ Date:   Thu Apr 20 18:53:17 2023 +0200
- - + + @@ -81839,8 +81839,8 @@ Date:   Thu Apr 20 18:53:17 2023 +0200
- - + + @@ -82009,9 +82009,10 @@ Date:   Thu Apr 20 18:53:17 2023 +0200
- - - + + + + @@ -82029,7 +82030,7 @@ Date:   Thu Apr 20 18:53:17 2023 +0200
- + @@ -82039,7 +82040,7 @@ Date:   Thu Apr 20 18:53:17 2023 +0200
- + @@ -82108,7 +82109,7 @@ Date:   Thu Apr 20 18:53:17 2023 +0200
- + @@ -82221,9 +82222,11 @@ Date:   Thu Apr 20 18:53:17 2023 +0200
- + - + + + @@ -82304,7 +82307,9 @@ Date:   Thu Apr 20 18:53:17 2023 +0200
- + + + @@ -82332,7 +82337,8 @@ Date:   Thu Apr 20 18:53:17 2023 +0200
- + + @@ -82340,7 +82346,7 @@ Date:   Thu Apr 20 18:53:17 2023 +0200
- + @@ -82407,11 +82413,11 @@ Date:   Thu Apr 20 18:53:17 2023 +0200
- - - + + + - + @@ -82428,9 +82434,10 @@ Date:   Thu Apr 20 18:53:17 2023 +0200
- - - + + + + @@ -82492,8 +82499,8 @@ Date:   Thu Apr 20 18:53:17 2023 +0200
- - + + @@ -82511,8 +82518,8 @@ Date:   Thu Apr 20 18:53:17 2023 +0200
- - + + @@ -82542,9 +82549,9 @@ Date:   Thu Apr 20 18:53:17 2023 +0200
- + - + @@ -82568,27 +82575,44 @@ Date:   Thu Apr 20 18:53:17 2023 +0200
+ + + + + + + + + +

+ ...das mach die Verwendung einfacher (kein "template"-Präfix vor jeder Methode) und speziell das Deleter-λ kann nun direkt auf die geerbte Factory mit dem eingebetteten Allokator durchgreifen; ja der Code ist inzwischen riesengroß, aber alles hängt irgendwie mit allem zusammen, und ich sehe nicht, wie ich hier eine Teilkomponente so extrahieren könnte, daß der Code wirklich einfacher wird. Jedenfalls die separate Strategy-Klasse ist es nicht... +

+ + +
- - - + + + + + - - + + - - + + - - + + @@ -82624,9 +82648,9 @@ Date:   Thu Apr 20 18:53:17 2023 +0200
- - - + + + @@ -82677,8 +82701,8 @@ Date:   Thu Apr 20 18:53:17 2023 +0200
- - + + @@ -82736,14 +82760,14 @@ Date:   Thu Apr 20 18:53:17 2023 +0200
- - - - + + + + - + @@ -82756,19 +82780,12 @@ Date:   Thu Apr 20 18:53:17 2023 +0200
- + - - - - - - - - - - - + + + + @@ -82792,6 +82809,25 @@ Date:   Thu Apr 20 18:53:17 2023 +0200
+ + + + + + + + + + +

+ Trick: capture der factory per value ⟹ Kopie des Allokators +

+ + +
+ +
+
@@ -82814,7 +82850,7 @@ Date:   Thu Apr 20 18:53:17 2023 +0200
- + @@ -82934,8 +82970,8 @@ Date:   Thu Apr 20 18:53:17 2023 +0200
- - + + @@ -82954,7 +82990,7 @@ Date:   Thu Apr 20 18:53:17 2023 +0200
- + @@ -83018,7 +83054,7 @@ Date:   Thu Apr 20 18:53:17 2023 +0200
- + @@ -83204,12 +83240,12 @@ Date:   Thu Apr 20 18:53:17 2023 +0200
- - + + - + @@ -83276,8 +83312,8 @@ Date:   Thu Apr 20 18:53:17 2023 +0200
- - + +