In the first commit, find() on a std::unordered_set has constant-to-linear (size of the container) complexity, whereas if you build a std::array instead, access with [] or .at will be constant time (and the array can be constexpr; git grep the codebase for "constexpr std::array" for examples).

nit suggestion

-            if (it == LogCategoryToStr.end()) {
-                s += "unknown";
-            } else {
-                s += it->second;
-            }
+            s += it == LogCategoryToStr.end() ? "unknown" : it->second;

I am not really sure how to do that.

LogCategory enum values are bit flags (ie 1 << i) with the exception of None (0) and All(~0) instead of the default enum values (ie 0,1,2,..,n-1) that easily correspond to indexes of an array.

Some solutions I can think of:

• Refactor LogCategory use the default enum values (ie 0,1,...) and create another helper function (ie LogCategoryToFlag(flag){ return 1 << flag;}
• Create a custom hash function for LogCategoryToStr unordered_map that returns a unique hash value for each enum value. This will guarantee that the find() will execute in constant time.
  • Hash function could possibly be flag % SOME_PRIME.

However, I am not too sure if this additional complexity is worth it as the size of LogCategoryToStr is bounded

Resolved the nit 👍🏻

Thanks for looking into it! I guess at some point this might be refactored but doesn't necessarily need to be done here.

In the first commit, a few suggestions

@@ -138,7 +138,7 @@ struct CLogCategoryDesc {
     }
 };
 
-const CLogCategoryDesc LogCategories[]{
+static CLogCategoryDesc LogCategories[]{
     {BCLog::NONE, "none"},
     {BCLog::NET, "net"},
     {BCLog::TOR, "tor"},
@@ -174,10 +174,10 @@ const CLogCategoryDesc LogCategories[]{
     {BCLog::ALL, "1"},
 };
 
-// Ignore last 2 extra mappings of NONE and ALL.
-const std::unordered_map<BCLog::LogFlags, std::string> LogCategoryToStr{
+static std::unordered_map<BCLog::LogFlags, std::string> LogCategoryToStr{
     LogCategories,
-    LogCategories + std::size(LogCategories) - 2};
+    LogCategories + std::size(LogCategories) - 2 // ignore last 2 extra mappings of NONE and ALL
+};

Wouldn't it make sense to do it the other way around?

First build LogCategoryToStr, then build LogCategoryDesc from that, adding the ALL=1 and NONE=0 instead of cutting them off. I think that's neater and less error prone.

In the first commit, a few suggestions

Done 👍🏻

First build LogCategoryToStr, then build LogCategoryDesc from that, adding the ALL=1 and NONE=0 instead of cutting them off. I think that's neater and less error prone.

Agreed, building LogCategories from LogCategoryToStr is definitely cleaner!

However, I am unable to think of a trivial method to implement it as currently both LogCategories and LogCategoryToStr are global variables where the values are added in its constructor. To build LogCategories from LogCategoryToStr, we will most probably need to change LogCategories type from an array to a class that allows us to add our own construction logic (build from LogCategoryToStr and add ALL=1 and NONE=0).

If changing LogCategories to a class is the desired approach, I will split the first commit into a separate PR.

Suggestions:

• order the params by in-param, then out-param
• pass the string in-param by reference to const rather than by value
• return a std::optional here instead of bool with a level out-param
• use a switch statement (with no default case, grep doc/developer-notes.md for "switch statement on an enumeration example")
• function can be static and is missing a line break before

Done, went with the return std::optional suggestion.

use a switch statement (with no default case, grep doc/developer-notes.md for "switch statement on an enumeration example")

I am unable to use a switch statement with level_str. If I am not wrong, c++ only allows switch statements with integral types and enumerators.

Oops, yes, you're right about switch statements.

Suggestion:

-- Allow configuration of global and category specific threshold log levels. (#25287)
+- A new `-loglevel` configuration option allows setting global and per-category
+  log severity levels: error, warning, info, debug (default: info). (#25287)

(This entry doesn't seem likely to cause a merge conflict, so it can probably be added directly to doc/release-notes.md)

Second iteration on this to hopefully make it shorter and clearer while providing the log levels at the start

$ ./src/bitcoind -h | grep -A9 loglevel
  -loglevel=<level>|<category>:<level>
       Set the global or per-category logging severity level: debug, none,
       info, warning, error (default=info). If <category>:<level> is
       supplied, the setting will override the global one and may be
       specified multiple times to set multiple category-specific
       levels. <category> can be: addrman, bench, blockstorage,
       cmpctblock, coindb, estimatefee, http, i2p, ipc, leveldb,
       libevent, lock, mempool, mempoolrej, net, proxy, prune, qt, rand,
       reindex, rpc, selectcoins, tor, util, validation, walletdb, zmq.

It might be better to construct this vector programmatically to have a single point of truth, i.e. BCLog::Level.

I'd find this data member naming clearer as m_severity_level or m_log_level (I prefer the latter), maybe that is just me.