#35322 logging: streamline Logger state and drop redundant methods
https://github.com/bitcoin/bitcoin/pull/35322 · · +205/-196 in 14 files, 14 commits · labels: none
Goal
- Avoid mutex acquisition and overhead during routine logging checks across the node
- Simplify internal logger state management and remove redundant category methods
Replaces the Logger class's three distinct state fields (an enabled bitmask, a global level, and a mutex-guarded hash map of category levels) with a two-element array of atomic bitmasks representing conditional levels (Debug and Trace). It eliminates lock acquisition from WillLogCategoryLevel and drops redundant methods such as EnableCategory, DisableCategory, WillLogCategory, LogLevel, SetLogLevel, and GetCategoryMask. It also updates callers across initialization, RPC, the Qt interface, and the kernel C API.
Problem: Logger previously tracked category enablement and verbosity using separate fields that required coordinated updates and acquired a mutex during category level checks, despite the node having only ~30 categories and two conditional levels.
Category: Kernel (libbitcoinkernel) (#15 of 18)
P4 · cleanup
- P4 because updates to the kernel C API and test exports are mechanical adaptations
- These adjustments are minor and will be superseded by a larger kernel logging rework
The kernel changes adapt the C API implementation to the new Logger methods and temporarily export internal symbols for kernel test coverage, which will be superseded by the kernel logging rework in #34374.
Membership: Modifies btck_logging_* implementations in src/kernel/bitcoinkernel.cpp, exports logging symbols for test_kernel in CMakeLists.txt, and updates kernel tests.
Factors: security/stability 0, bug 0, performance 0, user value 0, leverage 1
Category: Utilities (logging, arguments, libraries) (#19 of 66)
P3 · cleanup
- P3 because it eliminates lock acquisition and map lookups on every category log check
- Improves performance across all logging callers while removing redundant state and methods
Streamlining the Logger state replaces mutex locking and hash map lookups on log checks with relaxed atomic bitwise operations, dropping several redundant methods while maintaining logging behavior.
Membership: Directly refactors the Logger class and logging utilities in src/logging.h and src/logging.cpp.
Factors: security/stability 0, bug 0, performance 1, user value 0, leverage 1
Reviewability: Ready
- Ready for review
- Cleanly rebased on master with passing CI and previous review feedback addressed
The PR is rebased on master, passes CI, and the author addressed all feedback from the previous review round.
Author status: active, last rebased and updated on 2026-08-15
Resolved concerns:
- l0rinc requested splitting the initial monolithic commit into discrete reviewable steps and dropping unused string overloads.
- l0rinc noted unexpected behavior changes where -loglevel without -debug previously had no effect but would now enable logging; ryanofsky restored the original behavior and added regression tests.
- l0rinc asked for explicit tests verifying runtime category enable behavior via RPC and kernel entry points, which ryanofsky added.
Agreement: Neutral
- Requested splitting commits and preserving flag behavior when debug mode is off (l0rinc)
- Requested explicit tests for category enablement through RPC and kernel interfaces (l0rinc)
- Author addressed all review comments and added tests, but follow-up review is pending
Neutral: l0rinc requested commit splitting and behavior fixes; author addressed all requests in updates
l0rinc provided initial review requesting commits be split and behavioral differences addressed. The author addressed these requests across subsequent pushes and added unit tests, but no reviewer has returned to give an ACK yet.
- l0rinc requested changes asking to split commits and preserve -loglevel behavior without -debug
- ryanofsky addressed all points across subsequent pushes and added unit tests covering the edge cases
Review verdicts (DrahtBot): 0
Dependencies
Files
166 lines under test/bench/ci.
- src/test/logging_tests.cpp +86/-60
- src/logging.cpp +26/-62
- src/logging.h +21/-41
- src/init/common.cpp +23/-6
- src/kernel/CMakeLists.txt +17/-0
- src/rpc/node.cpp +3/-9
- src/kernel/bitcoinkernel.cpp +3/-8
- src/test/i2p_tests.cpp +5/-5
- doc/release-notes-35322.md +8/-0
- src/test/kernel/test_kernel.cpp +8/-0
- src/interfaces/node.h +2/-2
- src/bench/logging.cpp +1/-1
- src/node/interfaces.cpp +1/-1
- src/qt/transactiondesc.cpp +1/-1
Card
This PR refactors the Logger class to replace three distinct state variables (a categories bitmask, a global log level, and a mutex-protected hash map of overrides) with two atomic bitmasks representing Debug and Trace levels. This removes mutex acquisition during category log checks, simplifies Logger internals, and drops several redundant methods. The only functional change is that dynamically enabling categories via RPC or kernel API now always enables them at Debug level rather than restoring previous -loglevel settings. Review is currently in a neutral state; earlier concerns regarding commit structure and behavioral edge cases were resolved by the author, and the PR is ready for re-review.