Refactor storing of advice to prepare for actually managing the storage

src/lib/advice.hpp

@@ -91,12 +91,14 @@
 
 #include <boost/noncopyable.hpp>
 
-using util::isSameObject;
-
 
 namespace lib    {
 namespace advice {
   
+  using lib::Symbol;
+  using util::isSameObject;
+  
+  
   /**
    * TODO type comment
    */
@@ -167,15 +169,18 @@ namespace advice {
     : public PointOfAdvice
     {
     protected:
-      const PointOfAdvice* publishProvision (PointOfAdvice*);
+      void publishProvision (PointOfAdvice*);
-      const PointOfAdvice* discardSolutions ();
+      void discardSolutions ();
       void publishRequestBindingChange(HashVal);
       
       void registerRequest();
       void deregisterRequest();
       
-      void* getBuffer(size_t);
+      static void* getBuffer(size_t);
-      void  releaseBuffer (const void*, size_t);
+      static void  releaseBuffer (const void*, size_t);
+      
+      typedef void (DeleterFunc)(void*);
+      static void manageAdviceData (PointOfAdvice*, DeleterFunc*);
       
     public:
       explicit
@@ -241,15 +246,13 @@ namespace advice {
       
       void setAdvice (AD const& pieceOfAdvice)
         {
-          maybe_deallocateOld (
+          publishProvision(
-              publishProvision(
+            storeCopy (pieceOfAdvice));
-                  storeCopy (pieceOfAdvice)));
         }
       
       void retractAdvice()
         {
-          maybe_deallocateOld(
+          discardSolutions();
-              discardSolutions());
         }
       
       void
@@ -261,7 +264,7 @@ namespace advice {
       
     private:
       PointOfAdvice* storeCopy (AD const& advice_given);
-      void maybe_deallocateOld(const PointOfAdvice*);
+      static void releaseAdviceData (void*);
       void maybe_rePublish ();
     };
   
@@ -314,27 +317,36 @@ namespace advice {
   Provision<AD>::storeCopy (AD const& advice_given)
   {
     typedef ActiveProvision<AD> Holder;
-    return new(getBuffer(sizeof(Holder))) Holder (*this, advice_given);
+    void* storage = getBuffer(sizeof(Holder));
+    try
+      {
+        Holder* storedProvision = new(storage) Holder (*this, advice_given);
+        manageAdviceData (storedProvision, &releaseAdviceData);
+        return storedProvision;
+      }
+    catch(...)
+      {
+        Symbol errID = lumiera_error();
+        releaseBuffer (storage, sizeof(Holder));
+        throw lumiera::error::Fatal ("Failure to store advice data", errID);
+      }
   }
   
   
   /** @internal assist the AdviceSystem with deallocating buffer storage.
    *  Problem is we need to know the exact size of the advice value holder,
    *  which information is available only here, in the fully typed context.
-   *  @note the assumption is that \em any binding created will automatically
-   *        contain a type guard, which ensures the existingEntry passed in here
-   *        originally was allocated by #storeCopy within the same typed context.
    */
   template<class AD>
   inline void
-  Provision<AD>::maybe_deallocateOld (const PointOfAdvice* existingEntry)
+  Provision<AD>::releaseAdviceData (void* entry)
   {
     typedef ActiveProvision<AD> Holder;
-    if (existingEntry)
+    if (entry)
       {
-        Holder* obj = (Holder*)existingEntry;
+        Holder* obj = static_cast<Holder*> (entry);
         obj->~Holder();
-        releaseBuffer (existingEntry, sizeof(Holder));
+        releaseBuffer (entry, sizeof(Holder));
       }
   }
   
@@ -351,9 +363,8 @@ namespace advice {
     AdviceProvision* solution = static_cast<AdviceProvision*> (getSolution());
     
     if (solution)    // create copy of the data holder, using the new binding 
-      maybe_deallocateOld (
+      publishProvision(
-          publishProvision(
+        storeCopy (solution->getAdvice()));
-              storeCopy (solution->getAdvice())));
   }
   
   

src/lib/advice/advice.cpp

@@ -61,10 +61,18 @@
  ** re-discover the specifically typed context; the type guards can only be checked for a match.
  ** Thus we need the help of the frontend objects, which need to provide a deleter function
  ** when providing concrete advice data; this deleter function will be saved as function pointer
- ** so to be able to deallocate all advice data when the AdviceSystem is shut down
+ ** so to be able to deallocate all advice data when the AdviceSystem is shut down.
+ ** 
+ ** @todo This approach is closely related to the decision to use a single global index table
+ ** for managing all advice collaborations. An alternative would be to use either a separate
+ ** table for each type, or to store an additional type descriptor with this memory management
+ ** information into each "bucket", which can be assumed to manage entries dealing with the
+ ** same kind of advice data, because each binding automatically includes a type guard.
+ ** If we ever happen to get a significant amount of advice data, but only a small
+ ** number of different advice types, we should reconsider this optimisation.
  ** 
  ** @todo currently this is unimplemented and we happily leak memory....
- ** @todo rewrite the allocation to use Lumiera's mpool instead of heap allocations
+ ** @todo rewrite the allocation to use Lumiera's MPool instead of heap allocations
  ** 
  ** \par synchronisation
  ** While the frontend objects are deliberately \em not threadsafe, the lookup implementation
@@ -112,6 +120,15 @@ namespace advice {
           {
             INFO (library, "Shutting down Advice system.");
           }
+       
+       
+       void discardEntry (const PointOfAdvice* storedProvision)
+         {
+           if (storedProvision)
+             {
+               UNIMPLEMENTED ("use stored management information to trigger deletion");
+             }
+         }
       };
     
     
@@ -155,6 +172,21 @@ namespace advice {
   }
   
   
+  /** Store a descriptor record to take ownership of the given allocation.
+   *  Problem is we need to know the exact size of the advice value holder,
+   *  which information is available initially, when the advice data is
+   *  copied into the system. The knowledge about the size of the allocation
+   *  is embodied into the deleter function. This allows later to discard
+   *  entries without needing to know their exact type. 
+   */
+  void
+  AdviceLink::manageAdviceData (PointOfAdvice* entry, DeleterFunc* how_to_delete)
+  {
+    UNIMPLEMENTED ("store memory management information");
+  }
+
+  
+  
   
   /** when the Provision actually sets advice data, this is copied
    *  into an internal buffer within the AdviceSystem. We then use the
@@ -168,7 +200,7 @@ namespace advice {
    *          and thus the size of the entry to deallocate.
    *          Returning \c NULL in case no old entry exists.
    */
-  const PointOfAdvice*
+  void
   AdviceLink::publishProvision (PointOfAdvice* newProvision)
   {
     const PointOfAdvice* previousProvision (getSolution());
@@ -183,7 +215,7 @@ namespace advice {
     if (previousProvision && !newProvision)
       aSys().removeProvision (*previousProvision);
     
-    return previousProvision;  // to be deallocated by caller if non-NULL
+    aSys().discardEntry (previousProvision);
   }
   
   
@@ -195,14 +227,15 @@ namespace advice {
    *          to be deallocated by the caller, which
    *          is assumed to know it's exact type.
    */
-  const PointOfAdvice*
+  void
   AdviceLink::discardSolutions ()
   {
     const PointOfAdvice* existingProvision (getSolution());
     this->setSolution ( NULL );
     if (existingProvision)
       aSys().removeProvision (*existingProvision);
-    return existingProvision;
+    
+    aSys().discardEntry (existingProvision);
   }