First version of factoring out CachingStore

implementations/caching/CachedBlockRef.cpp

@@ -8,17 +8,13 @@ using std::function;
 namespace blockstore {
 namespace caching {
 
-CachedBlockRef::CachedBlockRef(Block *baseBlock, CachingBlockStore *blockStore)
+CachedBlockRef::CachedBlockRef(Block *baseBlock)
-  //TODO We store key twice here - once in OpenBlock, once in the underlying baseBlock.
-  //     Should we move that to make CachedBlockRef::key() call _baseBlock.key()?
   :Block(baseBlock->key()),
-   _baseBlock(baseBlock),
+   _baseBlock(baseBlock) {
-   _blockStore(blockStore) {
 }
 
 CachedBlockRef::~CachedBlockRef() {
   _baseBlock->flush();
-  _blockStore->release(_baseBlock);
 }
 
 const void *CachedBlockRef::data() const {

implementations/caching/CachedBlockRef.h

@@ -1,8 +1,9 @@
 #pragma once
-#ifndef BLOCKSTORE_IMPLEMENTATIONS_SYNCHRONIZED_CACHEDBLOCKREF_H_
+#ifndef BLOCKSTORE_IMPLEMENTATIONS_CACHING_CACHEDBLOCKREF_H_
-#define BLOCKSTORE_IMPLEMENTATIONS_SYNCHRONIZED_CACHEDBLOCKREF_H_
+#define BLOCKSTORE_IMPLEMENTATIONS_CACHING_CACHEDBLOCKREF_H_
 
 #include "../../interface/Block.h"
+#include "CachingStore.h"
 
 #include "messmer/cpp-utils/macros.h"
 #include <memory>
@@ -11,9 +12,9 @@ namespace blockstore {
 namespace caching {
 class CachingBlockStore;
 
-class CachedBlockRef: public Block {
+class CachedBlockRef: public Block, public CachingStore<Block, CachedBlockRef, Key>::CachedResource {
 public:
-  CachedBlockRef(Block *baseBlock, CachingBlockStore *blockStore);
+  CachedBlockRef(Block *baseBlock);
   virtual ~CachedBlockRef();
 
   const void *data() const override;
@@ -25,7 +26,6 @@ public:
 
 private:
   Block *_baseBlock;
-  CachingBlockStore *_blockStore;
 
   DISALLOW_COPY_AND_ASSIGN(CachedBlockRef);
 };

implementations/caching/CachingBlockStore.cpp

@@ -13,60 +13,33 @@ namespace blockstore {
 namespace caching {
 
 CachingBlockStore::CachingBlockStore(unique_ptr<BlockStore> baseBlockStore)
- : _baseBlockStore(std::move(baseBlockStore)),
+ : _baseBlockStore(std::move(baseBlockStore)) {
-   _openBlocks() {
 }
 
 unique_ptr<Block> CachingBlockStore::create(size_t size) {
   auto block = _baseBlockStore->create(size);
-  lock_guard<mutex> lock(_mutex);
+  return CachingStore::add(std::move(block));
-  return _addOpenBlock(std::move(block));
-}
-
-unique_ptr<Block> CachingBlockStore::_addOpenBlock(unique_ptr<Block> block) {
-  auto insertResult = _openBlocks.emplace(block->key(), std::move(block));
-  assert(true == insertResult.second);
-  return make_unique<CachedBlockRef>(insertResult.first->second.getReference(), this);
 }
 
 unique_ptr<Block> CachingBlockStore::load(const Key &key) {
-  lock_guard<mutex> lock(_mutex);
+  return CachingStore::load(key);
-  auto found = _openBlocks.find(key);
-  if (found == _openBlocks.end()) {
-	auto block = _baseBlockStore->load(key);
-	if (block.get() == nullptr) {
-	  return nullptr;
-	}
-	return _addOpenBlock(std::move(block));
-  } else {
-	return make_unique<CachedBlockRef>(found->second.getReference(), this);
-  }
 }
 
-void CachingBlockStore::release(const Block *block) {
-  lock_guard<mutex> lock(_mutex);
-  Key key = block->key();
-  auto found = _openBlocks.find(key);
-  assert (found != _openBlocks.end());
-  found->second.releaseReference();
-  if (found->second.refCount == 0) {
-	auto foundToRemove = _blocksToRemove.find(key);
-	if (foundToRemove != _blocksToRemove.end()) {
-	  foundToRemove->second.set_value(std::move(found->second.block));
-	}
-	_openBlocks.erase(found);
-  }
-}
 
 void CachingBlockStore::remove(unique_ptr<Block> block) {
-  auto insertResult = _blocksToRemove.emplace(block->key(), promise<unique_ptr<Block>>());
+  return CachingStore::remove(std::move(block));
-  assert(true == insertResult.second);
+}
-  block.reset();
 
-  //Wait for last block user to release it
+const Key &CachingBlockStore::getKey(const Block &block) const {
-  auto blockToRemove = insertResult.first->second.get_future().get();
+  return block.key();
+}
 
-  _baseBlockStore->remove(std::move(blockToRemove));
+unique_ptr<Block> CachingBlockStore::loadFromBaseStore(const Key &key) {
+  return _baseBlockStore->load(key);
+}
+
+void CachingBlockStore::removeFromBaseStore(unique_ptr<Block> block) {
+  return _baseBlockStore->remove(std::move(block));
 }
 
 uint64_t CachingBlockStore::numBlocks() const {

implementations/caching/CachingBlockStore.h

@@ -1,19 +1,16 @@
 #pragma once
-#ifndef BLOCKSTORE_IMPLEMENTATIONS_SYNCHRONIZED_SYNCHRONIZEDBLOCKSTORE_H_
+#ifndef BLOCKSTORE_IMPLEMENTATIONS_CACHING_CACHINGBLOCKSTORE_H_
-#define BLOCKSTORE_IMPLEMENTATIONS_SYNCHRONIZED_SYNCHRONIZEDBLOCKSTORE_H_
+#define BLOCKSTORE_IMPLEMENTATIONS_CACHIN_CACHINGBLOCKSTORE_H_
 
-#include "messmer/cpp-utils/macros.h"
+#include "CachingStore.h"
-#include <memory>
-#include <mutex>
-#include <map>
-#include <future>
 
 #include "../../interface/BlockStore.h"
+#include "CachedBlockRef.h"
 
 namespace blockstore {
 namespace caching {
 
-class CachingBlockStore: public BlockStore {
+class CachingBlockStore: public BlockStore, private CachingStore<Block, CachedBlockRef, Key> {
 public:
   CachingBlockStore(std::unique_ptr<BlockStore> baseBlockStore);
 
@@ -22,27 +19,13 @@ public:
   void remove(std::unique_ptr<Block> block) override;
   uint64_t numBlocks() const override;
 
-  void release(const Block *block);
+protected:
+  const Key &getKey(const Block &block) const override;
+  std::unique_ptr<Block> loadFromBaseStore(const Key &key) override;
+  void removeFromBaseStore(std::unique_ptr<Block> block) override;
 
 private:
-  struct OpenBlock {
-	OpenBlock(std::unique_ptr<Block> block_): block(std::move(block_)), refCount(0) {}
-	Block *getReference() {
-	  ++refCount;
-	  return block.get();
-	}
-	void releaseReference() {
-	  --refCount;
-	}
-    std::unique_ptr<Block> block;
-    uint32_t refCount;
-  };
   std::unique_ptr<BlockStore> _baseBlockStore;
-  std::map<Key, OpenBlock> _openBlocks;
-  std::mutex _mutex;
-  std::map<Key, std::promise<std::unique_ptr<Block>>> _blocksToRemove;
-
-  std::unique_ptr<Block> _addOpenBlock(std::unique_ptr<Block> block);
 
   DISALLOW_COPY_AND_ASSIGN(CachingBlockStore);
 };

implementations/caching/CachingStore.h

@@ -0,0 +1,146 @@
+#ifndef MESSMER_BLOCKSTORE_IMPLEMENTATIONS_CACHING_CACHINGSTORE_H_
+#define MESSMER_BLOCKSTORE_IMPLEMENTATIONS_CACHING_CACHINGSTORE_H_
+
+#include <mutex>
+#include <memory>
+#include <map>
+#include <future>
+#include <cassert>
+#include <messmer/cpp-utils/macros.h>
+
+template<class Resource, class CachedResourceRef, class Key>
+class CachingStore {
+public:
+  CachingStore() {} //TODO Init member variables
+
+  //TODO Enforce CachedResourceRef inherits from CachedResource
+  class CachedResource {
+  public:
+	//TODO Better way to initialize
+	CachedResource(): _cachingStore(nullptr), _key(Key::CreateRandomKey()) {}
+	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<Resource> add(std::unique_ptr<Resource> resource);
+  std::unique_ptr<Resource> load(const Key &key);
+  void remove(std::unique_ptr<Resource> block);
+
+protected:
+  //TODO Template instead of virtual for getKey?
+  virtual const Key &getKey(const Resource &resource) const = 0;
+  virtual std::unique_ptr<Resource> loadFromBaseStore(const Key &key) = 0;
+  virtual void removeFromBaseStore(std::unique_ptr<Resource> resource) = 0;
+
+private:
+  class OpenResource {
+  public:
+	OpenResource(std::unique_ptr<Resource> 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<Resource> moveResourceOut() {
+	  return std::move(_resource);
+	}
+  private:
+	std::unique_ptr<Resource> _resource;
+	uint32_t _refCount;
+  };
+
+  std::mutex _mutex;
+  std::map<Key, OpenResource> _openResources;
+  std::map<Key, std::promise<std::unique_ptr<Resource>>> _resourcesToRemove;
+
+  std::unique_ptr<Resource> _add(std::unique_ptr<Resource> resource);
+  std::unique_ptr<Resource> _createCachedResourceRef(Resource *resource, const Key &key);
+
+  void release(const Key &key);
+  friend class CachedResource;
+
+  DISALLOW_COPY_AND_ASSIGN(CachingStore);
+};
+
+template<class Resource, class CachedResourceRef, class Key>
+std::unique_ptr<Resource> CachingStore<Resource, CachedResourceRef, Key>::add(std::unique_ptr<Resource> resource) {
+  std::lock_guard<std::mutex> lock(_mutex);
+  return _add(std::move(resource));
+}
+
+template<class Resource, class CachedResourceRef, class Key>
+std::unique_ptr<Resource> CachingStore<Resource, CachedResourceRef, Key>::_add(std::unique_ptr<Resource> resource) {
+  auto insertResult = _openResources.emplace(getKey(*resource), std::move(resource));
+  assert(true == insertResult.second);
+  return _createCachedResourceRef(insertResult.first->second.getReference(), getKey(*resource));
+}
+
+template<class Resource, class CachedResourceRef, class Key>
+std::unique_ptr<Resource> CachingStore<Resource, CachedResourceRef, Key>::_createCachedResourceRef(Resource *resource, const Key &key) {
+  auto resourceRef = std::make_unique<CachedResourceRef>(resource);
+  resourceRef->init(this, getKey(*resource));
+  return std::move(resourceRef);
+}
+
+template<class Resource, class CachedResourceRef, class Key>
+std::unique_ptr<Resource> CachingStore<Resource, CachedResourceRef, Key>::load(const Key &key) {
+  std::lock_guard<std::mutex> lock(_mutex);
+  auto found = _openResources.find(key);
+  if (found == _openResources.end()) {
+	auto resource = loadFromBaseStore(key);
+	if (resource.get() == nullptr) {
+	  return nullptr;
+	}
+	return _add(std::move(resource));
+  } else {
+	return _createCachedResourceRef(found->second.getReference(), key);
+  }
+}
+
+template<class Resource, class CachedResourceRef, class Key>
+void CachingStore<Resource, CachedResourceRef, Key>::remove(std::unique_ptr<Resource> resource) {
+  auto insertResult = _resourcesToRemove.emplace(getKey(*resource), std::promise<std::unique_ptr<Resource>>());
+  assert(true == insertResult.second);
+  resource.reset();
+
+  //Wait for last resource user to release it
+  auto resourceToRemove = insertResult.first->second.get_future().get();
+
+  removeFromBaseStore(std::move(resourceToRemove));
+}
+
+template<class Resource, class CachedResourceRef, class Key>
+void CachingStore<Resource, CachedResourceRef, Key>::release(const Key &key) {
+  std::lock_guard<std::mutex> 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