5a96767e3f depends, libevent: Do not install *.pc files and remove patches for them (Hennadii Stepanov)
ffda355b5a cmake, refactor: Move `HAVE_EVHTTP_...` to `libevent` interface (Hennadii Stepanov)
b619bdc330 cmake: Revamp `FindLibevent` module (Hennadii Stepanov)
Pull request description:
This PR generalizes the use of `find_package` / `pkg_check_modules`, prioritizing the former.
Addresses https://github.com/bitcoin/bitcoin/pull/30903#issuecomment-2444700876:
> We should also follow up with refactoring the libevent module, to more generically use CMake/pkg-config, rather than restricting the CMake usage to `vcpkg`. At that point, we'd likely be able to dump pkg-config for the depends path entirely.
Similar to https://github.com/bitcoin/bitcoin/pull/30903.
ACKs for top commit:
fanquake:
ACK 5a96767e3f
Tree-SHA512: 181020c16ccd2821e718c73f264badcdc5e62980c4a8d9691e759efe2ea00da2326e26308d1dcfdeac01e9e27930428ecace9f36941deee951b751b138d7266c
0de3e96e33 tracing: use bitcoind pid in bcc tracing examples (0xb10c)
411c6cfc6c tracing: only prepare tracepoint args if attached (0xb10c)
d524c1ec06 tracing: dedup TRACE macros & rename to TRACEPOINT (0xb10c)
Pull request description:
Currently, if the tracepoints are compiled (e.g. in depends and release builds), we always prepare the tracepoint arguments regardless of the tracepoints being used or not. We made sure that the argument preparation is as cheap as possible, but we can almost completely eliminate any overhead for users not interested in the tracepoints (the vast majority), by gating the tracepoint argument preparation with an `if(something is attached to this tracepoint)`. To achieve this, we use the optional semaphore feature provided by SystemTap.
The first commit simplifies and deduplicates our tracepoint macros from 13 TRACEx macros to a single TRACEPOINT macro. This makes them easier to use and also avoids more duplicate macro definitions in the second commit.
The Linux tracing tools I'm aware of (bcc, bpftrace, libbpf, and systemtap) all support the semaphore gating feature. Thus, all existing tracepoints and their argument preparation is gated in the second commit. For details, please refer to the commit messages and the updated documentation in `doc/tracing.md`.
Also adding unit tests that include all tracepoint macros to make sure there are no compiler problems with them (e.g. some varadiac extension not supported).
Reviewers might want to check:
- Do the tracepoints still work for you? Do the examples in `contrib/tracing/` run on your system (as bpftrace frequently breaks on every new version, please test master too if it should't work for you)? Do the CI interface tests still pass?
- Is the new documentation clear?
- The `TRACEPOINT_SEMAPHORE(event, context)` macros places global variables in our global namespace. Is this something we strictly want to avoid or maybe move to all `TRACEPOINT_SEMAPHORE`s to a separate .cpp file or even namespace? I like having the `TRACEPOINT_SEMAPHORE()` in same file as the `TRACEPOINT()`, but open for suggestion on alternative approaches.
- Are newly added tracepoints in the unit tests visible when using `readelf -n build/src/test/test_bitcoin`? You can run the new unit tests with `./build/src/test/test_bitcoin --run_test=util_trace_tests* --log_level=all`.
<details><summary>Two of the added unit tests demonstrate that we are only processing the tracepoint arguments when attached by having a test-failure condition in the tracepoint argument preparation. The following bpftrace script can be used to demonstrate that the tests do indeed fail when attached to the tracepoints.</summary>
`fail_tests.bt`:
```c
#!/usr/bin/env bpftrace
usdt:./build/src/test/test_bitcoin:test:check_if_attached {
printf("the 'check_if_attached' test should have failed\n");
}
usdt:./build/src/test/test_bitcoin:test:expensive_section {
printf("the 'expensive_section' test should have failed\n");
}
```
Run the unit tests with `./build/src/test/test_bitcoin` and start `bpftrace fail_tests.bt -p $(pidof test_bitcoin)` in a separate terminal. The unit tests should fail with:
```
Running 594 test cases...
test/util_trace_tests.cpp(31): error: in "util_trace_tests/test_tracepoint_check_if_attached": check false has failed
test/util_trace_tests.cpp(51): error: in "util_trace_tests/test_tracepoint_manual_tracepoint_active_check": check false has failed
*** 2 failures are detected in the test module "Bitcoin Core Test Suite"
```
</details>
These links might provide more contextual information for reviewers:
- [How SystemTap Userspace Probes Work by eklitzke](https://eklitzke.org/how-sytemtap-userspace-probes-work) (actually an example on Bitcoin Core; mentions that with semaphores "the overhead for an untraced process is effectively zero.")
- [libbpf comment on USDT semaphore handling](1596a09b5d/src/usdt.c (L83-L92)) (can recommend the whole comment for background on how the tracepoints and tracing tools work together)
- https://sourceware.org/systemtap/wiki/UserSpaceProbeImplementation#Semaphore_Handling
ACKs for top commit:
willcl-ark:
utACK 0de3e96e33
laanwj:
re-ACK 0de3e96e33
jb55:
utACK 0de3e96e33
vasild:
ACK 0de3e96e33
Tree-SHA512: 0e5e0dc5e0353beaf5c446e4be03d447e64228b1be71ee9972fde1d6fac3fac71a9d73c6ce4fa68975f87db2b2bf6eee2009921a2a145e24d83a475d007a559b
915640e191 depends: zeromq: don't install .pc files and remove patches for them (Cory Fields)
6b8a74463b cmake: Add `FindZeroMQ` module (Hennadii Stepanov)
Pull request description:
This PR introduces the `FindZeroMQ` module, which first attempts to find the `libzmq` library using CMake's `find_package()` and falls back to `pkg_check_modules()` if unsuccessful.
Addresses https://github.com/bitcoin/bitcoin/issues/30876 for the ZeroMQ package.
ACKs for top commit:
fanquake:
ACK 915640e191
Tree-SHA512: 2f17bae21be5d3f280a13425d22f5d1b2e23837a8aaf5ec89c433767509de030a42d598b261e102bdb5b860d8ede98013c124c3d25e081e956d4ee3a81b2584f
The use of `PACKAGE_NAME` for the project's variable name is
problematic, as this name is commonly used in CMake's interface
variables. If third-party CMake code handles with scopes improperly,
our `PACKAGE_NAME` variable could end up with an unexpected value.
This change avoids such conflicts by renaming all `PACKAGE_*` variables
to `CLIENT_*`.
This check was added for vcpkg, given how it packages Boost. However, we
don't need to run the check for other platforms, and it's quite slow.
So, scope it to VCPKG. On my machine, this reduces the time to run
`cmake -B build` from ~12 seconds, to ~6 seconds.
Fixes: #30787
faecca9a85 test: Use span for raw data (MarcoFalke)
fac973647d test: Use string_view for json_tests (MarcoFalke)
Pull request description:
The build system converts raw data into a C++ header file for tests.
This change modernizes the code to use the convenience wrappers `std::span` and `std::string_view`, so that redundant copies can be avoided.
ACKs for top commit:
fjahr:
re-ACK faecca9a85
TheCharlatan:
ACK faecca9a85
stickies-v:
ACK faecca9a85
hebasto:
ACK faecca9a85, I have reviewed the code and the generated headers.
Tree-SHA512: 1f4951c54aff11ba27c41fb70f2821bdb79e06ca0abae734b970bd0d64dda9d8cced824a891fd51b3e9d4e5715ee9eb49ed5d369010a45eca7c3bec9f8641235
This change allows to drop brittle sizeof calls in favor of the
std::span::size method.
Other improvements include:
* Use of a namespace to mark test and bench data
* Use of the modern std::byte
* Drop of a no longer used std::vector copy and the bench/data module
