Issue75 rebased

cachelib/allocator/CMakeLists.txt

@@ -121,6 +121,7 @@ if (BUILD_TESTS)
   add_test (tests/MemoryTiersTest.cpp)
   add_test (tests/MultiAllocatorTest.cpp)
   add_test (tests/NvmAdmissionPolicyTest.cpp)
+  add_test (tests/CacheAllocatorConfigTest.cpp)
   add_test (nvmcache/tests/NvmItemTests.cpp)
   add_test (nvmcache/tests/InFlightPutsTest.cpp)
   add_test (nvmcache/tests/TombStoneTests.cpp)

cachelib/allocator/CacheAllocator-inl.h

@@ -202,10 +202,24 @@ ShmSegmentOpts CacheAllocator<CacheTrait>::createShmCacheOpts(TierId tid) {
   ShmSegmentOpts opts;
   opts.alignment = sizeof(Slab);
   opts.typeOpts = memoryTierConfigs[tid].getShmTypeOpts();
+  if (auto *v = std::get_if<PosixSysVSegmentOpts>(&opts.typeOpts)) {
+    v->usePosix = config_.usePosixShm;
+  }
 
   return opts;
 }
 
+template <typename CacheTrait>
+size_t CacheAllocator<CacheTrait>::memoryTierSize(TierId tid) const
+{
+  auto partitions = std::accumulate(memoryTierConfigs.begin(), memoryTierConfigs.end(), 0UL,
+  [](const size_t i, const MemoryTierCacheConfig& config){
+    return i + config.getRatio();
+  });
+
+  return memoryTierConfigs[tid].calculateTierSize(config_.getCacheSize(), partitions);
+}
+
 template <typename CacheTrait>
 std::unique_ptr<MemoryAllocator>
 CacheAllocator<CacheTrait>::createNewMemoryAllocator(TierId tid) {
@@ -216,7 +230,8 @@ CacheAllocator<CacheTrait>::createNewMemoryAllocator(TierId tid) {
                       config_.getCacheSize(), config_.slabMemoryBaseAddr,
                       createShmCacheOpts(tid))
           .addr,
-      memoryTierConfigs[tid].getSize());
+          memoryTierSize(tid)
+      );
 }
 
 template <typename CacheTrait>
@@ -227,7 +242,7 @@ CacheAllocator<CacheTrait>::restoreMemoryAllocator(TierId tid) {
       shmManager_
           ->attachShm(detail::kShmCacheName + std::to_string(tid),
             config_.slabMemoryBaseAddr, createShmCacheOpts(tid)).addr,
-      memoryTierConfigs[tid].getSize(),
+      memoryTierSize(tid),
       config_.disableFullCoredump);
 }
 
