diff --git a/src/steam/engine/buffer-metadata.hpp b/src/steam/engine/buffer-metadata.hpp index 5af1ebef0..dfc70c647 100644 --- a/src/steam/engine/buffer-metadata.hpp +++ b/src/steam/engine/buffer-metadata.hpp @@ -36,8 +36,15 @@ ** configuration tag attached to the buffer access, allowing to re-access a context ** within the buffer provider implementation. ** - ** @todo 2/2026 This code has gotten overly tricky and driven by expected effect. - ** It is mandatory to factor-out an interface! //////////////////////////////////////////////////////TICKET #1410 : clean-up BufferProvider base implementation + ** @todo 2/2026 After refactoring the BufferProvider and splitting up the implementation + ** in two realms (state and storage), the purpose of this code her became much + ** clearer. The problem remains however, that the BufferMetadata directly links + ** to the one central metadata hashtable. We'll have to reconsider this aspect + ** once we have to build a metadata store that works well in a massively + ** concurrent environment, where requests for state transition emanate + ** any time form within all worker threads! + ** @todo 2/2026 Do we really have to _store_ the actual buffer address into the + ** metadata::Entry, as opposed to just using this information for hash computation? ** ** @see buffer-provider.hpp ** @see BufferMetadata_test diff --git a/src/steam/engine/buffer-provider.cpp b/src/steam/engine/buffer-provider.cpp index 3396e6f2b..bccf91647 100644 --- a/src/steam/engine/buffer-provider.cpp +++ b/src/steam/engine/buffer-provider.cpp @@ -13,7 +13,16 @@ /** @file buffer-provider.cpp - ** Implementation details related to buffer lifecycle management in the render engine. + ** Implementation details related to buffer allocation and lifecycle management in the render engine. + ** This translation unit comprises the "back side" of the public front-end, where calls from the + ** client are received and relayed to the appropriate part of the implementation back-end. + ** Furthermore, some of the API functions on the [Buffer Descriptor](\ref BuffDescr) and the + ** [Buffer Handle](\ref BuffHandle) are also directly implemented here, by forwarding them + ** through the related implementation APIs. The actual backend implementation however is + ** split out and isolated by the internal interfaces BufferProvider::BufferStage (for the + ** state and lifecycle management) and BufferProvider::BufferStore (for memory handling). + ** + ** The actual backend implementations to use are configured in subclasses. */ @@ -30,14 +39,9 @@ namespace engine { // storage for the default-marker constants const TypeHandler TypeHandler::RAW{}; - const LocalTag LocalTag::UNKNOWN{}; + const LocalTag LocalTag::UNKNOWN{}; const metadata::Key metadata::Key::INVALID{HashVal(0), size_t(0)}; //////OOO consider to relocate those marker constants to somewhere more obvious - namespace { // impl. details and definitions - - const uint DEFAULT_DESCRIPTOR = 0; - - } LUMIERA_ERROR_DEFINE (BUFFER_MANAGEMENT, "Problem providing working buffers"); @@ -170,7 +174,7 @@ namespace engine { * @note EX_FREE */ void - BufferProvider::releaseBuffer (BuffHandle const& handle) + BufferProvider::releaseBuffer (BuffHandle& handle) try { // might invoke embedded dtor function auto& stateKey = bufferStage_->mark_released (handle); bufferStore_->detachBuffer (stateKey.storageSize(), stateKey.parentKey() @@ -188,7 +192,7 @@ namespace engine { * @note EX_FREE */ void - BufferProvider::emergencyCleanup (BuffHandle const& handle, bool invokeDtor) + BufferProvider::emergencyCleanup (BuffHandle& handle, bool invokeDtor) try { auto& stateKey = bufferStage_->abandon (handle, invokeDtor); bufferStore_->detachBuffer (stateKey.storageSize(), stateKey.parentKey() @@ -199,18 +203,18 @@ namespace engine { - bool - BufferProvider::was_created_by_this_provider (BuffDescr const& descr) const - { - return isSameAdr (this, descr.provider_); - } - /* === BuffDescr and BuffHandle === */ + bool + BufferProvider::was_created_by_this_provider (BuffDescr const& descr) const + { + return isSameAdr (this, descr.provider_); + } + /** * A Buffer Descriptor is considered _valid_ * iff it represents a registered buffer type. diff --git a/src/steam/engine/buffer-provider.hpp b/src/steam/engine/buffer-provider.hpp index 7acfc9536..d4821c51c 100644 --- a/src/steam/engine/buffer-provider.hpp +++ b/src/steam/engine/buffer-provider.hpp @@ -88,6 +88,18 @@ ** implementation; when used from the actual [Render Environment](\ref render-environment.hpp), ** this will be backed by a production-strenght and concurrency-safe implementation. ** + ** \par Design critique + ** In its current shape, BufferProvider is both a rather large front-end facade, + ** while also containing some »code behind« to translate the API invocations into + ** implementation calls. Furthermore, it exposes functions that can both be used + ** directly, but are also used by the two handle types. And, last but not least, + ** proper usage requires to stick to a _Protocol_ that is not immediately obvious + ** from the API alone. It would be conceivable to factor-out a technical or access + ** API, and to expose only the handles and a simple factory function to the user. + ** Yet given the very limited actual usage situation within the engine, and the + ** highly complex setup for the production-grade implementation variant, attempts + ** towards such a refactoring were postponed, after re-evaluation 2/2026. + ** ** @see buffer-provider-protocol-test.cpp ** @see output-slot.hpp ** @see engine-ctx.hpp @@ -137,33 +149,12 @@ namespace engine { * - "locking" a buffer to yield a buffer handle * - then dereferencing the obtained smart-handle * - * @warning all of BufferProvider is assumed to run within a threadsafe environment. - * - * @todo as of 6/2011 buffer management within the engine is still a bit vague * @todo as of 11/11 thread safety within the engine remains to be clarified ////////////////////////////TICKET #854 */ class BufferProvider : util::NonCopyable { - protected: /* === for Implementation by concrete providers === */ - - public: - virtual ~BufferProvider(); ///< this is an ABC - - - uint announce (uint count, BuffDescr const&); - - BuffHandle lockBuffer (BuffDescr const&); - void emitBuffer (BuffHandle const&); - void releaseBuffer (BuffHandle const&); - - template - BuffHandle lockBufferFor (ARGS ...args); - - void emergencyCleanup (BuffHandle const& target, bool invokeDtor =false); - - /** describe the kind of buffer managed by this provider */ BuffDescr getDescriptorFor(size_t storageSize=0); BuffDescr getDescriptorFor(size_t storageSize, TypeHandler specialTreatment); @@ -171,6 +162,16 @@ namespace engine { template BuffDescr getDescriptor (ARGS ...args); + /** give advance notice of the intention to request some buffers */ + uint announce (uint count, BuffDescr const&); + + /** claim storage for exclusive use, based on a descriptor */ + BuffHandle lockBuffer (BuffDescr const&); + + /** directly claim storage and construct an object, + * without preparing a descriptor beforehand */ + template + BuffHandle lockBufferFor (ARGS ...args); /* === API for BuffHandle internal access === */ @@ -179,7 +180,14 @@ namespace engine { bool isAccessible (HashVal) const; size_t getBufferSize (HashVal) const; + void emitBuffer (BuffHandle const&); + void releaseBuffer (BuffHandle &); + void emergencyCleanup (BuffHandle&, bool invokeDtor =false); + + protected: + ~BufferProvider(); + bool was_created_by_this_provider (BuffDescr const&) const; using Buff = StreamType::ImplFacade::DataBuffer; ///< marker type for an actual data buffer @@ -218,7 +226,7 @@ namespace engine { virtual void detachBuffer (size_t,HashVal, Slot alloc) =0; }; - unique_ptr bufferStage_; ///////////////////////////////////////////////////////////TICKET #1410 : must be turned into an internal interface + unique_ptr bufferStage_; unique_ptr bufferStore_; }; diff --git a/src/steam/engine/buffhandle.hpp b/src/steam/engine/buffhandle.hpp index df35af116..fd9518e66 100644 --- a/src/steam/engine/buffhandle.hpp +++ b/src/steam/engine/buffhandle.hpp @@ -147,7 +147,6 @@ namespace engine { return pBuffer_; } - private: void emergencyCleanup(); }; diff --git a/src/steam/engine/diagnostic-buffer-provider.hpp b/src/steam/engine/diagnostic-buffer-provider.hpp index b6063f674..79f0d92ef 100644 --- a/src/steam/engine/diagnostic-buffer-provider.hpp +++ b/src/steam/engine/diagnostic-buffer-provider.hpp @@ -118,7 +118,7 @@ namespace engine { inline BufferDiagnostic watch (BufferProvider& thePro) { - return BufferDiagnostic{dynamic_cast (thePro)}; + return BufferDiagnostic{static_cast (thePro)}; } diff --git a/src/steam/play/output-slot-connection.hpp b/src/steam/play/output-slot-connection.hpp index 932962882..d2183cfcf 100644 --- a/src/steam/play/output-slot-connection.hpp +++ b/src/steam/play/output-slot-connection.hpp @@ -93,9 +93,9 @@ namespace play { virtual BuffHandle claimBufferFor(FrameID) =0; virtual bool isTimely (FrameID, TimeValue) =0; - virtual void transfer (BuffHandle const&) =0; - virtual void pushout (BuffHandle const&) =0; - virtual void discard (BuffHandle const&) =0; + virtual void transfer (BuffHandle &) =0; + virtual void pushout (BuffHandle &) =0; + virtual void discard (BuffHandle &) =0; virtual void shutDown () =0; }; diff --git a/src/steam/play/output-slot.cpp b/src/steam/play/output-slot.cpp index 7aed3ca3a..a1396c13b 100644 --- a/src/steam/play/output-slot.cpp +++ b/src/steam/play/output-slot.cpp @@ -99,7 +99,7 @@ namespace play { void - DataSink::emit (FrameID frameNr, BuffHandle const& data2emit, TimeValue currentTime) + DataSink::emit (FrameID frameNr, BuffHandle& data2emit, TimeValue currentTime) { OutputSlot::Connection& connection = impl(); if (connection.isTimely(frameNr,currentTime)) diff --git a/src/steam/play/output-slot.hpp b/src/steam/play/output-slot.hpp index e1f10c447..762a410bc 100644 --- a/src/steam/play/output-slot.hpp +++ b/src/steam/play/output-slot.hpp @@ -162,7 +162,7 @@ namespace play { public: BuffHandle lockBufferFor(FrameID); - void emit(FrameID, BuffHandle const&, TimeValue currentTime = Time::ANYTIME); + void emit(FrameID, BuffHandle&, TimeValue currentTime = Time::ANYTIME); friend bool operator== (DataSink const& sink1, DataSink const& sink2) diff --git a/tests/core/steam/play/diagnostic-output-slot.hpp b/tests/core/steam/play/diagnostic-output-slot.hpp index f49ec6215..1bcacb06b 100644 --- a/tests/core/steam/play/diagnostic-output-slot.hpp +++ b/tests/core/steam/play/diagnostic-output-slot.hpp @@ -152,7 +152,7 @@ namespace play { } void - transfer (BuffHandle const& filledBuffer) + transfer (BuffHandle & filledBuffer) { TRACE (test, "Con=%p : transfer buffer %zu" , this, HashVal(filledBuffer)); @@ -162,7 +162,7 @@ namespace play { } void - pushout (BuffHandle const& data4output) + pushout (BuffHandle & data4output) { REQUIRE (!closed_); buffProvider_.emitBuffer (data4output); @@ -170,7 +170,7 @@ namespace play { } void - discard (BuffHandle const& superseededData) + discard (BuffHandle & superseededData) { REQUIRE (!closed_); buffProvider_.releaseBuffer (superseededData); diff --git a/tests/core/steam/play/dummy-builder-context.hpp b/tests/core/steam/play/dummy-builder-context.hpp index 8c9fe6e8f..fe2d8b689 100644 --- a/tests/core/steam/play/dummy-builder-context.hpp +++ b/tests/core/steam/play/dummy-builder-context.hpp @@ -68,9 +68,9 @@ namespace test { { BuffHandle claimBufferFor(FrameID) override { UNIMPLEMENTED ("claimBufferFor(FrameID)"); } bool isTimely (FrameID, TimeValue) override { return true; } - void transfer (BuffHandle const&) override { UNIMPLEMENTED ("transfer (BuffHandle const&)"); } - void pushout (BuffHandle const&) override { UNIMPLEMENTED ("pushout (BuffHandle const&)"); } - void discard (BuffHandle const&) override { UNIMPLEMENTED ("discard (BuffHandle const&)"); } + void transfer (BuffHandle &) override { UNIMPLEMENTED ("transfer (BuffHandle const&)"); } + void pushout (BuffHandle &) override { UNIMPLEMENTED ("pushout (BuffHandle const&)"); } + void discard (BuffHandle &) override { UNIMPLEMENTED ("discard (BuffHandle const&)"); } void shutDown () override { UNIMPLEMENTED ("shutDown() Connection"); } public: diff --git a/wiki/thinkPad.ichthyo.mm b/wiki/thinkPad.ichthyo.mm index 0003434ef..020a62137 100644 --- a/wiki/thinkPad.ichthyo.mm +++ b/wiki/thinkPad.ichthyo.mm @@ -112834,7 +112834,8 @@ StM_bind(Builder<R1> b1, Extension<R1,R2> extension) - + + @@ -112871,6 +112872,43 @@ StM_bind(Builder<R1> b1, Extension<R1,R2> extension) + + + + + + + + + + + + + + + + + + + + + + + + + + + + +

