logging: use bitset class for categories

... rather than integer bitmasks such as (1 << 28). This removes the limit of 32 logging categories. Adds `AtomicBitSet` class, which is similar to `std::bitset` but supports atomic (lock-free) operation, which is important for performance. Co-authored-by: Anthony Towns <[email protected]>
LarryRuane · Jul 26, 2024 · 79e7f94 · 79e7f94
1 parent 30cef53
commit 79e7f94
Show file tree

Hide file tree

Showing 6 changed files with 138 additions and 47 deletions.
diff --git a/src/interfaces/node.h b/src/interfaces/node.h
@@ -7,6 +7,7 @@
 
 #include <common/settings.h>
 #include <consensus/amount.h>          // For CAmount
+#include <logging.h>                   // For BCLog
 #include <net.h>                       // For NodeId
 #include <net_types.h>                 // For banmap_t
 #include <netaddress.h>                // For Network
@@ -84,7 +85,7 @@ class Node
     virtual int getExitStatus() = 0;
 
     // Get log flags.
-    virtual uint32_t getLogCategories() = 0;
+    virtual const BCLog::LogFlagsBitset& getLogCategories() = 0;
 
     //! Initialize app dependencies.
     virtual bool baseInitialize() = 0;

diff --git a/src/logging.cpp b/src/logging.cpp
@@ -5,6 +5,7 @@
 
 #include <logging.h>
 #include <memusage.h>
+#include <util/check.h>
 #include <util/fs.h>
 #include <util/string.h>
 #include <util/threadnames.h>
@@ -103,7 +104,6 @@ void BCLog::Logger::DisconnectTestLogger()
     m_cur_buffer_memusage = 0;
     m_buffer_lines_discarded = 0;
     m_msgs_before_open.clear();
-
 }
 
 void BCLog::Logger::DisableLogging()
@@ -120,7 +120,16 @@ void BCLog::Logger::DisableLogging()
 
 void BCLog::Logger::EnableCategory(BCLog::LogFlags flag)
 {
-    m_categories |= flag;
+    switch (flag) {
+    case BCLog::NONE:
+        return;
+    case BCLog::ALL:
+        // with no argument, set() sets all the bits to true
+        m_categories.set();
+        return;
+    default: break;
+    }
+    if (Assume(flag < BCLog::ALL)) m_categories.set(flag);
 }
 
 bool BCLog::Logger::EnableCategory(std::string_view str)
@@ -133,7 +142,16 @@ bool BCLog::Logger::EnableCategory(std::string_view str)
 
 void BCLog::Logger::DisableCategory(BCLog::LogFlags flag)
 {
-    m_categories &= ~flag;
+    switch (flag) {
+    case BCLog::NONE:
+        return;
+    case BCLog::ALL:
+        // set all the category flags to false
+        m_categories.reset();
+        return;
+    default: break;
+    }
+    if (Assume(flag < BCLog::ALL)) m_categories.reset(flag);
 }
 
 bool BCLog::Logger::DisableCategory(std::string_view str)
@@ -146,7 +164,10 @@ bool BCLog::Logger::DisableCategory(std::string_view str)
 
 bool BCLog::Logger::WillLogCategory(BCLog::LogFlags category) const
 {
-    return (m_categories.load(std::memory_order_relaxed) & category) != 0;
+    if (Assume(category < BCLog::ALL)) {
+        return m_categories.test(category);
+    }
+    return false;
 }
 
 bool BCLog::Logger::WillLogCategoryLevel(BCLog::LogFlags category, BCLog::Level level) const