@@ -2250,12 +2265,27 @@ PoolId CacheAllocator<CacheTrait>::addPool(
   folly::SharedMutex::WriteHolder w(poolsResizeAndRebalanceLock_);
 
   PoolId pid = 0;
-  auto tierConfigs = config_.getMemoryTierConfigs();
+  std::vector<size_t> tierPoolSizes;
+  const auto &tierConfigs = config_.getMemoryTierConfigs();
+  size_t totalCacheSize = 0;
+
   for (TierId tid = 0; tid < numTiers_; tid++) {
-    auto tierSizeRatio = static_cast<double>(
-        tierConfigs[tid].getSize()) / config_.getCacheSize();
-    auto tierPoolSize = static_cast<size_t>(tierSizeRatio * size);
-    auto res = allocator_[tid]->addPool(name, tierPoolSize, allocSizes, ensureProvisionable);
+    totalCacheSize += allocator_[tid]->getMemorySize();
+  }
+
+  for (TierId tid = 0; tid < numTiers_; tid++) {
+    auto tierSizeRatio =
+        static_cast<double>(allocator_[tid]->getMemorySize()) / totalCacheSize;
+    size_t tierPoolSize = static_cast<size_t>(tierSizeRatio * size);
+
+    tierPoolSizes.push_back(tierPoolSize);
+  }
+
+  for (TierId tid = 0; tid < numTiers_; tid++) {
+    // TODO: what if we manage to add pool only in one tier?
+    // we should probably remove that on failure
+    auto res = allocator_[tid]->addPool(
+        name, tierPoolSizes[tid], allocSizes, ensureProvisionable);
     XDCHECK(tid == 0 || res == pid);
     pid = res;
   }
@@ -2418,6 +2448,16 @@ const std::string CacheAllocator<CacheTrait>::getCacheName() const {
   return config_.cacheName;
 }
 
+template <typename CacheTrait>
+size_t CacheAllocator<CacheTrait>::getPoolSize(PoolId poolId) const {
+  size_t poolSize = 0;
+  for (auto& allocator: allocator_) {
+    const auto& pool = allocator->getPool(poolId);
+    poolSize += pool.getPoolSize();
+  }
+  return poolSize;
+}
+
 template <typename CacheTrait>
 PoolStats CacheAllocator<CacheTrait>::getPoolStats(PoolId poolId) const {
   const auto& pool = allocator_[currentTier()]->getPool(poolId);

cachelib/allocator/CacheAllocator.h

@@ -1045,6 +1045,9 @@ class CacheAllocator : public CacheBase {
   // get cache name
   const std::string getCacheName() const override final;
 
+  // combined pool size for all memory tiers
+  size_t getPoolSize(PoolId pid) const;
+
   // pool stats by pool id
   PoolStats getPoolStats(PoolId pid) const override final;
 
@@ -1578,6 +1581,8 @@ class CacheAllocator : public CacheBase {
   //         handle to the item. On failure an empty handle. 
   WriteHandle tryEvictToNextMemoryTier(Item& item);
 
+  size_t memoryTierSize(TierId tid) const;
+
   // Deserializer CacheAllocatorMetadata and verify the version
   //
   // @param  deserializer   Deserializer object

cachelib/allocator/CacheAllocatorConfig.h

@@ -28,6 +28,7 @@
 #include "cachelib/allocator/MemoryTierCacheConfig.h"
 #include "cachelib/allocator/MM2Q.h"
 #include "cachelib/allocator/MemoryMonitor.h"
+#include "cachelib/allocator/MemoryTierCacheConfig.h"
 #include "cachelib/allocator/NvmAdmissionPolicy.h"
 #include "cachelib/allocator/PoolOptimizeStrategy.h"
 #include "cachelib/allocator/RebalanceStrategy.h"
@@ -205,15 +206,15 @@ class CacheAllocatorConfig {
   // cachePersistence().
   CacheAllocatorConfig& usePosixForShm();
 
-  // 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
+  // Configures cache memory tiers. Each tier represents a cache region inside
+  // byte-addressable memory such as DRAM, Pmem, CXLmem.
+  // Accepts vector of MemoryTierCacheConfig. Each vector element describes
+  // configuration for a single memory cache tier. Tier sizes are specified as
+  // ratios, the number of parts of total cache size each tier would occupy.
   CacheAllocatorConfig& configureMemoryTiers(const MemoryTierConfigs& configs);
 
-  // Return vector of memory tier configs.
-  MemoryTierConfigs getMemoryTierConfigs() const;
+  // Return reference to MemoryTierCacheConfigs.
+  const MemoryTierConfigs& getMemoryTierConfigs() const;
 
   // This turns on a background worker that periodically scans through the
   // access container and look for expired items and remove them.
@@ -352,7 +353,7 @@ class CacheAllocatorConfig {
 
   const std::string& getCacheName() const noexcept { return cacheName; }
 
-  size_t getCacheSize() const noexcept;
+  size_t getCacheSize() const noexcept { return size; }
 
   bool isUsingPosixShm() const noexcept { return usePosixShm; }
 
@@ -367,13 +368,19 @@ class CacheAllocatorConfig {
   bool validateStrategy(
       const std::shared_ptr<PoolOptimizeStrategy>& strategy) const;
 
+  // check that memory tier ratios are set properly
+  const CacheAllocatorConfig& validateMemoryTiers() const;
+
   // @return a map representation of the configs
   std::map<std::string, std::string> serialize() const;
 
+  // The max number of memory cache tiers
+  inline static const size_t kMaxCacheMemoryTiers = 2;
+
   // Cache name for users to indentify their own cache.
   std::string cacheName{""};
 
-  // Amount of memory for this cache instance
+  // Amount of memory for this cache instance (sum of all memory tiers' sizes)
   size_t size = 1 * 1024 * 1024 * 1024;
 
   // Directory for shared memory related metadata
@@ -581,8 +588,6 @@ class CacheAllocatorConfig {
   friend CacheT;
 
  private:
-  void validateMemoryTiersWithSize(const MemoryTierConfigs&, size_t) const;
-
   // Configuration for memory tiers.
   MemoryTierConfigs memoryTierConfigs{
     {MemoryTierCacheConfig::fromShm().setRatio(1)}
@@ -606,8 +611,6 @@ 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) {
@@ -861,57 +864,24 @@ CacheAllocatorConfig<T>& CacheAllocatorConfig<T>::enableItemReaperInBackground(
 
 template <typename T>
 CacheAllocatorConfig<T>& CacheAllocatorConfig<T>::configureMemoryTiers(
-      const MemoryTierConfigs& config) {
-  if (!config.size()) {
-    throw std::invalid_argument("There must be at least one memory tier.");
+    const MemoryTierConfigs& config) {
+  if (config.size() > kMaxCacheMemoryTiers) {
+    throw std::invalid_argument(folly::sformat(
+        "Too many memory tiers. The number of supported tiers is {}.",
+        kMaxCacheMemoryTiers));
   }
-
-  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.");
-    }
+  if (!config.size()) {
+    throw std::invalid_argument(
+        "There must be at least one memory tier config.");
   }
-
-  validateMemoryTiersWithSize(config, this->size);
-
   memoryTierConfigs = config;
-
   return *this;
 }
 
 template <typename T>
-typename CacheAllocatorConfig<T>::MemoryTierConfigs
+const 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();
-  }
-
-  return config;
+  return memoryTierConfigs;
 }
 
 template <typename T>
@@ -1037,46 +1007,6 @@ CacheAllocatorConfig<T>::setSkipPromoteChildrenWhenParentFailed() {
   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
@@ -1101,23 +1031,7 @@ const CacheAllocatorConfig<T>& CacheAllocatorConfig<T>::validate() const {
         "It's not allowed to enable both RemoveCB and ItemDestructor.");
   }
 
-  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;
+  return validateMemoryTiers();
 }
 
 template <typename T>
@@ -1144,6 +1058,24 @@ bool CacheAllocatorConfig<T>::validateStrategy(
          (type != PoolOptimizeStrategy::MarginalHits || trackTailHits);
 }
 
+template <typename T>
+const CacheAllocatorConfig<T>& CacheAllocatorConfig<T>::validateMemoryTiers()
+    const {
+  size_t parts = 0;
+  for (const auto& tierConfig : memoryTierConfigs) {
+    if (!tierConfig.getRatio()) {
+      throw std::invalid_argument("Tier ratio must be an integer number >=1.");
+    }
+    parts += tierConfig.getRatio();
+  }
+
+  if (parts > size) {
+    throw std::invalid_argument(
+        "Sum of tier ratios must be less than total cache size.");
+  }
+  return *this;
+}
+
 template <typename T>
 std::map<std::string, std::string> CacheAllocatorConfig<T>::serialize() const {
   std::map<std::string, std::string> configMap;