Skip to content

Commit 3b987d0

Browse files
ryanofskyryanofsky
committed
Merge bitcoin/bitcoin#29419: log: deduplicate category names and improve logging.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/bitcoin#29415 but they make sense on their own and would be good to have them, regardless of the fate of bitcoin/bitcoin#29415. Also, if this is merged, that would reduce the size of bitcoin/bitcoin#29415, thus the current standalone PR. ACKs for top commit: davidgumberg: crACK bitcoin/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
2 parents 6dabb31 + b0344c2 commit 3b987d0

File tree

2 files changed

+65
-134
lines changed

2 files changed

+65
-134
lines changed

‎src/logging.cpp

Lines changed: 61 additions & 129 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,8 @@
99
#include <util/threadnames.h>
1010
#include <util/time.h>
1111

12-
#include <algorithm>
1312
#include <array>
14-
#include <mutex>
13+
#include <map>
1514
#include <optional>
1615

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

145-
struct CLogCategoryDesc {
146-
BCLog::LogFlags flag;
147-
std::string category;
148-
};
149-
150-
const CLogCategoryDesc LogCategories[] =
151-
{
152-
{BCLog::NONE, "0"},
153-
{BCLog::NONE, ""},
154-
{BCLog::NET, "net"},
155-
{BCLog::TOR, "tor"},
156-
{BCLog::MEMPOOL, "mempool"},
157-
{BCLog::HTTP, "http"},
158-
{BCLog::BENCH, "bench"},
159-
{BCLog::ZMQ, "zmq"},
160-
{BCLog::WALLETDB, "walletdb"},
161-
{BCLog::RPC, "rpc"},
162-
{BCLog::ESTIMATEFEE, "estimatefee"},
163-
{BCLog::ADDRMAN, "addrman"},
164-
{BCLog::SELECTCOINS, "selectcoins"},
165-
{BCLog::REINDEX, "reindex"},
166-
{BCLog::CMPCTBLOCK, "cmpctblock"},
167-
{BCLog::RAND, "rand"},
168-
{BCLog::PRUNE, "prune"},
169-
{BCLog::PROXY, "proxy"},
170-
{BCLog::MEMPOOLREJ, "mempoolrej"},
171-
{BCLog::LIBEVENT, "libevent"},
172-
{BCLog::COINDB, "coindb"},
173-
{BCLog::QT, "qt"},
174-
{BCLog::LEVELDB, "leveldb"},
175-
{BCLog::VALIDATION, "validation"},
176-
{BCLog::I2P, "i2p"},
177-
{BCLog::IPC, "ipc"},
144+
static const std::map<std::string, BCLog::LogFlags> LOG_CATEGORIES_BY_STR{
145+
{"0", BCLog::NONE},
146+
{"", BCLog::NONE},
147+
{"net", BCLog::NET},
148+
{"tor", BCLog::TOR},
149+
{"mempool", BCLog::MEMPOOL},
150+
{"http", BCLog::HTTP},
151+
{"bench", BCLog::BENCH},
152+
{"zmq", BCLog::ZMQ},
153+
{"walletdb", BCLog::WALLETDB},
154+
{"rpc", BCLog::RPC},
155+
{"estimatefee", BCLog::ESTIMATEFEE},
156+
{"addrman", BCLog::ADDRMAN},
157+
{"selectcoins", BCLog::SELECTCOINS},
158+
{"reindex", BCLog::REINDEX},
159+
{"cmpctblock", BCLog::CMPCTBLOCK},
160+
{"rand", BCLog::RAND},
161+
{"prune", BCLog::PRUNE},
162+
{"proxy", BCLog::PROXY},
163+
{"mempoolrej", BCLog::MEMPOOLREJ},
164+
{"libevent", BCLog::LIBEVENT},
165+
{"coindb", BCLog::COINDB},
166+
{"qt", BCLog::QT},
167+
{"leveldb", BCLog::LEVELDB},
168+
{"validation", BCLog::VALIDATION},
169+
{"i2p", BCLog::I2P},
170+
{"ipc", BCLog::IPC},
178171
#ifdef DEBUG_LOCKCONTENTION
179-
{BCLog::LOCK, "lock"},
172+
{"lock", BCLog::LOCK},
180173
#endif
181-
{BCLog::UTIL, "util"},
182-
{BCLog::BLOCKSTORAGE, "blockstorage"},
183-
{BCLog::TXRECONCILIATION, "txreconciliation"},
184-
{BCLog::SCAN, "scan"},
185-
{BCLog::TXPACKAGES, "txpackages"},
186-
{BCLog::ALL, "1"},
187-
{BCLog::ALL, "all"},
174+
{"blockstorage", BCLog::BLOCKSTORAGE},
175+
{"txreconciliation", BCLog::TXRECONCILIATION},
176+
{"scan", BCLog::SCAN},
177+
{"txpackages", BCLog::TXPACKAGES},
178+
{"1", BCLog::ALL},
179+
{"all", BCLog::ALL},
180+
};
181+
182+
static const std::unordered_map<BCLog::LogFlags, std::string> LOG_CATEGORIES_BY_FLAG{
183+
// Swap keys and values from LOG_CATEGORIES_BY_STR.
184+
[](const std::map<std::string, BCLog::LogFlags>& in) {
185+
std::unordered_map<BCLog::LogFlags, std::string> out;
186+
for (const auto& [k, v] : in) {
187+
switch (v) {
188+
case BCLog::NONE: out.emplace(BCLog::NONE, ""); break;
189+
case BCLog::ALL: out.emplace(BCLog::ALL, "all"); break;
190+
default: out.emplace(v, k);
191+
}
192+
}
193+
return out;
194+
}(LOG_CATEGORIES_BY_STR)
188195
};
189196

190197
bool GetLogCategory(BCLog::LogFlags& flag, const std::string& str)
@@ -193,11 +200,10 @@ bool GetLogCategory(BCLog::LogFlags& flag, const std::string& str)
193200
flag = BCLog::ALL;
194201
return true;
195202
}
196-
for (const CLogCategoryDesc& category_desc : LogCategories) {
197-
if (category_desc.category == str) {
198-
flag = category_desc.flag;
199-
return true;
200-
}
203+
auto it = LOG_CATEGORIES_BY_STR.find(str);
204+
if (it != LOG_CATEGORIES_BY_STR.end()) {
205+
flag = it->second;
206+
return true;
201207
}
202208
return false;
203209
}
@@ -221,76 +227,9 @@ std::string BCLog::Logger::LogLevelToStr(BCLog::Level level)
221227

