ichthyo/LUMIERA.clone
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

BufferProvider: reconsider and rearrange the public API

Browse source 
In the end I refrained from applying most of the changes I had intended.
I am aware that `BufferProvider` is still quite a massive structure,
yet with the implementation refactorings done thus far, it seems
to be reasonably clear and self-contained now.

It would be possible to totally overthrow the grown structure and
expose only the two handles (BuffDescr and BuffHandle), together with
a factory function, but I cannot see how this would help to make the
code clearer and more approachable. Maybe it would make the usage
more obvious from the client's perspective — yet, to be honest,
there are not ''that much clients'' to justify such a move.

Furthermore, the implementation is far from finished: for both of
the back-end lines, a fully production-strength implementation
must be created, that is able to operate in a highly concurrent
environment; very likely some lock-free messaging and hand-over
between threads will become necessary, adding to the complexity.

Thus it seems better to note the ''design critique'', but to
postpone any further refactorings to a point where we can watch
a fully developed Render Engine under production load.
This commit is contained in:
Fischlurch 2026-02-16 20:50:04 +01:00
parent 75913725a8
commit 3d6e2fea75
11 changed files with 150 additions and 56 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

11
src/steam/engine/buffer-metadata.hpp
View file

@ -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

34
src/steam/engine/buffer-provider.cpp
View file

@ -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.

52
src/steam/engine/buffer-provider.hpp
View file

@ -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<typename BU, typename...ARGS>
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<typename BU, typename...ARGS>
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<typename BU, typename...ARGS>
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> bufferStage_; ///////////////////////////////////////////////////////////TICKET #1410 : must be turned into an internal interface
unique_ptr<BufferStage> bufferStage_;
unique_ptr<BufferStore> bufferStore_;
};

1
src/steam/engine/buffhandle.hpp
View file

@ -147,7 +147,6 @@ namespace engine {
return pBuffer_;
}
private:
void emergencyCleanup();
};

2
src/steam/engine/diagnostic-buffer-provider.hpp
View file

@ -118,7 +118,7 @@ namespace engine {
inline BufferDiagnostic
watch (BufferProvider& thePro)
{
return BufferDiagnostic{dynamic_cast<DiagnosticBufferProvider&> (thePro)};
return BufferDiagnostic{static_cast<DiagnosticBufferProvider&> (thePro)};
}

6
src/steam/play/output-slot-connection.hpp
View file

@ -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;
};

2
src/steam/play/output-slot.cpp
View file

@ -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))

2
src/steam/play/output-slot.hpp
View file

@ -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)

6
tests/core/steam/play/diagnostic-output-slot.hpp
View file

@ -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);

6
tests/core/steam/play/dummy-builder-context.hpp
View file

@ -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:

84
wiki/thinkPad.ichthyo.mm
View file

