From 4add7f3d80aea8b2e84077c64d3b49f18648421c Mon Sep 17 00:00:00 2001 From: Sebastian Messmer Date: Mon, 21 Aug 2017 22:44:35 +0100 Subject: [PATCH] Start implementing new cache --- src/blockstore/CMakeLists.txt | 1 + .../caching2/CachingBlockStore2.cpp | 144 ++++++++++++++++++ .../caching2/CachingBlockStore2.h | 61 ++++++++ test/blockstore/CMakeLists.txt | 2 + .../CachingBlockStore2Test_Generic.cpp | 40 +++++ .../CachingBlockStore2Test_Specific.cpp | 58 +++++++ 6 files changed, 306 insertions(+) create mode 100644 src/blockstore/implementations/caching2/CachingBlockStore2.cpp create mode 100644 src/blockstore/implementations/caching2/CachingBlockStore2.h create mode 100644 test/blockstore/implementations/caching2/CachingBlockStore2Test_Generic.cpp create mode 100644 test/blockstore/implementations/caching2/CachingBlockStore2Test_Specific.cpp diff --git a/src/blockstore/CMakeLists.txt b/src/blockstore/CMakeLists.txt index ada4f8e8..bc1b0744 100644 --- a/src/blockstore/CMakeLists.txt +++ b/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 diff --git a/src/blockstore/implementations/caching2/CachingBlockStore2.cpp b/src/blockstore/implementations/caching2/CachingBlockStore2.cpp new file mode 100644 index 00000000..562eef79 --- /dev/null +++ b/src/blockstore/implementations/caching2/CachingBlockStore2.cpp @@ -0,0 +1,144 @@ +#include "CachingBlockStore2.h" +#include +#include +#include + +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 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 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(this, key, data.copy(), true)); + unique_lock 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> 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(this, key, std::move(*loaded), false); + } +} + +optional CachingBlockStore2::load(const Key &key) const { + auto loaded = _loadFromCacheOrBaseStore(key); + if (loaded == boost::none) { + // TODO Cache non-existence? + return boost::none; + } + optional 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(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 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 callback) const { + { + unique_lock lock(_cachedBlocksNotInBaseStoreMutex); + for (const Key &key : _cachedBlocksNotInBaseStore) { + callback(key); + } + } + _baseBlockStore->forEachBlock(std::move(callback)); +} + +} +} diff --git a/src/blockstore/implementations/caching2/CachingBlockStore2.h b/src/blockstore/implementations/caching2/CachingBlockStore2.h new file mode 100644 index 00000000..1c688f13 --- /dev/null +++ b/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 +#include "../caching/cache/Cache.h" +#include + +namespace blockstore { +namespace caching { + +class CachingBlockStore2 final: public BlockStore2 { +public: + CachingBlockStore2(cpputils::unique_ref baseBlockStore); + + bool tryCreate(const Key &key, const cpputils::Data &data) override; + bool remove(const Key &key) override; + boost::optional 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 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> _loadFromCacheOrBaseStore(const Key &key) const; + + cpputils::unique_ref _baseBlockStore; + friend class CachedBlock; + + // TODO Store CachedBlock directly, without unique_ref + mutable std::mutex _cachedBlocksNotInBaseStoreMutex; + mutable std::unordered_set _cachedBlocksNotInBaseStore; + mutable Cache, 1000> _cache; + + DISALLOW_COPY_AND_ASSIGN(CachingBlockStore2); +}; + +} +} + +#endif diff --git a/test/blockstore/CMakeLists.txt b/test/blockstore/CMakeLists.txt index bba29d1f..d4c3b762 100644 --- a/test/blockstore/CMakeLists.txt +++ b/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 diff --git a/test/blockstore/implementations/caching2/CachingBlockStore2Test_Generic.cpp b/test/blockstore/implementations/caching2/CachingBlockStore2Test_Generic.cpp new file mode 100644 index 00000000..3c8a815a --- /dev/null +++ b/test/blockstore/implementations/caching2/CachingBlockStore2Test_Generic.cpp @@ -0,0 +1,40 @@ +#include +#include "blockstore/implementations/caching2/CachingBlockStore2.h" +#include "blockstore/implementations/inmemory/InMemoryBlockStore2.h" +#include "../../testutils/BlockStoreTest.h" +#include "../../testutils/BlockStore2Test.h" +#include + +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 createBlockStore() override { + return make_unique_ref( + make_unique_ref(make_unique_ref()) + ); + } +}; + +INSTANTIATE_TYPED_TEST_CASE_P(Caching2, BlockStoreTest, CachingBlockStoreTestFixture); + + +class CachingBlockStore2TestFixture: public BlockStore2TestFixture { +public: + unique_ref createBlockStore() override { + return make_unique_ref(make_unique_ref()); + } +}; + +INSTANTIATE_TYPED_TEST_CASE_P(Caching, BlockStore2Test, CachingBlockStore2TestFixture); diff --git a/test/blockstore/implementations/caching2/CachingBlockStore2Test_Specific.cpp b/test/blockstore/implementations/caching2/CachingBlockStore2Test_Specific.cpp new file mode 100644 index 00000000..c9478a35 --- /dev/null +++ b/test/blockstore/implementations/caching2/CachingBlockStore2Test_Specific.cpp @@ -0,0 +1,58 @@ +#include +#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(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)