use try_lock to delete the evicted entry from hashmap on a best effort basis

Author: Ravi Nagarjun Akella

include/sisl/cache/simple_cache.hpp

@@ -34,7 +34,6 @@ class SimpleCache {
     uint32_t m_per_value_size;
 
     static thread_local std::set< K > t_failed_keys;
-    static thread_local std::set< K > t_evicted_keys;
 
 public:
     SimpleCache(const std::shared_ptr< Evictor >& evictor, uint32_t num_buckets, uint32_t per_val_size,
@@ -46,7 +45,7 @@ class SimpleCache {
         m_record_family_id = m_evictor->register_record_family(std::move(evict_cb), [this](const CacheRecord& record) {
             V const value = reinterpret_cast<SingleEntryHashNode< V >*>(const_cast< CacheRecord* >(&record))->m_value;
             K key = m_key_extract_cb(value);
-            t_evicted_keys.insert(key);
+            return m_map.try_erase(key);
         });
     }
 
@@ -55,14 +54,6 @@ class SimpleCache {
     bool insert(const V& value) {
         K k = m_key_extract_cb(value);
         bool ret = m_map.insert(k, value);
-        // erase evicted keys
-        if (t_evicted_keys.size() > 0) {
-            for (const auto& key : t_evicted_keys) {
-                V out_val;
-                m_map.erase(key, out_val);
-            }
-            t_evicted_keys.clear();
-        }
         return ret;
     }
 
@@ -116,6 +107,4 @@ class SimpleCache {
 template < typename K, typename V >
 thread_local std::set< K > SimpleCache< K, V >::t_failed_keys;
 
-template < typename K, typename V >
-thread_local std::set< K > SimpleCache< K, V >::t_evicted_keys;
 } // namespace sisl

include/sisl/cache/simple_hashmap.hpp

@@ -73,6 +73,7 @@ class SimpleHashMap {
     bool upsert(const K& key, const V& value);
     bool get(const K& input_key, V& out_val);
     bool erase(const K& key, V& out_val);
+    bool try_erase(const K& key);
     bool update(const K& key, auto&& update_cb);
     bool upsert_or_delete(const K& key, auto&& update_or_delete_cb);
 
@@ -102,6 +103,7 @@ class SimpleHashMap {
 template < typename V >
 struct SingleEntryHashNode : public ValueEntryBase, public boost::intrusive::slist_base_hook<> {
     V m_value;
+    bool evicted{false};
     SingleEntryHashNode(const V& value) : m_value{value} {}
 };
 
@@ -184,27 +186,21 @@ class SimpleHashBucket {
 #ifndef GLOBAL_HASHSET_LOCK
         folly::SharedMutexWritePriority::WriteHolder holder(m_lock);
 #endif
-        SingleEntryHashNode< V >* n = nullptr;
-
-        auto it = m_list.begin();
-        for (auto itend{m_list.end()}; it != itend; ++it) {
-            const K k = SimpleHashMap< K, V >::extractor_cb()(it->m_value);
-            if (input_key > k) {
-                break;
-            } else if (input_key == k) {
-                n = &*it;
-                break;
-            }
-        }
+        return erase_unsafe(input_key, out_val, true /* call_access_cb */);
+    }
 
-        if (n) {
-            access_cb(*n, input_key, hash_op_t::DELETE);
-            out_val = n->m_value;
-            m_list.erase(it);
-            delete n;
-            return true;
+    bool try_erase(const K& input_key) {
+        V dummy_val;
+#ifndef GLOBAL_HASHSET_LOCK
+        if(m_lock.try_lock()) {
+            bool ret = erase_unsafe(input_key, dummy_val, false /* call_access_cb */);
+            m_lock.unlock();
+            return ret;
+        } else {
+            return false;
         }
-        return false;
+#endif
+        return erase_unsafe(input_key, dummy_val, false);
     }
 
     bool upsert_or_delete(const K& input_key, auto&& update_or_delete_cb) {
@@ -266,9 +262,33 @@ class SimpleHashBucket {
     static void access_cb(const SingleEntryHashNode< V >& node, const K& key, hash_op_t op) {
         SimpleHashMap< K, V >::call_access_cb((const ValueEntryBase&)node, key, op);
     }
+
+    bool erase_unsafe(const K& input_key, V& out_val, bool call_access_cb) {
+        SingleEntryHashNode< V >* n = nullptr;
+
+        auto it = m_list.begin();
+        for (auto itend{m_list.end()}; it != itend; ++it) {
+            const K k = SimpleHashMap< K, V >::extractor_cb()(it->m_value);
+            if (input_key > k) {
+                break;
+            } else if (input_key == k) {
+                n = &*it;
+                break;
+            }
+        }
+
+        if (n) {
+            if (call_access_cb) { access_cb(*n, input_key, hash_op_t::DELETE); }
+            out_val = n->m_value;
+            m_list.erase(it);
+            delete n;
+            return true;
+        }
+        return false;
+    }
 };
 
-///////////////////////////////////////////// RangeHashMap Definitions ///////////////////////////////////
+///////////////////////////////////////////// SimpleHashMap Definitions ///////////////////////////////////
 template < typename K, typename V >
 SimpleHashMap< K, V >::SimpleHashMap(uint32_t nBuckets, const key_extractor_cb_t< K, V >& extract_cb,
                                      key_access_cb_t< K > access_cb) :
@@ -317,6 +337,12 @@ bool SimpleHashMap< K, V >::erase(const K& key, V& out_val) {
     return get_bucket(key).erase(key, out_val);
 }
 
+template < typename K, typename V >
+bool SimpleHashMap< K, V >::try_erase(const K& key) {
+    set_current_instance(this);
+    return get_bucket(key).try_erase(key);
+} 
+
 /// This is a special atomic operation where user can insert_or_update_or_erase based on condition atomically. It
 /// performs differently based on certain conditions.
 ///

src/cache/tests/test_simple_cache.cpp

@@ -231,7 +231,6 @@ TEST(SimpleCacheSize, TriggerEvict) {
             ++cache_hits;
         }
     }
-    ASSERT_EQ(cache_hits, num_partitions * max_nodes_per_partition);
 }
 
 TEST(SimpleCacheSize, MultithreadedEviction) {