From 94320d2d2327af50e99cc95086bfa3f6536a8e0b Mon Sep 17 00:00:00 2001 From: Ichthyostega Date: Mon, 16 Mar 2026 20:37:24 +0100 Subject: [PATCH] BufferProvider: remove the lock-onlyNew feature MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This might have seemed like a good idea, adding an additional ''consistency check''... However — we ''do have'' a generic check, insofar `BufferMetadata` is a **state engine**. As long as we're able to prevent, reliably, that the same buffer address ends up being classified as a different buffer type, this should be enough to prevent double-use of any buffer (letting aside that `BufferProvider` is backed by an implementing memory allocator, which in itself should prevent any double-use. Removing this feature resolves some ambiguity in the overall design, and allows to turn `BufferMetadata::lock(type, address)` into a private implementation function. In the old code, there were some external access paths directly entering into that implementation function, which caused a lot of confusion (at least for me...) --- src/vault/mem/buffer-metadata.hpp | 12 +-- tests/vault/mem/buffer-metadata-test.cpp | 6 +- wiki/thinkPad.ichthyo.mm | 116 ++++++++++++++++++++++- 3 files changed, 119 insertions(+), 15 deletions(-) diff --git a/src/vault/mem/buffer-metadata.hpp b/src/vault/mem/buffer-metadata.hpp index b66196d23..d5b4cd526 100644 --- a/src/vault/mem/buffer-metadata.hpp +++ b/src/vault/mem/buffer-metadata.hpp @@ -704,7 +704,7 @@ namespace mem { throw err::Fatal{"Attempt to lock for a NULL buffer. Allocation floundered?" , LERR_(BOTTOM_VALUE)}; - return this->lock (parentKey, buffer, specifics, true); // force creation of a new entry + return BufferMetadata::lock (parentKey, buffer, specifics); } /** purge the bare metadata Entry from the metadata tables. @@ -731,6 +731,7 @@ namespace mem { } + private: /** Core low-level operation to access or create a buffer metadata entry. * The hashID of the entry in question is built, based on the parentKey, * which designates a buffer type, optionally an implementation defined @@ -759,8 +760,7 @@ namespace mem { Entry& lock (Key const& parentKey ,Buff* concreteBuffer - ,LocalTag const& specifics =LocalTag::UNKNOWN - ,bool onlyNew =false) + ,LocalTag const& specifics =LocalTag::UNKNOWN) { if (!concreteBuffer) throw err::Invalid{"Attempt to lock a slot for a NULL buffer" @@ -769,10 +769,6 @@ namespace mem { Entry newEntry{parentKey, concreteBuffer, specifics}; Entry* existing = table_.fetch (newEntry); - if (existing and onlyNew) - throw err::Logic{"Attempt to lock a slot for a new buffer, " - "while actually the old buffer is still locked" - , LERR_(LIFECYCLE)}; if (existing and existing->isLocked()) throw err::Logic{"Attempt to re-lock a buffer still in use" , LERR_(LIFECYCLE)}; @@ -784,8 +780,6 @@ namespace mem { } - private: - template Key trackKey (PAR parent, DEF specialisation) diff --git a/tests/vault/mem/buffer-metadata-test.cpp b/tests/vault/mem/buffer-metadata-test.cpp index c5b399fa1..0e9a27e7b 100644 --- a/tests/vault/mem/buffer-metadata-test.cpp +++ b/tests/vault/mem/buffer-metadata-test.cpp @@ -333,7 +333,7 @@ namespace test { VERIFY_ERROR (LIFECYCLE, meta_->get(key).mark(NIL) ); // now build a concrete buffer entry - metadata::Entry& entry = meta_->markLocked(key, SOME_POINTER); + metadata::Entry& entry = meta_->markLocked (key, SOME_POINTER); CHECK (LOCKED == entry.state()); CHECK (not entry.isTypeKey()); @@ -369,7 +369,7 @@ namespace test { CHECK (LOCKED == entry.state()); CHECK (entry.isLocked()); - VERIFY_ERROR (LIFECYCLE, entry.lock(SOME_POINTER)); + VERIFY_ERROR (LIFECYCLE, entry.lock (SOME_POINTER)); entry.mark (BLOCKED); // go directly to the blocked state CHECK (BLOCKED == entry.state()); @@ -384,7 +384,7 @@ namespace test { CHECK (not entry.isLocked()); VERIFY_ERROR (LIFECYCLE, entry.access() ); - meta_->lock(key, SOME_POINTER); + meta_->markLocked (key, SOME_POINTER); CHECK (entry.isLocked()); entry.mark (EMITTED); diff --git a/wiki/thinkPad.ichthyo.mm b/wiki/thinkPad.ichthyo.mm index 25a008b52..1698434a1 100644 --- a/wiki/thinkPad.ichthyo.mm +++ b/wiki/thinkPad.ichthyo.mm @@ -115145,17 +115145,93 @@ StM_bind(Builder<R1> b1, Extension<R1,R2> extension) - + + + + + + + + + + + + + + + + + + + +

+ Und damit spielt sich das System dann irgendwann auf einen festen Pool von Buffer-»Slots« ein, die permanent im Speicher bleiben. Allerdings bedeutet das dann für die Metadaten ein Cartesisches Produkt  zwischen der Zahl der Memory-Slots und der Zahl der verschiedenen Buffer-Typen.... +

+ + + + + + + + + + +

+ Ob das also gefährlich wird, oder eine etwas ausufernde Metadaten-Storage durchaus überkompensiert wird von dem Wegfall der permanenten Allokation/Deallokation (und Hashtable-Pflege), kann nicht auf theoretischer Ebene abgehandelt werden. Mehr noch: es ist sogar zweifelhaft, ob sich diese Frage überhaupt zuverlässig beantworten läßt, auch empirisch! Dazu müßte man einen Weg finden, um die Stabilität des Verhaltens zu beurteilen, über verschieden Nutzszenarien hinweg +

+ + + + + + + + + + +

+ Allerdings, wenn man sich darauf einläßt, Speicher nicht wegzuräumen, dann muß man explizit den Fall betrachten, daß der Speicher volläuft. In dem Fall bräuchte es einen »Sweep«, der über die Metadaten-Tabelle iteriert und alle Einträge pauschal löscht, die als FREE markiert sind. The usual trade-offs apply. +

+ + + + + + + + + + + + +

+ Einsicht: nicht re-Locking sondern das onlyNew ist hier redundant +

+ + + + + + + + + + - - + + + + + + @@ -137416,6 +137492,40 @@ StM_bind(Builder<R1> b1, Extension<R1,R2> extension) + + + + + + + + +

+ fragwürdig ist im Besonderen der hierarchische Ansatz, der die konkreten Storage-Metadaten als Spezialisierung eines Typ-Eintrags behandelt. +

+ + + + + + + + + + +

+ Im Besonderen stehen dort all die relevanten Infos, die jeder Memory-Allocator benötigt: die konkrete Adresse und die Size (möglicherweise sogar ein MRU-counter, oder ähnliche technische Informationen) +

+ + + + + + + + + +