Issue75 rebased (#88)

* #75: Use actual tier sizes (rounded down to slab size and decreased by header size) when creating new memory pools

* Added getPoolSize method to calculate combined pool size for all tiers; added pool size validation to tests

* Explicitly specified type for totalCacheSize to avoid overflow

* Minor test change

* Reworked tests

* Minor change

* Deleted redundant tests

* Deleted unused constant

* First set of changes to cache configuration API to enable multi-tier caches (facebook#138)

Summary:
These changes introduce per-tier cache configuration required to implement features discussed here: facebook#102. These specific changes enable single DRAM tier configs only which are compatible with the current version of cachelib. Configuration API will be expanded as multi-tier changes in other parts of the library are introduced.

Pull Request resolved: facebook#138

Reviewed By: therealgymmy

Differential Revision: D36189766

Pulled By: jiayuebao

fbshipit-source-id: 947aa0cd800ea6accffc1b7b6b0c9693aa7fc0a5

Co-authored-by: Victoria McGrath <[email protected]>

Author: 2 people

cachelib/allocator/CacheAllocator-inl.h

@@ -125,6 +125,9 @@ 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;
 }
@@ -2508,6 +2511,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

@@ -1112,6 +1112,9 @@ class CacheAllocator : public CacheBase {
   // whether it is object-cache
   bool isObjectCache() const override final { return false; }
 
+  // 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;
 

cachelib/allocator/CacheAllocatorConfig.h

@@ -26,7 +26,6 @@
 #include <string>
 
 #include "cachelib/allocator/Cache.h"
-#include "cachelib/allocator/MemoryTierCacheConfig.h"
 #include "cachelib/allocator/MM2Q.h"
 #include "cachelib/allocator/MemoryMonitor.h"
 #include "cachelib/allocator/MemoryTierCacheConfig.h"
@@ -392,7 +391,6 @@ class CacheAllocatorConfig {
   std::map<std::string, std::string> serialize() const;
 
   // The max number of memory cache tiers
-  // TODO: increase this number when multi-tier configs are enabled
   inline static const size_t kMaxCacheMemoryTiers = 2;
 
   // Cache name for users to indentify their own cache.
@@ -901,11 +899,6 @@ CacheAllocatorConfig<T>& CacheAllocatorConfig<T>::configureMemoryTiers(
 template <typename T>
 const typename CacheAllocatorConfig<T>::MemoryTierConfigs&
 CacheAllocatorConfig<T>::getMemoryTierConfigs() const {
-  for (auto &tier_config: memoryTierConfigs) {
-    if (auto *v = std::get_if<PosixSysVSegmentOpts>(&tier_config.shmOpts)) {
-      const_cast<PosixSysVSegmentOpts*>(v)->usePosix = usePosixShm;
-    }
-  }
   return memoryTierConfigs;
 }
 

cachelib/allocator/MemoryTierCacheConfig.h

@@ -71,6 +71,7 @@ class MemoryTierCacheConfig {
 
   const ShmTypeOpts& getShmTypeOpts() const noexcept { return shmOpts; }
 
+private:
   // Ratio is a number of parts of the total cache size to be allocated for this
   // tier. E.g. if X is a total cache size, Yi are ratios specified for memory
   // tiers, and Y is the sum of all Yi, then size of the i-th tier
@@ -81,10 +82,6 @@ class MemoryTierCacheConfig {
   // Options specific to shm type
   ShmTypeOpts shmOpts;
 
- private:
-  // TODO: introduce a container for tier settings when adding support for
-  // file-mapped memory
-
   MemoryTierCacheConfig() = default;
 };
 } // namespace cachelib

cachelib/allocator/memory/SlabAllocator.cpp

@@ -40,7 +40,7 @@
 using namespace facebook::cachelib;
 
 namespace {
-size_t roundDownToSlabSize(size_t size) { return size - (size % sizeof(Slab)); }
+static inline size_t roundDownToSlabSize(size_t size) { return size - (size % sizeof(Slab)); }
 } // namespace
 
 // definitions to avoid ODR violation.

cachelib/allocator/tests/AllocatorMemoryTiersTest.cpp

@@ -23,7 +23,9 @@ namespace tests {
 using LruAllocatorMemoryTiersTest = AllocatorMemoryTiersTest<LruAllocator>;
 
 // TODO(MEMORY_TIER): add more tests with different eviction policies
-TEST_F(LruAllocatorMemoryTiersTest, MultiTiers) { this->testMultiTiers(); }
+TEST_F(LruAllocatorMemoryTiersTest, MultiTiersInvalid) { this->testMultiTiersInvalid(); }
+TEST_F(LruAllocatorMemoryTiersTest, MultiTiersValid) { this->testMultiTiersValid(); }
+TEST_F(LruAllocatorMemoryTiersTest, MultiTiersValidMixed) { this->testMultiTiersValidMixed(); }
 
 } // end of namespace tests
 } // end of namespace cachelib

cachelib/allocator/tests/AllocatorMemoryTiersTest.h

@@ -27,7 +27,7 @@ namespace tests {
 template <typename AllocatorT>
 class AllocatorMemoryTiersTest : public AllocatorTest<AllocatorT> {
  public:
-  void testMultiTiers() {
+  void testMultiTiersInvalid() {
     typename AllocatorT::Config config;
     config.setCacheSize(100 * Slab::kSize);
     config.configureMemoryTiers({
@@ -41,6 +41,48 @@ class AllocatorMemoryTiersTest : public AllocatorTest<AllocatorT> {
     ASSERT_THROW(std::make_unique<AllocatorT>(AllocatorT::SharedMemNew, config),
                  std::invalid_argument);
   }
+
+  void testMultiTiersValid() {
+    typename AllocatorT::Config config;
+    config.setCacheSize(100 * Slab::kSize);
+    config.enableCachePersistence("/tmp");
+    config.usePosixForShm();
+    config.configureMemoryTiers({
+        MemoryTierCacheConfig::fromFile("/tmp/a" + std::to_string(::getpid()))
+            .setRatio(1),
+        MemoryTierCacheConfig::fromFile("/tmp/b" + std::to_string(::getpid()))
+            .setRatio(1)
+    });
+
+    auto alloc = std::make_unique<AllocatorT>(AllocatorT::SharedMemNew, config);
+    ASSERT(alloc != nullptr);
+
+    auto pool = alloc->addPool("default", alloc->getCacheMemoryStats().cacheSize);
+    auto handle = alloc->allocate(pool, "key", std::string("value").size());
+    ASSERT(handle != nullptr);
+    ASSERT_NO_THROW(alloc->insertOrReplace(handle));
+  }
+
+  void testMultiTiersValidMixed() {
+    typename AllocatorT::Config config;
+    config.setCacheSize(100 * Slab::kSize);
+    config.enableCachePersistence("/tmp");
+    config.usePosixForShm();
+    config.configureMemoryTiers({
+        MemoryTierCacheConfig::fromShm()
+            .setRatio(1),
+        MemoryTierCacheConfig::fromFile("/tmp/b" + std::to_string(::getpid()))
+            .setRatio(1)
+    });
+
+    auto alloc = std::make_unique<AllocatorT>(AllocatorT::SharedMemNew, config);
+    ASSERT(alloc != nullptr);
+
+    auto pool = alloc->addPool("default", alloc->getCacheMemoryStats().cacheSize);
+    auto handle = alloc->allocate(pool, "key", std::string("value").size());
+    ASSERT(handle != nullptr);
+    ASSERT_NO_THROW(alloc->insertOrReplace(handle));
+  }
 };
 } // namespace tests
 } // namespace cachelib