diff --git a/src/vault/mem/engine-buffer-manager.cpp b/src/vault/mem/engine-buffer-manager.cpp index 25f1f2b43..3275bdc06 100644 --- a/src/vault/mem/engine-buffer-manager.cpp +++ b/src/vault/mem/engine-buffer-manager.cpp @@ -23,6 +23,7 @@ ** blocks of memory. The contract is that the local memory user has to ** return those blocks reliably to the EngineBuffrerManager, which ** itself is also an \ref AllocReceiver. + ** @todo 5/2026 the plan is to use pooling allocators for the render buffers //////////////////////////////TICKET #1430 ** ** @see EnginePufferManager_test ** @see LocalBufferStore_test @@ -76,12 +77,69 @@ namespace mem { requestQueue_.consume_all ([this](AllocRequest request) { REQUIRE (request.receiver); - BufferAllocator allocator; - Alloc alloc = allocator.allocate (request.sizRequest); - ENSURE (alloc.mem); + Alloc alloc = doPerformAllocation (request.sizRequest); + ENSURE (not alloc.empty()); request.receiver->supply (alloc); }); } + Alloc + EngineBufferManager::requestAllocation (size_t sizRequest) + { + processPendingRequests(); + return doPerformAllocation (sizRequest); + } + + + /** + * @todo 5/2026 this is a preliminary placeholder implementation, + * since any requests will just be passed-through to the heap allocator. + * The plan is to use a system of buffer pools eventually... ///////////////////////////////////////TICKET #1430 + */ + Alloc + EngineBufferManager::doPerformAllocation (size_t sizRequest) + { + BufferAllocator allocator; + Alloc alloc = allocator.allocate (sizRequest); + if (not alloc.empty()) + { + allocated_ += alloc.siz; + leased_ += alloc.siz; + ++cnt_; + } + return alloc; + } + + + void + EngineBufferManager::doRedeemAllocation (Alloc alloc) + { + REQUIRE (not alloc.empty()); + ENSURE (allocated_ >= alloc.siz); + ENSURE (leased_ >= alloc.siz); + ENSURE (cnt_ >= 1 ); + + BufferAllocator allocator; + allocated_ -= alloc.siz; + leased_ -= alloc.siz; + --cnt_; + allocator.deallocate (alloc); + } + + + /** @remark this API becomes relevant if we actually use a pool */ + size_t + EngineBufferManager::size() const + { + return cnt_; + } + + bool + EngineBufferManager::empty() const + { + return 0 == cnt_; + } + + }} // namespace vault::mem diff --git a/src/vault/mem/engine-buffer-manager.hpp b/src/vault/mem/engine-buffer-manager.hpp index 96ccb05be..c4baba0ea 100644 --- a/src/vault/mem/engine-buffer-manager.hpp +++ b/src/vault/mem/engine-buffer-manager.hpp @@ -70,6 +70,8 @@ namespace mem { { Buff* mem; size_t siz; + + bool empty() const { return not bool(mem); } }; @@ -118,17 +120,124 @@ namespace mem { using RequestQueue = boost::lockfree::queue; RequestQueue requestQueue_; + /// diagnostic bookkeeping + size_t allocated_{0}; + size_t leased_{0}; + size_t cnt_{0}; + public: EngineBufferManager() : AllocReceiver{} , requestQueue_{INQUEUE_SIZ} { } - + + + size_t size() const; + bool empty() const; + + + /****************************************************************//** + * Core function: satisfy a request for allocation + * @return an \ref Alloc struct representing the desired allocation, + * can be empty to signal that the request can not be satisfied. + * @warning whoever accepts a non-empty result from this call + * is **responsible** to [return](\ref AllocReceiver::supply) + * this allocation eventually, to prevent memory leakage. + */ + Alloc requestAllocation (size_t sizRequest); + + /** + * Standard access point: request an allocation + * that will be dispatched asynchronously to the given receiver. + * @remark without any temporal guarantees, this request will be + * processed eventually, placing an \ref Alloc into the + * [receiver's inqueue](\ref AllocReceiver::supply) + * @warning when the returned Alloc is [non-empty](\ref Alloc::empty), + * the receiver is responsible for returning it after use. + */ + void async_requestAllocation (AllocRequest const&); + + /** + * Handle all enqueued asynchronous requests + * and perform pending management and allocation work. + * @remark this function can be invoked as a batch job; + * it acquires a lock and then handles the backlog. + */ + void processPendingRequests(); + private: /** process all pending requests in the queue */ void handleAllocRequests(); + + /** performs the actual allocation work */ + Alloc doPerformAllocation (size_t sizRequest); + void doRedeemAllocation (Alloc); + + /** a »backdoor« for unit testing */ + friend class BufferManagerDiagnostic; }; + inline void + EngineBufferManager::async_requestAllocation (AllocRequest const& req) + { + + } + + inline void + EngineBufferManager::processPendingRequests() + { + inQueue_.consume_all ([this](Alloc alloc){ doRedeemAllocation (alloc); }); + handleAllocRequests(); + } + + + + /** wrapper to inspect internals from a unit test */ + class BufferManagerDiagnostic + : util::MoveOnly + { + EngineBufferManager const& globalPool_; + + public: + BufferManagerDiagnostic (EngineBufferManager const& manager) + : globalPool_{manager} + { } + + bool + isEmpty() + { + return globalPool_.empty(); + } + + size_t + size() + { + return globalPool_.size(); + } + + size_t + bytesAllocd() + { + UNIMPLEMENTED ("alloc state diagnostic"); + } + + size_t + bytesLeased() + { + UNIMPLEMENTED ("alloc state diagnostic"); + } + + /////////////////////////////////////////////////////////////////////////////////////////////////////TICKET #1430 : add pool diagnostics here if we actually switch to pooled allocations + }; + + + /** entrance point to inspection for test */ + inline BufferManagerDiagnostic + watch (EngineBufferManager const& manager) + { + return BufferManagerDiagnostic{manager}; + } + }} // namespace vault::mem #endif /*VAULT_MEM_ENGINE_BUFFR_MANAGA_H*/ diff --git a/tests/vault/mem/engine-buffer-manager-test.cpp b/tests/vault/mem/engine-buffer-manager-test.cpp index b77c9c21c..ef3527bed 100644 --- a/tests/vault/mem/engine-buffer-manager-test.cpp +++ b/tests/vault/mem/engine-buffer-manager-test.cpp @@ -24,11 +24,14 @@ //#include "lib/depend-inject.hpp" //#include "lib/thread.hpp" //#include "lib/error.hpp" +#include "lib/symbol.hpp" #include "test/diagnostic-output.hpp"////////////TODO +#include //using std::this_thread::yield; //using lib::Thread; +using lib::Literal; namespace vault { namespace mem { @@ -66,7 +69,8 @@ namespace test { // seedRand(); demonstrate_AllocatorInterface(); - verify_DataTransfer(); + verify_syncRequest(); + verify_asyncRequest(); verify_lulz(); } @@ -107,9 +111,34 @@ namespace test { - /** @test */ + /** @test show that a direct request for allocation is honored */ void - verify_DataTransfer() + verify_syncRequest() + { + EngineBufferManager manager; +SHOW_EXPR(watch(manager).isEmpty()) + + Alloc alloc = manager.requestAllocation (555); + CHECK (not alloc.empty()); + CHECK (alloc.siz >= 555); + +SHOW_EXPR(watch(manager).bytesAllocd()) +SHOW_EXPR(watch(manager).bytesLeased()) + + // can use that memory + char* buf = reinterpret_cast (alloc.mem); + string ranS{lib::randStr(555-1)}; + auto* p = std::copy (ranS.begin(), ranS.end(), buf); +SHOW_EXPR(p - buf) + *p = '\0'; + CHECK (ranS == buf); + } + + + + /** @test show that an working allocation request across thread boundaries */ + void + verify_asyncRequest() { } diff --git a/wiki/thinkPad.ichthyo.mm b/wiki/thinkPad.ichthyo.mm index 5e9fd4bc0..1f64aab39 100644 --- a/wiki/thinkPad.ichthyo.mm +++ b/wiki/thinkPad.ichthyo.mm @@ -116126,6 +116126,11 @@ StM_bind(Builder<R1> b1, Extension<R1,R2> extension) + + + + + @@ -146174,7 +146179,9 @@ StM_bind(Builder<R1> b1, Extension<R1,R2> extension) - + + + @@ -146543,6 +146550,8 @@ StM_bind(Builder<R1> b1, Extension<R1,R2> extension) + + @@ -146568,14 +146577,13 @@ StM_bind(Builder<R1> b1, Extension<R1,R2> extension)