@@ -164,7 +185,7 @@ bool BCLog::Logger::WillLogCategoryLevel(BCLog::LogFlags category, BCLog::Level
 
 bool BCLog::Logger::DefaultShrinkDebugFile() const
 {
-    return m_categories == BCLog::NONE;
+    return m_categories.is_none();
 }
 
 static const std::map<std::string, BCLog::LogFlags, std::less<>> LOG_CATEGORIES_BY_STR{

diff --git a/src/logging.h b/src/logging.h
@@ -13,6 +13,7 @@
 #include <util/time.h>
 
 #include <atomic>
+#include <bitset>
 #include <cstdint>
 #include <functional>
 #include <list>
@@ -37,41 +38,108 @@ struct LogCategory {
 };
 
 namespace BCLog {
-    enum LogFlags : uint32_t {
-        NONE        = 0,
-        NET         = (1 <<  0),
-        TOR         = (1 <<  1),
-        MEMPOOL     = (1 <<  2),
-        HTTP        = (1 <<  3),
-        BENCH       = (1 <<  4),
-        ZMQ         = (1 <<  5),
-        WALLETDB    = (1 <<  6),
-        RPC         = (1 <<  7),
-        ESTIMATEFEE = (1 <<  8),
-        ADDRMAN     = (1 <<  9),
-        SELECTCOINS = (1 << 10),
-        REINDEX     = (1 << 11),
-        CMPCTBLOCK  = (1 << 12),
-        RAND        = (1 << 13),
-        PRUNE       = (1 << 14),
-        PROXY       = (1 << 15),
-        MEMPOOLREJ  = (1 << 16),
-        LIBEVENT    = (1 << 17),
-        COINDB      = (1 << 18),
-        QT          = (1 << 19),
-        LEVELDB     = (1 << 20),
-        VALIDATION  = (1 << 21),
-        I2P         = (1 << 22),
-        IPC         = (1 << 23),
+    enum LogFlags {
+        NET,
+        TOR,
+        MEMPOOL,
+        HTTP,
+        BENCH,
+        ZMQ,
+        WALLETDB,
+        RPC,
+        ESTIMATEFEE,
+        ADDRMAN,
+        SELECTCOINS,
+        REINDEX,
+        CMPCTBLOCK,
+        RAND,
+        PRUNE,
+        PROXY,
+        MEMPOOLREJ,
+        LIBEVENT,
+        COINDB,
+        QT,
+        LEVELDB,
+        VALIDATION,
+        I2P,
+        IPC,
 #ifdef DEBUG_LOCKCONTENTION
-        LOCK        = (1 << 24),
+        LOCK,
 #endif
-        BLOCKSTORAGE = (1 << 25),
-        TXRECONCILIATION = (1 << 26),
-        SCAN        = (1 << 27),
-        TXPACKAGES  = (1 << 28),
-        ALL         = ~(uint32_t)0,
+        BLOCKSTORAGE,
+        TXRECONCILIATION,
+        SCAN,
+        TXPACKAGES,
+        // Add new entries before this line.
+
+        // The following have no representation in m_categories:
+        ALL, // this is also the size of the bitset
+        NONE,
     };
+
+    template<typename FlagType, size_t BITS, typename T=uint64_t>
+    class AtomicBitSet {
+    private:
+        // bits in a single T (an integer type)
+        static constexpr size_t BITS_PER_T = 8 * sizeof(T);
+
+        // number of Ts needed for BITS bits (round up)
+        static constexpr size_t NT = (BITS + BITS_PER_T - 1) / BITS_PER_T;
+
+        std::atomic<T> bits[NT]{0};
+
+    public:
+        AtomicBitSet() = default;
+        AtomicBitSet(FlagType f) { set(f); }
+
+        AtomicBitSet(const AtomicBitSet&) = delete;
+        AtomicBitSet(AtomicBitSet&&) = delete;
+        AtomicBitSet& operator=(const AtomicBitSet&) = delete;
+        AtomicBitSet& operator=(AtomicBitSet&&) = delete;
+        ~AtomicBitSet() = default;
+
+        constexpr size_t size() const { return BITS; }
+
+        bool is_any() const
+        {
+            for (const auto& i : bits) {
+                if (i > 0) return true;
+            }
+            return false;
+        }
+        bool is_none() const { return !is_any(); }
+        void set()
+        {
+            for (size_t i = 0; i < NT; ++i) {
+                bits[i] = ~T{0};
+            }
+        }
+        void reset()
+        {
+            for (size_t i = 0; i < NT; ++i) {
+                bits[i] = T{0};
+            }
+        }
+        void set(FlagType f)
+        {
+            if (f < 0 || f >= BITS) return;
+            bits[f/BITS_PER_T] |= (T{1} << (f % BITS_PER_T));
+        }
+        void reset(FlagType f)
+        {
+            if (f < 0 || f >= BITS) return;
+            bits[f/BITS_PER_T] &= ~(T{1} << (f % BITS_PER_T));
+        }
+        bool test(FlagType f) const
+        {
+            if (f < 0 || f >= BITS) return false;
+            const T i{bits[f/BITS_PER_T].load(std::memory_order_relaxed)};
+            return i & (T{1} << (f % BITS_PER_T));
+
+        }
+    };
+    using LogFlagsBitset = AtomicBitSet<LogFlags, LogFlags::ALL, uint32_t>;
+
     enum class Level {
         Trace = 0, // High-volume or detailed logging for development/debugging
         Debug,     // Reasonably noisy logging, but still usable in production
@@ -119,7 +187,7 @@ namespace BCLog {
         std::atomic<Level> m_log_level{DEFAULT_LOG_LEVEL};
 
         /** Log categories bitfield. */
-        std::atomic<uint32_t> m_categories{0};
+        LogFlagsBitset m_categories;
 
         void FormatLogStrInPlace(std::string& str, LogFlags category, Level level, std::string_view source_file, int source_line, std::string_view logging_function, std::string_view threadname, SystemClock::time_point now, std::chrono::seconds mocktime) const;
 
@@ -204,7 +272,10 @@ namespace BCLog {
         void SetLogLevel(Level level) { m_log_level = level; }
         bool SetLogLevel(std::string_view level);
 
-        uint32_t GetCategoryMask() const { return m_categories.load(); }
+        const LogFlagsBitset& GetCategoryMask() const
+        {
+            return m_categories;
+        }
 
         void EnableCategory(LogFlags flag);
         bool EnableCategory(std::string_view str);

diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp
@@ -100,7 +100,7 @@ class NodeImpl : public Node
     void initParameterInteraction() override { InitParameterInteraction(args()); }
     bilingual_str getWarnings() override { return Join(Assert(m_context->warnings)->GetMessages(), Untranslated("<hr />")); }
     int getExitStatus() override { return Assert(m_context)->exit_status.load(); }
-    uint32_t getLogCategories() override { return LogInstance().GetCategoryMask(); }
+    const BCLog::LogFlagsBitset& getLogCategories() override { return LogInstance().GetCategoryMask(); }
     bool baseInitialize() override
     {
         if (!AppInitBasicSetup(args(), Assert(context())->exit_status)) return false;

diff --git a/src/qt/transactiondesc.cpp b/src/qt/transactiondesc.cpp
@@ -336,8 +336,7 @@ QString TransactionDesc::toHTML(interfaces::Node& node, interfaces::Wallet& wall
     //
     // Debug view
     //
-    if (node.getLogCategories() != BCLog::NONE)
-    {
+    if (node.getLogCategories().is_any()) {
         strHTML += "<hr><br>" + tr("Debug information") + "<br><br>";
         for (const CTxIn& txin : wtx.tx->vin)
             if(wallet.txinIsMine(txin))

diff --git a/src/rpc/node.cpp b/src/rpc/node.cpp
@@ -245,18 +245,17 @@ static RPCHelpMan logging()
                 },
         [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
 {
-    uint32_t original_log_categories = LogInstance().GetCategoryMask();
+    const bool original_log_libevent = LogInstance().GetCategoryMask().test(BCLog::LIBEVENT);
     if (request.params[0].isArray()) {
         EnableOrDisableLogCategories(request.params[0], true);
     }
     if (request.params[1].isArray()) {
         EnableOrDisableLogCategories(request.params[1], false);
     }
-    uint32_t updated_log_categories = LogInstance().GetCategoryMask();
-    uint32_t changed_log_categories = original_log_categories ^ updated_log_categories;
 
     // Update libevent logging if BCLog::LIBEVENT has changed.
-    if (changed_log_categories & BCLog::LIBEVENT) {
+    const bool updated_log_libevent = LogInstance().GetCategoryMask().test(BCLog::LIBEVENT);
+    if (original_log_libevent != updated_log_libevent) {
         UpdateHTTPServerLogging(LogInstance().WillLogCategory(BCLog::LIBEVENT));
     }