Start implementing new cache

src/blockstore/CMakeLists.txt

@@ -18,6 +18,7 @@ set(SOURCES
   implementations/encrypted/EncryptedBlockStore2.cpp
   implementations/ondisk/OnDiskBlockStore2.cpp
   implementations/caching/CachingBlockStore.cpp
+  implementations/caching2/CachingBlockStore2.cpp
   implementations/caching/cache/PeriodicTask.cpp
   implementations/caching/cache/CacheEntry.cpp
   implementations/caching/cache/Cache.cpp

src/blockstore/implementations/caching2/CachingBlockStore2.cpp

@@ -0,0 +1,144 @@
+#include "CachingBlockStore2.h"
+#include <memory>
+#include <cpp-utils/assert/assert.h>
+#include <cpp-utils/system/get_total_memory.h>
+
+using std::make_unique;
+using std::string;
+using std::mutex;
+using std::lock_guard;
+using std::piecewise_construct;
+using std::make_tuple;
+using std::make_pair;
+using std::vector;
+using cpputils::Data;
+using cpputils::unique_ref;
+using cpputils::make_unique_ref;
+using boost::optional;
+using boost::none;
+using std::unique_lock;
+using std::mutex;
+
+namespace blockstore {
+namespace caching {
+
+CachingBlockStore2::CachedBlock::CachedBlock(const CachingBlockStore2* blockStore, const Key& key, cpputils::Data data, bool isDirty)
+    : _blockStore(blockStore), _key(key), _data(std::move(data)), _dirty(isDirty) {
+}
+
+CachingBlockStore2::CachedBlock::~CachedBlock() {
+  if (_dirty) {
+    _blockStore->_baseBlockStore->store(_key, _data);
+  }
+  // remove it from the list of blocks not in the base store, if it's on it
+  unique_lock<mutex> lock(_blockStore->_cachedBlocksNotInBaseStoreMutex);
+  _blockStore->_cachedBlocksNotInBaseStore.erase(_key);
+}
+
+const Data& CachingBlockStore2::CachedBlock::read() const {
+  return _data;
+}
+
+bool CachingBlockStore2::CachedBlock::remove() && {
+  _dirty = false; // Prevent writing it back into the base store
+  return _blockStore->_baseBlockStore->remove(_key);
+}
+
+void CachingBlockStore2::CachedBlock::write(Data data) {
+  _data = std::move(data);
+  _dirty = true;
+}
+
+CachingBlockStore2::CachingBlockStore2(cpputils::unique_ref<BlockStore2> baseBlockStore)
+: _baseBlockStore(std::move(baseBlockStore)), _cachedBlocksNotInBaseStoreMutex(), _cachedBlocksNotInBaseStore(), _cache() {
+}
+
+bool CachingBlockStore2::tryCreate(const Key &key, const Data &data) {
+  //TODO Check if block exists in base store
+  auto popped = _cache.pop(key);
+  if (popped != boost::none) {
+    // entry already exists in cache
+    _cache.push(key, std::move(*popped));
+    return false;
+  } else {
+    _cache.push(key, make_unique_ref<CachingBlockStore2::CachedBlock>(this, key, data.copy(), true));
+    unique_lock<mutex> lock(_cachedBlocksNotInBaseStoreMutex);
+    _cachedBlocksNotInBaseStore.insert(key);
+    return true;
+  }
+}
+
+bool CachingBlockStore2::remove(const Key &key) {
+  // TODO Don't write-through but cache remove operations
+  auto popped = _cache.pop(key);
+  if (popped != boost::none) {
+    return std::move(**popped).remove();
+  } else {
+    return _baseBlockStore->remove(key);
+  }
+}
+
+optional<unique_ref<CachingBlockStore2::CachedBlock>> CachingBlockStore2::_loadFromCacheOrBaseStore(const Key &key) const {
+  auto popped = _cache.pop(key);
+  if (popped != boost::none) {
+    return std::move(*popped);
+  } else {
+    auto loaded = _baseBlockStore->load(key);
+    if (loaded == boost::none) {
+      return boost::none;
+    }
+    return make_unique_ref<CachingBlockStore2::CachedBlock>(this, key, std::move(*loaded), false);
+  }
+}
+
+optional<Data> CachingBlockStore2::load(const Key &key) const {
+  auto loaded = _loadFromCacheOrBaseStore(key);
+  if (loaded == boost::none) {
+    // TODO Cache non-existence?
+    return boost::none;
+  }
+  optional<Data> result = (*loaded)->read().copy();
+  _cache.push(key, std::move(*loaded));
+  return result;
+}
+
+void CachingBlockStore2::store(const Key &key, const Data &data) {
+  auto popped = _cache.pop(key);
+  if (popped != boost::none) {
+    (*popped)->write(data.copy());
+  } else {
+    popped = make_unique_ref<CachingBlockStore2::CachedBlock>(this, key, data.copy(), true);
+    // TODO If block doesn't exist in base store, we have to add it to _cachedBlocksNotInBaseStore
+  }
+  _cache.push(key, std::move(*popped));
+}
+
+uint64_t CachingBlockStore2::numBlocks() const {
+  uint64_t numInCacheButNotInBaseStore = 0;
+  {
+    unique_lock<mutex> lock(_cachedBlocksNotInBaseStoreMutex);
+    numInCacheButNotInBaseStore = _cachedBlocksNotInBaseStore.size();
+  }
+  return _baseBlockStore->numBlocks() + numInCacheButNotInBaseStore;
+}
+
+uint64_t CachingBlockStore2::estimateNumFreeBytes() const {
+  return _baseBlockStore->estimateNumFreeBytes();
+}
+
+uint64_t CachingBlockStore2::blockSizeFromPhysicalBlockSize(uint64_t blockSize) const {
+  return _baseBlockStore->blockSizeFromPhysicalBlockSize(blockSize);
+}
+
+void CachingBlockStore2::forEachBlock(std::function<void (const Key &)> callback) const {
+  {
+    unique_lock<mutex> lock(_cachedBlocksNotInBaseStoreMutex);
+    for (const Key &key : _cachedBlocksNotInBaseStore) {
+      callback(key);
+    }
+  }
+  _baseBlockStore->forEachBlock(std::move(callback));
+}
+
+}
+}

src/blockstore/implementations/caching2/CachingBlockStore2.h

@@ -0,0 +1,61 @@
+#pragma once
+#ifndef MESSMER_BLOCKSTORE_IMPLEMENTATIONS_CACHING_CACHINGBLOCKSTORE2_H_
+#define MESSMER_BLOCKSTORE_IMPLEMENTATIONS_CACHING_CACHINGBLOCKSTORE2_H_
+
+#include "../../interface/BlockStore2.h"
+#include <cpp-utils/macros.h>
+#include "../caching/cache/Cache.h"
+#include <unordered_set>
+
+namespace blockstore {
+namespace caching {
+
+class CachingBlockStore2 final: public BlockStore2 {
+public:
+  CachingBlockStore2(cpputils::unique_ref<BlockStore2> baseBlockStore);
+
+  bool tryCreate(const Key &key, const cpputils::Data &data) override;
+  bool remove(const Key &key) override;
+  boost::optional<cpputils::Data> load(const Key &key) const override;
+  void store(const Key &key, const cpputils::Data &data) override;
+  uint64_t numBlocks() const override;
+  uint64_t estimateNumFreeBytes() const override;
+  uint64_t blockSizeFromPhysicalBlockSize(uint64_t blockSize) const override;
+  void forEachBlock(std::function<void (const Key &)> callback) const override;
+
+private:
+  // TODO Is a cache implementation with onEvict callback instead of destructor simpler?
+  class CachedBlock final {
+  public:
+    CachedBlock(const CachingBlockStore2* blockStore, const Key& key, cpputils::Data data, bool isDirty);
+    ~CachedBlock();
+
+    const cpputils::Data& read() const;
+    void write(cpputils::Data data);
+    bool remove() &&; // only on rvalue because the destructor should be called after calling remove(). It shouldn't be put back into the cache.
+  private:
+    const CachingBlockStore2* _blockStore;
+    Key _key;
+    cpputils::Data _data;
+    bool _dirty;
+
+    DISALLOW_COPY_AND_ASSIGN(CachedBlock);
+  };
+
+  boost::optional<cpputils::unique_ref<CachedBlock>> _loadFromCacheOrBaseStore(const Key &key) const;
+
+  cpputils::unique_ref<BlockStore2> _baseBlockStore;
+  friend class CachedBlock;
+
+  // TODO Store CachedBlock directly, without unique_ref
+  mutable std::mutex _cachedBlocksNotInBaseStoreMutex;
+  mutable std::unordered_set<Key> _cachedBlocksNotInBaseStore;
+  mutable Cache<Key, cpputils::unique_ref<CachedBlock>, 1000> _cache;
+
+  DISALLOW_COPY_AND_ASSIGN(CachingBlockStore2);
+};
+
+}
+}
+
+#endif

test/blockstore/CMakeLists.txt

@@ -19,6 +19,8 @@ set(SOURCES
     implementations/ondisk/OnDiskBlockTest/OnDiskBlockCreateTest.cpp
     implementations/ondisk/OnDiskBlockTest/OnDiskBlockFlushTest.cpp
     implementations/ondisk/OnDiskBlockTest/OnDiskBlockLoadTest.cpp
+    implementations/caching2/CachingBlockStore2Test_Generic.cpp
+        implementations/caching2/CachingBlockStore2Test_Specific.cpp
     implementations/caching/CachingBlockStoreTest_Generic.cpp
     implementations/caching/CachingBlockStoreTest_Specific.cpp
     implementations/caching/cache/QueueMapTest_Values.cpp

test/blockstore/implementations/caching2/CachingBlockStore2Test_Generic.cpp

@@ -0,0 +1,40 @@
+#include <blockstore/implementations/low2highlevel/LowToHighLevelBlockStore.h>
+#include "blockstore/implementations/caching2/CachingBlockStore2.h"
+#include "blockstore/implementations/inmemory/InMemoryBlockStore2.h"
+#include "../../testutils/BlockStoreTest.h"
+#include "../../testutils/BlockStore2Test.h"
+#include <gtest/gtest.h>
+
+using ::testing::Test;
+
+using blockstore::BlockStore;
+using blockstore::BlockStore2;
+using blockstore::caching::CachingBlockStore2;
+using blockstore::lowtohighlevel::LowToHighLevelBlockStore;
+using blockstore::inmemory::InMemoryBlockStore2;
+
+using cpputils::Data;
+using cpputils::DataFixture;
+using cpputils::make_unique_ref;
+using cpputils::unique_ref;
+
+class CachingBlockStoreTestFixture: public BlockStoreTestFixture {
+public:
+  unique_ref<BlockStore> createBlockStore() override {
+    return make_unique_ref<LowToHighLevelBlockStore>(
+        make_unique_ref<CachingBlockStore2>(make_unique_ref<InMemoryBlockStore2>())
+    );
+  }
+};
+
+INSTANTIATE_TYPED_TEST_CASE_P(Caching2, BlockStoreTest, CachingBlockStoreTestFixture);
+
+
+class CachingBlockStore2TestFixture: public BlockStore2TestFixture {
+public:
+  unique_ref<BlockStore2> createBlockStore() override {
+    return make_unique_ref<CachingBlockStore2>(make_unique_ref<InMemoryBlockStore2>());
+  }
+};
+
+INSTANTIATE_TYPED_TEST_CASE_P(Caching, BlockStore2Test, CachingBlockStore2TestFixture);

test/blockstore/implementations/caching2/CachingBlockStore2Test_Specific.cpp

@@ -0,0 +1,58 @@
+#include <gtest/gtest.h>
+#include "blockstore/implementations/caching/CachingBlockStore.h"
+#include "blockstore/implementations/testfake/FakeBlockStore.h"
+
+using ::testing::Test;
+
+using cpputils::Data;
+using cpputils::unique_ref;
+using cpputils::make_unique_ref;
+
+using blockstore::testfake::FakeBlockStore;
+
+using namespace blockstore::caching;
+
+class CachingBlockStore2Test: public Test {
+public:
+  CachingBlockStore2Test():
+      baseBlockStore(new FakeBlockStore),
+      blockStore(std::move(cpputils::nullcheck(std::unique_ptr<FakeBlockStore>(baseBlockStore)).value()))  {
+  }
+  FakeBlockStore *baseBlockStore;
+  CachingBlockStore blockStore;
+
+  blockstore::Key CreateBlockReturnKey(const Data &initData) {
+    auto block = blockStore.create(initData);
+    block->flush();
+    return block->key();
+  }
+};
+
+TEST_F(CachingBlockStore2Test, PhysicalBlockSize_zerophysical) {
+  EXPECT_EQ(0u, blockStore.blockSizeFromPhysicalBlockSize(0));
+}
+
+TEST_F(CachingBlockStore2Test, PhysicalBlockSize_zerovirtual) {
+  auto key = CreateBlockReturnKey(Data(0));
+  auto base = baseBlockStore->load(key).value();
+  EXPECT_EQ(0u, blockStore.blockSizeFromPhysicalBlockSize(base->size()));
+}
+
+TEST_F(CachingBlockStore2Test, PhysicalBlockSize_negativeboundaries) {
+  // This tests that a potential if/else in blockSizeFromPhysicalBlockSize that catches negative values has the
+  // correct boundary set. We test the highest value that is negative and the smallest value that is positive.
+  auto physicalSizeForVirtualSizeZero = baseBlockStore->load(CreateBlockReturnKey(Data(0))).value()->size();
+  if (physicalSizeForVirtualSizeZero > 0) {
+    EXPECT_EQ(0u, blockStore.blockSizeFromPhysicalBlockSize(physicalSizeForVirtualSizeZero - 1));
+  }
+  EXPECT_EQ(0u, blockStore.blockSizeFromPhysicalBlockSize(physicalSizeForVirtualSizeZero));
+  EXPECT_EQ(1u, blockStore.blockSizeFromPhysicalBlockSize(physicalSizeForVirtualSizeZero + 1));
+}
+
+TEST_F(CachingBlockStore2Test, PhysicalBlockSize_positive) {
+  auto key = CreateBlockReturnKey(Data(10*1024));
+  auto base = baseBlockStore->load(key).value();
+  EXPECT_EQ(10*1024u, blockStore.blockSizeFromPhysicalBlockSize(base->size()));
+}
+
+// TODO Add test cases that flushing the block store doesn't destroy things (i.e. all test cases from BlockStoreTest, but with flushes inbetween)