polling vs. tasks options

Author: byrnedj

cachelib/allocator/BackgroundEvictor-inl.h

@@ -35,15 +35,31 @@ BackgroundEvictor<CacheT>::~BackgroundEvictor() { stop(std::chrono::seconds(0));
 
 template <typename CacheT>
 void BackgroundEvictor<CacheT>::work() {
-  try {
-    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);
+  if (strategy_->poll_) {
+    try {
+      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());
     }
-  } 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;
+      }
+
   }
 }
 

cachelib/allocator/BackgroundEvictor.h

@@ -17,6 +17,7 @@
 #pragma once
 
 #include <gtest/gtest_prod.h>
+#include <folly/concurrency/UnboundedQueue.h>
 
 #include "cachelib/allocator/CacheStats.h"
 #include "cachelib/common/PeriodicWorker.h"
@@ -53,7 +54,10 @@ class BackgroundEvictor : public PeriodicWorker {
                     unsigned int tid);
 
   ~BackgroundEvictor() override;
-
+  
+  void schedule(size_t pid, size_t cid) {
+      tasks_.enqueue(std::make_pair(pid,cid));
+  }
   BackgroundEvictorStats getStats() const noexcept;
 
  private:
@@ -64,6 +68,7 @@ class BackgroundEvictor : public PeriodicWorker {
   Cache& cache_;
   std::shared_ptr<BackgroundEvictorStrategy> strategy_;
   unsigned int tid_;
+  folly::UMPSCQueue<std::pair<size_t,size_t>,true> tasks_;
 
   // implements the actual logic of running the background evictor
   void work() override final;

cachelib/allocator/BackgroundEvictorStrategy.h

@@ -21,21 +21,14 @@
 namespace facebook {
 namespace cachelib {
 
-//struct RebalanceContext {
-//  // Victim and Receiver must belong to the same pool
-//  ClassId victimClassId{Slab::kInvalidClassId};
-//  ClassId receiverClassId{Slab::kInvalidClassId};
-//
-//  RebalanceContext() = default;
-//  RebalanceContext(ClassId victim, ClassId receiver)
-//      : victimClassId(victim), receiverClassId(receiver) {}
-//};
 
 // Base class for background eviction strategy.
 class BackgroundEvictorStrategy {
 
 public:
 
+  BackgroundEvictorStrategy(bool poll) : poll_{poll} {};
+
   virtual unsigned int calculateBatchSize(const CacheBase& cache,
                                        unsigned int tid,
                                        PoolId pid,
@@ -46,6 +39,8 @@ class BackgroundEvictorStrategy {
                                        PoolId pid,
                                        ClassId cid ) = 0;
 
+  // if we should poll every n ms or wait for tasks in queue
+  bool poll_{false};
 
 };
 

cachelib/allocator/BackgroundEvictorStrategy.old

@@ -0,0 +1,42 @@
+/*
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "cachelib/allocator/BackgroundEvictorStrategy.h"
+
+#include <folly/logging/xlog.h>
+
+namespace facebook {
+namespace cachelib {
+
+
+bool BackgroundEvictorStrategy::shouldEvict(const CacheBase& cache,
+                                       unsigned int tid,
+                                       PoolId pid,
+                                       ClassId cid ) {
+  return true;
+}
+
+
+unsigned int BackgroundEvictorStrategy::calculateBatchSize(const CacheBase& cache,
+                                       unsigned int tid,
+                                       PoolId pid,
+                                       ClassId cid ) {
+  return 1;
+}
+
+
+} // namespace cachelib
+} // namespace facebook

cachelib/allocator/CacheAllocator-inl.h

@@ -382,6 +382,7 @@ 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);
   }
 
   ItemHandle handle;

cachelib/allocator/CacheAllocator.h

@@ -1789,11 +1789,6 @@ class CacheAllocator : public CacheBase {
       if(toReleaseHandle) {
         movedToNextTier = true;
       } else {
-        //toReleaseHandle =
-        //    itr->isChainedItem()
-        //        ? advanceIteratorAndTryEvictChainedItem(tid, pid, itr)
-        //        : advanceIteratorAndTryEvictRegularItem(tid, pid, mmContainer, itr);
-        //just bump the iterator and move on with life
         ++itr;
       }
 
@@ -1805,7 +1800,6 @@ class CacheAllocator : public CacheBase {
         }
         ++evictions;
 
-
         // we must be the last handle and for chained items, this will be
         // the parent.
         XDCHECK(toReleaseHandle.get() == candidate || candidate->isChainedItem());
@@ -1816,7 +1810,7 @@ class CacheAllocator : public CacheBase {
         // an already zero refcount, which will throw exception
         auto& itemToRelease = *toReleaseHandle.release();
 
-        // Decrementing the refcount because we want to recycle the item
+        // Decrementing the refcount because we will call releaseBackToAllocator
         const auto ref = decRef(itemToRelease);
         XDCHECK_EQ(0u, ref);
 

cachelib/allocator/FreeThresholdStrategy.cpp

@@ -23,8 +23,8 @@ namespace cachelib {
 
 
 
-FreeThresholdStrategy::FreeThresholdStrategy(double freeThreshold) 
-    : freeThreshold_(freeThreshold) {}
+FreeThresholdStrategy::FreeThresholdStrategy(double freeThreshold, bool poll) 
+    : BackgroundEvictorStrategy(poll), freeThreshold_(freeThreshold) {}
 
 bool FreeThresholdStrategy::shouldEvict(const CacheBase& cache,
                                        unsigned int tid,

cachelib/allocator/FreeThresholdStrategy.h

@@ -28,7 +28,7 @@ class FreeThresholdStrategy : public BackgroundEvictorStrategy {
 
 public:
 
-  FreeThresholdStrategy(double freeThreshold);
+  FreeThresholdStrategy(double freeThreshold, bool poll);
 
   ~FreeThresholdStrategy() {}
 

cachelib/allocator/KeepFreeStrategy.cpp

@@ -22,8 +22,8 @@ namespace facebook {
 namespace cachelib {
 
 
-KeepFreeStrategy::KeepFreeStrategy(unsigned int nKeepFree)
-   : nKeepFree_(nKeepFree) {} 
+KeepFreeStrategy::KeepFreeStrategy(unsigned int nKeepFree, bool poll)
+   : BackgroundEvictorStrategy(poll), nKeepFree_(nKeepFree) {} 
 
 bool KeepFreeStrategy::shouldEvict(const CacheBase& cache,
                                        unsigned int tid,

cachelib/allocator/KeepFreeStrategy.h

@@ -26,7 +26,7 @@ class KeepFreeStrategy : public BackgroundEvictorStrategy {
 
 public:
 
-  KeepFreeStrategy(unsigned int nKeepFree);
+  KeepFreeStrategy(unsigned int nKeepFree, bool poll);
 
   ~KeepFreeStrategy() {}