From d3b3af90343b7671231afd7dff87e87ff86d31d7 Mon Sep 17 00:00:00 2001 From: Vasil Dimov Date: Thu, 14 Dec 2023 12:23:36 +0100 Subject: [PATCH 1/2] log: deduplicate category names and improve logging.cpp The code in `logging.cpp` needs to: * Get the category name given the flag (e.g. `BCLog::PRUNE` -> `"prune"`) * Get the flag given the category name (e.g. `"prune"` -> `BCLog::PRUNE`) * Get the list of category names sorted in alphabetical order Achieve this by using the proper std containers. The result is * less code (this diff is +62 / -129) * faster code (to linear search and no copy+sort) * more maintainable code (the categories are no longer duplicated in `LogCategories[]` and `LogCategoryToStr()`) This behavior is preserved: `BCLog::NONE` -> `""` (lookup by `LogCategoryToStr()`) `""` -> `BCLog::ALL` (lookup by `GetLogCategory("")`) --- src/logging.cpp | 191 ++++++++++++++++-------------------------------- 1 file changed, 62 insertions(+), 129 deletions(-) diff --git a/src/logging.cpp b/src/logging.cpp index 42f100ded6e..2e3cffb8c18 100644 --- a/src/logging.cpp +++ b/src/logging.cpp @@ -9,9 +9,8 @@ #include #include -#include #include -#include +#include #include const char * const DEFAULT_DEBUGLOGFILE = "debug.log"; @@ -142,49 +141,58 @@ bool BCLog::Logger::DefaultShrinkDebugFile() const return m_categories == BCLog::NONE; } -struct CLogCategoryDesc { - BCLog::LogFlags flag; - std::string category; +static const std::map LOG_CATEGORIES_BY_STR{ + {"0", BCLog::NONE}, + {"", BCLog::NONE}, + {"net", BCLog::NET}, + {"tor", BCLog::TOR}, + {"mempool", BCLog::MEMPOOL}, + {"http", BCLog::HTTP}, + {"bench", BCLog::BENCH}, + {"zmq", BCLog::ZMQ}, + {"walletdb", BCLog::WALLETDB}, + {"rpc", BCLog::RPC}, + {"estimatefee", BCLog::ESTIMATEFEE}, + {"addrman", BCLog::ADDRMAN}, + {"selectcoins", BCLog::SELECTCOINS}, + {"reindex", BCLog::REINDEX}, + {"cmpctblock", BCLog::CMPCTBLOCK}, + {"rand", BCLog::RAND}, + {"prune", BCLog::PRUNE}, + {"proxy", BCLog::PROXY}, + {"mempoolrej", BCLog::MEMPOOLREJ}, + {"libevent", BCLog::LIBEVENT}, + {"coindb", BCLog::COINDB}, + {"qt", BCLog::QT}, + {"leveldb", BCLog::LEVELDB}, + {"validation", BCLog::VALIDATION}, + {"i2p", BCLog::I2P}, + {"ipc", BCLog::IPC}, +#ifdef DEBUG_LOCKCONTENTION + {"lock", BCLog::LOCK}, +#endif + {"util", BCLog::UTIL}, + {"blockstorage", BCLog::BLOCKSTORAGE}, + {"txreconciliation", BCLog::TXRECONCILIATION}, + {"scan", BCLog::SCAN}, + {"txpackages", BCLog::TXPACKAGES}, + {"1", BCLog::ALL}, + {"all", BCLog::ALL}, }; -const CLogCategoryDesc LogCategories[] = -{ - {BCLog::NONE, "0"}, - {BCLog::NONE, ""}, - {BCLog::NET, "net"}, - {BCLog::TOR, "tor"}, - {BCLog::MEMPOOL, "mempool"}, - {BCLog::HTTP, "http"}, - {BCLog::BENCH, "bench"}, - {BCLog::ZMQ, "zmq"}, - {BCLog::WALLETDB, "walletdb"}, - {BCLog::RPC, "rpc"}, - {BCLog::ESTIMATEFEE, "estimatefee"}, - {BCLog::ADDRMAN, "addrman"}, - {BCLog::SELECTCOINS, "selectcoins"}, - {BCLog::REINDEX, "reindex"}, - {BCLog::CMPCTBLOCK, "cmpctblock"}, - {BCLog::RAND, "rand"}, - {BCLog::PRUNE, "prune"}, - {BCLog::PROXY, "proxy"}, - {BCLog::MEMPOOLREJ, "mempoolrej"}, - {BCLog::LIBEVENT, "libevent"}, - {BCLog::COINDB, "coindb"}, - {BCLog::QT, "qt"}, - {BCLog::LEVELDB, "leveldb"}, - {BCLog::VALIDATION, "validation"}, - {BCLog::I2P, "i2p"}, - {BCLog::IPC, "ipc"}, -#ifdef DEBUG_LOCKCONTENTION - {BCLog::LOCK, "lock"}, -#endif - {BCLog::UTIL, "util"}, - {BCLog::BLOCKSTORAGE, "blockstorage"}, - {BCLog::TXRECONCILIATION, "txreconciliation"}, - {BCLog::SCAN, "scan"}, - {BCLog::TXPACKAGES, "txpackages"}, - {BCLog::ALL, "1"}, - {BCLog::ALL, "all"}, +static const std::unordered_map LOG_CATEGORIES_BY_FLAG{ + // Swap keys and values from LOG_CATEGORIES_BY_STR. + [](const std::map& in) { + std::unordered_map out; + for (const auto& [k, v] : in) { + switch (v) { + case BCLog::NONE: out.emplace(BCLog::NONE, ""); break; + case BCLog::ALL: out.emplace(BCLog::ALL, "all"); break; + default: out.emplace(v, k); + } + } + return out; + }(LOG_CATEGORIES_BY_STR) }; bool GetLogCategory(BCLog::LogFlags& flag, const std::string& str) @@ -193,11 +201,10 @@ bool GetLogCategory(BCLog::LogFlags& flag, const std::string& str) flag = BCLog::ALL; return true; } - for (const CLogCategoryDesc& category_desc : LogCategories) { - if (category_desc.category == str) { - flag = category_desc.flag; - return true; - } + auto it = LOG_CATEGORIES_BY_STR.find(str); + if (it != LOG_CATEGORIES_BY_STR.end()) { + flag = it->second; + return true; } return false; } @@ -221,76 +228,9 @@ std::string BCLog::Logger::LogLevelToStr(BCLog::Level level) std::string LogCategoryToStr(BCLog::LogFlags category) { - // Each log category string representation should sync with LogCategories - switch (category) { - case BCLog::LogFlags::NONE: - return ""; - case BCLog::LogFlags::NET: - return "net"; - case BCLog::LogFlags::TOR: - return "tor"; - case BCLog::LogFlags::MEMPOOL: - return "mempool"; - case BCLog::LogFlags::HTTP: - return "http"; - case BCLog::LogFlags::BENCH: - return "bench"; - case BCLog::LogFlags::ZMQ: - return "zmq"; - case BCLog::LogFlags::WALLETDB: - return "walletdb"; - case BCLog::LogFlags::RPC: - return "rpc"; - case BCLog::LogFlags::ESTIMATEFEE: - return "estimatefee"; - case BCLog::LogFlags::ADDRMAN: - return "addrman"; - case BCLog::LogFlags::SELECTCOINS: - return "selectcoins"; - case BCLog::LogFlags::REINDEX: - return "reindex"; - case BCLog::LogFlags::CMPCTBLOCK: - return "cmpctblock"; - case BCLog::LogFlags::RAND: - return "rand"; - case BCLog::LogFlags::PRUNE: - return "prune"; - case BCLog::LogFlags::PROXY: - return "proxy"; - case BCLog::LogFlags::MEMPOOLREJ: - return "mempoolrej"; - case BCLog::LogFlags::LIBEVENT: - return "libevent"; - case BCLog::LogFlags::COINDB: - return "coindb"; - case BCLog::LogFlags::QT: - return "qt"; - case BCLog::LogFlags::LEVELDB: - return "leveldb"; - case BCLog::LogFlags::VALIDATION: - return "validation"; - case BCLog::LogFlags::I2P: - return "i2p"; - case BCLog::LogFlags::IPC: - return "ipc"; -#ifdef DEBUG_LOCKCONTENTION - case BCLog::LogFlags::LOCK: - return "lock"; -#endif - case BCLog::LogFlags::UTIL: - return "util"; - case BCLog::LogFlags::BLOCKSTORAGE: - return "blockstorage"; - case BCLog::LogFlags::TXRECONCILIATION: - return "txreconciliation"; - case BCLog::LogFlags::SCAN: - return "scan"; - case BCLog::LogFlags::TXPACKAGES: - return "txpackages"; - case BCLog::LogFlags::ALL: - return "all"; - } - assert(false); + auto it = LOG_CATEGORIES_BY_FLAG.find(category); + assert(it != LOG_CATEGORIES_BY_FLAG.end()); + return it->second; } static std::optional GetLogLevel(const std::string& level_str) @@ -312,18 +252,11 @@ static std::optional GetLogLevel(const std::string& level_str) std::vector BCLog::Logger::LogCategoriesList() const { - // Sort log categories by alphabetical order. - std::array categories; - std::copy(std::begin(LogCategories), std::end(LogCategories), categories.begin()); - std::sort(categories.begin(), categories.end(), [](auto a, auto b) { return a.category < b.category; }); - std::vector ret; - for (const CLogCategoryDesc& category_desc : categories) { - if (category_desc.flag == BCLog::NONE || category_desc.flag == BCLog::ALL) continue; - LogCategory catActive; - catActive.category = category_desc.category; - catActive.active = WillLogCategory(category_desc.flag); - ret.push_back(catActive); + for (const auto& [category, flag] : LOG_CATEGORIES_BY_STR) { + if (flag != BCLog::NONE && flag != BCLog::ALL) { + ret.push_back(LogCategory{.category = category, .active = WillLogCategory(flag)}); + } } return ret; } From b0344c219a641b759fb0cc4f53afebe675b8ca27 Mon Sep 17 00:00:00 2001 From: Vasil Dimov Date: Sun, 11 Feb 2024 15:24:46 +0100 Subject: [PATCH 2/2] logging: remove unused BCLog::UTIL Suggested by: David Gumberg (https://github.com/bitcoin/bitcoin/pull/29415#discussion_r1485310634) --- src/logging.cpp | 1 - src/logging.h | 9 ++++----- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/src/logging.cpp b/src/logging.cpp index 2e3cffb8c18..578650f856e 100644 --- a/src/logging.cpp +++ b/src/logging.cpp @@ -171,7 +171,6 @@ static const std::map LOG_CATEGORIES_BY_STR{ #ifdef DEBUG_LOCKCONTENTION {"lock", BCLog::LOCK}, #endif - {"util", BCLog::UTIL}, {"blockstorage", BCLog::BLOCKSTORAGE}, {"txreconciliation", BCLog::TXRECONCILIATION}, {"scan", BCLog::SCAN}, diff --git a/src/logging.h b/src/logging.h index 525e0aec6d7..54af9d3a424 100644 --- a/src/logging.h +++ b/src/logging.h @@ -65,11 +65,10 @@ namespace BCLog { #ifdef DEBUG_LOCKCONTENTION LOCK = (1 << 24), #endif - UTIL = (1 << 25), - BLOCKSTORAGE = (1 << 26), - TXRECONCILIATION = (1 << 27), - SCAN = (1 << 28), - TXPACKAGES = (1 << 29), + BLOCKSTORAGE = (1 << 25), + TXRECONCILIATION = (1 << 26), + SCAN = (1 << 27), + TXPACKAGES = (1 << 28), ALL = ~(uint32_t)0, }; enum class Level {