Make Cache a generic over Key/Value. Furthermore, add test skeleton for Cache

implementations/caching/Cache.cpp

@@ -1,56 +0,0 @@
-#include "Cache.h"
-#include "PeriodicTask.h"
-
-using std::unique_ptr;
-using std::make_unique;
-using std::mutex;
-using std::lock_guard;
-using std::pair;
-
-namespace blockstore {
-namespace caching {
-
-constexpr uint32_t Cache::MAX_ENTRIES;
-constexpr double Cache::PURGE_LIFETIME_SEC;
-constexpr double Cache::PURGE_INTERVAL;
-constexpr double Cache::MAX_LIFETIME_SEC;
-
-Cache::Cache(): _cachedBlocks(), _timeoutFlusher(nullptr) {
-  //Don't initialize timeoutFlusher in the initializer list,
-  //because it then might already call Cache::popOldEntries() before Cache is done constructing
-  _timeoutFlusher = make_unique<PeriodicTask>(std::bind(&Cache::_popOldEntries, this), PURGE_INTERVAL);
-}
-
-Cache::~Cache() {
-}
-
-unique_ptr<Block> Cache::pop(const Key &key) {
-  lock_guard<mutex> lock(_mutex);
-  auto found = _cachedBlocks.pop(key);
-  if (!found) {
-    return nullptr;
-  }
-  auto block = found->releaseBlock();
-  return block;
-}
-
-void Cache::push(unique_ptr<Block> block) {
-  lock_guard<mutex> lock(_mutex);
-  assert(_cachedBlocks.size() <= MAX_ENTRIES);
-  if (_cachedBlocks.size() == MAX_ENTRIES) {
-    _cachedBlocks.pop();
-    assert(_cachedBlocks.size() == MAX_ENTRIES-1);
-  }
-  Key key = block->key();
-  _cachedBlocks.push(key, CacheEntry(std::move(block)));
-}
-
-void Cache::_popOldEntries() {
-  lock_guard<mutex> lock(_mutex);
-  while(_cachedBlocks.size() > 0 && _cachedBlocks.peek()->ageSeconds() > PURGE_LIFETIME_SEC) {
-    _cachedBlocks.pop();
-  }
-}
-
-}
-}

implementations/caching/Cache.h

@@ -1,42 +0,0 @@
-#pragma once
-#ifndef MESSMER_BLOCKSTORE_IMPLEMENTATIONS_CACHING_CACHE_H_
-#define MESSMER_BLOCKSTORE_IMPLEMENTATIONS_CACHING_CACHE_H_
-
-#include "CacheEntry.h"
-#include "QueueMap.h"
-#include "../../interface/Block.h"
-#include <memory>
-#include <mutex>
-
-namespace blockstore {
-namespace caching {
-class PeriodicTask;
-
-//TODO Test
-
-class Cache {
-public:
-  static constexpr uint32_t MAX_ENTRIES = 1000;
-  //TODO Experiment with good values
-  static constexpr double PURGE_LIFETIME_SEC = 0.5; //When an entry has this age, it will be purged from the cache
-  static constexpr double PURGE_INTERVAL = 0.5; // With this interval, we check for entries to purge
-  static constexpr double MAX_LIFETIME_SEC = PURGE_LIFETIME_SEC + PURGE_INTERVAL; // This is the oldest age an entry can reach (given purging works in an ideal world, i.e. with the ideal interval and in zero time)
-
-  Cache();
-  virtual ~Cache();
-
-  void push(std::unique_ptr<Block> block);
-  std::unique_ptr<Block> pop(const Key &key);
-
-private:
-  void _popOldEntries();
-
-  mutable std::mutex _mutex;
-  QueueMap<Key, CacheEntry> _cachedBlocks;
-  std::unique_ptr<PeriodicTask> _timeoutFlusher;
-};
-
-}
-}
-
-#endif

implementations/caching/CacheEntry.cpp

@@ -1,2 +0,0 @@
-#include "CacheEntry.h"
-#include "../../interface/Block.h"

implementations/caching/CachingBlockStore.cpp

