Library: AllocationCluster and SeveralBuilder logic tweaks

...identified as part of bug investigation

 * make clear that reserve() prepares for an absolute capacity
 * clarify that, to the contrary, ensureStorageCapaciy() means the delta

Moreover, it turns out that the assertion regarding storage limits
triggers frequently while writing the test code; so we can conclude
that the `AllocationCluster` interface lures into allocating without
previous check. Consequently, this check now throws a runtime exception.

As an aside, the size limitation should be accessible on the interface,
similar to `std::vector::max_size()`
This commit is contained in:
Fischlurch 2024-06-19 15:22:26 +02:00
parent 9709309186
commit 39e9ecd90e
8 changed files with 217 additions and 40 deletions

View file

@ -40,20 +40,20 @@
#include "lib/allocation-cluster.hpp"
#include "lib/linked-elements.hpp"
#include "lib/format-string.hpp"
#include "lib/util-quant.hpp"
#include "lib/util.hpp"
using util::unConst;
using util::isPow2;
using util::isnil;
using util::_Fmt;
using std::byte;
namespace lib {
namespace {// Configuration constants
const size_t EXTENT_SIZ = 256;
const size_t OVERHEAD = 2 * sizeof(void*);
const size_t STORAGE_SIZ = EXTENT_SIZ - OVERHEAD;
namespace {// Internals...
/**
* Special allocator-policy for lib::LinkedElements
@ -77,7 +77,10 @@ namespace lib {
}
};
}
}//(End)configuration and internals
/**
@ -105,7 +108,7 @@ namespace lib {
{
Extent* next;
Destructors dtors;
std::byte storage[STORAGE_SIZ];
std::byte storage[max_size()];
};
using Extents = lib::LinkedElements<Extent>;
@ -167,8 +170,8 @@ namespace lib {
size_t
calcAllocInCurrentBlock() const
{
ENSURE (STORAGE_SIZ >= view_.storage.rest);
return STORAGE_SIZ - view_.storage.rest;
ENSURE (max_size() >= view_.storage.rest);
return max_size() - view_.storage.rest;
}
@ -197,7 +200,7 @@ namespace lib {
{
view_.extents.emplace();
view_.storage.pos = & view_.extents.top().storage;
view_.storage.rest = STORAGE_SIZ;
view_.storage.rest = max_size();
}
};
@ -246,7 +249,7 @@ namespace lib {
void
AllocationCluster::expandStorage (size_t allocRequest)
{
ENSURE (allocRequest + OVERHEAD <= EXTENT_SIZ);
ENSURE (allocRequest <= max_size());
StorageManager::access(*this).addBlock();
}
@ -259,21 +262,34 @@ namespace lib {
/* === diagnostics helpers === */
bool
AllocationCluster::_is_within_limits (size_t size, size_t align)
/**
* Allocation cluster uses a comparatively small tile size for its extents,
* which turns out to be a frequently encountered limitation in practice.
* This was deemed acceptable, due to its orientation towards performance.
* @throws err::Fatal when a desired allocation can not be accommodated
*/
void
AllocationCluster::__enforce_limits (size_t allocSiz, size_t align)
{
auto isPower2 = [](size_t n){ return !(n & (n-1)); };
return 0 < size
and 0 < align
and size <= STORAGE_SIZ
and align <= STORAGE_SIZ
and isPower2 (align);
REQUIRE (allocSiz);
REQUIRE (align);
REQUIRE (isPow2 (align));
if (allocSiz > max_size())
throw err::Fatal{_Fmt{"AllocationCluster: desired allocation of %d bytes "
"exceeds the fixed extent size of %d"} % allocSiz % max_size()
,LERR_(CAPACITY)};
if (align > max_size())
throw err::Fatal{_Fmt{"AllocationCluster: data requires alignment at %d bytes, "
"which is beyond the fixed extent size of %d"} % align % max_size()
,LERR_(CAPACITY)};
}
/* === diagnostics helpers === */
size_t
AllocationCluster::numExtents() const
{
@ -291,7 +307,7 @@ namespace lib {
size_t extents = numExtents();
if (not extents) return 0;
size_t bytes = StorageManager::access (unConst(*this)).calcAllocInCurrentBlock();
return (extents - 1) * STORAGE_SIZ + bytes;
return (extents - 1) * max_size() + bytes;
}

View file

@ -115,6 +115,11 @@ namespace lib {
AllocationCluster ();
~AllocationCluster () noexcept;
/** hard wired size of storage extents */
static size_t constexpr EXTENT_SIZ = 256;
static size_t constexpr max_size();
/* === diagnostics === */
size_t numExtents() const;
size_t numBytes() const;
@ -161,7 +166,7 @@ namespace lib {
void*
allotMemory (size_t bytes, size_t alignment)
{
ENSURE (_is_within_limits (bytes, alignment));
__enforce_limits (bytes, alignment);
void* loc = storage_.allot(bytes, alignment);
if (loc) return loc;
expandStorage (bytes);
@ -201,7 +206,7 @@ namespace lib {
void expandStorage (size_t);
void registerDestructor (Destructor&);
bool _is_within_limits (size_t,size_t);
void __enforce_limits (size_t,size_t);
friend class test::AllocationCluster_test;
};
@ -212,6 +217,21 @@ namespace lib {
//-----implementation-details------------------------
/**
* Maximum individual allocation size that can be handled.
* @remark AllocationCluser expands its storage buffer in steps
* of fixed sized _tiles_ or _extents._ Doing so can be beneficial
* when clusters are frequently created and thrown away (which is the
* intended usage pattern). However, using such extents is inherently
* wasteful, and thus the size must be rather tightly limited.
*/
size_t constexpr
AllocationCluster::max_size()
{
size_t ADMIN_OVERHEAD = 2 * sizeof(void*);
return EXTENT_SIZ - ADMIN_OVERHEAD;
}
/**
* Factory function: place a new instance into this AllocationCluster,
* but *without invoking its destructor* on clean-up (for performance reasons).

View file

@ -136,6 +136,7 @@ namespace lib {
using util::max;
using util::min;
using util::_Fmt;
using util::positiveDiff;
using std::is_nothrow_move_constructible_v;
using std::is_trivially_move_constructible_v;
using std::is_trivially_destructible_v;
@ -369,14 +370,15 @@ namespace lib {
auto withAllocator (ARGS&& ...args);
/** ensure sufficient memory allocation up-front */
/** ensure up-front that a desired capacity is allocated */
template<typename TY =E>
SeveralBuilder&&
reserve (size_t cntElm =1
,size_t elmSiz =reqSiz<TY>())
{
size_t extraElm = positiveDiff (cntElm, Coll::size());
ensureElementCapacity<TY> (elmSiz);
ensureStorageCapacity<TY> (elmSiz,cntElm);
ensureStorageCapacity<TY> (elmSiz,extraElm);
elmSiz = max (elmSiz, Coll::spread());
adjustStorage (cntElm, elmSiz);
return move(*this);
@ -503,7 +505,7 @@ namespace lib {
% util::typeStr<TY>() % requiredSiz % Coll::spread()};
}
/** ensure sufficient storage reserve or verify the ability to re-allocate */
/** ensure sufficient storage reserve for \a newElms or verify the ability to re-allocate */
template<class TY>
void
ensureStorageCapacity (size_t requiredSiz =reqSiz<TY>(), size_t newElms =1)

View file

@ -38,6 +38,13 @@
namespace util {
template<typename N>
inline bool constexpr
isPow2 (N n)
{
return n > 0 and !(n & (n-1));
}; // at each power of 2, a new bit is set for the first time
/** helper to treat int or long division uniformly */
template<typename I>

View file

@ -112,6 +112,14 @@ namespace util {
and val <= upperBound;
}
template <typename UN, typename N2>
inline UN constexpr
positiveDiff (N2 newVal, UN refVal)
{
return UN(newVal) > refVal? UN(newVal) - refVal
: UN(0);
}
/** positive integral number from textual representation
* @return always a number, 0 in case of unparseable text,
* limited to 0 <= num <= LUMIERA_MAX_ORDINAL_NUMBER */

View file

@ -63,7 +63,7 @@ namespace test {
const uint NUM_TYPES = 20;
const uint NUM_OBJECTS = 500;
const size_t BLOCKSIZ = 256; ///< @warning actually defined in allocation-cluster.cpp
const size_t EXTSIZ = AllocationCluster::EXTENT_SIZ;
int64_t checksum = 0; // validate proper pairing of ctor/dtor calls
@ -242,17 +242,17 @@ namespace test {
CHECK (2 == clu.numBytes());
CHECK (clu.storage_.pos != nullptr);
CHECK (clu.storage_.pos == (& i1) + 1 ); // points directly behind the allocated integer
CHECK (clu.storage_.rest == BLOCKSIZ - (2*sizeof(void*) + sizeof(uint16_t)));
CHECK (clu.storage_.rest == EXTSIZ - (2*sizeof(void*) + sizeof(uint16_t)));
// Demonstration: how to reconstruct the start of the current extent
byte* blk = static_cast<std::byte*>(clu.storage_.pos);
blk += clu.storage_.rest - BLOCKSIZ;
blk += clu.storage_.rest - EXTSIZ;
CHECK(size_t(blk) < size_t(clu.storage_.pos));
// some abbreviations for navigating the raw storage blocks...
auto currBlock = [&]{
byte* blk = static_cast<std::byte*>(clu.storage_.pos);
blk += clu.storage_.rest - BLOCKSIZ;
blk += clu.storage_.rest - EXTSIZ;
return blk;
};
auto posOffset = [&]{
@ -273,7 +273,7 @@ namespace test {
uint16_t i1pre = i1;
auto& i2 = clu.create<uint16_t> (55555);
CHECK (posOffset() == 2 * sizeof(void*) + 2 * sizeof(uint16_t));
CHECK (clu.storage_.rest == BLOCKSIZ - posOffset());
CHECK (clu.storage_.rest == EXTSIZ - posOffset());
// existing storage unaffected
CHECK (i1 == i1pre);
CHECK (i2 == 55555);
@ -295,8 +295,8 @@ namespace test {
for (uint i=clu.storage_.rest; i>0; --i)
clu.create<uchar> (i);
CHECK (clu.storage_.rest == 0); // no space left in current extent
CHECK (posOffset() == BLOCKSIZ);
CHECK (clu.numBytes() == BLOCKSIZ - 2*sizeof(void*)); // now using all the rest behind the admin »slots«
CHECK (posOffset() == EXTSIZ);
CHECK (clu.numBytes() == EXTSIZ - 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
@ -305,8 +305,8 @@ namespace test {
char& c2 = clu.create<char> ('U');
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());
CHECK (clu.numBytes() == BLOCKSIZ - 2*sizeof(void*) + 1); // accounted allocation for the full first block + one byte
CHECK (clu.storage_.rest == EXTSIZ - posOffset());
CHECK (clu.numBytes() == EXTSIZ - 2*sizeof(void*) + 1); // accounted allocation for the full first block + one byte
CHECK (clu.numExtents() == 2); // we have two extents now
CHECK (slot(0) == size_t(blk)); // first »slot« of the current block points back to previous block
CHECK (i1 == i1pre);

View file

@ -610,8 +610,8 @@ namespace test{
{
auto builder = makeSeveral<Dummy>()
.withAllocator(clu)
.reserve(5)
.fillElm(5);
.reserve(4)
.fillElm(4);
size_t buffSiz = sizeof(Dummy) * builder.capacity();
size_t headerSiz = sizeof(ArrayBucket<Dummy>);
@ -631,6 +631,15 @@ SHOW_EXPR(buffSiz + headerSiz)
SHOW_EXPR(builder.size())
SHOW_EXPR(builder.capacity())
SHOW_EXPR(clu.numExtents())
SHOW_EXPR(clu.numBytes())
// now perform another unrelated allocation
Dummy& extraDummy = clu.create<Dummy>(55);
SHOW_EXPR(clu.numExtents())
SHOW_EXPR(clu.numBytes())
builder.reserve(9);
SHOW_EXPR(builder.size())
SHOW_EXPR(builder.capacity())
SHOW_EXPR(clu.numExtents())
SHOW_EXPR(clu.numBytes())
}
}

View file

@ -84196,8 +84196,123 @@ Date:&#160;&#160;&#160;Thu Apr 20 18:53:17 2023 +0200<br/>
</node>
<node BACKGROUND_COLOR="#eee5c3" COLOR="#990000" CREATED="1718730299524" ID="ID_1847366954" MODIFIED="1718730388370" TEXT="verifizieren, da&#xdf; die dynamische Anpassung stattgefunden hat">
<icon BUILTIN="flag-yellow"/>
<node BACKGROUND_COLOR="#eee5c3" COLOR="#990000" CREATED="1718761161744" ID="ID_532446008" MODIFIED="1718761180110" TEXT="auch demonstrieren, da&#xdf; das nach einer Zwischen-Allokaiton nicht mehr geht">
<icon BUILTIN="flag-yellow"/>
</node>
<node BACKGROUND_COLOR="#eee5c3" COLOR="#990000" CREATED="1718730317115" ID="ID_1509954190" MODIFIED="1718730388371" TEXT="es sollte sich genau die gleiche Speicherbelegung ergeben">
<node BACKGROUND_COLOR="#fafe99" COLOR="#fa002a" CREATED="1718761232124" ID="ID_482647269" MODIFIED="1718761241262" TEXT="Speicherbeschr&#xe4;nkung spricht an">
<icon BUILTIN="broken-line"/>
<node BACKGROUND_COLOR="#fdfdcf" COLOR="#ff0000" CREATED="1718761242724" ID="ID_103960271" MODIFIED="1718761290782" TEXT="sollte sie hier nicht">
<richcontent TYPE="NOTE"><html>
<head>
</head>
<body>
<p>
denn das eine zus&#228;tzliche Element w&#252;rde locker reinpassen &#8212; und zudem sollte ja nun der Block in den n&#228;chsten Extent migrieren
</p>
</body>
</html>
</richcontent>
<icon BUILTIN="yes"/>
</node>
<node CREATED="1718797226426" ID="ID_1835898397" MODIFIED="1718797232563" TEXT="Beobachtungen">
<node BACKGROUND_COLOR="#c8c0b6" COLOR="#990033" CREATED="1718797237918" ID="ID_652382356" MODIFIED="1718800423280" TEXT="verwemdet ensureStorageCapacity den Aufruf Coll::hasReserve korrekt?">
<icon BUILTIN="help"/>
<node CREATED="1718797275521" ID="ID_1971364291" MODIFIED="1718797292795" TEXT="geht es hier um ein Delta oder die absolute Forderung"/>
<node CREATED="1718797293664" ID="ID_1513751405" MODIFIED="1718797312042" TEXT="der umgebende Code behandelt die absolute Anforderung"/>
<node BACKGROUND_COLOR="#e0ceaa" COLOR="#690f14" CREATED="1718799189604" ID="ID_1391548509" MODIFIED="1718800485267" TEXT="bereits die dar&#xfc;berliegende ensureStorageCapacity() ist zweideutig">
<icon BUILTIN="messagebox_warning"/>
<node CREATED="1718799220944" ID="ID_767486342" MODIFIED="1718799361796" TEXT="sie wird zweimal verwendet">
<richcontent TYPE="NOTE"><html>
<head>
</head>
<body>
<p>
einmal aus emplaceNewElm() und einmal aus reserve()
</p>
</body>
</html>
</richcontent>
<icon BUILTIN="info"/>
</node>
<node CREATED="1718799228581" ID="ID_278040714" MODIFIED="1718799338634" TEXT="und in zwei verschiedenen Bedeutungen">
<icon BUILTIN="smiley-oh"/>
</node>
</node>
<node COLOR="#338800" CREATED="1718799242437" ID="ID_186272424" MODIFIED="1718800434682" TEXT="Festlegung: da soll es um die zus&#xe4;tzliche Kapazit&#xe4;t gehen">
<richcontent TYPE="NOTE"><html>
<head>
</head>
<body>
<p>
das ist es n&#228;mlich, worauf die Implementierung hinausl&#228;uft, und dieser Grebrauch ist auch naheliegend und korrekt, sofern es sich um das Einf&#252;gen neuer Datenelemente handelt. Insofern mu&#223; sich der andere Gebrauch in reserve() daran anpassen
</p>
</body>
</html></richcontent>
<icon BUILTIN="yes"/>
</node>
</node>
<node CREATED="1718797389002" ID="ID_487967037" MODIFIED="1718797406916" TEXT="adjustStorage versucht exponentielle Erweiterung">
<node CREATED="1718797408528" ID="ID_1641482989" MODIFIED="1718797416963" TEXT="obwohl explizit weniger gefordert ist"/>
<node BACKGROUND_COLOR="#fafe99" COLOR="#fa002a" CREATED="1718797462569" ID="ID_1609091342" MODIFIED="1718797572998" TEXT="es wird nur das Safety-Limit gepr&#xfc;ft, nicht aber der Allokator angefragt">
<icon BUILTIN="broken-line"/>
</node>
<node CREATED="1718797473087" ID="ID_1313216059" MODIFIED="1718797490217" TEXT="konkret: versucht stets von 128 auf 256 zu vergr&#xf6;&#xdf;ern"/>
<node BACKGROUND_COLOR="#e0ceaa" COLOR="#690f14" CREATED="1718797491900" ID="ID_1858549185" MODIFIED="1718797543741" TEXT="was dann scheiern mu&#xdf; (da der Extent nur Platz f&#xfc;r 240 hat">
<icon BUILTIN="clanbomber"/>
</node>
</node>
<node COLOR="#435e98" CREATED="1718798106197" ID="ID_285198133" MODIFIED="1718802934257" TEXT="nebenbei bemerkt: hier triggert eine Assertion">
<icon BUILTIN="messagebox_warning"/>
<node CREATED="1718798120521" ID="ID_1558829409" MODIFIED="1718798132829" TEXT="und zwar zeigen die Tests, da&#xdf; die regelm&#xe4;&#xdf;ig triggert"/>
<node CREATED="1718798134610" ID="ID_1437311418" MODIFIED="1718798148283" TEXT="das bedeutet: es ist kein dynamischer Check im Codepfad">
<node CREATED="1718798206756" ID="ID_1802549345" MODIFIED="1718798263474" TEXT="eigentlich hatte ich ja gedacht...">
<richcontent TYPE="NOTE"><html>
<head>
</head>
<body>
<p>
...da&#223; da immer noch high-level-Code dar&#252;ber liegt, der eine inhaltliche Pr&#252;fung gemacht hat, so da&#223; hier nur noch ein Konsistenzcheck vonn&#246;ten ist
</p>
</body>
</html>
</richcontent>
</node>
<node CREATED="1718798266263" ID="ID_1888657389" MODIFIED="1718798419479" TEXT="realistisch betrachtet &#x2014; zu gut">
<richcontent TYPE="NOTE"><html>
<head>
</head>
<body>
<p>
...aber realistisch betrachtet ist der AllocationCluster viel zu einfach zu verwenden, als da&#223; man da &#252;berhaupt daran denkt, noch explizit zu pr&#252;fen &#8212; und durch den standard-Allocator-Adapter gibt es unvermeidbar einen direkten Eingang &#252;ber allot().
</p>
</body>
</html>
</richcontent>
</node>
</node>
<node BACKGROUND_COLOR="#e0ceaa" COLOR="#690f14" CREATED="1718798156446" ID="ID_726993777" MODIFIED="1718798198816" TEXT="&#x27f9; im RELEASE-Mode w&#xfc;rde wir munter in eine Endlosschleife oder Memory-Corruption laufen">
<icon BUILTIN="clanbomber"/>
</node>
<node COLOR="#435e98" CREATED="1718798426017" ID="ID_1434519803" MODIFIED="1718802927818" TEXT="also mu&#xdf; an dieser Stelle eine Out-of-Memory-Exception geworfen werden">
<icon BUILTIN="yes"/>
<node CREATED="1718802913460" ID="ID_486402094" MODIFIED="1718802925724" TEXT="error::Fatal mit LUMIERA_ERROR_CAPACITY"/>
</node>
<node COLOR="#338800" CREATED="1718802895310" ID="ID_490815993" MODIFIED="1718802910357" TEXT="umformulieren in ein __enforce_limits()">
<icon BUILTIN="button_ok"/>
</node>
<node COLOR="#338800" CREATED="1718801745458" ID="ID_345486433" MODIFIED="1718801764080" TEXT="au&#xdf;erdem: AllocationCluster::max_size() extern zug&#xe4;nglich machen">
<icon BUILTIN="button_ok"/>
</node>
</node>
</node>
</node>
</node>
<node BACKGROUND_COLOR="#eee5c3" COLOR="#990000" CREATED="1718730317115" ID="ID_1509954190" MODIFIED="1718761158043" TEXT="es sollte sich genau die erwartete Speicherbelegung ergeben">
<icon BUILTIN="flag-yellow"/>
</node>
<node BACKGROUND_COLOR="#eee5c3" COLOR="#990000" CREATED="1718730353373" ID="ID_819034170" MODIFIED="1718730388372" TEXT="Objekt-Cleanup findet statt &#x2014; aber keine de-Allokation">