Midpoint insertions in ClockCache (facebook#10305)

Summary: When an element is first inserted into the ClockCache, it is now assigned either medium or high clock priority, depending on whether its cache priority is low or high, respectively. This is a variant of LRUCache's midpoint insertions. The main difference is that LRUCache can specify the allocated capacity for high-priority elements via the ``high_pri_pool_ratio`` parameter. Contrarily, in ClockCache, low- and high-priority elements compete for all cache slots, and one group can take over the other (of course, it takes more low-priority insertions to push out high-priority elements). However, just as LRUCache, ClockCache provides the following guarantee: a high-priority element will not be evicted before a low-priority element that was inserted earlier in time. Pull Request resolved: facebook#10305 Test Plan: ``make -j24 check`` Reviewed By: pdillinger Differential Revision: D37607787 Pulled By: guidotag fbshipit-source-id: 24d9f2523d2f4e6415e7f0029cc061fa275c2040
Zagitta · Jul 7, 2022 · c277aeb · c277aeb
1 parent 8debfe2
commit c277aeb
Show file tree

Hide file tree

Showing 3 changed files with 59 additions and 30 deletions.
diff --git a/cache/clock_cache.cc b/cache/clock_cache.cc
@@ -109,7 +109,7 @@ void ClockHandleTable::Assign(int slot, ClockHandle* h) {
   dst->displacements = disp;
   dst->SetIsVisible(true);
   dst->SetIsElement(true);
-  dst->SetPriority(ClockHandle::ClockPriority::NONE);
+  dst->SetClockPriority(ClockHandle::ClockPriority::NONE);
   occupancy_++;
 }
 
@@ -243,14 +243,18 @@ void ClockCacheShard::ApplyToSomeEntries(
 
 void ClockCacheShard::ClockRemove(ClockHandle* h) {
   assert(h->IsInClockList());
-  h->SetPriority(ClockHandle::ClockPriority::NONE);
+  h->SetClockPriority(ClockHandle::ClockPriority::NONE);
   assert(clock_usage_ >= h->total_charge);
   clock_usage_ -= h->total_charge;
 }
 
 void ClockCacheShard::ClockInsert(ClockHandle* h) {
   assert(!h->IsInClockList());
-  h->SetPriority(ClockHandle::ClockPriority::HIGH);
+  bool is_high_priority =
+      h->HasHit() || h->GetCachePriority() == Cache::Priority::HIGH;
+  h->SetClockPriority(static_cast<ClockHandle::ClockPriority>(
+      is_high_priority * ClockHandle::ClockPriority::HIGH +
+      (1 - is_high_priority) * ClockHandle::ClockPriority::MEDIUM));
   clock_usage_ += h->total_charge;
 }
 
@@ -264,15 +268,15 @@ void ClockCacheShard::EvictFromClock(size_t charge,
     if (!old->IsInClockList()) {
       continue;
     }
-    if (old->GetPriority() == ClockHandle::ClockPriority::LOW) {
+    if (old->GetClockPriority() == ClockHandle::ClockPriority::LOW) {
       ClockRemove(old);
       table_.Remove(old);
       assert(usage_ >= old->total_charge);
       usage_ -= old->total_charge;
       deleted->push_back(*old);
       return;
     }
-    old->DecreasePriority();
+    old->DecreaseClockPriority();
   }
 }
 
@@ -319,7 +323,7 @@ void ClockCacheShard::SetStrictCapacityLimit(bool strict_capacity_limit) {
 Status ClockCacheShard::Insert(const Slice& key, uint32_t hash, void* value,
                                size_t charge, Cache::DeleterFn deleter,
                                Cache::Handle** handle,
-                               Cache::Priority /*priority*/) {
+                               Cache::Priority priority) {
   if (key.size() != kCacheKeySize) {
     return Status::NotSupported("ClockCache only supports key size " +
                                 std::to_string(kCacheKeySize) + "B");
@@ -330,6 +334,7 @@ Status ClockCacheShard::Insert(const Slice& key, uint32_t hash, void* value,
   tmp.deleter = deleter;
   tmp.hash = hash;
   tmp.CalcTotalCharge(charge, metadata_charge_policy_);
+  tmp.SetCachePriority(priority);
   for (int i = 0; i < kCacheKeySize; i++) {
     tmp.key_data[i] = key.data()[i];
   }
@@ -415,6 +420,7 @@ Cache::Handle* ClockCacheShard::Lookup(const Slice& key, uint32_t /* hash */) {
         ClockRemove(h);
       }
       h->Ref();
+      h->SetHit();
     }
   }
   return reinterpret_cast<Cache::Handle*>(h);

diff --git a/cache/clock_cache.h b/cache/clock_cache.h
@@ -65,6 +65,8 @@ struct ClockHandle {
   static constexpr int kIsVisibleOffset = 0;
   static constexpr int kIsElementOffset = 1;
   static constexpr int kClockPriorityOffset = 2;
+  static constexpr int kIsHitOffset = 4;
+  static constexpr int kCachePriorityOffset = 5;
 
   enum Flags : uint8_t {
     // Whether the handle is visible to Lookups.
@@ -74,6 +76,9 @@ struct ClockHandle {
     // Clock priorities. Represents how close a handle is from
     // being evictable.
     CLOCK_PRIORITY = (3 << kClockPriorityOffset),
+    // Whether the handle has been looked up after its insertion.
+    HAS_HIT = (1 << kIsHitOffset),
+    CACHE_PRIORITY = (1 << kCachePriorityOffset),
   };
   uint8_t flags;
 
@@ -82,7 +87,7 @@ struct ClockHandle {
     LOW = (1 << kClockPriorityOffset),   // Immediately evictable.
     MEDIUM = (2 << kClockPriorityOffset),
     HIGH = (3 << kClockPriorityOffset)
-    // Priority is CLOCK_NONE if and only if
+    // Priority is NONE if and only if
     // (i) the handle is not an element, or
     // (ii) the handle is an element but it is being referenced.
   };
@@ -102,7 +107,8 @@ struct ClockHandle {
     flags = 0;
     SetIsVisible(false);
     SetIsElement(false);
-    SetPriority(ClockPriority::NONE);
+    SetClockPriority(ClockPriority::NONE);
+    SetCachePriority(Cache::Priority::LOW);
     displacements = 0;
     key_data.fill(0);
   }
@@ -142,20 +148,36 @@ struct ClockHandle {
     }
   }
 
-  ClockPriority GetPriority() const {
-    return static_cast<ClockPriority>(flags & Flags::CLOCK_PRIORITY);
-  }
+  bool HasHit() const { return flags & HAS_HIT; }
+
+  void SetHit() { flags |= HAS_HIT; }
 
   bool IsInClockList() const {
-    return GetPriority() != ClockHandle::ClockPriority::NONE;
+    return GetClockPriority() != ClockHandle::ClockPriority::NONE;
+  }
+
+  Cache::Priority GetCachePriority() const {
+    return static_cast<Cache::Priority>(flags & CACHE_PRIORITY);
+  }
+
+  void SetCachePriority(Cache::Priority priority) {
+    if (priority == Cache::Priority::HIGH) {
+      flags |= Flags::CACHE_PRIORITY;
+    } else {
+      flags &= ~Flags::CACHE_PRIORITY;
+    }
+  }
+
+  ClockPriority GetClockPriority() const {
+    return static_cast<ClockPriority>(flags & Flags::CLOCK_PRIORITY);
   }
 
-  void SetPriority(ClockPriority priority) {
+  void SetClockPriority(ClockPriority priority) {
     flags &= ~Flags::CLOCK_PRIORITY;
     flags |= priority;
   }
 
-  void DecreasePriority() {
+  void DecreaseClockPriority() {
     uint8_t p = static_cast<uint8_t>(flags & Flags::CLOCK_PRIORITY) >>
                 kClockPriorityOffset;
     assert(p > 0);

diff --git a/cache/lru_cache_test.cc b/cache/lru_cache_test.cc
@@ -448,28 +448,29 @@ TEST_F(ClockCacheTest, Validate) {
 
 TEST_F(ClockCacheTest, ClockPriorityTest) {
   clock_cache::ClockHandle handle;
-  EXPECT_EQ(handle.GetPriority(),
+  EXPECT_EQ(handle.GetClockPriority(),
             clock_cache::ClockHandle::ClockPriority::NONE);
-  handle.SetPriority(clock_cache::ClockHandle::ClockPriority::HIGH);
-  EXPECT_EQ(handle.GetPriority(),
+  handle.SetClockPriority(clock_cache::ClockHandle::ClockPriority::HIGH);
+  EXPECT_EQ(handle.GetClockPriority(),
             clock_cache::ClockHandle::ClockPriority::HIGH);
-  handle.DecreasePriority();
-  EXPECT_EQ(handle.GetPriority(),
+  handle.DecreaseClockPriority();
+  EXPECT_EQ(handle.GetClockPriority(),
             clock_cache::ClockHandle::ClockPriority::MEDIUM);
-  handle.DecreasePriority();
-  EXPECT_EQ(handle.GetPriority(), clock_cache::ClockHandle::ClockPriority::LOW);
-  handle.SetPriority(clock_cache::ClockHandle::ClockPriority::MEDIUM);
-  EXPECT_EQ(handle.GetPriority(),
+  handle.DecreaseClockPriority();
+  EXPECT_EQ(handle.GetClockPriority(),
+            clock_cache::ClockHandle::ClockPriority::LOW);
+  handle.SetClockPriority(clock_cache::ClockHandle::ClockPriority::MEDIUM);
+  EXPECT_EQ(handle.GetClockPriority(),
             clock_cache::ClockHandle::ClockPriority::MEDIUM);
-  handle.SetPriority(clock_cache::ClockHandle::ClockPriority::NONE);
-  EXPECT_EQ(handle.GetPriority(),
+  handle.SetClockPriority(clock_cache::ClockHandle::ClockPriority::NONE);
+  EXPECT_EQ(handle.GetClockPriority(),
             clock_cache::ClockHandle::ClockPriority::NONE);
-  handle.SetPriority(clock_cache::ClockHandle::ClockPriority::MEDIUM);
-  EXPECT_EQ(handle.GetPriority(),
+  handle.SetClockPriority(clock_cache::ClockHandle::ClockPriority::MEDIUM);
+  EXPECT_EQ(handle.GetClockPriority(),
             clock_cache::ClockHandle::ClockPriority::MEDIUM);
-  handle.DecreasePriority();
-  handle.DecreasePriority();
-  EXPECT_EQ(handle.GetPriority(),
+  handle.DecreaseClockPriority();
+  handle.DecreaseClockPriority();
+  EXPECT_EQ(handle.GetClockPriority(),
             clock_cache::ClockHandle::ClockPriority::NONE);
 }