222228
std::string LogCategoryToStr(BCLog::LogFlags category)
223229
{
224-
// Each log category string representation should sync with LogCategories
225-
switch (category) {
226-
case BCLog::LogFlags::NONE:
227-
return "";
228-
case BCLog::LogFlags::NET:
229-
return "net";
230-
case BCLog::LogFlags::TOR:
231-
return "tor";
232-
case BCLog::LogFlags::MEMPOOL:
233-
return "mempool";
234-
case BCLog::LogFlags::HTTP:
235-
return "http";
236-
case BCLog::LogFlags::BENCH:
237-
return "bench";
238-
case BCLog::LogFlags::ZMQ:
239-
return "zmq";
240-
case BCLog::LogFlags::WALLETDB:
241-
return "walletdb";
242-
case BCLog::LogFlags::RPC:
243-
return "rpc";
244-
case BCLog::LogFlags::ESTIMATEFEE:
245-
return "estimatefee";
246-
case BCLog::LogFlags::ADDRMAN:
247-
return "addrman";
248-
case BCLog::LogFlags::SELECTCOINS:
249-
return "selectcoins";
250-
case BCLog::LogFlags::REINDEX:
251-
return "reindex";
252-
case BCLog::LogFlags::CMPCTBLOCK:
253-
return "cmpctblock";
254-
case BCLog::LogFlags::RAND:
255-
return "rand";
256-
case BCLog::LogFlags::PRUNE:
257-
return "prune";
258-
case BCLog::LogFlags::PROXY:
259-
return "proxy";
260-
case BCLog::LogFlags::MEMPOOLREJ:
261-
return "mempoolrej";
262-
case BCLog::LogFlags::LIBEVENT:
263-
return "libevent";
264-
case BCLog::LogFlags::COINDB:
265-
return "coindb";
266-
case BCLog::LogFlags::QT:
267-
return "qt";
268-
case BCLog::LogFlags::LEVELDB:
269-
return "leveldb";
270-
case BCLog::LogFlags::VALIDATION:
271-
return "validation";
272-
case BCLog::LogFlags::I2P:
273-
return "i2p";
274-
case BCLog::LogFlags::IPC:
275-
return "ipc";
276-
#ifdef DEBUG_LOCKCONTENTION
277-
case BCLog::LogFlags::LOCK:
278-
return "lock";
279-
#endif
280-
case BCLog::LogFlags::UTIL:
281-
return "util";
282-
case BCLog::LogFlags::BLOCKSTORAGE:
283-
return "blockstorage";
284-
case BCLog::LogFlags::TXRECONCILIATION:
285-
return "txreconciliation";
286-
case BCLog::LogFlags::SCAN:
287-
return "scan";
288-
case BCLog::LogFlags::TXPACKAGES:
289-
return "txpackages";
290-
case BCLog::LogFlags::ALL:
291-
return "all";
292-
}
293-
assert(false);
230+
auto it = LOG_CATEGORIES_BY_FLAG.find(category);
231+
assert(it != LOG_CATEGORIES_BY_FLAG.end());
232+
return it->second;
294233
}
295234

