From ea0ae5f78e72ded456e527cd5b7f611ac1118474 Mon Sep 17 00:00:00 2001 From: Ichthyostega Date: Mon, 16 Mar 2026 22:33:48 +0100 Subject: [PATCH] BufferProvider: propagate the actual buffer-size info This is a nasty new requirement, discovered as an aside while looking into ways to resolve the ''crucial mismatch'' between the `OutputSlot` and `BufferProvider`, using the proxy-implementation of `BufferProvider` (thereby expanding on the recent draft with the `BufferProxyAdaptor`) The point is, if the BufferMetadata will be used beyond the internal coordination within BufferProvider, then it will be used as part of actual buffer storage. And this implies that the actual size of the allocation may differ (be greater) than the nominally required size (since we'll very likely employ some kind of memory tiling, for reasons of performance). This leads again to an analysis of `BufferMetadata` usage, with the conclusion to add the actual storage size directly into the API, but treat it as an ''extra'' and not integrated into the ''buffer type'' Phew ... quite complex alltogether --- src/vault/mem/buffer-metadata.hpp | 52 +++++++++-- src/vault/mem/buffer-provider.cpp | 10 +- src/vault/mem/buffer-provider.hpp | 35 ++++--- src/vault/mem/buffhandle.hpp | 81 +++++++++------- src/vault/mem/diagnostic-buffer-provider.cpp | 6 +- .../mem/simple-buffer-state-registry.hpp | 3 +- tests/vault/mem/buffer-metadata-test.cpp | 3 +- wiki/thinkPad.ichthyo.mm | 93 ++++++++++++------- 8 files changed, 185 insertions(+), 98 deletions(-) diff --git a/src/vault/mem/buffer-metadata.hpp b/src/vault/mem/buffer-metadata.hpp index d5b4cd526..c12efba4d 100644 --- a/src/vault/mem/buffer-metadata.hpp +++ b/src/vault/mem/buffer-metadata.hpp @@ -286,13 +286,18 @@ namespace mem { protected: Entry (Key const& parent - ,Buff* bufferPtr =nullptr + ,Buff* bufferPtr =nullptr + ,size_t actualSize =0 ,LocalTag const& specialTag =LocalTag::UNKNOWN ) : Key{Key::forEntry (parent, bufferPtr, specialTag)} , state_{bufferPtr? LOCKED:NIL} , buffer_{bufferPtr} - { } + { + if (bufferPtr and actualSize) + Key::storageSize_ = actualSize; + ENSURE (Key::storageSize() >= parent.storageSize()); + } /// BufferMetadata is allowed to create friend class vault::mem::BufferMetadata; @@ -372,10 +377,11 @@ namespace mem { } Entry& - lock (Buff* newBuffer) + lock (Buff* newBuffer, size_t actualSize =0) { __must_be_FREE(); buffer_ = newBuffer; + storageSize_ = std::max (actualSize, storageSize_); return mark (LOCKED); } @@ -692,19 +698,44 @@ namespace mem { * metadata Entry to account for this fact. This might include * invoking a constructor function, in case the type (Key) * defines a (nontrivial) TypeHandler. + * @param parentKey designates the _buffer type_ to base on + * @param buffer an actual memory address, which defines a distinct Entry + * @param size the size of the actual allocation. When size ≡ 0, + * the nominal size is assumed, as defined through parentKey. + * Otherwise, the actual size must be _larger_. + * @param specifics an optional qualification marker, which is managed + * and evaluated by the BufferStore implementation and treated as + * opaque data, as far as BufferMetadata is concerned. However, + * the LocalTag becomes part of the identity of the metadata record. + * Thus, either the parentKey must not include a LocalTag, or the + * LocalTag given here must be identical to what was associate + * with the _buffer type_ represented by parentKey. * @throw error::Fatal when locking a NULL buffer * @throw exceptions which might be raised by a TypeHandler's * constructor function. In this case, the Entry remains * created, but is marked as FREE */ Entry& - markLocked (Key const& parentKey, Buff* buffer, LocalTag const& specifics =LocalTag::UNKNOWN) + markLocked (Key const& parentKey + ,Buff* buffer + ,size_t actualSize =0 + ,LocalTag const& specifics =LocalTag::UNKNOWN) { - if (!buffer) + if (not buffer) throw err::Fatal{"Attempt to lock for a NULL buffer. Allocation floundered?" , LERR_(BOTTOM_VALUE)}; + if (not actualSize) + actualSize = parentKey.storageSize(); + REQUIRE (parentKey.storageSize() <= actualSize); - return BufferMetadata::lock (parentKey, buffer, specifics); + return BufferMetadata::lock (parentKey, buffer, actualSize, specifics); + } + + /** variation to use when a LocalTag is given, yet size is implicitly taken from parent */ + Entry& + markLocked (Key const& parentKey, Buff* buffer, LocalTag const& specifics) + { + return markLocked (parentKey, buffer, size_t(0), specifics); } /** purge the bare metadata Entry from the metadata tables. @@ -760,13 +791,14 @@ namespace mem { Entry& lock (Key const& parentKey ,Buff* concreteBuffer + ,size_t actualSize ,LocalTag const& specifics =LocalTag::UNKNOWN) { - if (!concreteBuffer) + if (not concreteBuffer) throw err::Invalid{"Attempt to lock a slot for a NULL buffer" , LERR_(BOTTOM_VALUE)}; - Entry newEntry{parentKey, concreteBuffer, specifics}; + Entry newEntry{parentKey, concreteBuffer, actualSize, specifics}; Entry* existing = table_.fetch (newEntry); if (existing and existing->isLocked()) @@ -774,9 +806,9 @@ namespace mem { , LERR_(LIFECYCLE)}; if (not existing) - return store_as_locked (newEntry); // actual creation + return store_as_locked (newEntry); // store new Entry marked as locked else - return existing->lock (concreteBuffer); + return existing->lock (concreteBuffer, actualSize); } diff --git a/src/vault/mem/buffer-provider.cpp b/src/vault/mem/buffer-provider.cpp index bdd5eaa17..e68778be0 100644 --- a/src/vault/mem/buffer-provider.cpp +++ b/src/vault/mem/buffer-provider.cpp @@ -149,12 +149,12 @@ namespace mem { BufferProvider::lockBuffer (BuffDescr const& type) { REQUIRE (was_created_by_this_provider (type)); - auto& typeKey = bufferStage_->lookup (type); - auto [storage, actualSize, localTag] = bufferStore_->provideBuffer (type,typeKey.storageSize(),typeKey.localTag()); - auto& stateKey = bufferStage_->mark_locked (typeKey, storage, localTag); + auto& typeKey = bufferStage_->lookup (type); + BuffAlloc alloc= bufferStore_->provideBuffer (type,typeKey.storageSize(),typeKey.localTag()); + auto& stateKey = bufferStage_->mark_locked (typeKey, alloc); - return BuffHandle (buildDescriptor(stateKey), storage); - } // NOTE: not the underlying parent descriptor! + return BuffHandle (buildDescriptor(stateKey), getAddr(alloc)); + } // NOTE: not the underlying parent descriptor! /** BufferProvider API: state transition to _emitted state_. diff --git a/src/vault/mem/buffer-provider.hpp b/src/vault/mem/buffer-provider.hpp index fd644db57..1a6a03db7 100644 --- a/src/vault/mem/buffer-provider.hpp +++ b/src/vault/mem/buffer-provider.hpp @@ -100,6 +100,15 @@ ** highly complex setup for the production-grade implementation variant, attempts ** towards such a refactoring were postponed, after re-evaluation 2/2026. ** + ** @warning buffer management via BufferProvider and BuffHandle does not automatically + ** _maintain proper alignment_. Rather, it relies on the storage allocator to + ** provide a buffer suitably aligned for the target type to hold. In most cases, + ** this target location will actually be storage located on the heap, yet some + ** processing might impose additional constraints regarding alignment to memory + ** blocks, causing degraded performance when violated; so this topic is a + ** possible subtle pitfall. + ** @todo 2026 we might consider adding alignment information to the associated TypeHandler. + ** ** @see buffer-provider-protocol-test.cpp ** @see output-slot.hpp ** @see engine-ctx.hpp @@ -142,6 +151,8 @@ namespace mem { * @remark LocalTag can be used by the storage-impl. */ using BuffAlloc = std::tuple; + inline Buff* getAddr (BuffAlloc& alloc) { return std::get<0> (alloc); } + inline size_t getSize (BuffAlloc& alloc) { return std::get<1> (alloc); } LUMIERA_ERROR_DECLARE (BUFFER_MANAGEMENT); ///< Problem providing working buffers @@ -207,14 +218,14 @@ namespace mem { virtual ~BufferStage() { } ///< this is an interface virtual ID defineBufferType (size_t, TypeHandler =TypeHandler::RAW, LocalTag =LocalTag::UNKNOWN) =0; - virtual ID lookup (HashVal) =0; - virtual ID mark_locked (ID typeKey, Buff*, LocalTag) =0; - virtual ID mark_emitted (HashVal stateKey) =0; - virtual ID mark_released (HashVal stateKey) =0; - virtual ID abandon (HashVal, bool destroy=false) =0; - virtual void discard (HashVal stateKey) =0; - virtual bool isAllotted (HashVal stateKey) const =0; - virtual bool isAccessible (HashVal stateKey) const =0; + virtual ID lookup (HashVal) =0; + virtual ID mark_locked (ID typeKey, BuffAlloc) =0; + virtual ID mark_emitted (HashVal stateKey) =0; + virtual ID mark_released (HashVal stateKey) =0; + virtual ID abandon (HashVal, bool destroy=false) =0; + virtual void discard (HashVal stateKey) =0; + virtual bool isAllotted (HashVal stateKey) const =0; + virtual bool isAccessible (HashVal stateKey) const =0; }; class BufferStore @@ -225,10 +236,10 @@ namespace mem { public: virtual ~BufferStore() { } ///< this is an interface - virtual uint prepareBuffers (HashVal typeID, uint cnt, size_t) =0; - virtual BuffAlloc provideBuffer (HashVal typeID, size_t, LocalTag) =0; - virtual void mark_emitted (HashVal typeID, BuffAlloc) =0; - virtual void detachBuffer (HashVal typeID, BuffAlloc) =0; + virtual uint prepareBuffers (HashVal typeID, uint cnt, size_t) =0; + virtual BuffAlloc provideBuffer (HashVal typeID, size_t, LocalTag) =0; + virtual void mark_emitted (HashVal typeID, BuffAlloc) =0; + virtual void detachBuffer (HashVal typeID, BuffAlloc) =0; }; unique_ptr bufferStage_; diff --git a/src/vault/mem/buffhandle.hpp b/src/vault/mem/buffhandle.hpp index 2db1f5db8..fe987f84b 100644 --- a/src/vault/mem/buffhandle.hpp +++ b/src/vault/mem/buffhandle.hpp @@ -15,28 +15,39 @@ ** A front-end to support the buffer management within the render nodes. ** When pulling data from predecessor nodes and calculating new data, each render node ** needs several input and output buffers. These may be allocated and provided by various - ** different "buffer providers" (for example the frame cache). Typically, the real buffers - ** will be passed as parameters to the actual job instance when scheduled, drawing on the - ** results of prerequisite jobs. Yet the actual job implementation remains agnostic with - ** respect to the way actual buffers are provided; the invocation just pushes BuffHandle - ** objects around. The actual render function gets an array of C-pointers to the actual - ** buffers, and for accessing those buffers, the node needs to keep a table of buffer - ** pointers, and for releasing the buffers later on, we utilise the buffer handles. + ** different [»buffer providers«](\ref buffer-provider.hpp) (for example the frame cache). + ** Buffers are classified by a _buffer type,_ which essentially is the size of the storage, + ** but could optionally also include a constructor and destructor function. Which type of + ** buffer is used at some node invocation and which provider is backing and managing the + ** allocation is considered static configuration of the Render Node Network and established + ** by the **Builder**. Typically the actual render job is unaware of these details and only + ** provides a parameter, that _just happens to select_ the proper source buffer for the + ** frame to be computed. ** - ** These buffer handles are based on a [Buffer Descriptor record](\ref BuffDescr), - ** which is opaque as far as the client is concerned. BuffDescr acts as a representation - ** of the type or kind of buffer. The only way to obtain such a BuffDescr is from a concrete - ** BufferProvider implementation. A back-link to this owning and managing provider is embedded - ** into the BuffDescr, which thus may be used as a _configuration tag,_ allowing to retrieve a - ** concrete buffer handle when needed, corresponding to an actual buffer provided and managed - ** behind the scenes. There is no automatic resource management; clients are responsible to - ** invoke BuffHandle#release when done. + ** However, the Node Network as such is strictly typed (at compile time). Information regarding + ** the _buffer type_ are encoded into a [Buffer Descriptor](\ref BuffDescr). These descriptors + ** are embedded into the _feed prototype_ of each node and used, by the actual _Node invocation_, + ** to retrieve a [Buffer Handle](\ref BuffHandle). On the other hand, the _feed prototype_ was + ** also attached to a typed context at build time, so that the invocation of the actual + ** _processing function_ can access the buffers and any further parameters strictly typed. ** - ** @warning buffer management via BuffHandle and BuffDescr does _not automatically maintain - ** proper alignment._ Rather, it relies on the storage allocator to provide a buffer - ** suitably aligned for the target type to hold. In most cases, this target location - ** will actually be storage maintained on heap through some STL collection; - ** this topic is a possible subtle pitfall non the less. + ** Usage of these descriptors and handles is organised in accordance to the »Buffer Provider Protocol«, + ** which also explains the structure of the BufferProvider API. + ** - the first step is to obtain a BuffDescr from the BufferProvider, which implies + ** to define at least the required storage size (optionally also a constructor/destructor). + ** - the usage of some buffers with a given type can be _announced_ optionally, which allows + ** to pre-arrange some storage space and associate it with the calling thread. + ** - an actual allocation is initiated by _locking_ the descriptor, which yields a BuffHandle. + ** - from this handle, the buffer memory can be accessed. + ** - the client is then responsible to [»emit«](\ref BuffHandle::emit) on the handle, + ** once the computation is complete and data can be published. This step can be omitted, + ** and it depends on the actual situation if invoking this operation matters. For example, + ** it is very relevant for a result sent to some output sink, or for a buffer attached + ** to the frame cache. Without _emitting_, the data will not be propagated in those cases. + ** - however, it is always mandatory to [»release«](\ref BuffHandle::release) the handle. + ** + ** @note both BuffDescr and BuffHandle store a back-link to the managing BufferProvider. + ** @warning BuffHandle is _not a smart-ptr_ — the client is responsible to `release()` ** ** @see BufferProvider ** @see BufferProviderProtocol_test usage demonstration @@ -53,8 +64,8 @@ #include "lib/hash-value.h" -namespace vault{ -namespace mem { +namespace vault { +namespace mem { namespace err = lumiera::error; using lib::HashVal; @@ -75,10 +86,10 @@ namespace mem { * An opaque descriptor to identify the type and further properties of a data buffer. * For each kind of buffer, there is somewhere a BufferProvider responsible for the * actual storage management. This provider may "lock" a buffer for actual use, - * returning a BuffHandle. - * @note this descriptor and especially the #subClassification_ is really owned - * by the BufferProvider, which may use (and even change) the opaque contents - * to organise the internal buffer management. + * returning a BuffHandle. BuffDescr is a convenient front-end to represent the arrangement + * for some type / kind of buffer, and to [allocate](\ref BuffDescr::lockBuffer) it actually. + * @note this descriptor and especially meaning of the #subClassification_ is + * implementation-defined and tied to a specific BufferProvider. */ class BuffDescr { @@ -111,8 +122,16 @@ namespace mem { /** - * Handle for a buffer for processing data, abstracting away the actual implementation. - * The real buffer pointer can be retrieved by dereferencing this smart-handle class. + * Handle to designate a buffer with data for processing, thereby abstracting from the + * memory management implementation. The buffer content can be accessed through an unchecked + * cast operation, assuming that the client has otherwise complete control over the data type + * to be accessed, since the only way to retrieve such a handle is from a BuffDescr. + * Besides some information function, BuffHandle exposes the important API functions + * - `emit()` : indicate that processing is complete and the data can be used / published + * - `release()` : indicate that the client will not touch this handle any more, so that + * storage can be repurposed. + * @note This is a low-level feature and not a *smart-handle* — it is the clien's sole + * responsibility to invoke the `release()` function reliably. */ class BuffHandle { @@ -123,9 +142,9 @@ namespace mem { public: /** @internal a buffer handle may be obtained by "locking" * a buffer from the corresponding BufferProvider */ - BuffHandle(BuffDescr const& typeInfo, void* storage = 0) - : descriptor_(typeInfo) - , pBuffer_(static_cast(storage)) + BuffHandle(BuffDescr const& typeInfo, Buff* storage =nullptr) + : descriptor_{typeInfo} + , pBuffer_{storage} { } // using standard copy operations diff --git a/src/vault/mem/diagnostic-buffer-provider.cpp b/src/vault/mem/diagnostic-buffer-provider.cpp index 34cd9f335..55be5ab1c 100644 --- a/src/vault/mem/diagnostic-buffer-provider.cpp +++ b/src/vault/mem/diagnostic-buffer-provider.cpp @@ -170,10 +170,10 @@ namespace mem { } ID - mark_locked (ID typeKey, Buff* storage, LocalTag implMark) override + mark_locked (ID typeKey, BuffAlloc storage) override { - ID stateKey = stage_->mark_locked (typeKey, storage, implMark); - tracker_.record_locked (stateKey, storage); + ID stateKey = stage_->mark_locked (typeKey, storage); + tracker_.record_locked (stateKey, getAddr(storage)); return stateKey; } diff --git a/src/vault/mem/simple-buffer-state-registry.hpp b/src/vault/mem/simple-buffer-state-registry.hpp index 6fcc6aa25..07b1ed975 100644 --- a/src/vault/mem/simple-buffer-state-registry.hpp +++ b/src/vault/mem/simple-buffer-state-registry.hpp @@ -72,8 +72,9 @@ namespace mem { } // deliberately: create storage, and return reference to it ID - mark_locked (ID typeKey, Buff* storage, LocalTag implMark) override + mark_locked (ID typeKey, BuffAlloc alloc) override { + auto& [storage,size,implMark] = alloc; return metadata_.markLocked (typeKey, storage, implMark); } diff --git a/tests/vault/mem/buffer-metadata-test.cpp b/tests/vault/mem/buffer-metadata-test.cpp index 0e9a27e7b..5ed75079c 100644 --- a/tests/vault/mem/buffer-metadata-test.cpp +++ b/tests/vault/mem/buffer-metadata-test.cpp @@ -155,7 +155,7 @@ namespace test { CHECK (LOCKED == m2.state()); CHECK (SOME_POINTER == m2.access()); // buffer pointer associated - // entries are unique and identifiable + // entries are unique and identifiable by a memory address HashVal keyX = meta_->key(key1, SOME_POINTER); CHECK (meta_->isLocked(keyX)); CHECK (keyX != key1); @@ -385,6 +385,7 @@ namespace test { VERIFY_ERROR (LIFECYCLE, entry.access() ); meta_->markLocked (key, SOME_POINTER); + //Note: we locked the key and this changed the existing entry CHECK (entry.isLocked()); entry.mark (EMITTED); diff --git a/wiki/thinkPad.ichthyo.mm b/wiki/thinkPad.ichthyo.mm index 1698434a1..2e67d8384 100644 --- a/wiki/thinkPad.ichthyo.mm +++ b/wiki/thinkPad.ichthyo.mm @@ -114290,6 +114290,9 @@ StM_bind(Builder<R1> b1, Extension<R1,R2> extension) + + + @@ -114593,8 +114596,8 @@ StM_bind(Builder<R1> b1, Extension<R1,R2> extension) - - + + @@ -115001,7 +115004,7 @@ StM_bind(Builder<R1> b1, Extension<R1,R2> extension) - + @@ -115015,7 +115018,7 @@ StM_bind(Builder<R1> b1, Extension<R1,R2> extension) - + @@ -115024,9 +115027,12 @@ StM_bind(Builder<R1> b1, Extension<R1,R2> extension)

+ - + + + @@ -115042,16 +115048,19 @@ StM_bind(Builder<R1> b1, Extension<R1,R2> extension) - - + + + - + + - + + @@ -115088,12 +115097,15 @@ StM_bind(Builder<R1> b1, Extension<R1,R2> extension) - + + - - - + + + + + @@ -115152,7 +115164,8 @@ StM_bind(Builder<R1> b1, Extension<R1,R2> extension) - + + @@ -115160,9 +115173,7 @@ 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.... @@ -115173,9 +115184,7 @@ StM_bind(Builder<R1> b1, Extension<R1,R2> extension) - - - +

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 @@ -115186,9 +115195,7 @@ StM_bind(Builder<R1> b1, Extension<R1,R2> extension) - - - +

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. @@ -115201,9 +115208,7 @@ StM_bind(Builder<R1> b1, Extension<R1,R2> extension) - - - +

Einsicht: nicht re-Locking sondern das onlyNew ist hier redundant @@ -115220,7 +115225,7 @@ StM_bind(Builder<R1> b1, Extension<R1,R2> extension) - + @@ -115228,12 +115233,24 @@ StM_bind(Builder<R1> b1, Extension<R1,R2> extension) - + - + + + + + + + + + + + + + @@ -117793,6 +117810,10 @@ StM_bind(Builder<R1> b1, Extension<R1,R2> extension) + + + + @@ -137496,9 +137517,7 @@ 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. @@ -137509,9 +137528,7 @@ StM_bind(Builder<R1> b1, Extension<R1,R2> extension) - - - +

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) @@ -137999,6 +138016,12 @@ StM_bind(Builder<R1> b1, Extension<R1,R2> extension) + + + + + +