+ Angemessen wäre so was, wenn wir hier über einen generischen Memory-Allokator reden würden, der überall in der Applikation zum Einsatz kommt. Siehe als Beispiel das lib::Several, dort habe ich so eine segmentierte Struktur gebaut, denn dort überwiegt m.E. der Nutzen so eines »insider-APIs« +

+ + + + + + @@ -113092,12 +113130,40 @@ StM_bind(Builder<R1> b1, Extension<R1,R2> extension) - + + + + + + + + +

+ BuffHandle hat keine Freunde ... das heißt, wenn man die release(handle) oder emegencyCleanup(handle) auf dem buffer-provider direkt aufruft, dann wird der Pointer im Handle nicht invalidiert! — das Handle ist aber danach trotzdem invalid und kann keine weitere State-transition mehr machen, weil der Entry aus den BufferMetada weggeworfen wurde. +

+ + + + + + + + + + +

+ Deshalb nenne ich es auch ein »Protocol« — wollten wir etwas wirklich wasserdichtes,  dann müßten wir die Struktur ganz anders anlegen, und vermutlich auch deutlich rigider, und mehr interne Zugriffe verwenden. +

+ + + + + - + @@ -113307,7 +113373,17 @@ StM_bind(Builder<R1> b1, Extension<R1,R2> extension) - + + + + + + + + + + +