Skip to content

Commit 8dde327

Browse files
igchorbyrnedj
authored andcommitted
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).. Enabled shared memory tier in cachebench.
1 parent cc13bd3 commit 8dde327

File tree

7 files changed

+52
-43
lines changed

7 files changed

+52
-43
lines changed

cachelib/allocator/CacheAllocator-inl.h

Lines changed: 10 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,8 @@ template <typename CacheTrait>
2323
CacheAllocator<CacheTrait>::CacheAllocator(Config config)
2424
: CacheAllocator(InitMemType::kNone, config) {
2525
// TODO(MEMORY_TIER)
26-
if (memoryTierConfigs.size()) {
26+
if (std::holds_alternative<FileShmSegmentOpts>(
27+
memoryTierConfigs[0].getShmTypeOpts())) {
2728
throw std::runtime_error(
2829
"Using custom memory tier is only supported for Shared Memory.");
2930
}
@@ -62,7 +63,7 @@ CacheAllocator<CacheTrait>::CacheAllocator(
6263
config_(config.validate()),
6364
memoryTierConfigs(config.getMemoryTierConfigs()),
6465
tempShm_(type == InitMemType::kNone && isOnShm_
65-
? std::make_unique<TempShmMapping>(config_.size)
66+
? std::make_unique<TempShmMapping>(config_.getCacheSize())
6667
: nullptr),
6768
shmManager_(type != InitMemType::kNone
6869
? std::make_unique<ShmManager>(config_.cacheDir,
@@ -121,16 +122,7 @@ ShmSegmentOpts CacheAllocator<CacheTrait>::createShmCacheOpts() {
121122

122123
ShmSegmentOpts opts;
123124
opts.alignment = sizeof(Slab);
124-
125-
// If memoryTierConfigs is empty, Fallback to Posix/SysV segment
126-
// to keep legacy bahavior
127-
// TODO(MEMORY_TIER) - guarantee there is always at least one mem
128-
// layer inside Config
129-
if (memoryTierConfigs.size()) {
130-
opts.typeOpts = FileShmSegmentOpts(memoryTierConfigs[0].path);
131-
} else {
132-
opts.typeOpts = PosixSysVSegmentOpts(config_.isUsingPosixShm());
133-
}
125+
opts.typeOpts = memoryTierConfigs[0].getShmTypeOpts();
134126

135127
return opts;
136128
}
@@ -141,10 +133,10 @@ CacheAllocator<CacheTrait>::createNewMemoryAllocator() {
141133
return std::make_unique<MemoryAllocator>(
142134
getAllocatorConfig(config_),
143135
shmManager_
144-
->createShm(detail::kShmCacheName, config_.size,
136+
->createShm(detail::kShmCacheName, config_.getCacheSize(),
145137
config_.slabMemoryBaseAddr, createShmCacheOpts())
146138
.addr,
147-
config_.size);
139+
config_.getCacheSize());
148140
}
149141

150142
template <typename CacheTrait>
@@ -155,7 +147,7 @@ CacheAllocator<CacheTrait>::restoreMemoryAllocator() {
155147
shmManager_
156148
->attachShm(detail::kShmCacheName, config_.slabMemoryBaseAddr,
157149
createShmCacheOpts()).addr,
158-
config_.size,
150+
config_.getCacheSize(),
159151
config_.disableFullCoredump);
160152
}
161153

@@ -261,10 +253,10 @@ std::unique_ptr<MemoryAllocator> CacheAllocator<CacheTrait>::initAllocator(
261253
if (type == InitMemType::kNone) {
262254
if (isOnShm_ == true) {
263255
return std::make_unique<MemoryAllocator>(
264-
getAllocatorConfig(config_), tempShm_->getAddr(), config_.size);
256+
getAllocatorConfig(config_), tempShm_->getAddr(), config_.getCacheSize());
265257
} else {
266258
return std::make_unique<MemoryAllocator>(getAllocatorConfig(config_),
267-
config_.size);
259+
config_.getCacheSize());
268260
}
269261
} else if (type == InitMemType::kMemNew) {
270262
return createNewMemoryAllocator();
@@ -2272,7 +2264,7 @@ PoolEvictionAgeStats CacheAllocator<CacheTrait>::getPoolEvictionAgeStats(
22722264
template <typename CacheTrait>
22732265
CacheMetadata CacheAllocator<CacheTrait>::getCacheMetadata() const noexcept {
22742266
return CacheMetadata{kCachelibVersion, kCacheRamFormatVersion,
2275-
kCacheNvmFormatVersion, config_.size};
2267+
kCacheNvmFormatVersion, config_.getCacheSize()};
22762268
}
22772269

22782270
template <typename CacheTrait>

cachelib/allocator/CacheAllocatorConfig.h

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -210,10 +210,13 @@ class CacheAllocatorConfig {
210210
// Accepts vector of MemoryTierCacheConfig. Each vector element describes
211211
// configuration for a single memory cache tier. Tier sizes are specified as
212212
// ratios, the number of parts of total cache size each tier would occupy.
213+
// @throw std::invalid_argument if:
214+
// - the size of configs is 0
215+
// - the size of configs is greater than kMaxCacheMemoryTiers
213216
CacheAllocatorConfig& configureMemoryTiers(const MemoryTierConfigs& configs);
214217

215218
// Return reference to MemoryTierCacheConfigs.
216-
const MemoryTierConfigs& getMemoryTierConfigs();
219+
const MemoryTierConfigs& getMemoryTierConfigs() const;
217220

218221
// This turns on a background worker that periodically scans through the
219222
// access container and look for expired items and remove them.
@@ -378,7 +381,7 @@ class CacheAllocatorConfig {
378381

379382
// The max number of memory cache tiers
380383
// TODO: increase this number when multi-tier configs are enabled
381-
inline static const size_t kMaxCacheMemoryTiers = 1;
384+
inline static const size_t kMaxCacheMemoryTiers = 2;
382385

383386
// Cache name for users to indentify their own cache.
384387
std::string cacheName{""};
@@ -880,7 +883,12 @@ CacheAllocatorConfig<T>& CacheAllocatorConfig<T>::configureMemoryTiers(
880883

881884
template <typename T>
882885
const typename CacheAllocatorConfig<T>::MemoryTierConfigs&
883-
CacheAllocatorConfig<T>::getMemoryTierConfigs() {
886+
CacheAllocatorConfig<T>::getMemoryTierConfigs() const {
887+
for (auto &tier_config: memoryTierConfigs) {
888+
if (auto *v = std::get_if<PosixSysVSegmentOpts>(&tier_config.shmOpts)) {
889+
const_cast<PosixSysVSegmentOpts*>(v)->usePosix = usePosixShm;
890+
}
891+
}
884892
return memoryTierConfigs;
885893
}
886894

cachelib/allocator/MemoryTierCacheConfig.h

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -26,15 +26,17 @@ class MemoryTierCacheConfig {
2626
public:
2727
// Creates instance of MemoryTierCacheConfig for Posix/SysV Shared memory.
2828
static MemoryTierCacheConfig fromShm() {
29-
return MemoryTierCacheConfig();
29+
MemoryTierCacheConfig config;
30+
config.shmOpts = PosixSysVSegmentOpts();
31+
return config;
3032
}
3133

3234
// Creates instance of MemoryTierCacheConfig for file-backed memory.
3335
// @param path to file which CacheLib will use to map memory from.
3436
// TODO: add fromDirectory, fromAnonymousMemory
3537
static MemoryTierCacheConfig fromFile(const std::string& _file) {
3638
MemoryTierCacheConfig config;
37-
config.path = _file;
39+
config.shmOpts = FileShmSegmentOpts(_file);
3840
return config;
3941
}
4042

@@ -67,7 +69,7 @@ class MemoryTierCacheConfig {
6769
return getRatio() * (totalCacheSize / partitionNum);
6870
}
6971

70-
const std::string& getPath() const noexcept { return path; }
72+
const ShmTypeOpts& getShmTypeOpts() const noexcept { return shmOpts; }
7173

7274
// Ratio is a number of parts of the total cache size to be allocated for this
7375
// tier. E.g. if X is a total cache size, Yi are ratios specified for memory
@@ -76,10 +78,8 @@ class MemoryTierCacheConfig {
7678
// tier is a half of the total cache size, set both tiers' ratios to 1.
7779
size_t ratio{1};
7880

79-
// Path to file for file system-backed memory tier
80-
// TODO: consider using variant<file, directory, NUMA> to support different
81-
// memory sources
82-
std::string path;
81+
// Options specific to shm type
82+
ShmTypeOpts shmOpts;
8383

8484
private:
8585
// TODO: introduce a container for tier settings when adding support for

cachelib/allocator/tests/AllocatorMemoryTiersTest.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ namespace tests {
2222

2323
using LruAllocatorMemoryTiersTest = AllocatorMemoryTiersTest<LruAllocator>;
2424

25+
// TODO(MEMORY_TIER): add more tests with different eviction policies
2526
TEST_F(LruAllocatorMemoryTiersTest, MultiTiers) { this->testMultiTiers(); }
2627

2728
} // end of namespace tests

cachelib/allocator/tests/BaseAllocatorTest.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1259,7 +1259,8 @@ class BaseAllocatorTest : public AllocatorTest<AllocatorT> {
12591259
this->testLruLength(alloc, poolId, sizes, keyLen, evictedKeys);
12601260
}
12611261

1262-
void testReaperShutDown(typename AllocatorT::Config::MemoryTierConfigs cfgs = {}) {
1262+
void testReaperShutDown(typename AllocatorT::Config::MemoryTierConfigs cfgs =
1263+
{MemoryTierCacheConfig::fromShm().setRatio(1)}) {
12631264
const size_t nSlabs = 20;
12641265
const size_t size = nSlabs * Slab::kSize;
12651266

@@ -1269,8 +1270,7 @@ class BaseAllocatorTest : public AllocatorTest<AllocatorT> {
12691270
config.setAccessConfig({8, 8});
12701271
config.enableCachePersistence(this->cacheDir_);
12711272
config.enableItemReaperInBackground(std::chrono::seconds(1), {});
1272-
if (cfgs.size())
1273-
config.configureMemoryTiers(cfgs);
1273+
config.configureMemoryTiers(cfgs);
12741274
std::vector<typename AllocatorT::Key> keys;
12751275
{
12761276
AllocatorT alloc(AllocatorT::SharedMemNew, config);

cachelib/allocator/tests/MemoryTiersTest.cpp

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,8 @@ class MemoryTiersTest: public AllocatorTest<Allocator> {
6060

6161
for(auto i = 0; i < configs.size(); ++i) {
6262
auto tierSize = configs[i].calculateTierSize(actualConfig.getCacheSize(), sum_ratios);
63-
EXPECT_EQ(configs[i].getPath(), expectedPaths[i]);
63+
auto &opt = std::get<FileShmSegmentOpts>(configs[i].getShmTypeOpts());
64+
EXPECT_EQ(opt.path, expectedPaths[i]);
6465
EXPECT_GT(tierSize, 0);
6566
if (configs[i].getRatio() && (i < configs.size() - 1)) {
6667
EXPECT_EQ(tierSize, partition_size * configs[i].getRatio());
@@ -99,43 +100,43 @@ class MemoryTiersTest: public AllocatorTest<Allocator> {
99100
using LruMemoryTiersTest = MemoryTiersTest<LruAllocator>;
100101

101102
TEST_F(LruMemoryTiersTest, TestValid1TierPmemRatioConfig) {
102-
LruAllocatorConfig cfg = createTestCacheConfig({defaultPmemPath}).validate();
103+
LruAllocatorConfig cfg = createTestCacheConfig({defaultPmemPath});
103104
basicCheck(cfg);
104105
}
105106

106107
TEST_F(LruMemoryTiersTest, TestValid1TierDaxRatioConfig) {
107-
LruAllocatorConfig cfg = createTestCacheConfig({defaultDaxPath}).validate();
108+
LruAllocatorConfig cfg = createTestCacheConfig({defaultDaxPath});
108109
basicCheck(cfg, {defaultDaxPath});
109110
}
110111

111112
TEST_F(LruMemoryTiersTest, TestValid2TierDaxPmemConfig) {
112113
LruAllocatorConfig cfg = createTestCacheConfig({defaultDaxPath, defaultPmemPath},
113-
{1, 1}).validate();
114+
{1, 1});
114115
basicCheck(cfg, {defaultDaxPath, defaultPmemPath});
115116
}
116117

117118
TEST_F(LruMemoryTiersTest, TestValid2TierDaxPmemRatioConfig) {
118119
LruAllocatorConfig cfg = createTestCacheConfig({defaultDaxPath, defaultPmemPath},
119-
{5, 2}).validate();
120+
{5, 2});
120121
basicCheck(cfg, {defaultDaxPath, defaultPmemPath});
121122
}
122123

123124
TEST_F(LruMemoryTiersTest, TestInvalid2TierConfigPosixShmNotSet) {
124125
LruAllocatorConfig cfg = createTestCacheConfig({defaultDaxPath, defaultPmemPath},
125126
{1, 1},
126-
/* setPosixShm */ false).validate();
127+
/* setPosixShm */ false);
127128
}
128129

129130
TEST_F(LruMemoryTiersTest, TestInvalid2TierConfigNumberOfPartitionsTooLarge) {
130131
EXPECT_THROW(createTestCacheConfig({defaultDaxPath, defaultPmemPath},
131-
{defaultTotalCacheSize, 1}),
132+
{defaultTotalCacheSize, 1}).validate(),
132133
std::invalid_argument);
133134
}
134135

135136
TEST_F(LruMemoryTiersTest, TestInvalid2TierConfigRatiosCacheSizeNotSet) {
136137
EXPECT_THROW(createTestCacheConfig({defaultDaxPath, defaultPmemPath},
137138
{1, 1},
138-
/* setPosixShm */ true, /* cacheSize */ 0),
139+
/* setPosixShm */ true, /* cacheSize */ 0).validate(),
139140
std::invalid_argument);
140141
}
141142

cachelib/cachebench/util/CacheConfig.h

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -46,15 +46,22 @@ struct MemoryTierConfig : public JSONConfig {
4646

4747
explicit MemoryTierConfig(const folly::dynamic& configJson);
4848
MemoryTierCacheConfig getMemoryTierCacheConfig() {
49-
if (file.empty()) {
50-
throw std::invalid_argument("Please specify valid path to memory mapped file.");
51-
}
52-
MemoryTierCacheConfig config = MemoryTierCacheConfig::fromFile(file).setRatio(ratio);
49+
MemoryTierCacheConfig config = memoryTierCacheConfigFromSource();
50+
config.setRatio(ratio);
5351
return config;
5452
}
5553

5654
std::string file{""};
5755
size_t ratio{0};
56+
57+
private:
58+
MemoryTierCacheConfig memoryTierCacheConfigFromSource() {
59+
if (file.empty()) {
60+
return MemoryTierCacheConfig::fromShm();
61+
} else {
62+
return MemoryTierCacheConfig::fromFile(file);
63+
}
64+
}
5865
};
5966

6067
struct CacheConfig : public JSONConfig {

0 commit comments

Comments
 (0)