BufferProvider: design a new diagnostics API

To push ahead with the refactoring,
I will build the test/diagnostics API completely anew,
and attach it solely to an instance of `DiagnosticBufferProvider`.

Unfortunately this ''leads us into chainsaw surgery...''
 * it uncovers a host of conceptual contradictions
 * actually, the `HeapMemProvider` was more an implementation
   of diagnostic tracking, than it was an actual `BufferProvider`
   implementation (hence the old name: `TrackingHeapBlockProivder`)
 * and this lured me into using these tracking functions directly,
   while the `DiagnosticBufferProvider` remained in unfinished state.

So basically I have now **disabled** all verification from the Unit-tests.
I'll have to rebuild them from scratch, thereby reproducing the logic/intention

src/steam/engine/diagnostic-buffer-provider.cpp

@@ -31,10 +31,19 @@
 namespace steam {
 namespace engine {
   
+  /**
+   * Implementation of tracking and instrumentation
+   */
+  class DiagnosticBufferProvider::BlockTracker
+    {
+      
+    };
+  
   
   DiagnosticBufferProvider::DiagnosticBufferProvider()
     : NaiveBufferSetup{}
     , heapStore_{dynamic_cast<HeapMemProvider&> (*bufferStore_)}
+    , tracker_{std::make_unique<BlockTracker>()}
     { }
   
   DiagnosticBufferProvider::~DiagnosticBufferProvider()
@@ -50,22 +59,45 @@ namespace engine {
   bool
   BufferDiagnostic::buffer_was_used (uint bufferID)
     {
-      return dbp_.heapStore_.access_emitted(bufferID).was_used();
+      NOTREACHED ("shall use new API only");
+//    return dbp_.heapStore_.access_emitted(bufferID).was_used();  //////////////////////////////////////////TICKET 1410 : switch to newly defined tracking-API
     }
   
   
   bool
   BufferDiagnostic::buffer_was_closed (uint bufferID)
     {
-      return dbp_.heapStore_.access_emitted(bufferID).was_closed();
+      NOTREACHED ("shall use new API only");
+//    return dbp_.heapStore_.access_emitted(bufferID).was_closed();//////////////////////////////////////////TICKET 1410 : switch to newly defined tracking-API
     }
   
   
   void*
   BufferDiagnostic::accessMemory (uint bufferID)
     {
-      return dbp_.heapStore_.access_emitted(bufferID).accessMemory();
+      NOTREACHED ("shall use new API only");
+//    return dbp_.heapStore_.access_emitted(bufferID).accessMemory(); ///////////////////////////////////////TICKET 1410 : switch to newly defined tracking-API
+    }
+  
+  diagn::StateReg&
+  BufferDiagnostic::created ()
+    {
+      UNIMPLEMENTED ("track all created buffer blocks");
+    }
+  
+  diagn::StateReg&
+  BufferDiagnostic::emitted ()
+    {
+      UNIMPLEMENTED ("track all emitted buffer blocks");
+    }
+  
+  diagn::StateReg&
+  BufferDiagnostic::released()
+    {
+      UNIMPLEMENTED ("track all released buffer blocks");
     }
   
 
+  
+
 }} // namespace engine

src/steam/engine/diagnostic-buffer-provider.hpp

@@ -35,9 +35,31 @@ namespace engine {
   
   namespace error = lumiera::error;
   
-  class HeapMemProvider;
+  class HeapMemProvider;  //////////////////////////////OOO fällt dann weg nach dem Umbau
   class BufferDiagnostic;
   
+  namespace diagn {// state descriptors for diagnostics....
+    
+    struct Block
+      : BuffHandle
+      , util::NonCopyable
+      {
+        
+        Block(BuffHandle const& handle) : BuffHandle{handle} { }
+        Block(BuffDescr const&  descr)  : BuffHandle{descr}  { }
+      };
+    
+    class StateReg
+      : util::NonCopyable
+      {
+      public:
+        size_t cnt()  const;
+        Block const& operator[] (size_t  seqNr)  const;
+        Block const& byHandle   (HashVal handle) const;
+      };
+    
+  }//(End)diagnostic descriptors.
+
   
   
   /****************************************************************//**
@@ -48,8 +70,9 @@ namespace engine {
   class DiagnosticBufferProvider
     : public NaiveBufferSetup
     {
-      HeapMemProvider& heapStore_;
+      HeapMemProvider& heapStore_;  //////////////////////////////OOO fällt dann weg nach dem Umbau
-      
+      class BlockTracker;
+      std::unique_ptr<BlockTracker> tracker_;
       
     public:
      ~DiagnosticBufferProvider();
@@ -70,10 +93,13 @@ namespace engine {
         { }
       
       
-      bool buffer_was_used (uint bufferID);
+      bool buffer_was_used (uint bufferID);   ///< @deprecated       ////////////////////////////////////////TICKET 1410
-      bool buffer_was_closed (uint bufferID);
+      bool buffer_was_closed (uint bufferID); ///< @deprecated       ////////////////////////////////////////TICKET 1410
-      void* accessMemory (uint bufferID);
+      void* accessMemory (uint bufferID);     ///< @deprecated       ////////////////////////////////////////TICKET 1410
       bool all_buffers_released();
+      diagn::StateReg& created ();
+      diagn::StateReg& emitted ();
+      diagn::StateReg& released();
     };
   
   inline BufferDiagnostic

src/steam/engine/heap-mem-buffer-store.cpp

@@ -41,6 +41,51 @@ namespace engine {
   using Buff = StreamType::ImplFacade::DataBuffer;
   
   
+/////////////////////////////////////////////////////////////////////////////////////////////////////////////TICKET 1410 : need to question what implementation structures are needed, after »tracking« was extracted...
+    /**
+     * Helper for implementing a diagnostic BufferProvider:
+     * A block of heap allocated storage, with the capability
+     * to store some additional tracking information.
+     */
+    class HeapMemProvider::Block
+      : util::NonCopyable
+      {
+        unique_ptr<char[]> storage_;
+        
+        bool was_released_;
+        
+      public:
+        explicit
+        Block(size_t bufferSize)
+          : storage_(bufferSize? new char[bufferSize] : NULL)
+          , was_released_(false)
+          { }
+        
+        bool
+        was_used()  const
+          {
+            return bool(storage_);
+          }
+        
+        bool
+        was_closed()  const
+          {
+            return was_released_;
+          }
+        
+        void*
+        accessMemory()  const
+          {
+            REQUIRE (storage_, "Block was never prepared for use");
+            return storage_.get();
+          }
+        
+        void
+        markReleased()
+          {
+            was_released_ = true;
+          }
+      };
   namespace { // implementation helpers...
     
     inline Buff*
@@ -50,7 +95,7 @@ namespace engine {
       }
     
     
-    using diagn::Block;
+    using Block = HeapMemProvider::Block;
     
     /** helper to find Block entries
      *  based on their raw memory address */
@@ -82,10 +127,11 @@ namespace engine {
   }
   
   
-  
+
-  namespace diagn {
+      
+/////////////////////////////////////////////////////////////////////////////////////////////////////////////TICKET 1410 : need to question what implementation structures are needed, after »tracking« was extracted...
     
-    using PoolBlocks = ScopedPtrVect<Block>;
+    using PoolBlocks = ScopedPtrVect<HeapMemProvider::Block>;
     
     /**
      * @internal Pool of allocated buffer Blocks of a specific size.
@@ -98,10 +144,11 @@ namespace engine {
      * and later gets "emitted" to an output sequence, where it remains for
      * later investigation and diagnostics.
      */
-    class BlockPool
+    class HeapMemProvider::BlockPool
       {
         uint maxAllocCount_;
         size_t memBlockSize_;
+        using PoolBlocks = ScopedPtrVect<Block>;
         PoolBlocks blockList_;
         
       public:
@@ -205,7 +252,7 @@ namespace engine {
               return not block.was_used() or block.was_closed();
             }
       };
-  }
+/////////////////////////////////////////////////////////////////////////////////////////////////////////////TICKET 1410 : need to question what implementation structures are needed, after »tracking« was extracted...
   
   
   
@@ -213,7 +260,7 @@ namespace engine {
     
     const uint MAX_BUFFERS = 50;
     
-    diagn::Block emptyPlaceholder(0);
+    Block emptyPlaceholder(0);
   
   } // (END) Details of allocation and accounting
   
@@ -223,7 +270,7 @@ namespace engine {
    * @internal create a memory tracking BufferProvider,
    */
   HeapMemProvider::HeapMemProvider()
-    : pool_(new diagn::PoolTable)
+    : pool_(new PoolTable)
     , outSeq_()
     { }
   
@@ -236,7 +283,7 @@ namespace engine {
   uint
   HeapMemProvider::prepareBuffers(uint requestedAmount, HashVal typeID)
   {
-    diagn::BlockPool& responsiblePool = getBlockPoolFor (typeID);
+    BlockPool& responsiblePool = getBlockPoolFor (typeID);
     return responsiblePool.prepare_for (requestedAmount);
   }
 
@@ -244,8 +291,8 @@ namespace engine {
   BuffHandle
   HeapMemProvider::provideLockedBuffer(HashVal typeID)
   {
-    diagn::BlockPool& blocks = getBlockPoolFor (typeID);
+    BlockPool& blocks = getBlockPoolFor (typeID);
-    diagn::Block& newBlock = blocks.createBlock();
+    Block& newBlock = blocks.createBlock();
     UNIMPLEMENTED ("Separate tasks into two distinct APIs");
 //  return buildHandle (typeID, asBuffer(newBlock.accessMemory()), &newBlock); /////////////////////////////OOO instead return a tuple (buffer, localTag)
   }
@@ -254,11 +301,11 @@ namespace engine {
   void
   HeapMemProvider::mark_emitted (HashVal typeID, LocalTag const& specifics)
   {
-    diagn::Block* block4buffer = locateBlock (typeID, specifics);
+    Block* block4buffer = locateBlock (typeID, specifics);
     if (!block4buffer)
       throw error::Logic ("Attempt to emit a buffer not known to this BufferProvider"
                          , LUMIERA_ERROR_BUFFER_MANAGEMENT);
-    diagn::BlockPool& pool = getBlockPoolFor (typeID);
+    BlockPool& pool = getBlockPoolFor (typeID);
     Block* active = pool.transferResponsibility (block4buffer);
     if (active)
       outSeq_.manage (active);
@@ -275,7 +322,7 @@ namespace engine {
   void
   HeapMemProvider::detachBuffer (HashVal typeID, LocalTag const& specifics, Buff& storage)
   {
-    diagn::Block* block4buffer = locateBlock (typeID, specifics);
+    Block* block4buffer = locateBlock (typeID, specifics);
     REQUIRE (block4buffer, "releasing a buffer not allocated through this provider");
     REQUIRE (util::isSameAdr (storage, block4buffer->accessMemory()));
     block4buffer->markReleased();
@@ -298,7 +345,7 @@ namespace engine {
          blockPool.discard();
   }
   
-  diagn::Block&
+  HeapMemProvider::Block&
   HeapMemProvider::access_emitted (uint bufferID)
   {
     if (!withinOutputSequence (bufferID))
@@ -316,26 +363,28 @@ namespace engine {
     return bufferID < outSeq_.size();
   }
   
-  diagn::BlockPool&
+  HeapMemProvider::BlockPool&
   HeapMemProvider::getBlockPoolFor (HashVal typeID)
   {
-    diagn::BlockPool& pool = (*pool_)[typeID];
+    BlockPool& pool = (*pool_)[typeID];
     UNIMPLEMENTED ("Separate tasks into two distinct APIs");
 //  if (!pool)
 //      pool.initialise(getBufferSize(typeID));   //////////////////////////OOO getBufferSize is a helper function in the BufferProvider protected API. Need to be "here"
     return pool;
   }
   
-  diagn::Block*
+  HeapMemProvider::Block*
   HeapMemProvider::locateBlock (HashVal typeID, void* storage)
   {
-    diagn::BlockPool& pool = getBlockPoolFor (typeID);
+    BlockPool& pool = getBlockPoolFor (typeID);
-    diagn::Block* block4buffer = pool.find (storage);                         ////////////////////////////////TICKET #856
+    Block* block4buffer = pool.find (storage);                                ////////////////////////////////TICKET #856
     return block4buffer? block4buffer
                        : searchInOutSeqeuence (storage);
   }
   
-  diagn::Block*
+  
+  
+  HeapMemProvider::Block*
   HeapMemProvider::searchInOutSeqeuence (void* blockLocation)
   {
     return pick_Block_by_storage (outSeq_, blockLocation);                    ////////////////////////////////TICKET #856

src/steam/engine/heap-mem-buffer-store.hpp

@@ -57,54 +57,6 @@ namespace engine {
     using std::unique_ptr;
 
     
-    /**
-     * Helper for implementing a diagnostic BufferProvider:
-     * A block of heap allocated storage, with the capability
-     * to store some additional tracking information.
-     */
-    class Block
-      : util::NonCopyable
-      {
-        unique_ptr<char[]> storage_;
-        
-        bool was_released_;
-        
-      public:
-        explicit
-        Block(size_t bufferSize)
-          : storage_(bufferSize? new char[bufferSize] : NULL)
-          , was_released_(false)
-          { }
-        
-        bool
-        was_used()  const
-          {
-            return bool(storage_);
-          }
-        
-        bool
-        was_closed()  const
-          {
-            return was_released_;
-          }
-        
-        void*
-        accessMemory()  const
-          {
-            REQUIRE (storage_, "Block was never prepared for use");
-            return storage_.get();
-          }
-        
-        void
-        markReleased()
-          {
-            was_released_ = true;
-          }
-      };
-      
-    class BlockPool;
-    
-    using PoolTable = std::unordered_map<HashVal,BlockPool>;
   }
   
   
@@ -120,8 +72,13 @@ namespace engine {
   class HeapMemProvider
     : public BufferProviderSetup::Store
     {
-      unique_ptr<diagn::PoolTable> pool_;
+    public: /////////////////////////////////////////////////////////////////////////////////////////////////TICKET #1410 : looks like I'll re-implement the storage from scratch, since »tracking« is no longer required here
-      ScopedPtrVect<diagn::Block> outSeq_;
+      class Block;
+      class BlockPool;
+    private:
+      using PoolTable = std::unordered_map<HashVal,BlockPool>;
+      unique_ptr<PoolTable> pool_;
+      ScopedPtrVect<Block> outSeq_;
       
     public:
       /* === BufferStore interface === */
@@ -137,7 +94,7 @@ namespace engine {
       
       size_t emittedCnt()  const;
       
-      diagn::Block& access_emitted (uint bufferID);
+      Block& access_emitted (uint bufferID);
       
       template<typename TY>
       TY&  accessAs (uint bufferID);
@@ -146,9 +103,9 @@ namespace engine {
       
     private:
       bool withinOutputSequence (uint bufferID)  const;
-      diagn::BlockPool& getBlockPoolFor (HashVal typeID);
+      BlockPool& getBlockPoolFor (HashVal typeID);
-      diagn::Block* locateBlock (HashVal typeID, void*);
+      Block* locateBlock (HashVal typeID, void*);
-      diagn::Block* searchInOutSeqeuence (void* storage);
+      Block* searchInOutSeqeuence (void* storage);
     };
   
   
@@ -168,11 +125,12 @@ namespace engine {
     if (!withinOutputSequence (bufferID))
       throw error::Invalid ("Buffer with the given ID not yet emitted");
     
-    diagn::Block& memoryBlock = access_emitted (bufferID);
+    Block& memoryBlock = access_emitted (bufferID);
-    TY* converted = std::launder (reinterpret_cast<TY*> (memoryBlock.accessMemory()));
+    NOTREACHED ("TICKET #1410 : tracking implementation -> DiagnosticBufferProvider");
+//  TY* converted = std::launder (reinterpret_cast<TY*> (memoryBlock.accessMemory()));  /////////////////////TICKET 1410 : my assumption is that Block will be implemented differently -- at least not used this way...
     
-    REQUIRE (converted);
+//  REQUIRE (converted);
-    return *converted;
+//  return *converted;
   }
   
   

tests/core/steam/engine/buffer-provider-protocol-test.cpp

@@ -80,8 +80,8 @@ namespace test  {
         {
           verifySimpleUsage();
           verifyRenderingUsage();
-          verifyObjectAttachment();
+//          verifyObjectAttachment();
-          verifyObjectAttachmentFailure();
+//          verifyObjectAttachmentFailure();  //////////////////////////OOO need a completely new test case to cover behaviour of failed ctor calls
         }
       
       
@@ -181,6 +181,7 @@ namespace test  {
         }
       
       
+#if false  //////////////////////////////////////////////////////////////////////////////////////////////////TICKET #1410 : disabled code to disentangle BufferProvider implementation
       void
       verifyObjectAttachment()
         {
@@ -256,6 +257,7 @@ namespace test  {
           VERIFY_ERROR (LIFECYCLE, handle_DD.accessAs<Dummy>() );
           VERIFY_ERROR (LIFECYCLE, handle_DD.create<Dummy>() );
         }
+#endif  /////////////////////////////////////////////////////////////////////////////////////////////////////TICKET #1410 : (end) disabled code
     };
   
   

tests/core/steam/engine/diagnostic-buffer-provider-test.cpp

@@ -42,18 +42,22 @@ namespace test  {
     bool 
     has_expectedContent (uint nr, diagn::Block& memoryBlock)
     {
+#if false  //////////////////////////////////////////////////////////////////////////////////////////////////TICKET #1410 : disabled code to disentangle BufferProvider implementation
       void* mem = memoryBlock.accessMemory();
       uint data = *static_cast<uint*> (mem);
       
       return data == testNumbers[nr];
+#endif  /////////////////////////////////////////////////////////////////////////////////////////////////////TICKET #1410 : (end) disabled code
     }
     
     bool
     verifyUsedBlock (uint nr, diagn::Block& memoryBlock)
     {
+#if false  //////////////////////////////////////////////////////////////////////////////////////////////////TICKET #1410 : disabled code to disentangle BufferProvider implementation
       return memoryBlock.was_used()
          and memoryBlock.was_closed()
          and has_expectedContent (nr, memoryBlock);
+#endif  /////////////////////////////////////////////////////////////////////////////////////////////////////TICKET #1410 : (end) disabled code
     }
   }
   

tests/core/steam/play/diagnostic-output-slot.hpp

@@ -212,6 +212,7 @@ namespace play {
 //          return 0;                                               ////////////////////////////////TICKET #856
         }
       
+#if false  //////////////////////////////////////////////////////////////////////////////////////////////////TICKET #1410 : disabled code to disentangle BufferProvider implementation
       diagn::Block const *
       accessEmittedBuffer (uint bufferNr)  const
         {
@@ -221,6 +222,7 @@ namespace play {
 //        else
 //          return 0;
         }
+#endif  /////////////////////////////////////////////////////////////////////////////////////////////////////TICKET #1410 : (end) disabled code
       
       bool
       wasAllocated (uint frameNr)  const
@@ -238,12 +240,14 @@ namespace play {
 //        return unConst(this)->buffProvider_.accessAs<TestFrame> (frameNr);      ///////////////////////////////OOO provide suitable diagnostic API!
         }
       
+#if false  //////////////////////////////////////////////////////////////////////////////////////////////////TICKET #1410 : disabled code to disentangle BufferProvider implementation
       diagn::Block const&
       accessBlock (uint bufferNr)  const
         {
           UNIMPLEMENTED ("suitable diagnostic API");
 //        return unConst(this)->buffProvider_.access_emitted (bufferNr);      ///////////////////////////////OOO provide suitable diagnostic API!
         }
+#endif  /////////////////////////////////////////////////////////////////////////////////////////////////////TICKET #1410 : (end) disabled code
     };
   
   
@@ -412,20 +416,24 @@ namespace play {
       bool
       output_was_emitted (uint channel, FrameID outputFrame)
         {
+#if false  //////////////////////////////////////////////////////////////////////////////////////////////////TICKET #1410 : disabled code to disentangle BufferProvider implementation
           diagn::Block const *block = accessSequence(channel)
                                         .accessEmittedBuffer(outputFrame);
           return block
              and block->was_used();
+#endif  /////////////////////////////////////////////////////////////////////////////////////////////////////TICKET #1410 : (end) disabled code
         }
       
       
       bool
       output_was_closed (uint channel, FrameID outputFrame)
         {
+#if false  //////////////////////////////////////////////////////////////////////////////////////////////////TICKET #1410 : disabled code to disentangle BufferProvider implementation
           diagn::Block const *block = accessSequence(channel)
                                         .accessEmittedBuffer(outputFrame);
           return block
              and block->was_closed();
+#endif  /////////////////////////////////////////////////////////////////////////////////////////////////////TICKET #1410 : (end) disabled code
         }
     };