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 <aj@erisian.com.au>

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 @@ public:
     virtual int getExitStatus() = 0;
 
     // Get log flags.
-    virtual uint32_t getLogCategories() = 0;
+    virtual const BCLog::LogCategoryMask& getLogCategories() = 0;
 
     //! Initialize app dependencies.
     virtual bool baseInitialize() = 0;

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.none();
 }
 
 static const std::map<std::string, BCLog::LogFlags, std::less<>> LOG_CATEGORIES_BY_STR{

src/logging.h

@@ -37,41 +37,111 @@ struct LogCategory {
 };
 
 namespace BCLog {
-    enum LogFlags : uint32_t {
+    enum LogFlags {
-        NONE        = 0,
+        NET,
-        NET         = (1 <<  0),
+        TOR,
-        TOR         = (1 <<  1),
+        MEMPOOL,
-        MEMPOOL     = (1 <<  2),
+        HTTP,
-        HTTP        = (1 <<  3),
+        BENCH,
-        BENCH       = (1 <<  4),
+        ZMQ,
-        ZMQ         = (1 <<  5),
+        WALLETDB,
-        WALLETDB    = (1 <<  6),
+        RPC,
-        RPC         = (1 <<  7),
+        ESTIMATEFEE,
-        ESTIMATEFEE = (1 <<  8),
+        ADDRMAN,
-        ADDRMAN     = (1 <<  9),
+        SELECTCOINS,
-        SELECTCOINS = (1 << 10),
+        REINDEX,
-        REINDEX     = (1 << 11),
+        CMPCTBLOCK,
-        CMPCTBLOCK  = (1 << 12),
+        RAND,
-        RAND        = (1 << 13),
+        PRUNE,
-        PRUNE       = (1 << 14),
+        PROXY,
-        PROXY       = (1 << 15),
+        MEMPOOLREJ,
-        MEMPOOLREJ  = (1 << 16),
+        LIBEVENT,
-        LIBEVENT    = (1 << 17),
+        COINDB,
-        COINDB      = (1 << 18),
+        QT,
-        QT          = (1 << 19),
+        LEVELDB,
-        LEVELDB     = (1 << 20),
+        VALIDATION,
-        VALIDATION  = (1 << 21),
+        I2P,
-        I2P         = (1 << 22),
+        IPC,
-        IPC         = (1 << 23),
 #ifdef DEBUG_LOCKCONTENTION
-        LOCK        = (1 << 24),
+        LOCK,
 #endif
-        BLOCKSTORAGE = (1 << 25),
+        BLOCKSTORAGE,
-        TXRECONCILIATION = (1 << 26),
+        TXRECONCILIATION,
-        SCAN        = (1 << 27),
+        SCAN,
-        TXPACKAGES  = (1 << 28),
+        TXPACKAGES,
-        ALL         = ~(uint32_t)0,
+        // Add new entries before this line, and also add a
+        // corresponding entry to LOG_CATEGORIES_BY_STR.
+
+        // 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>
+    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> m_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; }
+
+        // Although any() and none() use all bits to calculate their
+        // result, the bits may not be sampled at the same instant.
+        bool any() const
+        {
+            for (const auto& i : m_bits) {
+                if (i > 0) return true;
+            }
+            return false;
+        }
+        bool none() const { return !any(); }
+
+        void set()
+        {
+            for (size_t i = 0; i < NT; ++i) {
+                m_bits[i] = ~T{0};
+            }
+        }
+        void reset()
+        {
+            for (size_t i = 0; i < NT; ++i) {
+                m_bits[i] = T{0};
+            }
+        }
+        void set(FlagType f)
+        {
+            if (f < 0 || f >= BITS) return;
+            m_bits[f/BITS_PER_T] |= (T{1} << (f % BITS_PER_T));
+        }
+        void reset(FlagType f)
+        {
+            if (f < 0 || f >= BITS) return;
+            m_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{m_bits[f/BITS_PER_T].load(std::memory_order_relaxed)};
+            return i & (T{1} << (f % BITS_PER_T));
+        }
+    };
+    using LogCategoryMask = AtomicBitSet<LogFlags, LogFlags::ALL, uint64_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 +189,7 @@ namespace BCLog {
         std::atomic<Level> m_log_level{DEFAULT_LOG_LEVEL};
 
         /** Log categories bitfield. */
-        std::atomic<uint32_t> m_categories{BCLog::NONE};
+        LogCategoryMask m_categories{BCLog::NONE};
 
         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 +274,7 @@ 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 LogCategoryMask& GetCategoryMask() const { return m_categories; }
 
         void EnableCategory(LogFlags flag);
         bool EnableCategory(std::string_view str);

src/node/interfaces.cpp

@@ -100,7 +100,7 @@ public:
     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::LogCategoryMask& getLogCategories() override { return LogInstance().GetCategoryMask(); }
     bool baseInitialize() override
     {
         if (!AppInitBasicSetup(args(), Assert(context())->exit_status)) return false;

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().any()) {
-    {
         strHTML += "<hr><br>" + tr("Debug information") + "<br><br>";
         for (const CTxIn& txin : wtx.tx->vin)
             if(wallet.txinIsMine(txin))

src/rpc/node.cpp

@@ -244,18 +244,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));
     }