@@ -28,13 +28,16 @@ unique_ptr<Block> CachingBlockStore::tryCreate(const Key &key, Data data) {
 }
 
 unique_ptr<Block> CachingBlockStore::load(const Key &key) {
-  auto block = _cache.pop(key);
-  if (block.get() != nullptr) {
-    return make_unique<CachedBlock>(std::move(block), this);
-  }
-  block = _baseBlockStore->load(key);
-  if (block.get() == nullptr) {
-    return nullptr;
+  boost::optional<unique_ptr<Block>> optBlock = _cache.pop(key);
+  unique_ptr<Block> block;
+  //TODO an optional<> class with .getOrElse() would make this code simpler. boost::optional<>::value_or_eval didn't seem to work with unique_ptr members.
+  if (optBlock) {
+    block = std::move(*optBlock);
+  } else {
+    block = _baseBlockStore->load(key);
+    if (block.get() == nullptr) {
+      return nullptr;
+    }
   }
   return make_unique<CachedBlock>(std::move(block), this);
 }
@@ -57,7 +60,8 @@ uint64_t CachingBlockStore::numBlocks() const {
 }
 
 void CachingBlockStore::release(unique_ptr<Block> block) {
-  _cache.push(std::move(block));
+  Key key = block->key();
+  _cache.push(key, std::move(block));
 }
 
 std::unique_ptr<Block> CachingBlockStore::tryCreateInBaseStore(const Key &key, Data data) {

implementations/caching/CachingBlockStore.h

@@ -2,7 +2,7 @@
 #ifndef BLOCKSTORE_IMPLEMENTATIONS_CACHING_CACHINGBLOCKSTORE_H_
 #define BLOCKSTORE_IMPLEMENTATIONS_CACHING_CACHINGBLOCKSTORE_H_
 
-#include "Cache.h"
+#include "cache/Cache.h"
 #include "../../interface/BlockStore.h"
 
 namespace blockstore {
@@ -26,7 +26,7 @@ public:
 
 private:
   std::unique_ptr<BlockStore> _baseBlockStore;
-  Cache _cache;
+  Cache<Key, std::unique_ptr<Block>> _cache;
   uint32_t _numNewBlocks;
 
   DISALLOW_COPY_AND_ASSIGN(CachingBlockStore);

implementations/caching/cache/Cache.cpp

@@ -0,0 +1 @@
+#include "Cache.h"

implementations/caching/cache/Cache.h

@@ -0,0 +1,88 @@
+#pragma once
+#ifndef MESSMER_BLOCKSTORE_IMPLEMENTATIONS_CACHING_CACHE_H_
+#define MESSMER_BLOCKSTORE_IMPLEMENTATIONS_CACHING_CACHE_H_
+
+#include "CacheEntry.h"
+#include "QueueMap.h"
+#include "PeriodicTask.h"
+#include <memory>
+#include <mutex>
+#include <boost/optional.hpp>
+
+namespace blockstore {
+namespace caching {
+
+//TODO Test
+
+template<class Key, class Value>
+class Cache {
+public:
+  static constexpr uint32_t MAX_ENTRIES = 1000;
+  //TODO Experiment with good values
+  static constexpr double PURGE_LIFETIME_SEC = 0.5; //When an entry has this age, it will be purged from the cache
+  static constexpr double PURGE_INTERVAL = 0.5; // With this interval, we check for entries to purge
+  static constexpr double MAX_LIFETIME_SEC = PURGE_LIFETIME_SEC + PURGE_INTERVAL; // This is the oldest age an entry can reach (given purging works in an ideal world, i.e. with the ideal interval and in zero time)
+
+  Cache();
+  virtual ~Cache();
+
+  void push(const Key &key, Value value);
+  boost::optional<Value> pop(const Key &key);
+
+private:
+  void _popOldEntries();
+
+  mutable std::mutex _mutex;
+  QueueMap<Key, CacheEntry<Key, Value>> _cachedBlocks;
+  std::unique_ptr<PeriodicTask> _timeoutFlusher;
+};
+
+template<class Key, class Value> constexpr uint32_t Cache<Key, Value>::MAX_ENTRIES;
+template<class Key, class Value> constexpr double Cache<Key, Value>::PURGE_LIFETIME_SEC;
+template<class Key, class Value> constexpr double Cache<Key, Value>::PURGE_INTERVAL;
+template<class Key, class Value> constexpr double Cache<Key, Value>::MAX_LIFETIME_SEC;
+
+template<class Key, class Value>
+Cache<Key, Value>::Cache(): _cachedBlocks(), _timeoutFlusher(nullptr) {
+  //Don't initialize timeoutFlusher in the initializer list,
+  //because it then might already call Cache::popOldEntries() before Cache is done constructing
+  _timeoutFlusher = std::make_unique<PeriodicTask>(std::bind(&Cache::_popOldEntries, this), PURGE_INTERVAL);
+}
+
+template<class Key, class Value>
+Cache<Key, Value>::~Cache() {
+}
+
+template<class Key, class Value>
+boost::optional<Value> Cache<Key, Value>::pop(const Key &key) {
+  std::lock_guard<std::mutex> lock(_mutex);
+  auto found = _cachedBlocks.pop(key);
+  if (!found) {
+    return boost::none;
+  }
+  return found->releaseValue();
+}
+
+template<class Key, class Value>
+void Cache<Key, Value>::push(const Key &key, Value value) {
+  std::lock_guard<std::mutex> lock(_mutex);
+  assert(_cachedBlocks.size() <= MAX_ENTRIES);
+  if (_cachedBlocks.size() == MAX_ENTRIES) {
+    _cachedBlocks.pop();
+    assert(_cachedBlocks.size() == MAX_ENTRIES-1);
+  }
+  _cachedBlocks.push(key, CacheEntry<Key, Value>(std::move(value)));
+}
+
+template<class Key, class Value>
+void Cache<Key, Value>::_popOldEntries() {
+  std::lock_guard<std::mutex> lock(_mutex);
+  while(_cachedBlocks.size() > 0 && _cachedBlocks.peek()->ageSeconds() > PURGE_LIFETIME_SEC) {
+    _cachedBlocks.pop();
+  }
+}
+
+}
+}
+
+#endif

implementations/caching/cache/CacheEntry.cpp

@@ -0,0 +1 @@
+#include "CacheEntry.h"

implementations/caching/cache/CacheEntry.h

@@ -8,12 +8,12 @@
 #include <boost/date_time/posix_time/posix_time_types.hpp>
 
 namespace blockstore {
-class Block;
 namespace caching {
 
+template<class Key, class Value>
 class CacheEntry {
 public:
-  explicit CacheEntry(std::unique_ptr<Block> block): _lastAccess(currentTime()), _block(std::move(block)) {
+  explicit CacheEntry(Value value): _lastAccess(currentTime()), _value(std::move(value)) {
   }
 
   CacheEntry(CacheEntry &&) = default;
@@ -22,13 +22,13 @@ public:
     return ((double)(currentTime() - _lastAccess).total_nanoseconds()) / ((double)1000000000);
   }
 
-  std::unique_ptr<Block> releaseBlock() {
-    return std::move(_block);
+  Value releaseValue() {
+    return std::move(_value);
   }
 
 private:
   boost::posix_time::ptime _lastAccess;
-  std::unique_ptr<Block> _block;
+  Value _value;
 
   static boost::posix_time::ptime currentTime() {
 	return boost::posix_time::microsec_clock::local_time();

test/implementations/caching/cache/CacheTest.cpp

@@ -0,0 +1,12 @@
+#include <google/gtest/gtest.h>
+
+#include "../../../../implementations/caching/cache/Cache.h"
+
+using ::testing::Test;
+
+using namespace blockstore::caching;
+
+class CacheTest: public Test {
+public:
+  Cache<int, int> cache;
+};

test/implementations/caching/cache/PeriodicTaskTest.cpp

@@ -1,6 +1,6 @@
 #include <google/gtest/gtest.h>
 
-#include "../../../implementations/caching/PeriodicTask.h"
+#include "../../../../implementations/caching/cache/PeriodicTask.h"
 
 #include <mutex>
 #include <condition_variable>

test/implementations/caching/cache/QueueMapTest.cpp

@@ -1,6 +1,6 @@
 #include <google/gtest/gtest.h>
 #include <messmer/cpp-utils/macros.h>
-#include "../../../implementations/caching/QueueMap.h"
+#include "../../../../implementations/caching/cache/QueueMap.h"
 #include <memory>
 #include <boost/optional/optional_io.hpp>