From: Mike Stepanek (mstepane) <mstepane@cisco.com>
Date: Fri, 17 Jan 2020 21:08:04 +0000 (+0000)
Subject: Merge pull request #1920 in SNORT/snort3 from ~SMINUT/snort3:host_cache_atomic_size... 
X-Git-Tag: 3.0.0-268~51
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=5ea345cca4a3c4122a615d8ce3218ee88f341d28;p=thirdparty%2Fsnort3.git

Merge pull request #1920 in SNORT/snort3 from ~SMINUT/snort3:host_cache_atomic_size to master

Squashed commit of the following:

commit 8e02a12362716ed4da4c267879e19eea5ebacfbb
Author: Silviu Minut <sminut@cisco.com>
Date:   Tue Jan 7 15:27:55 2020 -0500

    host_tracker: make current_size atomic to save some locks.
---

diff --git a/src/hash/lru_cache_shared.h b/src/hash/lru_cache_shared.h
index 155bee1ab..1c3e990c0 100644
--- a/src/hash/lru_cache_shared.h
+++ b/src/hash/lru_cache_shared.h
@@ -24,6 +24,7 @@
 // LruCacheShared -- Implements a thread-safe unordered map where the
 // least-recently-used (LRU) entries are removed once a fixed size is hit.
 
+#include <atomic>
 #include <cassert>
 #include <list>
 #include <memory>
@@ -142,7 +143,7 @@ protected:
     size_t max_size;   // Once max_size elements are in the cache, start to
                        // remove the least-recently-used elements.
 
-    size_t current_size;    // Number of entries currently in the cache.
+    std::atomic<size_t> current_size;// Number of entries currently in the cache.
 
     std::mutex cache_mutex;
     LruList list;  //  Contains key/data pairs. Maintains LRU order with
@@ -151,7 +152,11 @@ protected:
 
     struct LruCacheSharedStats stats;
 
-    // These get called only from within the LRU and assume the LRU is locked.
+    // The reason for these functions is to allow derived classes to do their
+    // size book keeping differently (e.g. host_cache). This effectively
+    // decouples the current_size variable from the actual size in memory,
+    // so these functions should only be called when something is actually
+    // added or removed from memory (e.g. in find_else_insert, remove, etc).
     virtual void increase_size()
     {
         current_size++;
diff --git a/src/host_tracker/host_cache.h b/src/host_tracker/host_cache.h
index 9060ac55f..70ed6edaf 100644
--- a/src/host_tracker/host_cache.h
+++ b/src/host_tracker/host_cache.h
@@ -60,7 +60,6 @@ public:
 
     size_t mem_size() override
     {
-        std::lock_guard<std::mutex> cache_lock(cache_mutex);
         return current_size;
     }
 
@@ -105,24 +104,25 @@ private:
     // In concrete terms, never have a standalone HostTracker object outside
     // the host cache add or remove stuff to itself, as that will incorrectly
     // change the current_size of the cache.
+
     void update(int size) override
     {
-        // Same idea as in LruCacheShared::remove(), use shared pointers
-        // to hold the pruned data until after the cache is unlocked.
-        // Do not change the order of data and cache_lock, as the data must
-        // self destruct after cache_lock.
-        std::list<Data> data;
-
-        std::lock_guard<std::mutex> cache_lock(cache_mutex);
-
-        if (size < 0)
-            assert( current_size >= (size_t) -size );
-        current_size += size;
-        if (current_size > max_size)
+        if ( size < 0 )
+        {
+            assert( current_size >= (size_t) -size);
+        }
+        if ( (current_size += size) > max_size )
+        {
+            // Same idea as in LruCacheShared::remove(), use shared pointers
+            // to hold the pruned data until after the cache is unlocked.
+            // Do not change the order of data and cache_lock, as the data must
+            // self destruct after cache_lock.
+            std::list<Data> data;
+            std::lock_guard<std::mutex> cache_lock(cache_mutex);
             LruBase::prune(data);
+        }
     }
 
-    // These get called only from within the LRU and assume the LRU is locked.
     void increase_size() override
     {
         current_size += mem_chunk;