Few cleanups from D85438965

Summary: Clean up some comments & names to be more appropriate.  Also simplify `updateNumSlabsToAdvise()` to be a bit less verbose.

Reviewed By: pbhandar2

Differential Revision: D87228068

fbshipit-source-id: f4834bf0772e50368cf69511dfd65558d8cf565b

Author: Rob Lyerly

cachelib/allocator/CacheAllocator.h

@@ -5880,7 +5880,7 @@ bool CacheAllocator<CacheTrait>::startNewMemMonitor(
     MemoryMonitor::Config config,
     std::shared_ptr<RebalanceStrategy> strategy) {
   if (!startNewWorker("MemoryMonitor", memMonitor_, interval, *this, config,
-                      strategy, allocator_->getNumSlabsToAdvise())) {
+                      strategy, allocator_->getNumSlabsAdvised())) {
     return false;
   }
 

cachelib/allocator/MemoryMonitor.cpp

@@ -80,21 +80,17 @@ void MemoryMonitor::work() {
 
 void MemoryMonitor::updateNumSlabsToAdvise(int32_t numSlabs) {
   auto curNumSlabsToAdvise = numSlabsToAdvise_.load(std::memory_order_acquire);
+  size_t newNumSlabsToAdvise;
   do {
+    newNumSlabsToAdvise = curNumSlabsToAdvise + numSlabs;
     if (numSlabs < 0 &&
         static_cast<uint64_t>(-numSlabs) > curNumSlabsToAdvise) {
       throw std::invalid_argument(
           folly::sformat("Invalid numSlabs {} to update numSlabsToAdvise {}",
                          numSlabs, curNumSlabsToAdvise));
     }
-
-    auto newNumSlabsToAdvise = curNumSlabsToAdvise + numSlabs;
-    if (numSlabsToAdvise_.compare_exchange_weak(curNumSlabsToAdvise,
-                                                newNumSlabsToAdvise,
-                                                std::memory_order_acq_rel)) {
-      break;
-    }
-  } while (true);
+  } while (!numSlabsToAdvise_.compare_exchange_weak(
+      curNumSlabsToAdvise, newNumSlabsToAdvise, std::memory_order_acq_rel));
 }
 
 void MemoryMonitor::checkFreeMemory() {

cachelib/allocator/MemoryMonitor.h

@@ -218,10 +218,9 @@ class MemoryMonitor : public PeriodicWorker {
   }
 
  private:
-  // updates the number of slabs to be advised by numSlabs. This would
-  // either increment (to advise away more slabs) or decrement (to reclaim
-  // some of the previously advised away slabs) the numSlabsToAdvise_ by
-  // numSlabs.
+  // updates the number of slabs to be advised by numSlabs. This would either
+  // increment (to advise away more slabs) or decrement (to reclaim some of the
+  // previously advised away slabs) numSlabsToAdvise_ by numSlabs.
   //
   // @param numSlabs   number of slabs to add-to/subtract-from numSlabToAdvise_
   //

cachelib/allocator/memory/MemoryAllocator.h

@@ -738,9 +738,9 @@ class MemoryAllocator {
     return alignedSize;
   }
 
-  // calculate total number of slabs that can be advised away across all pools
-  size_t getNumSlabsToAdvise() const noexcept {
-    return memoryPoolManager_.getNumSlabsToAdvise();
+  // get the total number of slabs advised away across all pools
+  size_t getNumSlabsAdvised() const noexcept {
+    return memoryPoolManager_.getNumSlabsAdvised();
   }
 
   // calculate the number of slabs to be advised/reclaimed in each pool

cachelib/allocator/memory/MemoryPoolManager.cpp

@@ -66,7 +66,7 @@ MemoryPoolManager::MemoryPoolManager(
     poolsByName_.insert(kv);
   }
 
-  auto slabsAdvised = getNumSlabsToAdvise();
+  auto slabsAdvised = getNumSlabsAdvised();
   auto slabAllocAdvised = slabAlloc_.numSlabsReclaimable();
   if (slabsAdvised != slabAllocAdvised) {
     throw std::logic_error(folly::sformat(
@@ -76,7 +76,7 @@ MemoryPoolManager::MemoryPoolManager(
   }
 }
 
-size_t MemoryPoolManager::getNumSlabsToAdvise() const noexcept {
+size_t MemoryPoolManager::getNumSlabsAdvised() const noexcept {
   size_t slabsAdvised = 0;
   auto maxPools = nextPoolId_.load(std::memory_order_acquire);
   for (auto i = 0; i < maxPools; i++) {

cachelib/allocator/memory/MemoryPoolManager.h

@@ -152,8 +152,8 @@ class MemoryPoolManager {
     return getRemainingSizeLocked();
   }
 
-  // calculate total number of slabs that can be advised away across all pools
-  size_t getNumSlabsToAdvise() const noexcept;
+  // get the total number of slabs advised away across all pools
+  size_t getNumSlabsAdvised() const noexcept;
 
   // return total memory currently advised away
   size_t getAdvisedMemorySize() const noexcept {
@@ -167,7 +167,8 @@ class MemoryPoolManager {
 
   // calculate the number of slabs to be advised/reclaimed in each pool
   //
-  // @param poolIds    list of pools to process
+  // @param numSlabsToAdvise  total number of slabs to be advised/reclaimed
+  // @param poolIds           list of pools to process
   //
   // @return   vector of pairs with first value as poolId and second value
   //           the number of slabs to advise or number of slabs to reclaim