Add MemoryTierCacheConfig::fromShm()

igchor · igchor · commit 6b4f46bd8552 · 2021-11-10T16:17:11.000-05:00
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)..
diff --git a/cachelib/allocator/CacheAllocator-inl.h b/cachelib/allocator/CacheAllocator-inl.h
@@ -27,14 +27,16 @@ CacheAllocator<CacheTrait>::CacheAllocator(Config config)
     : memoryTierConfigs(config.getMemoryTierConfigs()),
       isOnShm_{config.memMonitoringEnabled()},
       config_(config.validate()),
-      tempShm_(isOnShm_ ? std::make_unique<TempShmMapping>(config_.size)
+      tempShm_(isOnShm_ ? std::make_unique<TempShmMapping>(
+                            config_.getCacheSize())
                         : nullptr),
       allocator_(isOnShm_ ? std::make_unique<MemoryAllocator>(
                                 getAllocatorConfig(config_),
                                 tempShm_->getAddr(),
-                                config_.size)
+                                config_.getCacheSize())
                           : std::make_unique<MemoryAllocator>(
-                                getAllocatorConfig(config_), config_.size)),
+                                getAllocatorConfig(config_),
+                                config_.getCacheSize())),
       compactCacheManager_(std::make_unique<CCacheManager>(*allocator_)),
       compressor_(createPtrCompressor()),
       accessContainer_(std::make_unique<AccessContainer>(
@@ -51,7 +53,8 @@ CacheAllocator<CacheTrait>::CacheAllocator(Config config)
       nvmCacheState_{config_.cacheDir, config_.isNvmCacheEncryptionEnabled(),
                      config_.isNvmCacheTruncateAllocSizeEnabled()} {
   // TODO(MEMORY_TIER)
-  if (memoryTierConfigs.size()) {
+  if (std::holds_alternative<FileShmSegmentOpts>(
+      memoryTierConfigs[0].getShmTypeOpts())) {
     throw std::runtime_error(
       "Using custom memory tier is only supported for Shared Memory.");
   }
@@ -165,16 +168,7 @@ ShmSegmentOpts CacheAllocator<CacheTrait>::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<CacheTrait>::createNewMemoryAllocator() {
   return std::make_unique<MemoryAllocator>(
       getAllocatorConfig(config_),
       shmManager_
-          ->createShm(detail::kShmCacheName, config_.size,
+          ->createShm(detail::kShmCacheName, config_.getCacheSize(),
                       config_.slabMemoryBaseAddr, createShmCacheOpts())
           .addr,
-      config_.size);
+      config_.getCacheSize());
 }
 
 template <typename CacheTrait>
@@ -199,7 +193,7 @@ CacheAllocator<CacheTrait>::restoreMemoryAllocator() {
       shmManager_
           ->attachShm(detail::kShmCacheName, config_.slabMemoryBaseAddr,
           createShmCacheOpts()).addr,
-      config_.size,
+      config_.getCacheSize(),
       config_.disableFullCoredump);
 }
 
@@ -2216,7 +2210,7 @@ PoolEvictionAgeStats CacheAllocator<CacheTrait>::getPoolEvictionAgeStats(
 template <typename CacheTrait>
 CacheMetadata CacheAllocator<CacheTrait>::getCacheMetadata() const noexcept {
   return CacheMetadata{kCachelibVersion, kCacheRamFormatVersion,
-                       kCacheNvmFormatVersion, config_.size};
+                       kCacheNvmFormatVersion, config_.getCacheSize()};
 }
 
 template <typename CacheTrait>
diff --git 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<std::string, std::string>& configMap,
       const std::map<std::string, std::string>& configMapToMerge,
@@ -576,6 +583,8 @@ CacheAllocatorConfig<T>& CacheAllocatorConfig<T>::setCacheName(
 
 template <typename T>
 CacheAllocatorConfig<T>& CacheAllocatorConfig<T>::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<T>& CacheAllocatorConfig<T>::enableItemReaperInBackground(
 template <typename T>
 CacheAllocatorConfig<T>& CacheAllocatorConfig<T>::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 <typename T>
-const typename CacheAllocatorConfig<T>::MemoryTierConfigs& CacheAllocatorConfig<T>::getMemoryTierConfigs() {
-  return memoryTierConfigs;
+typename CacheAllocatorConfig<T>::MemoryTierConfigs
+CacheAllocatorConfig<T>::getMemoryTierConfigs() const {
+  MemoryTierConfigs config = memoryTierConfigs;
+  size_t sum_ratios = 0;
+
+  for (auto &tier_config: config) {
+    if (auto *v = std::get_if<PosixSysVSegmentOpts>(&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 <typename T>
@@ -997,6 +1000,46 @@ CacheAllocatorConfig<T>& CacheAllocatorConfig<T>::setNvmAdmissionMinTTL(
   return *this;
 }
 
+template <typename T>
+size_t CacheAllocatorConfig<T>::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 <typename T>
+void CacheAllocatorConfig<T>::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 <typename T>
 const CacheAllocatorConfig<T>& CacheAllocatorConfig<T>::validate() const {
   // we can track tail hits only if MMType is MM2Q
@@ -1018,6 +1061,23 @@ const CacheAllocatorConfig<T>& CacheAllocatorConfig<T>::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
@@ -18,6 +18,8 @@
 
 #include <string>
 
+#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<file, directory, NUMA> 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
@@ -22,6 +22,7 @@ namespace tests {
 
 using LruAllocatorMemoryTiersTest = AllocatorMemoryTiersTest<LruAllocator>;
 
+// 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
@@ -1140,7 +1140,8 @@ class BaseAllocatorTest : public AllocatorTest<AllocatorT> {
     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<AllocatorT> {
     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<typename AllocatorT::Key> keys;
     {
       AllocatorT alloc(AllocatorT::SharedMemNew, config);
diff --git a/cachelib/allocator/tests/MemoryTiersTest.cpp b/cachelib/allocator/tests/MemoryTiersTest.cpp
diff --git a/cachelib/shm/ShmCommon.h b/cachelib/shm/ShmCommon.h
