Simplify BG evictor code and add possibility to wakeup it

Author: igchor

cachelib/allocator/BackgroundEvictor-inl.h

@@ -27,67 +27,61 @@ BackgroundEvictor<CacheT>::BackgroundEvictor(Cache& cache,
     : cache_(cache),
       strategy_(strategy),
       tid_(tid) {
-
 }
 
 template <typename CacheT>
 BackgroundEvictor<CacheT>::~BackgroundEvictor() { stop(std::chrono::seconds(0)); }
 
 template <typename CacheT>
 void BackgroundEvictor<CacheT>::work() {
-  if (strategy_->poll_) {
-    try {
+  try {
+    if (!tasks_.empty()) {
+      while (!tasks_.empty()) {
+        auto [pid, cid] = tasks_.dequeue();
+        auto batch = strategy_->calculateBatchSize(cache_, tid_, pid, cid);
+        auto evicted = BackgroundEvictorAPIWrapper<CacheT>::traverseAndEvictItems(cache_,
+                tid_,pid,cid,batch);
+        numEvictedItemsFromSchedule_.fetch_add(1, std::memory_order_relaxed);
+        runCount_.fetch_add(1, std::memory_order_relaxed);
+      }
+    } else {
       for (const auto pid : cache_.getRegularPoolIds()) {
         //check if the pool is full - probably should be if tier is full
         if (cache_.getPoolByTid(pid,tid_).allSlabsAllocated()) {
           checkAndRun(pid);
         }
       }
-    } catch (const std::exception& ex) {
-      XLOGF(ERR, "BackgroundEvictor interrupted due to exception: {}", ex.what());
     }
-  } else {
-      //when an eviction for a given pid,cid at tier 0 is triggered this will be run
-      while (1) {
-        std::pair p = tasks_.dequeue();
-        unsigned int pid = p.first;
-        unsigned int cid = p.second;
-        unsigned int batch = strategy_->calculateBatchSize(cache_,tid_,pid,cid);
-        //try evicting BATCH items from the class in order to reach free target
-        unsigned int evicted = 
-            BackgroundEvictorAPIWrapper<CacheT>::traverseAndEvictItems(cache_,
-                tid_,pid,cid,batch);
-        runCount_ = runCount_ + 1;
-      }
-
+  } catch (const std::exception& ex) {
+    XLOGF(ERR, "BackgroundEvictor interrupted due to exception: {}", ex.what());
   }
 }
 
-
 // Look for classes that exceed the target memory capacity
 // and return those for eviction
 template <typename CacheT>
-void BackgroundEvictor<CacheT>::checkAndRun(PoolId pid) {
-    
+void BackgroundEvictor<CacheT>::checkAndRun(PoolId pid) {    
   const auto& mpStats = cache_.getPoolByTid(pid,tid_).getStats();
   for (auto& cid : mpStats.classIds) {
-    if (strategy_->shouldEvict(cache_,tid_,pid,cid)) {
-        unsigned int batch = strategy_->calculateBatchSize(cache_,tid_,pid,cid);
-        //try evicting BATCH items from the class in order to reach free target
-        unsigned int evicted = 
-            BackgroundEvictorAPIWrapper<CacheT>::traverseAndEvictItems(cache_,
-                tid_,pid,cid,batch);
-        numEvictedItems_ += evicted;
-    }
+      auto batch = strategy_->calculateBatchSize(cache_,tid_,pid,cid);
+      if (!batch)
+        continue;
+
+      //try evicting BATCH items from the class in order to reach free target
+      auto evicted =
+          BackgroundEvictorAPIWrapper<CacheT>::traverseAndEvictItems(cache_,
+              tid_,pid,cid,batch);
+      numEvictedItems_.fetch_add(evicted, std::memory_order_relaxed);
   }
-  runCount_ = runCount_ + 1;
+  runCount_.fetch_add(1, std::memory_order_relaxed);
 }
 
 template <typename CacheT>
 BackgroundEvictorStats BackgroundEvictor<CacheT>::getStats() const noexcept {
   BackgroundEvictorStats stats;
   stats.numEvictedItems = numEvictedItems_.load(std::memory_order_relaxed);
   stats.numTraversals = runCount_.load(std::memory_order_relaxed);
+  stats.numEvictedItemsFromSchedule = numEvictedItemsFromSchedule_.load(std::memory_order_relaxed);
   return stats;
 }
 

cachelib/allocator/BackgroundEvictor.h

@@ -32,8 +32,8 @@ namespace cachelib {
 template <typename C>
 struct BackgroundEvictorAPIWrapper {
 
-  static unsigned int traverseAndEvictItems(C& cache,
-          unsigned int tid, unsigned int pid, unsigned int cid, unsigned int batch) {
+  static size_t traverseAndEvictItems(C& cache,
+          unsigned int tid, unsigned int pid, unsigned int cid, size_t batch) {
     return cache.traverseAndEvictItems(tid,pid,cid,batch);
   }
 };
@@ -75,6 +75,7 @@ class BackgroundEvictor : public PeriodicWorker {
   void checkAndRun(PoolId pid);
 
   std::atomic<uint64_t> numEvictedItems_{0};
+  std::atomic<uint64_t> numEvictedItemsFromSchedule_{0};
   std::atomic<uint64_t> runCount_{0};
 };
 } // namespace cachelib

cachelib/allocator/BackgroundEvictorStrategy.h

@@ -26,22 +26,10 @@ namespace cachelib {
 class BackgroundEvictorStrategy {
 
 public:
-
-  BackgroundEvictorStrategy(bool poll) : poll_{poll} {};
-
-  virtual unsigned int calculateBatchSize(const CacheBase& cache,
-                                       unsigned int tid,
-                                       PoolId pid,
-                                       ClassId cid ) = 0;
-
-  virtual bool shouldEvict(const CacheBase& cache,
+  virtual size_t calculateBatchSize(const CacheBase& cache,
                                        unsigned int tid,
                                        PoolId pid,
                                        ClassId cid ) = 0;
-
-  // if we should poll every n ms or wait for tasks in queue
-  bool poll_{false};
-
 };
 
 } // namespace cachelib

cachelib/allocator/BackgroundEvictorStrategy.old

@@ -382,7 +382,13 @@ CacheAllocator<CacheTrait>::allocateInternalTier(TierId tid,
   //       Should we support eviction between memory tiers (e.g. from DRAM to PMEM)?
   if (memory == nullptr && !config_.disableEviction) {
     memory = findEviction(tid, pid, cid);
-    backgroundEvictor_->schedule(pid,cid);
+
+    if (config_.scheduleEviction && backgroundEvictor_) {
+      backgroundEvictor_->schedule(pid,cid);
+      if (config_.wakeupBgEvictor) {
+        backgroundEvictor_->wakeUp();
+      }
+    }
   }
 
   ItemHandle handle;

cachelib/allocator/CacheAllocator-inl.h

@@ -1769,13 +1769,12 @@ class CacheAllocator : public CacheBase {
 
   // exposed for the background evictor to iterate through the memory and evict
   // in batch. This should improve insertion path for tiered memory config
-  unsigned int traverseAndEvictItems(unsigned int tid, unsigned int pid, unsigned int cid, unsigned int batch) {
-
+  size_t traverseAndEvictItems(unsigned int tid, unsigned int pid, unsigned int cid, size_t batch) {
     auto& mmContainer = getMMContainer(tid, pid, cid);
-    unsigned int evictions = 0;
+    size_t evictions = 0;
     auto itr = mmContainer.getEvictionIterator();
-    while (evictions < batch && itr) {
 
+    while (evictions < batch && itr) {
       Item* candidate = itr.get();
       if (candidate == NULL) {
           break;
@@ -1785,14 +1784,9 @@ class CacheAllocator : public CacheBase {
       // recycles the child we intend to.
 
       ItemHandle toReleaseHandle = tryEvictToNextMemoryTier(tid, pid, itr);
-      bool movedToNextTier = false;
-      if(toReleaseHandle) {
-        movedToNextTier = true;
-      } else {
+      if (!toReleaseHandle) {
         ++itr;
-      }
-
-      if (toReleaseHandle) {
+      } else {
         if (toReleaseHandle->hasChainedItem()) {
           (*stats_.chainedItemEvictions)[pid][cid].inc();
         } else {
@@ -1817,11 +1811,9 @@ class CacheAllocator : public CacheBase {
         // check if by releasing the item we intend to, we actually
         // recycle the candidate.
         const auto res = releaseBackToAllocator(itemToRelease, RemoveContext::kEviction,
-                              /* isNascent */ movedToNextTier);
+                              /* isNascent */ true);
         XDCHECK(res == ReleaseRes::kReleased);
-
       } 
-
     }
 
     // Invalidate iterator since later on we may use this mmContainer

cachelib/allocator/CacheAllocator.h

@@ -597,6 +597,12 @@ class CacheAllocatorConfig {
   // skip promote children items in chained when parent fail to promote
   bool skipPromoteChildrenWhenParentFailed{false};
 
+  // wakeupBg evictor each time there is no memory to allocate new item in topmost tier
+  bool wakeupBgEvictor {false};
+
+  // every time there is no space to allocate for particual cid,pid pass this information to BG worker
+  bool scheduleEviction {false};
+
   friend CacheT;
 
  private:

cachelib/allocator/CacheAllocatorConfig.h

@@ -287,9 +287,12 @@ struct ReaperStats {
 
 // Stats for background evictor
 struct BackgroundEvictorStats {
-  // the total number of items the reaper has visited.
+  // the number of items this worker evicted by looking at pools/classes stats
   uint64_t numEvictedItems{0};
 
+  // the number of items this worker evicted for pools/classes requested by schedule call
+  uint64_t numEvictedItemsFromSchedule{0};
+
   // number of times we went executed the thread //TODO: is this def correct?
   uint64_t numTraversals{0};
 

cachelib/allocator/CacheStats.h

@@ -23,39 +23,17 @@ namespace cachelib {
 
 
 
-FreeThresholdStrategy::FreeThresholdStrategy(double freeThreshold, bool poll) 
-    : BackgroundEvictorStrategy(poll), freeThreshold_(freeThreshold) {}
+FreeThresholdStrategy::FreeThresholdStrategy(double freeThreshold) 
+    : freeThreshold_(freeThreshold) {}
 
-bool FreeThresholdStrategy::shouldEvict(const CacheBase& cache,
+size_t FreeThresholdStrategy::calculateBatchSize(const CacheBase& cache,
                                        unsigned int tid,
                                        PoolId pid,
                                        ClassId cid ) {
-
-  const auto& mpStats = cache.getPoolByTid(pid,tid).getStats();
-  size_t allocSize = mpStats.acStats.at(cid).allocSize;
-  size_t totalMem = mpStats.acStats.at(cid).getTotalMemory() / allocSize; 
-  if (totalMem > 0) {
-    size_t freeMem = mpStats.acStats.at(cid).getTotalFreeMemory() / allocSize;
-    return ((double)freeMem / (double)totalMem) < freeThreshold_;
-  }
-  return false;
-
-}
-
-unsigned int FreeThresholdStrategy::calculateBatchSize(const CacheBase& cache,
-                                       unsigned int tid,
-                                       PoolId pid,
-                                       ClassId cid ) {
-  const auto& mpStats = cache.getPoolByTid(pid,tid).getStats();
-  size_t totalMem = mpStats.acStats.at(cid).getTotalMemory(); 
-  size_t freeMem = mpStats.acStats.at(cid).getTotalFreeMemory();
-
-  size_t targetMem = (freeThreshold_ * totalMem) - freeMem;
-  unsigned int batch = (targetMem / mpStats.acStats.at(cid).allocSize);
-
-  return batch;
+  const auto& mpStats = cache.getPoolByTid(pid,tid).getStats().acStats.at(cid);
+  size_t targetMem = (freeThreshold_ * mpStats.getTotalMemory()) - mpStats.getTotalFreeMemory();
+  return std::max(0UL, targetMem / mpStats.allocSize);
 }
 
-
 } // namespace cachelib
 } // namespace facebook

cachelib/allocator/FreeThresholdStrategy.cpp

@@ -27,26 +27,15 @@ namespace cachelib {
 class FreeThresholdStrategy : public BackgroundEvictorStrategy {
 
 public:
-
-  FreeThresholdStrategy(double freeThreshold, bool poll);
-
+  FreeThresholdStrategy(double freeThreshold);
   ~FreeThresholdStrategy() {}
 
-  unsigned int calculateBatchSize(const CacheBase& cache,
-                                       unsigned int tid,
-                                       PoolId pid,
-                                       ClassId cid );
-
-  bool shouldEvict(const CacheBase& cache,
+  size_t calculateBatchSize(const CacheBase& cache,
                                        unsigned int tid,
                                        PoolId pid,
                                        ClassId cid );
-
 private:
-
   double freeThreshold_;
-
-
 };
 
 } // namespace cachelib