diff --git a/CachingBaseStore.h b/CachingBaseStore.h deleted file mode 100644 index 3a7d1ed3..00000000 --- a/CachingBaseStore.h +++ /dev/null @@ -1,18 +0,0 @@ -#ifndef MESSMER_CACHINGSTORE_CACHINGBASESTORE_H_ -#define MESSMER_CACHINGSTORE_CACHINGBASESTORE_H_ - -#include - -namespace cachingstore { - -template -class CachingBaseStore { -public: - virtual ~CachingBaseStore() {} - virtual std::unique_ptr loadFromBaseStore(const Key &key) = 0; - virtual void removeFromBaseStore(std::unique_ptr block) = 0; -}; - -} - -#endif diff --git a/CachingStore.h b/CachingStore.h deleted file mode 100644 index 05940c26..00000000 --- a/CachingStore.h +++ /dev/null @@ -1,158 +0,0 @@ -#ifndef MESSMER_BLOCKSTORE_IMPLEMENTATIONS_CACHING_CACHINGSTORE_H_ -#define MESSMER_BLOCKSTORE_IMPLEMENTATIONS_CACHING_CACHINGSTORE_H_ - -#include -#include -#include -#include -#include -#include - -#include "CachingBaseStore.h" - -//TODO Refactor -//TODO Test cases - -namespace cachingstore { - -template -class CachingStore { -public: - CachingStore(std::unique_ptr> baseStore) - : _mutex(), - _baseStore(std::move(baseStore)), - _openResources(), - _resourcesToRemove() { - } - - //TODO Enforce CachedResourceRef inherits from CachedResource - - class CachedResource { - public: - //TODO Better way to initialize - CachedResource(): _cachingStore(nullptr), _key(Key::CreateRandom()) {} - void init(CachingStore *cachingStore, const Key &key) { - _cachingStore = cachingStore; - _key = key; - } - virtual ~CachedResource() { - _cachingStore->release(_key); - } - private: - CachingStore *_cachingStore; - //TODO We're storing Key twice (here and in the base resource). Rather use getKey() on the base resource if possible somehow. - Key _key; - }; - - std::unique_ptr add(const Key &key, std::unique_ptr resource); - std::unique_ptr load(const Key &key); - void remove(const Key &key, std::unique_ptr block); - -private: - class OpenResource { - public: - OpenResource(std::unique_ptr resource): _resource(std::move(resource)), _refCount(0) {} - - Resource *getReference() { - ++_refCount; - return _resource.get(); - } - - void releaseReference() { - --_refCount; - } - - bool refCountIsZero() const { - return 0 == _refCount; - } - - std::unique_ptr moveResourceOut() { - return std::move(_resource); - } - private: - std::unique_ptr _resource; - uint32_t _refCount; - }; - - std::mutex _mutex; - std::unique_ptr> _baseStore; - - std::map _openResources; - std::map>> _resourcesToRemove; - - std::unique_ptr _add(const Key &key, std::unique_ptr resource); - std::unique_ptr _createCachedResourceRef(Resource *resource, const Key &key); - - void release(const Key &key); - friend class CachedResource; - - DISALLOW_COPY_AND_ASSIGN(CachingStore); -}; - -template -std::unique_ptr CachingStore::add(const Key &key, std::unique_ptr resource) { - std::lock_guard lock(_mutex); - return _add(key, std::move(resource)); -} - -template -std::unique_ptr CachingStore::_add(const Key &key, std::unique_ptr resource) { - auto insertResult = _openResources.emplace(key, std::move(resource)); - assert(true == insertResult.second); - return _createCachedResourceRef(insertResult.first->second.getReference(), key); -} - -template -std::unique_ptr CachingStore::_createCachedResourceRef(Resource *resource, const Key &key) { - auto resourceRef = std::make_unique(resource); - resourceRef->init(this, key); - return std::move(resourceRef); -} - -template -std::unique_ptr CachingStore::load(const Key &key) { - //TODO This lock doesn't allow loading different blocks in parallel. Can we do something with futures maybe? - std::lock_guard lock(_mutex); - auto found = _openResources.find(key); - if (found == _openResources.end()) { - auto resource = _baseStore->loadFromBaseStore(key); - if (resource.get() == nullptr) { - return nullptr; - } - return _add(key, std::move(resource)); - } else { - return _createCachedResourceRef(found->second.getReference(), key); - } -} - -template -void CachingStore::remove(const Key &key, std::unique_ptr resource) { - auto insertResult = _resourcesToRemove.emplace(key, std::promise>()); - assert(true == insertResult.second); - resource.reset(); - - //Wait for last resource user to release it - auto resourceToRemove = insertResult.first->second.get_future().get(); - _resourcesToRemove.erase(key); //TODO Is this erase causing a race condition? - - _baseStore->removeFromBaseStore(std::move(resourceToRemove)); -} - -template -void CachingStore::release(const Key &key) { - std::lock_guard lock(_mutex); - auto found = _openResources.find(key); - assert (found != _openResources.end()); - found->second.releaseReference(); - if (found->second.refCountIsZero()) { - auto foundToRemove = _resourcesToRemove.find(key); - if (foundToRemove != _resourcesToRemove.end()) { - foundToRemove->second.set_value(found->second.moveResourceOut()); - } - _openResources.erase(found); - } -} - -} - -#endif diff --git a/ParallelAccessBaseStore.h b/ParallelAccessBaseStore.h new file mode 100644 index 00000000..033441da --- /dev/null +++ b/ParallelAccessBaseStore.h @@ -0,0 +1,18 @@ +#ifndef MESSMER_PARALLELACCESSSTORE_PARALLELACCESSBASESTORE_H_ +#define MESSMER_PARALLELACCESSSTORE_PARALLELACCESSBASESTORE_H_ + +#include + +namespace parallelaccessstore { + +template +class ParallelAccessBaseStore { +public: + virtual ~ParallelAccessBaseStore() {} + virtual std::unique_ptr loadFromBaseStore(const Key &key) = 0; + virtual void removeFromBaseStore(std::unique_ptr block) = 0; +}; + +} + +#endif diff --git a/ParallelAccessStore.h b/ParallelAccessStore.h new file mode 100644 index 00000000..acad7c37 --- /dev/null +++ b/ParallelAccessStore.h @@ -0,0 +1,161 @@ +#ifndef MESSMER_PARALLELACCESSSTORE_IMPLEMENTATIONS_PARALLELACCESS_PARALLELACCESSSTORE_H_ +#define MESSMER_PARALLELACCESSSTORE_IMPLEMENTATIONS_PARALLELACCESS_PARALLELACCESSSTORE_H_ + +#include +#include +#include +#include +#include +#include +#include +#include "ParallelAccessBaseStore.h" + + +//TODO Refactor +//TODO Test cases + +namespace parallelaccessstore { + +template +class ParallelAccessStore { +public: + ParallelAccessStore(std::unique_ptr> baseStore); + + class ResourceRefBase { + public: + //TODO Better way to initialize + ResourceRefBase(): _cachingStore(nullptr), _key(Key::CreateRandom()) {} + void init(ParallelAccessStore *cachingStore, const Key &key) { + _cachingStore = cachingStore; + _key = key; + } + virtual ~ResourceRefBase() { + _cachingStore->release(_key); + } + private: + ParallelAccessStore *_cachingStore; + //TODO We're storing Key twice (here and in the base resource). Rather use getKey() on the base resource if possible somehow. + Key _key; + }; + + std::unique_ptr add(const Key &key, std::unique_ptr resource); + std::unique_ptr load(const Key &key); + void remove(const Key &key, std::unique_ptr block); + +private: + class OpenResource { + public: + OpenResource(std::unique_ptr resource): _resource(std::move(resource)), _refCount(0) {} + + Resource *getReference() { + ++_refCount; + return _resource.get(); + } + + void releaseReference() { + --_refCount; + } + + bool refCountIsZero() const { + return 0 == _refCount; + } + + std::unique_ptr moveResourceOut() { + return std::move(_resource); + } + private: + std::unique_ptr _resource; + uint32_t _refCount; + }; + + std::mutex _mutex; + std::unique_ptr> _baseStore; + + std::map _openResources; + std::map>> _resourcesToRemove; + + std::unique_ptr _add(const Key &key, std::unique_ptr resource); + std::unique_ptr _createResourceRef(Resource *resource, const Key &key); + + void release(const Key &key); + friend class CachedResource; + + DISALLOW_COPY_AND_ASSIGN(ParallelAccessStore); +}; + +template +ParallelAccessStore::ParallelAccessStore(std::unique_ptr> baseStore) + : _mutex(), + _baseStore(std::move(baseStore)), + _openResources(), + _resourcesToRemove() { + static_assert(std::is_base_of::value, "ResourceRef must inherit from ResourceRefBase"); +} + +template +std::unique_ptr ParallelAccessStore::add(const Key &key, std::unique_ptr resource) { + std::lock_guard lock(_mutex); + return _add(key, std::move(resource)); +} + +template +std::unique_ptr ParallelAccessStore::_add(const Key &key, std::unique_ptr resource) { + auto insertResult = _openResources.emplace(key, std::move(resource)); + assert(true == insertResult.second); + return _createResourceRef(insertResult.first->second.getReference(), key); +} + +template +std::unique_ptr ParallelAccessStore::_createResourceRef(Resource *resource, const Key &key) { + auto resourceRef = std::make_unique(resource); + resourceRef->init(this, key); + return std::move(resourceRef); +} + +template +std::unique_ptr ParallelAccessStore::load(const Key &key) { + //TODO This lock doesn't allow loading different blocks in parallel. Can we do something with futures maybe? + std::lock_guard lock(_mutex); + auto found = _openResources.find(key); + if (found == _openResources.end()) { + auto resource = _baseStore->loadFromBaseStore(key); + if (resource.get() == nullptr) { + return nullptr; + } + return _add(key, std::move(resource)); + } else { + return _createResourceRef(found->second.getReference(), key); + } +} + +template +void ParallelAccessStore::remove(const Key &key, std::unique_ptr resource) { + auto insertResult = _resourcesToRemove.emplace(key, std::promise>()); + assert(true == insertResult.second); + resource.reset(); + + //Wait for last resource user to release it + auto resourceToRemove = insertResult.first->second.get_future().get(); + _resourcesToRemove.erase(key); //TODO Is this erase causing a race condition? + + _baseStore->removeFromBaseStore(std::move(resourceToRemove)); +} + +template +void ParallelAccessStore::release(const Key &key) { + std::lock_guard lock(_mutex); + auto found = _openResources.find(key); + assert (found != _openResources.end()); + found->second.releaseReference(); + if (found->second.refCountIsZero()) { + auto foundToRemove = _resourcesToRemove.find(key); + if (foundToRemove != _resourcesToRemove.end()) { + foundToRemove->second.set_value(found->second.moveResourceOut()); + } + _openResources.erase(found); + } +} + +} + +#endif diff --git a/README.md b/README.md index 1d47f2ee..b1fa0047 100644 --- a/README.md +++ b/README.md @@ -1,2 +1,2 @@ -# cachingstore +# parallelaccessstore diff --git a/biicode.conf b/biicode.conf index b6d0173e..24ae36cd 100644 --- a/biicode.conf +++ b/biicode.conf @@ -6,7 +6,7 @@ messmer/cpp-utils: 2 [parent] - messmer/cachingstore: 0 + messmer/parallelaccessstore: 0 [paths] # Local directories to look for headers (within block) # / diff --git a/test/CachingBaseStoreTest.cpp b/test/CachingBaseStoreTest.cpp index 9e05e76b..edc0aece 100644 --- a/test/CachingBaseStoreTest.cpp +++ b/test/CachingBaseStoreTest.cpp @@ -1,3 +1,3 @@ -#include "../CachingBaseStore.h" +#include "../ParallelAccessBaseStore.h" // Test that CachingBaseStore.h can be included without errors