@ -112834,7 +112834,8 @@ StM_bind(Builder&lt;R1&gt; b1, Extension&lt;R1,R2&gt; extension)
<node CREATED="1771032680433" ID="ID_49382511" MODIFIED="1771032697968" TEXT="und die Typ-Registrierung via Buffer-Descriptor"/>
</node>
</node>
<node CREATED="1770950134226" ID="ID_210815021" MODIFIED="1770950145147" TEXT="mu&#xdf; die korrekte Tracker-Registry ansprechen">
<node BACKGROUND_COLOR="#fdfdcf" COLOR="#ff0000" CREATED="1770950134226" ID="ID_210815021" MODIFIED="1771202650391" TEXT="mu&#xdf; die korrekte Tracker-Registry ansprechen">
<icon BUILTIN="flag-pink"/>
<node CREATED="1770950151506" ID="ID_1595932224" MODIFIED="1770950161653" TEXT="eigentlich immer zuerst die created-Registry"/>
<node CREATED="1770950167400" ID="ID_1883755499" MODIFIED="1770950182484" TEXT="die Tracking-Funktion mu&#xdf; den Zustands&#xfc;bergang erkennen"/>
<node CREATED="1770950209162" ID="ID_1174437807" MODIFIED="1770950232620" TEXT="und dann den Block aktualisieren und in die emitted/released-Registry &#xfc;bernehmen"/>
@ -112871,6 +112872,43 @@ StM_bind(Builder&lt;R1&gt; b1, Extension&lt;R1,R2&gt; extension)
<arrowlink COLOR="#5f6c81" DESTINATION="ID_17538120" ENDARROW="Default" ENDINCLINATION="107;-147;" ID="Arrow_ID_196819766" STARTARROW="None" STARTINCLINATION="261;19;"/>
<icon BUILTIN="button_ok"/>
</node>
<node COLOR="#5b280f" CREATED="1771267184811" ID="ID_693750944" MODIFIED="1771267514738" TEXT="man k&#xf6;nnte die API-Funktionen auf BufferProvider etwas restriktiver machen">
<linktarget COLOR="#6d4975" DESTINATION="ID_693750944" ENDARROW="Default" ENDINCLINATION="-1078;42;" ID="Arrow_ID_480988688" SOURCE="ID_19949747" STARTARROW="None" STARTINCLINATION="-465;29;"/>
<icon BUILTIN="help"/>
<icon BUILTIN="button_cancel"/>
<node CREATED="1771267312969" ID="ID_1046056200" MODIFIED="1771267341952" TEXT="emit(), release() und emergencyCleanup() k&#xf6;nnten nur noch f&#xfc;r friends zug&#xe4;nglich sein"/>
<node CREATED="1771267347707" ID="ID_371233128" MODIFIED="1771267362974" TEXT="dann w&#xe4;re BuffHandle ein friend"/>
<node CREATED="1771267368894" ID="ID_722396409" MODIFIED="1771267381129" TEXT="lie&#xdf;e sich auch auf lock() ausweiten">
<node CREATED="1771267388082" ID="ID_1139758444" MODIFIED="1771267403040" TEXT="dann w&#xfc;rde auch BuffDescriptor ein friend"/>
<node BACKGROUND_COLOR="#e0ceaa" COLOR="#690f14" CREATED="1771267438063" ID="ID_228071001" MODIFIED="1771267459433" TEXT="Die Flexibilit&#xe4;t m&#xfc;&#xdf;te dann aber wegfallen">
<icon BUILTIN="messagebox_warning"/>
<node CREATED="1771267464709" ID="ID_889141096" MODIFIED="1771267476371" TEXT="bisher kann man einen Buffer direkt LOCKen"/>
<node CREATED="1771267477309" ID="ID_544155148" MODIFIED="1771267496288" TEXT="k&#xfc;nftig m&#xfc;&#xdf;te man stets zuerst einen Buffer-Descriptor erstellen"/>
</node>
</node>
<node COLOR="#5b280f" CREATED="1771267498644" ID="ID_246739333" MODIFIED="1771267511548" TEXT="Nein! ich bin dagegen....">
<icon BUILTIN="stop-sign"/>
<node CREATED="1771267518908" ID="ID_1568205940" MODIFIED="1771267530759" TEXT="denn dadurch wird das API nicht klarer"/>
<node CREATED="1771267531787" ID="ID_254635901" MODIFIED="1771267552756" TEXT="sondern man braucht eine Bedienungsanleitung dazu"/>
<node CREATED="1771267698117" ID="ID_109470235" MODIFIED="1771267707567" TEXT="besser w&#xfc;rde es mit einem separaten Callback-API">
<node CREATED="1771267748278" ID="ID_150866207" MODIFIED="1771267764386" TEXT="das w&#xfc;rde nur die BuffDescr / BuffHandle-Implementierung sehen"/>
<node CREATED="1771267790382" ID="ID_819177459" MODIFIED="1771267825431" TEXT="im Gegenzug m&#xfc;&#xdf;te man BuffDescr / BuffHandle direkt beim Front-end sichtbar machen"/>
</node>
<node CREATED="1771267839810" ID="ID_583729267" MODIFIED="1771267977789" TEXT="ich halte das f&#xfc;r &#xfc;bertrieben &#x2014; eingedenk der doch sehr limitierten Verwendung">
<richcontent TYPE="NOTE"><html>
<head>
</head>
<body>
<p>
Angemessen w&#228;re so was, wenn wir hier &#252;ber einen generischen Memory-Allokator reden w&#252;rden, der &#252;berall in der Applikation zum Einsatz kommt. Siehe als Beispiel das lib::Several, dort habe ich so eine segmentierte Struktur gebaut, denn dort &#252;berwiegt m.E. der Nutzen so eines &#187;insider-APIs&#171;
</p>
</body>
</html>
</richcontent>
</node>
</node>
</node>
<node BACKGROUND_COLOR="#fdfdcf" COLOR="#ff0000" CREATED="1771195835031" ID="ID_465237496" MODIFIED="1771195857844" TEXT="BufferProvider &#x27f6; in Vault umziehen">
<icon BUILTIN="yes"/>
<node CREATED="1771196048101" ID="ID_1022260808" MODIFIED="1771196056256" TEXT="TypeHandler kann auch dorting"/>
@ -113092,12 +113130,40 @@ StM_bind(Builder&lt;R1&gt; b1, Extension&lt;R1,R2&gt; extension)
</body>
</html></richcontent>
</node>
<node BACKGROUND_COLOR="#f0d5c5" COLOR="#990033" CREATED="1771117541849" ID="ID_1494781008" MODIFIED="1771117561620" TEXT="KL&#xc4;REN: wer setzt den storage-ptr. im Handle auf NULL?">
<node BACKGROUND_COLOR="#e0ceaa" COLOR="#690f14" CREATED="1771117541849" ID="ID_1494781008" MODIFIED="1771267178579" TEXT="wer setzt den storage-ptr. im Handle auf NULL?">
<icon BUILTIN="help"/>
<node CREATED="1771266770433" ID="ID_347345361" MODIFIED="1771266774908" TEXT="das Handle selber"/>
<node CREATED="1771266775875" ID="ID_1781625402" MODIFIED="1771266906677" TEXT="niemand sonst kann das tun">
<richcontent TYPE="NOTE"><html>
<head>
</head>
<body>
<p>
BuffHandle hat keine Freunde ... das hei&#223;t, wenn man die release(handle) oder emegencyCleanup(handle) auf dem buffer-provider direkt aufruft, dann wird der Pointer im Handle <i>nicht invalidiert!</i>&#160;&#8212; das Handle ist aber danach trotzdem invalid und kann keine weitere State-transition mehr machen, weil der Entry aus den BufferMetada weggeworfen wurde.
</p>
</body>
</html>
</richcontent>
</node>
<node CREATED="1771266908318" ID="ID_19949747" MODIFIED="1771267284665" TEXT="das ist nicht sch&#xf6;n &#x2014; aber pa&#xdf;t ins Konzept">
<richcontent TYPE="NOTE"><html>
<head>
</head>
<body>
<p>
Deshalb nenne ich es auch ein &#187;Protocol&#171; &#8212; wollten wir etwas wirklich <i>wasserdichtes,</i>&#160; dann m&#252;&#223;ten wir die Struktur ganz anders anlegen, und vermutlich auch deutlich rigider, und mehr interne Zugriffe verwenden.
</p>
</body>
</html>
</richcontent>
<arrowlink COLOR="#6d4975" DESTINATION="ID_693750944" ENDARROW="Default" ENDINCLINATION="-1078;42;" ID="Arrow_ID_480988688" STARTARROW="None" STARTINCLINATION="-465;29;"/>
</node>
</node>
</node>
<node CREATED="1771040196671" ID="ID_952067605" MODIFIED="1771040209557" TEXT="release(stateKey) verwirft diesen">
<node BACKGROUND_COLOR="#f8f1cb" COLOR="#a50125" CREATED="1771116086464" ID="ID_1151511037" MODIFIED="1771116133806" TEXT="Problem mit Type-Mapping">
<node BACKGROUND_COLOR="#e0ceaa" COLOR="#690f14" CREATED="1771116086464" ID="ID_1151511037" MODIFIED="1771266757146" TEXT="Problem mit Type-Mapping">
<icon BUILTIN="messagebox_warning"/>
<node CREATED="1771116137414" ID="ID_13506948" MODIFIED="1771116158973" TEXT="entweder zus&#xe4;tzlicher Hashtable-Lookup"/>
<node CREATED="1771116160984" ID="ID_1585303726" MODIFIED="1771116180128" TEXT="oder aber Verzicht auf Santiy-Check"/>
@ -113307,7 +113373,17 @@ StM_bind(Builder&lt;R1&gt; b1, Extension&lt;R1,R2&gt; extension)
</node>
</node>
</node>
<node CREATED="1771174242074" ID="ID_926200360" MODIFIED="1771174255852" TEXT="man k&#xf6;nnte das separate release() durch ein Lambda los werden"/>
<node COLOR="#5b280f" CREATED="1771174242074" ID="ID_926200360" MODIFIED="1771266319798" TEXT="man k&#xf6;nnte das separate release() durch ein Lambda los werden">
<icon BUILTIN="button_cancel"/>
<node CREATED="1771264739102" ID="ID_1851042941" MODIFIED="1771264745752" TEXT="geht leider nicht"/>
<node CREATED="1771264746791" ID="ID_465393386" MODIFIED="1771264760004" TEXT="da liegt ein Interface (VTable) dazwischen"/>
<node CREATED="1771264797285" ID="ID_836823668" MODIFIED="1771264859535" TEXT="das m&#xfc;&#xdf;te dann eine std::function akzeptieren">
<icon BUILTIN="smiley-oh"/>
</node>
<node CREATED="1771264832993" ID="ID_579998808" MODIFIED="1771264854809" TEXT="und diese bindet mehrere Argumente &#x27f9; Heap storage">
<icon BUILTIN="smily_bad"/>
</node>
</node>
<node COLOR="#338800" CREATED="1771187666261" ID="ID_200153250" MODIFIED="1771201072828" TEXT="die Validit&#xe4;ts-Checks nun konsistent gemacht">
<arrowlink COLOR="#39bef0" DESTINATION="ID_17538120" ENDARROW="Default" ENDINCLINATION="-605;990;" ID="Arrow_ID_600202029" STARTARROW="None" STARTINCLINATION="-326;19;"/>
<icon BUILTIN="button_ok"/>