diff --git a/src/lib/advice.hpp b/src/lib/advice.hpp index 063584533..d76193a22 100644 --- a/src/lib/advice.hpp +++ b/src/lib/advice.hpp @@ -91,12 +91,14 @@ #include -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*); - const PointOfAdvice* discardSolutions (); + void publishProvision (PointOfAdvice*); + void discardSolutions (); void publishRequestBindingChange(HashVal); void registerRequest(); void deregisterRequest(); - void* getBuffer(size_t); - void releaseBuffer (const void*, size_t); + static void* getBuffer(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( - storeCopy (pieceOfAdvice))); + publishProvision( + 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::storeCopy (AD const& advice_given) { typedef ActiveProvision 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 inline void - Provision::maybe_deallocateOld (const PointOfAdvice* existingEntry) + Provision::releaseAdviceData (void* entry) { typedef ActiveProvision Holder; - if (existingEntry) + if (entry) { - Holder* obj = (Holder*)existingEntry; + Holder* obj = static_cast (entry); obj->~Holder(); - releaseBuffer (existingEntry, sizeof(Holder)); + releaseBuffer (entry, sizeof(Holder)); } } @@ -351,9 +363,8 @@ namespace advice { AdviceProvision* solution = static_cast (getSolution()); if (solution) // create copy of the data holder, using the new binding - maybe_deallocateOld ( - publishProvision( - storeCopy (solution->getAdvice()))); + publishProvision( + storeCopy (solution->getAdvice())); } diff --git a/src/lib/advice/advice.cpp b/src/lib/advice/advice.cpp index 6d0e32b06..660ff9ef8 100644 --- a/src/lib/advice/advice.cpp +++ b/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); }