+
- - - +

das ist zwar nur ein Grenzfall @@ -146584,50 +146592,38 @@ StM_bind(Builder<R1> b1, Extension<R1,R2> extension)  — aber er stellt das gesamte Konzept in Frage

- -
+ - - - +

Es handelt sich ganz klassisch um einen »Race« — das heißt, diese Situation ist in keinster Weise regulär vorgesehen, und dürfrte auch auch schwierig zu reproduzieren sein; sie sollte daher nicht der Maßstab sein, an dem das Konstrukt insgeseamt ausgerichtet wird. Leider aber führt diese Situation zur memory corruption, wenn wir das bisher ausgearbeitete Konzept umsetzen

- -
+ - - - +

denn andernfalls könnte man versuchen, über eine Spezialbehandlung im Destruktor die gesendete Speicher-Anforderung irgendwie stornieren (wiewohl auch dabei noch ein Race auftreten könnte)

- -
+
- - - +

So ähnlich wie der ZombieCheck...

- -
+
- - - +

Erläuterung... @@ -146653,10 +146649,7 @@ StM_bind(Builder<R1> b1, Extension<R1,R2> extension) - - - - + @@ -146689,12 +146682,31 @@ StM_bind(Builder<R1> b1, Extension<R1,R2> extension) + + + + + + + + - + + + + +

