From ee63ef37b9227b3affb8ee2d8d4f6247714e3be1 Mon Sep 17 00:00:00 2001 From: Igor Chorazewicz Date: Fri, 29 Oct 2021 20:23:46 -0400 Subject: [PATCH 1/3] Integrate Memory Tier config API with CacheAllocator. --- cachelib/allocator/CMakeLists.txt | 1 + cachelib/allocator/CacheAllocator-inl.h | 66 +++++++++++++------ cachelib/allocator/CacheAllocator.h | 4 ++ cachelib/allocator/CacheAllocatorConfig.h | 1 - .../tests/AllocatorMemoryTiersTest.cpp | 29 ++++++++ .../tests/AllocatorMemoryTiersTest.h | 47 +++++++++++++ .../allocator/tests/AllocatorTypeTest.cpp | 7 ++ cachelib/allocator/tests/BaseAllocatorTest.h | 4 +- cachelib/shm/ShmCommon.h | 3 +- 9 files changed, 138 insertions(+), 24 deletions(-) create mode 100644 cachelib/allocator/tests/AllocatorMemoryTiersTest.cpp create mode 100644 cachelib/allocator/tests/AllocatorMemoryTiersTest.h diff --git a/cachelib/allocator/CMakeLists.txt b/cachelib/allocator/CMakeLists.txt index 688bf09134..367a02caa3 100644 --- a/cachelib/allocator/CMakeLists.txt +++ b/cachelib/allocator/CMakeLists.txt @@ -109,6 +109,7 @@ if (BUILD_TESTS) add_test (tests/ChainedHashTest.cpp) add_test (tests/AllocatorResizeTypeTest.cpp) add_test (tests/AllocatorHitStatsTypeTest.cpp) + add_test (tests/AllocatorMemoryTiersTest.cpp) add_test (tests/MemoryTiersTest.cpp) add_test (tests/MultiAllocatorTest.cpp) add_test (tests/NvmAdmissionPolicyTest.cpp) diff --git a/cachelib/allocator/CacheAllocator-inl.h b/cachelib/allocator/CacheAllocator-inl.h index ce9dcc3c52..d285de2fef 100644 --- a/cachelib/allocator/CacheAllocator-inl.h +++ b/cachelib/allocator/CacheAllocator-inl.h @@ -24,7 +24,8 @@ namespace cachelib { template CacheAllocator::CacheAllocator(Config config) - : isOnShm_{config.memMonitoringEnabled()}, + : memoryTierConfigs(config.getMemoryTierConfigs()), + isOnShm_{config.memMonitoringEnabled()}, config_(config.validate()), tempShm_(isOnShm_ ? std::make_unique(config_.size) : nullptr), @@ -49,15 +50,21 @@ CacheAllocator::CacheAllocator(Config config) cacheCreationTime_{util::getCurrentTimeSec()}, nvmCacheState_{config_.cacheDir, config_.isNvmCacheEncryptionEnabled(), config_.isNvmCacheTruncateAllocSizeEnabled()} { + // TODO(MEMORY_TIER) + if (memoryTierConfigs.size()) { + throw std::runtime_error( + "Using custom memory tier is only supported for Shared Memory."); + } initCommon(false); } template CacheAllocator::CacheAllocator(SharedMemNewT, Config config) - : isOnShm_{true}, + : memoryTierConfigs(config.getMemoryTierConfigs()), + isOnShm_{true}, config_(config.validate()), shmManager_( - std::make_unique(config_.cacheDir, config_.usePosixShm)), + std::make_unique(config_.cacheDir, config_.isUsingPosixShm())), allocator_(createNewMemoryAllocator()), compactCacheManager_(std::make_unique(*allocator_)), compressor_(createPtrCompressor()), @@ -69,7 +76,7 @@ CacheAllocator::CacheAllocator(SharedMemNewT, Config config) config_.accessConfig.getNumBuckets()), nullptr, ShmSegmentOpts(config_.accessConfig.getPageSize(), - false, config_.usePosixShm)) + false, config_.isUsingPosixShm())) .addr, compressor_, [this](Item* it) -> ItemHandle { return acquire(it); })), @@ -81,7 +88,7 @@ CacheAllocator::CacheAllocator(SharedMemNewT, Config config) config_.chainedItemAccessConfig.getNumBuckets()), nullptr, ShmSegmentOpts(config_.accessConfig.getPageSize(), - false, config_.usePosixShm)) + false, config_.isUsingPosixShm())) .addr, compressor_, [this](Item* it) -> ItemHandle { return acquire(it); })), @@ -92,12 +99,13 @@ CacheAllocator::CacheAllocator(SharedMemNewT, Config config) config_.isNvmCacheTruncateAllocSizeEnabled()} { initCommon(false); shmManager_->removeShm(detail::kShmInfoName, - PosixSysVSegmentOpts(config_.usePosixShm)); + PosixSysVSegmentOpts(config_.isUsingPosixShm())); } template CacheAllocator::CacheAllocator(SharedMemAttachT, Config config) - : isOnShm_{true}, + : memoryTierConfigs(config.getMemoryTierConfigs()), + isOnShm_{true}, config_(config.validate()), shmManager_( std::make_unique(config_.cacheDir, config_.usePosixShm)), @@ -111,14 +119,14 @@ CacheAllocator::CacheAllocator(SharedMemAttachT, Config config) deserializer_->deserialize(), config_.accessConfig, shmManager_->attachShm(detail::kShmHashTableName, nullptr, - ShmSegmentOpts(PageSizeT::NORMAL, false, config_.usePosixShm)), + ShmSegmentOpts(PageSizeT::NORMAL, false, config_.isUsingPosixShm())), compressor_, [this](Item* it) -> ItemHandle { return acquire(it); })), chainedItemAccessContainer_(std::make_unique( deserializer_->deserialize(), config_.chainedItemAccessConfig, shmManager_->attachShm(detail::kShmChainedItemHashTableName, nullptr, - ShmSegmentOpts(PageSizeT::NORMAL, false, config_.usePosixShm)), + ShmSegmentOpts(PageSizeT::NORMAL, false, config_.isUsingPosixShm())), compressor_, [this](Item* it) -> ItemHandle { return acquire(it); })), chainedItemLocks_(config_.chainedItemsLockPower, @@ -136,7 +144,7 @@ CacheAllocator::CacheAllocator(SharedMemAttachT, Config config) // this info shm segment here and the new info shm segment's size is larger // than this one, creating new one will fail. shmManager_->removeShm(detail::kShmInfoName, - PosixSysVSegmentOpts(config_.usePosixShm)); + PosixSysVSegmentOpts(config_.isUsingPosixShm())); } template @@ -150,16 +158,35 @@ CacheAllocator::~CacheAllocator() { } template -std::unique_ptr -CacheAllocator::createNewMemoryAllocator() { +ShmSegmentOpts CacheAllocator::createShmCacheOpts() { + if (memoryTierConfigs.size() > 1) { + throw std::invalid_argument("CacheLib only supports a single memory tier"); + } + ShmSegmentOpts opts; opts.alignment = sizeof(Slab); - opts.typeOpts = PosixSysVSegmentOpts(config_.usePosixShm); + + // If memoryTierConfigs is empty, Fallback to Posix/SysV segment + // to keep legacy bahavior + // TODO(MEMORY_TIER) - guarantee there is always at least one mem + // layer inside Config + if (memoryTierConfigs.size()) { + opts.typeOpts = FileShmSegmentOpts(memoryTierConfigs[0].path); + } else { + opts.typeOpts = PosixSysVSegmentOpts(config_.isUsingPosixShm()); + } + + return opts; +} + +template +std::unique_ptr +CacheAllocator::createNewMemoryAllocator() { return std::make_unique( getAllocatorConfig(config_), shmManager_ ->createShm(detail::kShmCacheName, config_.size, - config_.slabMemoryBaseAddr, opts) + config_.slabMemoryBaseAddr, createShmCacheOpts()) .addr, config_.size); } @@ -167,14 +194,11 @@ CacheAllocator::createNewMemoryAllocator() { template std::unique_ptr CacheAllocator::restoreMemoryAllocator() { - ShmSegmentOpts opts; - opts.alignment = sizeof(Slab); - opts.typeOpts = PosixSysVSegmentOpts(config_.usePosixShm); return std::make_unique( deserializer_->deserialize(), shmManager_ - ->attachShm(detail::kShmCacheName, config_.slabMemoryBaseAddr, opts) - .addr, + ->attachShm(detail::kShmCacheName, config_.slabMemoryBaseAddr, + createShmCacheOpts()).addr, config_.size, config_.disableFullCoredump); } @@ -274,7 +298,7 @@ void CacheAllocator::initWorkers() { template std::unique_ptr CacheAllocator::createDeserializer() { auto infoAddr = shmManager_->attachShm(detail::kShmInfoName, nullptr, - ShmSegmentOpts(PageSizeT::NORMAL, false, config_.usePosixShm)); + ShmSegmentOpts(PageSizeT::NORMAL, false, config_.isUsingPosixShm())); return std::make_unique( reinterpret_cast(infoAddr.addr), reinterpret_cast(infoAddr.addr) + infoAddr.size); @@ -3051,7 +3075,7 @@ void CacheAllocator::saveRamCache() { ioBuf->coalesce(); ShmSegmentOpts opts; - opts.typeOpts = PosixSysVSegmentOpts(config_.usePosixShm); + opts.typeOpts = PosixSysVSegmentOpts(config_.isUsingPosixShm()); void* infoAddr = shmManager_->createShm(detail::kShmInfoName, ioBuf->length(), nullptr, opts).addr; diff --git a/cachelib/allocator/CacheAllocator.h b/cachelib/allocator/CacheAllocator.h index 9b2831a0dd..27cf7b0ca6 100644 --- a/cachelib/allocator/CacheAllocator.h +++ b/cachelib/allocator/CacheAllocator.h @@ -1607,6 +1607,8 @@ class CacheAllocator : public CacheBase { std::unique_ptr& worker, std::chrono::seconds timeout = std::chrono::seconds{0}); + ShmSegmentOpts createShmCacheOpts(); + std::unique_ptr createNewMemoryAllocator(); std::unique_ptr restoreMemoryAllocator(); std::unique_ptr restoreCCacheManager(); @@ -1714,6 +1716,8 @@ class CacheAllocator : public CacheBase { const Config config_{}; + const typename Config::MemoryTierConfigs memoryTierConfigs; + // Manages the temporary shared memory segment for memory allocator that // is not persisted when cache process exits. std::unique_ptr tempShm_; diff --git a/cachelib/allocator/CacheAllocatorConfig.h b/cachelib/allocator/CacheAllocatorConfig.h index 1bd3c75056..a61b1f07b4 100644 --- a/cachelib/allocator/CacheAllocatorConfig.h +++ b/cachelib/allocator/CacheAllocatorConfig.h @@ -877,7 +877,6 @@ CacheAllocatorConfig& CacheAllocatorConfig::configureMemoryTiers( return *this; } -//const std::vector& CacheAllocatorConfig::getMemoryTierConfigs() { template const typename CacheAllocatorConfig::MemoryTierConfigs& CacheAllocatorConfig::getMemoryTierConfigs() { return memoryTierConfigs; diff --git a/cachelib/allocator/tests/AllocatorMemoryTiersTest.cpp b/cachelib/allocator/tests/AllocatorMemoryTiersTest.cpp new file mode 100644 index 0000000000..b784729157 --- /dev/null +++ b/cachelib/allocator/tests/AllocatorMemoryTiersTest.cpp @@ -0,0 +1,29 @@ +/* + * Copyright (c) Intel Corporation. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "cachelib/allocator/tests/AllocatorMemoryTiersTest.h" + +namespace facebook { +namespace cachelib { +namespace tests { + +using LruAllocatorMemoryTiersTest = AllocatorMemoryTiersTest; + +TEST_F(LruAllocatorMemoryTiersTest, MultiTiers) { this->testMultiTiers(); } + +} // end of namespace tests +} // end of namespace cachelib +} // end of namespace facebook diff --git a/cachelib/allocator/tests/AllocatorMemoryTiersTest.h b/cachelib/allocator/tests/AllocatorMemoryTiersTest.h new file mode 100644 index 0000000000..8208c6b19f --- /dev/null +++ b/cachelib/allocator/tests/AllocatorMemoryTiersTest.h @@ -0,0 +1,47 @@ +/* + * Copyright (c) Facebook, Inc. and its affiliates. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#pragma once + +#include "cachelib/allocator/CacheAllocatorConfig.h" +#include "cachelib/allocator/MemoryTierCacheConfig.h" +#include "cachelib/allocator/tests/TestBase.h" + +namespace facebook { +namespace cachelib { +namespace tests { + +template +class AllocatorMemoryTiersTest : public AllocatorTest { + public: + void testMultiTiers() { + typename AllocatorT::Config config; + config.setCacheSize(100 * Slab::kSize); + config.configureMemoryTiers({ + MemoryTierCacheConfig::fromFile("/tmp/a" + std::to_string(::getpid())) + .setRatio(1), + MemoryTierCacheConfig::fromFile("/tmp/b" + std::to_string(::getpid())) + .setRatio(1) + }); + + // More than one tier is not supported + ASSERT_THROW(std::make_unique(AllocatorT::SharedMemNew, config), + std::invalid_argument); + } +}; +} // namespace tests +} // namespace cachelib +} // namespace facebook diff --git a/cachelib/allocator/tests/AllocatorTypeTest.cpp b/cachelib/allocator/tests/AllocatorTypeTest.cpp index f8bb1df9eb..18c4f64044 100644 --- a/cachelib/allocator/tests/AllocatorTypeTest.cpp +++ b/cachelib/allocator/tests/AllocatorTypeTest.cpp @@ -16,6 +16,7 @@ #include "cachelib/allocator/tests/BaseAllocatorTest.h" #include "cachelib/allocator/tests/TestBase.h" +#include "cachelib/allocator/MemoryTierCacheConfig.h" namespace facebook { namespace cachelib { @@ -215,6 +216,12 @@ TYPED_TEST(BaseAllocatorTest, ReaperOutOfBound) { } TYPED_TEST(BaseAllocatorTest, ReaperShutDown) { this->testReaperShutDown(); } +TYPED_TEST(BaseAllocatorTest, ReaperShutDownFile) { + this->testReaperShutDown({ + MemoryTierCacheConfig::fromFile("/tmp/a" + std::to_string(::getpid())) + .setRatio(1) + }); +} TYPED_TEST(BaseAllocatorTest, ShutDownWithActiveHandles) { this->testShutDownWithActiveHandles(); diff --git a/cachelib/allocator/tests/BaseAllocatorTest.h b/cachelib/allocator/tests/BaseAllocatorTest.h index 8bbb380891..c1898d0c0c 100644 --- a/cachelib/allocator/tests/BaseAllocatorTest.h +++ b/cachelib/allocator/tests/BaseAllocatorTest.h @@ -1140,7 +1140,7 @@ class BaseAllocatorTest : public AllocatorTest { this->testLruLength(alloc, poolId, sizes, keyLen, evictedKeys); } - void testReaperShutDown() { + void testReaperShutDown(typename AllocatorT::Config::MemoryTierConfigs cfgs = {}) { const size_t nSlabs = 20; const size_t size = nSlabs * Slab::kSize; @@ -1150,6 +1150,8 @@ class BaseAllocatorTest : public AllocatorTest { config.setAccessConfig({8, 8}); config.enableCachePersistence(this->cacheDir_); config.enableItemReaperInBackground(std::chrono::seconds(1), {}); + if (cfgs.size()) + config.configureMemoryTiers(cfgs); std::vector keys; { AllocatorT alloc(AllocatorT::SharedMemNew, config); diff --git a/cachelib/shm/ShmCommon.h b/cachelib/shm/ShmCommon.h index c3363f4e34..b574c3d0fb 100644 --- a/cachelib/shm/ShmCommon.h +++ b/cachelib/shm/ShmCommon.h @@ -57,7 +57,8 @@ struct ShmSegmentOpts { PageSizeT pageSize{PageSizeT::NORMAL}; bool readOnly{false}; size_t alignment{1}; // alignment for mapping. - ShmTypeOpts typeOpts{}; // opts specific to segment type + // opts specific to segment type + ShmTypeOpts typeOpts{PosixSysVSegmentOpts(false)}; explicit ShmSegmentOpts(PageSizeT p) : pageSize(p) {} explicit ShmSegmentOpts(PageSizeT p, bool ro) : pageSize(p), readOnly(ro) {} From 6b4f46bd8552745566a04073db5d392a22d30fa6 Mon Sep 17 00:00:00 2001 From: Igor Chorazewicz Date: Fri, 5 Nov 2021 21:03:17 -0400 Subject: [PATCH 2/3] Add MemoryTierCacheConfig::fromShm() to allow using new configureMemoryTiers() API with legacy behavior. Move validation code for memory tiers to validate() method and convert ratios to sizes lazily (on get).. --- cachelib/allocator/CacheAllocator-inl.h | 30 ++-- cachelib/allocator/CacheAllocatorConfig.h | 166 ++++++++++++------ cachelib/allocator/MemoryTierCacheConfig.h | 23 +-- .../tests/AllocatorMemoryTiersTest.cpp | 1 + cachelib/allocator/tests/BaseAllocatorTest.h | 6 +- cachelib/allocator/tests/MemoryTiersTest.cpp | 27 +-- cachelib/shm/ShmCommon.h | 1 - 7 files changed, 159 insertions(+), 95 deletions(-) diff --git a/cachelib/allocator/CacheAllocator-inl.h b/cachelib/allocator/CacheAllocator-inl.h index d285de2fef..fc485c2ae9 100644 --- a/cachelib/allocator/CacheAllocator-inl.h +++ b/cachelib/allocator/CacheAllocator-inl.h @@ -27,14 +27,16 @@ CacheAllocator::CacheAllocator(Config config) : memoryTierConfigs(config.getMemoryTierConfigs()), isOnShm_{config.memMonitoringEnabled()}, config_(config.validate()), - tempShm_(isOnShm_ ? std::make_unique(config_.size) + tempShm_(isOnShm_ ? std::make_unique( + config_.getCacheSize()) : nullptr), allocator_(isOnShm_ ? std::make_unique( getAllocatorConfig(config_), tempShm_->getAddr(), - config_.size) + config_.getCacheSize()) : std::make_unique( - getAllocatorConfig(config_), config_.size)), + getAllocatorConfig(config_), + config_.getCacheSize())), compactCacheManager_(std::make_unique(*allocator_)), compressor_(createPtrCompressor()), accessContainer_(std::make_unique( @@ -51,7 +53,8 @@ CacheAllocator::CacheAllocator(Config config) nvmCacheState_{config_.cacheDir, config_.isNvmCacheEncryptionEnabled(), config_.isNvmCacheTruncateAllocSizeEnabled()} { // TODO(MEMORY_TIER) - if (memoryTierConfigs.size()) { + if (std::holds_alternative( + memoryTierConfigs[0].getShmTypeOpts())) { throw std::runtime_error( "Using custom memory tier is only supported for Shared Memory."); } @@ -165,16 +168,7 @@ ShmSegmentOpts CacheAllocator::createShmCacheOpts() { ShmSegmentOpts opts; opts.alignment = sizeof(Slab); - - // If memoryTierConfigs is empty, Fallback to Posix/SysV segment - // to keep legacy bahavior - // TODO(MEMORY_TIER) - guarantee there is always at least one mem - // layer inside Config - if (memoryTierConfigs.size()) { - opts.typeOpts = FileShmSegmentOpts(memoryTierConfigs[0].path); - } else { - opts.typeOpts = PosixSysVSegmentOpts(config_.isUsingPosixShm()); - } + opts.typeOpts = memoryTierConfigs[0].getShmTypeOpts(); return opts; } @@ -185,10 +179,10 @@ CacheAllocator::createNewMemoryAllocator() { return std::make_unique( getAllocatorConfig(config_), shmManager_ - ->createShm(detail::kShmCacheName, config_.size, + ->createShm(detail::kShmCacheName, config_.getCacheSize(), config_.slabMemoryBaseAddr, createShmCacheOpts()) .addr, - config_.size); + config_.getCacheSize()); } template @@ -199,7 +193,7 @@ CacheAllocator::restoreMemoryAllocator() { shmManager_ ->attachShm(detail::kShmCacheName, config_.slabMemoryBaseAddr, createShmCacheOpts()).addr, - config_.size, + config_.getCacheSize(), config_.disableFullCoredump); } @@ -2216,7 +2210,7 @@ PoolEvictionAgeStats CacheAllocator::getPoolEvictionAgeStats( template CacheMetadata CacheAllocator::getCacheMetadata() const noexcept { return CacheMetadata{kCachelibVersion, kCacheRamFormatVersion, - kCacheNvmFormatVersion, config_.size}; + kCacheNvmFormatVersion, config_.getCacheSize()}; } template diff --git a/cachelib/allocator/CacheAllocatorConfig.h b/cachelib/allocator/CacheAllocatorConfig.h index a61b1f07b4..cb578717cb 100644 --- a/cachelib/allocator/CacheAllocatorConfig.h +++ b/cachelib/allocator/CacheAllocatorConfig.h @@ -200,10 +200,13 @@ class CacheAllocatorConfig { // Configures cache memory tiers. Accepts vector of MemoryTierCacheConfig. // Each vector element describes configuration for a single memory cache tier. + // @throw std::invalid_argument if: + // - the size of configs is 0 + // - memory tiers use both size and ratio parameters CacheAllocatorConfig& configureMemoryTiers(const MemoryTierConfigs& configs); - // Return reference to MemoryTierCacheConfigs. - const MemoryTierConfigs& getMemoryTierConfigs(); + // Return vector of memory tier configs. + MemoryTierConfigs getMemoryTierConfigs() const; // This turns on a background worker that periodically scans through the // access container and look for expired items and remove them. @@ -334,7 +337,7 @@ class CacheAllocatorConfig { const std::string& getCacheName() const noexcept { return cacheName; } - size_t getCacheSize() const noexcept { return size; } + size_t getCacheSize() const noexcept; bool isUsingPosixShm() const noexcept { return usePosixShm; } @@ -552,12 +555,16 @@ class CacheAllocatorConfig { // cache. uint64_t nvmAdmissionMinTTL{0}; - // Configuration for memory tiers. - MemoryTierConfigs memoryTierConfigs; - friend CacheT; private: + void validateMemoryTiersWithSize(const MemoryTierConfigs&, size_t) const; + + // Configuration for memory tiers. + MemoryTierConfigs memoryTierConfigs{ + {MemoryTierCacheConfig::fromShm().setRatio(1)} + }; + void mergeWithPrefix( std::map& configMap, const std::map& configMapToMerge, @@ -576,6 +583,8 @@ CacheAllocatorConfig& CacheAllocatorConfig::setCacheName( template CacheAllocatorConfig& CacheAllocatorConfig::setCacheSize(size_t _size) { + validateMemoryTiersWithSize(this->memoryTierConfigs, _size); + size = _size; constexpr size_t maxCacheSizeWithCoredump = 64'424'509'440; // 60GB if (size <= maxCacheSizeWithCoredump) { @@ -818,68 +827,62 @@ CacheAllocatorConfig& CacheAllocatorConfig::enableItemReaperInBackground( template CacheAllocatorConfig& CacheAllocatorConfig::configureMemoryTiers( const MemoryTierConfigs& config) { - memoryTierConfigs = config; - size_t sum_ratios = 0; - size_t sum_sizes = 0; + if (!config.size()) { + throw std::invalid_argument("There must be at least one memory tier."); + } - for (auto tier_config: memoryTierConfigs) { + for (auto tier_config: config) { auto tier_size = tier_config.getSize(); auto tier_ratio = tier_config.getRatio(); if ((!tier_size and !tier_ratio) || (tier_size and tier_ratio)) { throw std::invalid_argument( "For each memory tier either size or ratio must be set."); } - sum_ratios += tier_ratio; - sum_sizes += tier_size; } - if (sum_ratios) { - if (!getCacheSize()) { - throw std::invalid_argument( - "Total cache size must be specified when size ratios are \ - used to specify memory tier sizes."); - } else { - if (getCacheSize() < sum_ratios) { - throw std::invalid_argument( - "Sum of all tier size ratios is greater than total cache size."); - } - // Convert ratios to sizes - sum_sizes = 0; - size_t partition_size = getCacheSize() / sum_ratios; - for (auto& tier_config: memoryTierConfigs) { - tier_config.setSize(partition_size * tier_config.getRatio()); - sum_sizes += tier_config.getSize(); - } - if (getCacheSize() != sum_sizes) { - // Adjust capacity of the last tier to account for rounding error - memoryTierConfigs.back().setSize(memoryTierConfigs.back().getSize() + \ - (getCacheSize() - sum_sizes)); - sum_sizes = getCacheSize(); - } - } - } else if (sum_sizes) { - if (getCacheSize() && sum_sizes != getCacheSize()) { - throw std::invalid_argument( - "Sum of tier sizes doesn't match total cache size. \ - Setting of cache total size is not required when per-tier \ - sizes are specified - it is calculated as sum of tier sizes."); - } - } else { - throw std::invalid_argument( - "Either sum of all memory tiers sizes or sum of all ratios \ - must be greater than 0."); - } + validateMemoryTiersWithSize(config, this->size); - if (sum_sizes && !getCacheSize()) { - setCacheSize(sum_sizes); - } + memoryTierConfigs = config; return *this; } template -const typename CacheAllocatorConfig::MemoryTierConfigs& CacheAllocatorConfig::getMemoryTierConfigs() { - return memoryTierConfigs; +typename CacheAllocatorConfig::MemoryTierConfigs +CacheAllocatorConfig::getMemoryTierConfigs() const { + MemoryTierConfigs config = memoryTierConfigs; + size_t sum_ratios = 0; + + for (auto &tier_config: config) { + if (auto *v = std::get_if(&tier_config.shmOpts)) { + v->usePosix = usePosixShm; + } + + sum_ratios += tier_config.getRatio(); + } + + if (sum_ratios == 0) + return config; + + // if ratios are used, size must be specified + XDCHECK(size); + + // Convert ratios to sizes, size must be non-zero + size_t sum_sizes = 0; + size_t partition_size = size / sum_ratios; + for (auto& tier_config: config) { + tier_config.setSize(partition_size * tier_config.getRatio()); + tier_config.setRatio(0); + sum_sizes += tier_config.getSize(); + } + + if (size != sum_sizes) { + // Adjust capacity of the last tier to account for rounding error + config.back().setSize( + config.back().getSize() + (getCacheSize() - sum_sizes)); + } + + return config; } template @@ -997,6 +1000,46 @@ CacheAllocatorConfig& CacheAllocatorConfig::setNvmAdmissionMinTTL( return *this; } +template +size_t CacheAllocatorConfig::getCacheSize() const noexcept { + if (size) + return size; + + size_t sum_sizes = 0; + for (const auto &tier_config : getMemoryTierConfigs()) { + sum_sizes += tier_config.getSize(); + } + + return sum_sizes; +} + +template +void CacheAllocatorConfig::validateMemoryTiersWithSize( + const MemoryTierConfigs &config, size_t size) const { + size_t sum_ratios = 0; + size_t sum_sizes = 0; + + for (const auto &tier_config: config) { + sum_ratios += tier_config.getRatio(); + sum_sizes += tier_config.getSize(); + } + + if (sum_ratios && sum_sizes) { + throw std::invalid_argument("Cannot mix ratios and sizes."); + } else if (sum_sizes) { + if (size && sum_sizes != size) { + throw std::invalid_argument( + "Sum of tier sizes doesn't match total cache size. " + "Setting of cache total size is not required when per-tier " + "sizes are specified - it is calculated as sum of tier sizes."); + } + } else if (!sum_ratios && !sum_sizes) { + throw std::invalid_argument( + "Either sum of all memory tiers sizes or sum of all ratios " + "must be greater than 0."); + } +} + template const CacheAllocatorConfig& CacheAllocatorConfig::validate() const { // we can track tail hits only if MMType is MM2Q @@ -1018,6 +1061,23 @@ const CacheAllocatorConfig& CacheAllocatorConfig::validate() const { size, maxCacheSize)); } + + size_t sum_ratios = 0; + for (auto tier_config: memoryTierConfigs) { + sum_ratios += tier_config.getRatio(); + } + + if (sum_ratios) { + if (!size) { + throw std::invalid_argument( + "Total cache size must be specified when size ratios are " + "used to specify memory tier sizes."); + } else if (size < sum_ratios) { + throw std::invalid_argument( + "Sum of all tier size ratios is greater than total cache size."); + } + } + return *this; } diff --git a/cachelib/allocator/MemoryTierCacheConfig.h b/cachelib/allocator/MemoryTierCacheConfig.h index 5e3604a0af..12fd2c91f0 100644 --- a/cachelib/allocator/MemoryTierCacheConfig.h +++ b/cachelib/allocator/MemoryTierCacheConfig.h @@ -18,6 +18,8 @@ #include +#include "cachelib/shm/ShmCommon.h" + namespace facebook { namespace cachelib { class MemoryTierCacheConfig { @@ -27,7 +29,14 @@ class MemoryTierCacheConfig { // TODO: add fromDirectory, fromAnonymousMemory static MemoryTierCacheConfig fromFile(const std::string& _file) { MemoryTierCacheConfig config; - config.path = _file; + config.shmOpts = FileShmSegmentOpts(_file); + return config; + } + + // Creates instance of MemoryTierCacheConfig for Posix/SysV Shared memory. + static MemoryTierCacheConfig fromShm() { + MemoryTierCacheConfig config; + config.shmOpts = PosixSysVSegmentOpts(); return config; } @@ -53,11 +62,7 @@ class MemoryTierCacheConfig { size_t getSize() const noexcept { return size; } - const std::string& getPath() const noexcept { return path; } - - bool isFileBacked() const { - return !path.empty(); - } + const ShmTypeOpts& getShmTypeOpts() const noexcept { return shmOpts; } // Size of this memory tiers size_t size{0}; @@ -67,10 +72,8 @@ class MemoryTierCacheConfig { // then size of the i-th tier Xi = (X / (Y1 + Y2)) * Yi and X = sum(Xi) size_t ratio{0}; - // Path to file for file system-backed memory tier - // TODO: consider using variant to support different - // memory sources - std::string path; + // Options specific to shm type + ShmTypeOpts shmOpts; private: MemoryTierCacheConfig() = default; diff --git a/cachelib/allocator/tests/AllocatorMemoryTiersTest.cpp b/cachelib/allocator/tests/AllocatorMemoryTiersTest.cpp index b784729157..b6db9ce168 100644 --- a/cachelib/allocator/tests/AllocatorMemoryTiersTest.cpp +++ b/cachelib/allocator/tests/AllocatorMemoryTiersTest.cpp @@ -22,6 +22,7 @@ namespace tests { using LruAllocatorMemoryTiersTest = AllocatorMemoryTiersTest; +// TODO(MEMORY_TIER): add more tests with different eviction policies TEST_F(LruAllocatorMemoryTiersTest, MultiTiers) { this->testMultiTiers(); } } // end of namespace tests diff --git a/cachelib/allocator/tests/BaseAllocatorTest.h b/cachelib/allocator/tests/BaseAllocatorTest.h index c1898d0c0c..dce17f7ceb 100644 --- a/cachelib/allocator/tests/BaseAllocatorTest.h +++ b/cachelib/allocator/tests/BaseAllocatorTest.h @@ -1140,7 +1140,8 @@ class BaseAllocatorTest : public AllocatorTest { this->testLruLength(alloc, poolId, sizes, keyLen, evictedKeys); } - void testReaperShutDown(typename AllocatorT::Config::MemoryTierConfigs cfgs = {}) { + void testReaperShutDown(typename AllocatorT::Config::MemoryTierConfigs cfgs = + {MemoryTierCacheConfig::fromShm().setRatio(1)}) { const size_t nSlabs = 20; const size_t size = nSlabs * Slab::kSize; @@ -1150,8 +1151,7 @@ class BaseAllocatorTest : public AllocatorTest { config.setAccessConfig({8, 8}); config.enableCachePersistence(this->cacheDir_); config.enableItemReaperInBackground(std::chrono::seconds(1), {}); - if (cfgs.size()) - config.configureMemoryTiers(cfgs); + config.configureMemoryTiers(cfgs); std::vector keys; { AllocatorT alloc(AllocatorT::SharedMemNew, config); diff --git a/cachelib/allocator/tests/MemoryTiersTest.cpp b/cachelib/allocator/tests/MemoryTiersTest.cpp index f578ed3ea3..6e5616fcdb 100644 --- a/cachelib/allocator/tests/MemoryTiersTest.cpp +++ b/cachelib/allocator/tests/MemoryTiersTest.cpp @@ -59,7 +59,8 @@ class MemoryTiersTest: public AllocatorTest { } for(auto i = 0; i < configs.size(); ++i) { - EXPECT_EQ(configs[i].getPath(), expectedPaths[i]); + auto &opt = std::get(configs[i].getShmTypeOpts()); + EXPECT_EQ(opt.path, expectedPaths[i]); EXPECT_GT(configs[i].getSize(), 0); if (configs[i].getRatio() && (i < configs.size() - 1)) { EXPECT_EQ(configs[i].getSize(), partition_size * configs[i].getRatio()); @@ -98,12 +99,12 @@ class MemoryTiersTest: public AllocatorTest { using LruMemoryTiersTest = MemoryTiersTest; TEST_F(LruMemoryTiersTest, TestValid1TierPmemRatioConfig) { - LruAllocatorConfig cfg = createTestCacheConfig({defaultPmemPath}).validate(); + LruAllocatorConfig cfg = createTestCacheConfig({defaultPmemPath}); basicCheck(cfg); } TEST_F(LruMemoryTiersTest, TestValid1TierDaxRatioConfig) { - LruAllocatorConfig cfg = createTestCacheConfig({defaultDaxPath}).validate(); + LruAllocatorConfig cfg = createTestCacheConfig({defaultDaxPath}); basicCheck(cfg, {defaultDaxPath}); } @@ -111,19 +112,22 @@ TEST_F(LruMemoryTiersTest, TestValid1TierDaxSizeConfig) { LruAllocatorConfig cfg = createTestCacheConfig({defaultDaxPath}, {std::make_tuple(0, defaultTotalCacheSize)}, /* setPosixShm */ true, - /* cacheSize */ 0).validate(); + /* cacheSize */ 0); basicCheck(cfg, {defaultDaxPath}); + + // Setting size after conifguringMemoryTiers with sizes is not allowed. + EXPECT_THROW(cfg.setCacheSize(defaultTotalCacheSize + 1), std::invalid_argument); } TEST_F(LruMemoryTiersTest, TestValid2TierDaxPmemConfig) { LruAllocatorConfig cfg = createTestCacheConfig({defaultDaxPath, defaultPmemPath}, - {std::make_tuple(1, 0), std::make_tuple(1, 0)}).validate(); + {std::make_tuple(1, 0), std::make_tuple(1, 0)}); basicCheck(cfg, {defaultDaxPath, defaultPmemPath}); } TEST_F(LruMemoryTiersTest, TestValid2TierDaxPmemRatioConfig) { LruAllocatorConfig cfg = createTestCacheConfig({defaultDaxPath, defaultPmemPath}, - {std::make_tuple(5, 0), std::make_tuple(2, 0)}).validate(); + {std::make_tuple(5, 0), std::make_tuple(2, 0)}); basicCheck(cfg, {defaultDaxPath, defaultPmemPath}); } @@ -131,19 +135,22 @@ TEST_F(LruMemoryTiersTest, TestValid2TierDaxPmemSizeConfig) { size_t size_1 = 4321, size_2 = 1234; LruAllocatorConfig cfg = createTestCacheConfig({defaultDaxPath, defaultPmemPath}, {std::make_tuple(0, size_1), std::make_tuple(0, size_2)}, - true, 0).validate(); + true, 0); basicCheck(cfg, {defaultDaxPath, defaultPmemPath}, size_1 + size_2); + + // Setting size after conifguringMemoryTiers with sizes is not allowed. + EXPECT_THROW(cfg.setCacheSize(size_1 + size_2 + 1), std::invalid_argument); } TEST_F(LruMemoryTiersTest, TestInvalid2TierConfigPosixShmNotSet) { LruAllocatorConfig cfg = createTestCacheConfig({defaultDaxPath, defaultPmemPath}, {std::make_tuple(1, 0), std::make_tuple(1, 0)}, - /* setPosixShm */ false).validate(); + /* setPosixShm */ false); } TEST_F(LruMemoryTiersTest, TestInvalid2TierConfigNumberOfPartitionsTooLarge) { EXPECT_THROW(createTestCacheConfig({defaultDaxPath, defaultPmemPath}, - {std::make_tuple(defaultTotalCacheSize, 0), std::make_tuple(1, 0)}), + {std::make_tuple(defaultTotalCacheSize, 0), std::make_tuple(1, 0)}).validate(), std::invalid_argument); } @@ -165,7 +172,7 @@ TEST_F(LruMemoryTiersTest, TestInvalid2TierConfigSizesAndRatioNotSet) { TEST_F(LruMemoryTiersTest, TestInvalid2TierConfigRatiosCacheSizeNotSet) { EXPECT_THROW(createTestCacheConfig({defaultDaxPath, defaultPmemPath}, {std::make_tuple(1, 0), std::make_tuple(1, 0)}, - /* setPosixShm */ true, /* cacheSize */ 0), + /* setPosixShm */ true, /* cacheSize */ 0).validate(), std::invalid_argument); } diff --git a/cachelib/shm/ShmCommon.h b/cachelib/shm/ShmCommon.h index b574c3d0fb..807237d6f5 100644 --- a/cachelib/shm/ShmCommon.h +++ b/cachelib/shm/ShmCommon.h @@ -40,7 +40,6 @@ enum PageSizeT { constexpr int kInvalidFD = -1; -// TODO(SHM_FILE): maybe we could use this inside the Tier Config class? struct FileShmSegmentOpts { FileShmSegmentOpts(std::string path = ""): path(path) {} std::string path; From f401f176cb42b46295e55cda7555eac7b9530117 Mon Sep 17 00:00:00 2001 From: Igor Chorazewicz Date: Mon, 8 Nov 2021 19:46:04 -0500 Subject: [PATCH 3/3] Fix test_shm_manager.cpp test It wrongly assumed that the only possible segment type is PosixSysV segment. --- cachelib/shm/tests/test_shm_manager.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cachelib/shm/tests/test_shm_manager.cpp b/cachelib/shm/tests/test_shm_manager.cpp index 014e93d04d..1343c84c77 100644 --- a/cachelib/shm/tests/test_shm_manager.cpp +++ b/cachelib/shm/tests/test_shm_manager.cpp @@ -797,8 +797,8 @@ void ShmManagerTest::testShutDown(bool posix) { ASSERT_NO_THROW(s.createShm(seg2, seg2Size, nullptr, seg2Opt)); ASSERT_EQ(s.getShmByName(seg2).getSize(), seg2Size); auto *v = std::get_if(&s.getShmTypeByName(seg2)); - ASSERT_TRUE(v); - ASSERT_EQ(v->usePosix, posix); + if (v) + ASSERT_EQ(v->usePosix, posix); ASSERT_TRUE(s.shutDown() == ShutDownRes::kSuccess); };