[WIP] tracing: lock contention #32952

1. 
   yuvicc commented at 8:55 am on July 12, 2025: contributor

   Currently there is a macro DEBUG_LOCKCONTENTION (see developer-notes.md) to enable logging of contention. The disadvantage of this method is that bitcoind needs to be especially compiled with that enabled, and when enabled this quickly produces huge log files, and has a relatively large overhead.

   This is a rework of PR #25081.

   Previously the binary(bitcoind) shows multiple ELF notes which is problematic as it can bloat the binary with redundant metadata, and can also confuse tracing tools(bcc, etc) when using binary path. see comment.

   I am keeping this as DRAFT for now and research more on the better way to handle the duplication of ELF notes, currently we use a check to omit multiple instantiation of TRACEPOINT. After a suitable workaround is found, I will be updating the commits with bpftrace scripts for lock analysis.

   CC 0xB10C

2. sync: Move UniqueLock and AnnotatedMixin to sync.cpp

   These are templated class which also adds explicit instantiation - because adding tracepoints
   in the header file may lead exhaust the limit of traces so we want to add tracepoints in the .cpp
   file

   dcb04c7fd2
3. Stores the arguments as members so they are available in the destructor.

   This will be useful later when adding tracepoints.
   
   Note that it is remembered as a char const*, so this only works for
   string literals where long lifetime is guaranteed. UniqueLock is only
   used with the LOCK/LOCK2 macro, so this is fine. Using
    would be an unnecessary overhead.

   4067cc8250
4. sync: add tracepoints to AnnotatedMixin and UniqueLock

   Note: To avoid duplication of ELF notes, we currently use a check for each tracepoint calls.

   a173eeeb6e
5. 
   DrahtBot commented at 8:55 am on July 12, 2025: contributor

   The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

   Code Coverage & Benchmarks

   For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/32952.

   Reviews

   See the guideline for information on the review process. A summary of reviews will appear here.

6. DrahtBot added the label CI failed on Jul 12, 2025
7. 
   DrahtBot commented at 9:54 am on July 12, 2025: contributor

   🚧 At least one of the CI tasks failed. Task lint: https://github.com/bitcoin/bitcoin/runs/45847110436 LLM reason (✨ experimental): The CI failure is caused by a commit message formatting error, specifically missing a blank line after the subject line.

   Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

   • Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.

   • A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.

   • An intermittent issue.

   Leave a comment here, if you need help tracking down a confusing failure.

8. 
   0xB10C commented at 8:44 am on July 14, 2025: contributor

   a bit of feedback since you cc’d me:

   • I’m not sure if opening this draft PR at this stage is worth it. There is nothing to experiment with, CI fails, and the feedback from the original PR is still a TODO. I’d recommend you finish your research first, come up with something that works, and then open a PR :)
   • You are obviously free to work on this, and I’m sure it’s interesting work, but I’m not sure if there is real developer interest for this feature. The original PR died down for a similar reason

   At the moment this looks like a “I want to work on this soon, so I open a PR for others to see” PR and I don’t think this adds value for anyone, for now

9. 
   yuvicc commented at 9:15 am on July 14, 2025: contributor

   I’m not sure if opening this draft PR at this stage is worth it. There is nothing to experiment with, CI fails, and the feedback from the original PR is still a TODO. I’d recommend you finish your research first, come up with something that > works, and then open a PR :) @0xB10C I think you’re right — I opened this draft PR a bit prematurely. I’ll take your advice and focus on finishing my research and having something more concrete before opening this PR again.

   Closing this for now. Thanks for taking the time to share your thoughts — really appreciate it!

10. yuvicc closed this on Jul 14, 2025

    ---

Contributors
yuvicc DrahtBot 0xB10C

Labels
CI failed