(kudu) branch master updated: KUDU-613: Micro-optimization in Erase()

[al...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

alexey pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git


The following commit(s) were added to refs/heads/master by this push:
     new 5151270ea KUDU-613: Micro-optimization in Erase()
5151270ea is described below

commit 5151270ea9d88bc40ad5ca2344c97f1eed5fadc9
Author: Mahesh Reddy <mr...@cloudera.com>
AuthorDate: Tue Dec 17 19:43:36 2024 -0500

    KUDU-613: Micro-optimization in Erase()
    
    Prior to this patch, the Erase() function on the shard level
    did not have a return value. The Erase() function on the
    ShardPair level called both shard-level functions.
    
    This patch changes the shard-level Erase() function to return
    a bool to indicate whether an entry was erased. This allows
    the ShardPair level function to only call Erase() on the
    protected shard when the probationary shard does not find and
    erase the appropriate entry.
    
    Change-Id: I9941b671f00d5f4dd71b59a77b45f9cdbd79b367
    Reviewed-on: http://gerrit.cloudera.org:8080/22229
    Reviewed-by: Alexey Serbin <al...@apache.org>
    Tested-by: Alexey Serbin <al...@apache.org>
---
 src/kudu/util/slru_cache.cc | 12 +++++++++---
 src/kudu/util/slru_cache.h  |  3 ++-
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/src/kudu/util/slru_cache.cc b/src/kudu/util/slru_cache.cc
index e319f8541..021f97c9a 100644
--- a/src/kudu/util/slru_cache.cc
+++ b/src/kudu/util/slru_cache.cc
@@ -27,6 +27,7 @@
 #include <gflags/gflags_declare.h>
 #include <glog/logging.h>
 
+#include "kudu/gutil/basictypes.h"
 #include "kudu/gutil/bits.h"
 #include "kudu/gutil/hash/city.h"
 #include "kudu/gutil/port.h"
@@ -354,15 +355,18 @@ void SLRUCacheShard<Segment::kProtected>::ReInsert(SLRUHandle* handle,
 }
 
 template<Segment segment>
-void SLRUCacheShard<segment>::Erase(const Slice& key, uint32_t hash, SLRUHandle** free_entry) {
+bool SLRUCacheShard<segment>::Erase(const Slice& key, uint32_t hash, SLRUHandle** free_entry) {
+  bool erased = false;
   SLRUHandle* e = table_.Remove(key, hash);
   if (e != nullptr) {
     RL_Remove(e);
+    erased = true;
     // Free entry if this is the last reference.
     if (Unref(e)) {
       *free_entry = e;
     }
   }
+  return erased;
 }
 
 template<>
@@ -515,8 +519,10 @@ void SLRUCacheShardPair::Erase(const Slice& key, uint32_t hash) {
   SLRUHandle* protected_free_entry = nullptr;
   {
     std::lock_guard l(mutex_);
-    probationary_shard_.Erase(key, hash, &probationary_free_entry);
-    protected_shard_.Erase(key, hash, &protected_free_entry);
+    // Only call Erase for protected shard if the entry was not erased from the probationary shard.
+    if (!probationary_shard_.Erase(key, hash, &probationary_free_entry)) {
+      ignore_result(protected_shard_.Erase(key, hash, &protected_free_entry));
+    }
   }
 
   // Free entry outside lock for performance reasons.
diff --git a/src/kudu/util/slru_cache.h b/src/kudu/util/slru_cache.h
index 5e60efe65..502ab273c 100644
--- a/src/kudu/util/slru_cache.h
+++ b/src/kudu/util/slru_cache.h
@@ -130,7 +130,8 @@ class SLRUCacheShard {
   // Reduces the entry's ref by one, frees the entry if no refs are remaining.
   void Release(Handle* handle);
   // Removes entry from shard, returns it to be freed if no refs are remaining.
-  void Erase(const Slice& key, uint32_t hash, SLRUHandle** free_entry);
+  // Returns a bool indicating whether the entry was erased.
+  bool Erase(const Slice& key, uint32_t hash, SLRUHandle** free_entry);
   // Like Erase, but underlying entry is not freed.
   // Necessary when upgrading entry to protected segment.
   void SoftErase(const Slice& key, uint32_t hash);