From 98a2fde6b17069341dd74c68b2be60b06f56b9c2 Mon Sep 17 00:00:00 2001 From: igchor Date: Wed, 4 May 2022 06:29:54 -0400 Subject: [PATCH 1/2] Extend cachbench with touch value The main purpose of this patch is to better simulate workloads in cachebench. Setting touchValue to true allows to see performance impact of using different mediums for memory cache. --- cachelib/cachebench/cache/Cache-inl.h | 36 ++++++++-------------- cachelib/cachebench/cache/Cache.h | 19 ++++++------ cachelib/cachebench/runner/CacheStressor.h | 6 ++-- cachelib/cachebench/util/Config.cpp | 2 +- cachelib/cachebench/util/Config.h | 4 +++ 5 files changed, 28 insertions(+), 39 deletions(-) diff --git a/cachelib/cachebench/cache/Cache-inl.h b/cachelib/cachebench/cache/Cache-inl.h index 34f65e1b15..a4526fbee2 100644 --- a/cachelib/cachebench/cache/Cache-inl.h +++ b/cachelib/cachebench/cache/Cache-inl.h @@ -50,8 +50,10 @@ uint64_t Cache::fetchNandWrites() const { template Cache::Cache(const CacheConfig& config, ChainedItemMovingSync movingSync, - std::string cacheDir) + std::string cacheDir, + bool touchValue) : config_(config), + touchValue_(touchValue), nandBytesBegin_{fetchNandWrites()}, itemRecords_(config_.enableItemDestructorCheck) { constexpr size_t MB = 1024ULL * 1024ULL; @@ -325,7 +327,6 @@ template void Cache::enableConsistencyCheck( const std::vector& keys) { XDCHECK(valueTracker_ == nullptr); - XDCHECK(!valueValidatingEnabled()); valueTracker_ = std::make_unique(ValueTracker::wrapStrings(keys)); for (const std::string& key : keys) { @@ -333,14 +334,6 @@ void Cache::enableConsistencyCheck( } } -template -void Cache::enableValueValidating( - const std::string &expectedValue) { - XDCHECK(!valueValidatingEnabled()); - XDCHECK(!consistencyCheckEnabled()); - this->expectedValue_ = expectedValue; -} - template typename Cache::RemoveRes Cache::remove(Key key) { if (!consistencyCheckEnabled()) { @@ -434,17 +427,15 @@ typename Cache::ItemHandle Cache::insertOrReplace( } template -void Cache::validateValue(const ItemHandle &it) const { - XDCHECK(valueValidatingEnabled()); - - const auto &expected = expectedValue_.value(); +void Cache::touchValue(const ItemHandle& it) const { + XDCHECK(touchValueEnabled()); auto ptr = reinterpret_cast(getMemory(it)); - auto cmp = std::memcmp(ptr, expected.data(), std::min(expected.size(), - getSize(it))); - if (cmp != 0) { - throw std::runtime_error("Value does not match!"); - } + + /* The accumulate call is intended to access all bytes of the value + * and nothing more. */ + auto sum = std::accumulate(ptr, ptr + getSize(it), 0ULL); + folly::doNotOptimizeAway(sum); } template @@ -459,9 +450,8 @@ typename Cache::ItemHandle Cache::find(Key key, auto it = cache_->find(key, mode); it.wait(); - if (valueValidatingEnabled()) { - XDCHECK(!consistencyCheckEnabled()); - validateValue(it); + if (touchValueEnabled()) { + touchValue(it); } return it; @@ -472,8 +462,6 @@ typename Cache::ItemHandle Cache::find(Key key, return it; } - XDCHECK(!valueValidatingEnabled()); - auto opId = valueTracker_->beginGet(key); auto it = findFn(); if (checkGet(opId, it)) { diff --git a/cachelib/cachebench/cache/Cache.h b/cachelib/cachebench/cache/Cache.h index c822c1bb89..344025f2b0 100644 --- a/cachelib/cachebench/cache/Cache.h +++ b/cachelib/cachebench/cache/Cache.h @@ -64,9 +64,11 @@ class Cache { // cache. // @param cacheDir optional directory for the cache to enable // persistence across restarts. + // @param touchValue read entire value on find explicit Cache(const CacheConfig& config, ChainedItemMovingSync movingSync = {}, - std::string cacheDir = ""); + std::string cacheDir = "", + bool touchValue = false); ~Cache(); @@ -168,8 +170,8 @@ class Cache { return getSize(item.get()); } - // checks if values stored in it matches expectedValue_. - void validateValue(const ItemHandle &it) const; + // read entire value on find. + void touchValue(const ItemHandle& it) const; // returns the size of the item, taking into account ItemRecords could be // enabled. @@ -228,14 +230,11 @@ class Cache { // @param keys list of keys that the stressor uses for the workload. void enableConsistencyCheck(const std::vector& keys); - // enables validating all values on find. Each value is compared to - // expected Value. - void enableValueValidating(const std::string &expectedValue); - // returns true if the consistency checking is enabled. bool consistencyCheckEnabled() const { return valueTracker_ != nullptr; } - bool valueValidatingEnabled() const { return expectedValue_.has_value(); } + // returns true if touching value is enabled. + bool touchValueEnabled() const { return touchValue_; } // return true if the key was previously detected to be inconsistent. This // is useful only when consistency checking is enabled by calling @@ -359,8 +358,8 @@ class Cache { // tracker for consistency monitoring. std::unique_ptr valueTracker_; - // exceptected value of all items in Cache. - std::optional expectedValue_; + // read entire value on find. + bool touchValue_{false}; // reading of the nand bytes written for the benchmark if enabled. const uint64_t nandBytesBegin_{0}; diff --git a/cachelib/cachebench/runner/CacheStressor.h b/cachelib/cachebench/runner/CacheStressor.h index be4a807900..74e5a0e8cd 100644 --- a/cachelib/cachebench/runner/CacheStressor.h +++ b/cachelib/cachebench/runner/CacheStressor.h @@ -93,7 +93,8 @@ class CacheStressor : public Stressor { cacheConfig.ticker = ticker_; } - cache_ = std::make_unique(cacheConfig, movingSync); + cache_ = std::make_unique(cacheConfig, movingSync, "", + config_.touchValue); if (config_.opPoolDistribution.size() > cache_->numPools()) { throw std::invalid_argument(folly::sformat( "more pools specified in the test than in the cache. " @@ -110,9 +111,6 @@ class CacheStressor : public Stressor { if (config_.checkConsistency) { cache_->enableConsistencyCheck(wg_->getAllKeys()); } - if (config_.validateValue) { - cache_->enableValueValidating(hardcodedString_); - } if (config_.opRatePerSec > 0) { rateLimiter_ = std::make_unique>( config_.opRatePerSec, config_.opRatePerSec); diff --git a/cachelib/cachebench/util/Config.cpp b/cachelib/cachebench/util/Config.cpp index 2166fe5e47..9dc6da1d1c 100644 --- a/cachelib/cachebench/util/Config.cpp +++ b/cachelib/cachebench/util/Config.cpp @@ -34,7 +34,7 @@ StressorConfig::StressorConfig(const folly::dynamic& configJson) { JSONSetVal(configJson, samplingIntervalMs); JSONSetVal(configJson, checkConsistency); - JSONSetVal(configJson, validateValue); + JSONSetVal(configJson, touchValue); JSONSetVal(configJson, numOps); JSONSetVal(configJson, numThreads); diff --git a/cachelib/cachebench/util/Config.h b/cachelib/cachebench/util/Config.h index 1a35c61b67..d1939aca59 100644 --- a/cachelib/cachebench/util/Config.h +++ b/cachelib/cachebench/util/Config.h @@ -195,6 +195,10 @@ struct StressorConfig : public JSONConfig { // Mutually exclusive with checkConsistency bool validateValue{false}; + // If enabled, each value will be read on find. This is useful for measuring + // performance of value access. + bool touchValue{false}; + uint64_t numOps{0}; // operation per thread uint64_t numThreads{0}; // number of threads that will run uint64_t numKeys{0}; // number of keys that will be used From 21c2e3119de5f1591c49e2d2f5fd1f9154a63582 Mon Sep 17 00:00:00 2001 From: Igor Chorazewicz Date: Wed, 15 Jun 2022 06:04:13 -0400 Subject: [PATCH 2/2] Enable touchValue by default --- cachelib/cachebench/cache/Cache.h | 4 ++-- cachelib/cachebench/util/Config.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/cachelib/cachebench/cache/Cache.h b/cachelib/cachebench/cache/Cache.h index 344025f2b0..96f52a9dcd 100644 --- a/cachelib/cachebench/cache/Cache.h +++ b/cachelib/cachebench/cache/Cache.h @@ -68,7 +68,7 @@ class Cache { explicit Cache(const CacheConfig& config, ChainedItemMovingSync movingSync = {}, std::string cacheDir = "", - bool touchValue = false); + bool touchValue = true); ~Cache(); @@ -359,7 +359,7 @@ class Cache { std::unique_ptr valueTracker_; // read entire value on find. - bool touchValue_{false}; + bool touchValue_{true}; // reading of the nand bytes written for the benchmark if enabled. const uint64_t nandBytesBegin_{0}; diff --git a/cachelib/cachebench/util/Config.h b/cachelib/cachebench/util/Config.h index d1939aca59..d7156416a3 100644 --- a/cachelib/cachebench/util/Config.h +++ b/cachelib/cachebench/util/Config.h @@ -197,7 +197,7 @@ struct StressorConfig : public JSONConfig { // If enabled, each value will be read on find. This is useful for measuring // performance of value access. - bool touchValue{false}; + bool touchValue{true}; uint64_t numOps{0}; // operation per thread uint64_t numThreads{0}; // number of threads that will run