296235
static std::optional<BCLog::Level> GetLogLevel(const std::string& level_str)
@@ -312,18 +251,11 @@ static std::optional<BCLog::Level> GetLogLevel(const std::string& level_str)
312251

313252
std::vector<LogCategory> BCLog::Logger::LogCategoriesList() const
314253
{
315-
// Sort log categories by alphabetical order.
316-
std::array<CLogCategoryDesc, std::size(LogCategories)> categories;
317-
std::copy(std::begin(LogCategories), std::end(LogCategories), categories.begin());
318-
std::sort(categories.begin(), categories.end(), [](auto a, auto b) { return a.category < b.category; });
319-
320254
std::vector<LogCategory> ret;
321-
for (const CLogCategoryDesc& category_desc : categories) {
322-
if (category_desc.flag == BCLog::NONE || category_desc.flag == BCLog::ALL) continue;
323-
LogCategory catActive;
324-
catActive.category = category_desc.category;
325-
catActive.active = WillLogCategory(category_desc.flag);
326-
ret.push_back(catActive);
255+
for (const auto& [category, flag] : LOG_CATEGORIES_BY_STR) {
256+
if (flag != BCLog::NONE && flag != BCLog::ALL) {
257+
ret.push_back(LogCategory{.category = category, .active = WillLogCategory(flag)});
258+
}
327259
}
328260
return ret;
329261
}

‎src/logging.h

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -65,11 +65,10 @@ namespace BCLog {
6565
#ifdef DEBUG_LOCKCONTENTION
6666
LOCK = (1 << 24),
6767
#endif
68-
UTIL = (1 << 25),
69-
BLOCKSTORAGE = (1 << 26),
70-
TXRECONCILIATION = (1 << 27),
71-
SCAN = (1 << 28),
72-
TXPACKAGES = (1 << 29),
68+
BLOCKSTORAGE = (1 << 25),
69+
TXRECONCILIATION = (1 << 26),
70+
SCAN = (1 << 27),
71+
TXPACKAGES = (1 << 28),
7372
ALL = ~(uint32_t)0,
7473
};
7574
enum class Level {

0 commit comments

Comments
 (0)