+ ...so daß es tatsächlich nur eine Lösung gibt, nämlich die Asynchrone, die dann hier aber sofort im aktuellen Thread komplett durchläuft. Wie sich zeigt, ist das nicht bloß ein Kniff, um Duplikation im Code zu vermeiden, sondern löst wie von selbst eine schwierige architektonische Aufgabe +

+ +
+ +
+ @@ -146715,8 +146727,100 @@ StM_bind(Builder<R1> b1, Extension<R1,R2> extension) - - + + + + + + + + + + + + + + + +

+ Möglicherweise ist diese Lösung gar nicht so dramatisch schlecht... +

+

+ Allerdings wird dadurch der Aufwand für Allokationen und Speichermanagement zum Blocker und kann auch andere Threads behindern; wie schädlich das aber tatsächlich in der Praxis wird, hängt davon ab wie gut der »local Pool«-Ansatz wirkt +

+ +
+
+ + + + +

+ Da wir nur N Kerne haben, muß letztlich immer dafür Rechenkapazität von einem Worker verdrängt werden (es sei denn, man hat so viele Kerne, daß man einfach mal einen Kern für »Sonstiges« beiseite stellen kann) +

+ +
+
+ + + + +

+ Das ist ein Kniff (für den ich einige Zeit gebraucht habe, um ihn zu sehen): wenn es gelingt, die synchrone Lösung komplett durch die asynchrone Lösung zu implementieren, dann impliziert der Aufruf dieser synchronen Lösung daß effektiv ein »sub-Job« mit der aktuell aufgesammelten Allokator-Arbeit im aktuellen Worker läuft. Das ist zwar ein ziemlich trickreicher Ansatz, hat aber die Schönheit daß man eigentlich gar nichts dafür tun muß — insofern wäre damit jede andere Lösung (Hintergrund-Thread, dedizierter Job) nur noch eine optionale Performance-Optimierung +

+ +
+ + +
+ + + + +

+ Dieser würde dann periodisch schlafen und die Queue bedienen. Nach außen bleibt das komplett transparent und der Code ist einfach; allerdings kann das dazu führen daß andere Worker zwischen den Cores migrieren, außerdem wird das System insgesamt komplexer (es braucht einen separaten Service mit eigenem Lebenszyklus). Sofern allerdings der Aufwand für die Allokations-Arbeit gegenüber dem Rendering vernachlässigbar ist, und weniger als ein OS-Schedule-Slice benötigt, dürfte sich der Einfluß auf die Performance in Grenzen halten — unter diesen Voraussetzungen allerdings könnte der jeweilge Worker diese Arbeit auch gleich selber machen +

+ +
+
+ + + + +

+ In dem Moment, wo so eine Anforderung auftritt, wird ein Service-Job abgesetzt, mit einer Priorität, die noch zu diskutieren wäre. Möglicherweise müßte die Steuerung sogar dynamisch sein, denn es handelt sich um eine »opportunistische« Parallelisierung, die nur sinnvoll ist, wenn der eigentliche Worker inzwischen schon mal anfangen kann. Denn sonst besteht die Gefahr von duplizierten Allokationen, wenn kurz darauf der Speicher definitiv im Worker gebraucht wird, und daher direkt, synchron angefordert. Das läßt sich nicht verhindern, da wir nicht »buch führen« über die Anforderungen, und auch nicht aggregieren, da der Aufwand hierfür im Regelfall verschwendet wäre. Eine weitere relevante Frage ist die nach dem insgesamt anfallenden Aufwand; da Anfragen, aber auch die die Rückgabe von Allokationen asynchron über Queues erfolgt, steht zu erwarten daß sich jeweils einge dieser »kleinen Handgriffe« ansammeln. Dem steht gegenüber, daß das Scheduling eines Jobs einen erheblichen Aufwand verursacht, der im Verhältnis um 1-2 Größenordnungen über dem Aufwand für ein Mutex-Lock und eine globale Allokation liegt. +

+ +
+ + + + + + + + + + + + + + + + +
+
+ + + + +
+ + + + + + @@ -146727,6 +146831,37 @@ StM_bind(Builder<R1> b1, Extension<R1,R2> extension) + + + + + + + + + + + + + + + + + + + + +

+ wenigstens demonstriert es die Nutzbarkeit +

+ +
+
+
+
+ + +
@@ -171421,6 +171556,36 @@ std::cout << tmpl.render({"what", "World"}) << s + + + + + + + + + + + + +

+ es ist nicht einmal klar wie wir vorläufig damit umgehen +

+ +
+ + + +

+ ...denn es muß hier etwas getan werden, sonst funktioniert das System überhaupt nicht — ist es sinnvoll, zunächst die Anforderungen stets synchron zu handhaben? +

+ +
+ + +
+
+