Skip to content

Commit

Permalink
Merge bitcoin#29419: log: deduplicate category names and improve logg…
Browse files Browse the repository at this point in the history 
…ing.cpp

b0344c2 logging: remove unused BCLog::UTIL (Vasil Dimov)
d3b3af9 log: deduplicate category names and improve logging.cpp (Vasil Dimov)

Pull request description:

  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 (the diff of the first commit 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("")`)

  ---

  Also remove unused `BCLog::UTIL`.

  ---

  These changes (modulo the `BCLog::UTIL` removal) are part of bitcoin#29415 but they make sense on their own and would be good to have them, regardless of the fate of bitcoin#29415. Also, if this is merged, that would reduce the size of bitcoin#29415, thus the current standalone PR.

ACKs for top commit:
  davidgumberg:
    crACK bitcoin@b0344c2
  pinheadmz:
    ACK b0344c2
  ryanofsky:
    Code review ACK b0344c2. Nice cleanup! Having to maintain multiple copies of the same mapping seemed messy and a like a possible footgun. I checked old and new mappings in both directions and confirmed no behavior should be changing.

Tree-SHA512: 57f87a090932f9b33dc8e075d1855dba9b71a3243a0758511745483dec2d9c46d3b532eadab297e78164c9b7caba370986ee380696a45f0778a841082f8e21a7
  • Loading branch information
@ryanofsky
ryanofsky committed Apr 2, 2024
2 parents 6dabb31 + b0344c2 commit 3b987d0
Show file tree
Hide file tree
Showing 2 changed files with 65 additions and 134 deletions.
190 changes: 61 additions & 129 deletions src/logging.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,8 @@
#include <util/threadnames.h>
#include <util/time.h>

#include <algorithm>
#include <array>
#include <mutex>
#include <map>
#include <optional>

const char * const DEFAULT_DEBUGLOGFILE = "debug.log";
Expand Down Expand Up @@ -142,49 +141,57 @@ bool BCLog::Logger::DefaultShrinkDebugFile() const
return m_categories == BCLog::NONE;
}

struct CLogCategoryDesc {
BCLog::LogFlags flag;
std::string category;
};

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"},
static const std::map<std::string, BCLog::LogFlags> 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
{BCLog::LOCK, "lock"},
{"lock", BCLog::LOCK},
#endif
{BCLog::UTIL, "util"},
{BCLog::BLOCKSTORAGE, "blockstorage"},
{BCLog::TXRECONCILIATION, "txreconciliation"},
{BCLog::SCAN, "scan"},
{BCLog::TXPACKAGES, "txpackages"},
{BCLog::ALL, "1"},
{BCLog::ALL, "all"},
{"blockstorage", BCLog::BLOCKSTORAGE},
{"txreconciliation", BCLog::TXRECONCILIATION},
{"scan", BCLog::SCAN},
{"txpackages", BCLog::TXPACKAGES},
{"1", BCLog::ALL},
{"all", BCLog::ALL},
};

static const std::unordered_map<BCLog::LogFlags, std::string> LOG_CATEGORIES_BY_FLAG{
// Swap keys and values from LOG_CATEGORIES_BY_STR.
[](const std::map<std::string, BCLog::LogFlags>& in) {
std::unordered_map<BCLog::LogFlags, std::string> 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)
Expand All @@ -193,11 +200,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;
}
Expand All @@ -221,76 +227,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<BCLog::Level> GetLogLevel(const std::string& level_str)
Expand All @@ -312,18 +251,11 @@ static std::optional<BCLog::Level> GetLogLevel(const std::string& level_str)

std::vector<LogCategory> BCLog::Logger::LogCategoriesList() const
{
// Sort log categories by alphabetical order.
std::array<CLogCategoryDesc, std::size(LogCategories)> 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<LogCategory> 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;
}
Expand Down
9 changes: 4 additions & 5 deletions src/logging.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down

0 comments on commit 3b987d0

Please sign in to comment.