#35355 Use atomics for determining whether trace logging is enabled
| Reviewability | Reviews | Agreement | Size | ||||
|---|---|---|---|---|---|---|---|
| #35355 Use atomics for determining whether trace logging is enabled | Review #34038 first | 0 | Crickets | 297 + 116 tests | |||
|
|
|
|
| |||
Reviewability: ReadyStalePaused Agreement: StrongPositivePositive w/ caveatsNeutralMildDisputedBlockedCrickets Reviews: current code-review ACKs, then (+stale ACKs) and -NACKs; greener = more ACKs. Size: lines added or modified outside tests, then in tests; greener = smaller. | |||||||
Goal
Replaces the logging system's category-level maps and global log level with atomic bitmasks for debug and trace categories, making category enablement checks (`ShouldDebugLog` and `ShouldTraceLog`) inlined and lock-free. Incorporates changes from #34038 that replace `-loglevel` with a `-trace` command-line option and update the `logging` RPC to support trace categories with backward-compatible deprecation output. Also makes various logger configuration fields atomic or mutex-guarded to ensure thread safety without initialization ordering assumptions.
Problem: Checking whether trace or debug logging is active for a category requires map lookups or mutex acquisition, imposing CPU and lock overhead on hot multi-threaded code paths even when logging is disabled. Additionally, configuring trace logging previously required an unintuitive combination of `-debug` and `-loglevel` options.
Evidence of importance:
- PR description: 'simplify and optimize the logging-disabled path for LogTrace() ... reduces ShouldDebugLog and ShouldTraceLog to lock-free checks on a single atomic. Also makes Enabled() lock-free, to waste a little less time in the case where debugging is completely disabled.'
- Commit 0572c28fe3: 'This allows WillLogCategoryLevel to avoid taking a mutex, potentially allowing for better performance in the event that multiple threads are running many LogDebug(..) statements over categories that do not have -debug enabled (or LogTrace/-trace).'
- Commit 36586ec1b3: 'Simplifies the util/log.h API so that log enablement does not require indirection and ShouldDebugLog and ShouldTraceLog can be fully inlined at logging call sites.'
Scope: The PR is stacked on top of open PR #34038; the first three commits (replacing `-loglevel` with `-trace` and the RPC changes) belong to that base PR.
Risk of a mistake: medium (rpc_api, other). Modifications to logging startup parameters and the logging RPC output structure could break external management tooling and node automation scripts.
Changed since the previous assessment: Moved category bitmasks into static constinit atomics in util/log.h, enabling ShouldDebugLog and ShouldTraceLog to be fully inlined at call sites rather than relying on indirection through logger instances, and adapted the kernel logger accordingly.
Reviewability
Agreement
Crickets
no substantive comment either way
Thread notes: Initial record. Draft PR opened by ajtowns with no reviewers or commenters yet.
Objections, suggestions, and questions: none recorded.
Dependencies
Files
215 lines under test/bench/ci.
- src/logging.cpp +47/-93
- src/logging.h +59/-62
- src/test/logging_tests.cpp +44/-59
- src/rpc/node.cpp +63/-24
- src/util/log.h +52/-14
- test/functional/feature_logging.py +37/-22
- src/init/common.cpp +23/-31
- src/kernel/bitcoinkernel.cpp +28/-11
- test/functional/rpc_misc.py +21/-8
- doc/release-notes-34038.md +13/-0
- src/test/i2p_tests.cpp +6/-4
- doc/developer-notes.md +4/-5
- src/bench/logging.cpp +3/-3
- test/functional/test_framework/test_node.py +4/-1
- src/test/util/setup_common.cpp +1/-2
- src/bitcoind.cpp +1/-1
- src/httpserver.cpp +1/-1
- src/node/abort.cpp +1/-1
- src/qt/guiutil.cpp +1/-1
- src/rpc/client.cpp +2/-0
- src/rpc/server.cpp +1/-1
- src/wallet/sqlite.cpp +1/-1
- src/init.cpp +0/-1
- src/init/common.h +0/-1
Card
The short text the ranking pass compares across PRs.
This PR optimizes the disabled-logging path by replacing category log level maps and global levels with atomic bitmasks, making ShouldDebugLog and ShouldTraceLog fully inlined and lock-free across all threads. It also pulls in changes from #34038 that replace -loglevel with a -trace command-line flag and update the logging RPC with trace toggling and deprecation fallback. The changes eliminate lock contention when evaluating inactive log statements in hot multi-threaded code. It is currently in draft and depends on #34038.
Data
thread read through 2026-05-22 (1 statements), head 36586ec1b3
Discussion read under schema 1 rules: suggestions and questions may be missing, and an objection may combine several harms (schema history).
labels: none · draft
Categories
Utilities (logging, arguments, libraries)
P3 · speedup (ranking pass: Moved down to sit with #34038, on which it depends, and with the overlapping #35322.)
The PR makes disabled debug and trace category checks lock-free and fully inlined to eliminate mutex overhead on hot code paths. As stated in the description, it 'reduces ShouldDebugLog and ShouldTraceLog to lock-free checks on a single atomic' to avoid locking or map lookups across threads.
Membership: Directly refactors the logging subsystem in src/logging.cpp, src/logging.h, and src/util/log.h to use atomic bitmasks for categories.
Factors: security/stability 0, bug 0, performance 1, user value 1, leverage 0