Skip to content

Commit 411c6cf

Browse files
committed
tracing: only prepare tracepoint args if attached
Before this commit, we would always prepare tracepoint arguments regardless of the tracepoint being used or not. While we already made sure not to include expensive arguments in our tracepoints, this commit introduces gating to make sure the arguments are only prepared if the tracepoints are actually used. This is a win-win improvement to our tracing framework. For users not interested in tracing, the overhead is reduced to a cheap 'greater than 0' compare. As the semaphore-gating technique used here is available in bpftrace, bcc, and libbpf, users interested in tracing don't have to change their tracing scripts while profiting from potential future tracepoints passing slightly more expensive arguments. An example are mempool tracepoints that pass serialized transactions. We've avoided the serialization in the past as it was too expensive. Under the hood, the semaphore-gating works by placing a 2-byte semaphore in the '.probes' ELF section. The address of the semaphore is contained in the ELF note providing the tracepoint information (`readelf -n ./src/bitcoind | grep NT_STAPSDT`). Tracing toolkits like bpftrace, bcc, and libbpf increase the semaphore at the address upon attaching to the tracepoint. We only prepare the arguments and reach the tracepoint if the semaphore is greater than zero. The semaphore is decreased when detaching from the tracepoint. This also extends the "Adding a new tracepoint" documentation to include information about the semaphores and updated step-by-step instructions on how to add a new tracepoint.
1 parent d524c1e commit 411c6cf

9 files changed

+130
-28
lines changed

doc/tracing.md

+43-23
Original file line numberDiff line numberDiff line change
@@ -270,21 +270,47 @@ Use the `TRACEPOINT` macro to add a new tracepoint. If not yet included, include
270270
Each tracepoint needs a `context` and an `event`. Please use `snake_case` and
271271
try to make sure that the tracepoint names make sense even without detailed
272272
knowledge of the implementation details. You can pass zero to twelve arguments
273-
to the tracepoint. Do not forget to update the tracepoint list in this
274-
document.
273+
to the tracepoint. Each tracepoint also needs a global semaphore. The semaphore
274+
gates the tracepoint arguments from being processed if we are not attached to
275+
the tracepoint. Add a `TRACEPOINT_SEMAPHORE(context, event)` with the `context`
276+
and `event` of your tracepoint in the top-level namespace at the beginning of
277+
the file. Do not forget to update the tracepoint list in this document.
275278

276-
For example, a tracepoint in the `net` context for the event `inbound_message`
277-
and six arguments:
279+
For example, the `net:outbound_message` tracepoint in `src/net.cpp` with six
280+
arguments.
278281

279282
```C++
280-
TRACEPOINT(net, inbound_message,
281-
pnode->GetId(),
282-
pnode->m_addr_name.c_str(),
283-
pnode->ConnectionTypeAsString().c_str(),
284-
sanitizedType.c_str(),
285-
msg.data.size(),
286-
msg.data.data()
287-
);
283+
// src/net.cpp
284+
TRACEPOINT_SEMAPHORE(net, outbound_message);
285+
286+
void CConnman::PushMessage(…) {
287+
288+
TRACEPOINT(net, outbound_message,
289+
pnode->GetId(),
290+
pnode->m_addr_name.c_str(),
291+
pnode->ConnectionTypeAsString().c_str(),
292+
sanitizedType.c_str(),
293+
msg.data.size(),
294+
msg.data.data()
295+
);
296+
297+
}
298+
```
299+
If needed, an extra `if (TRACEPOINT_ACTIVE(context, event)) {...}` check can be
300+
used to prepare somewhat expensive arguments right before the tracepoint. While
301+
the tracepoint arguments are only prepared when we attach something to the
302+
tracepoint, an argument preparation should never hang the process. Hashing and
303+
serialization of data structures is probably fine, a `sleep(10s)` not.
304+
305+
```C++
306+
// An example tracepoint with an expensive argument.
307+
308+
TRACEPOINT_SEMAPHORE(example, gated_expensive_argument);
309+
310+
if (TRACEPOINT_ACTIVE(example, gated_expensive_argument)) {
311+
expensive_argument = expensive_calulation();
312+
TRACEPOINT(example, gated_expensive_argument, expensive_argument);
313+
}
288314
```
289315

290316
### Guidelines and best practices
@@ -302,12 +328,6 @@ the tracepoint. See existing examples in [contrib/tracing/].
302328

303329
[contrib/tracing/]: ../contrib/tracing/
304330

305-
#### No expensive computations for tracepoints
306-
Data passed to the tracepoint should be inexpensive to compute. Although the
307-
tracepoint itself only has overhead when enabled, the code to compute arguments
308-
is always run - even if the tracepoint is not used. For example, avoid
309-
serialization and parsing.
310-
311331
#### Semi-stable API
312332
Tracepoints should have a semi-stable API. Users should be able to rely on the
313333
tracepoints for scripting. This means tracepoints need to be documented, and the
@@ -354,9 +374,9 @@ $ gdb ./build/src/bitcoind
354374
355375
(gdb) info probes
356376
Type Provider Name Where Semaphore Object
357-
stap net inbound_message 0x000000000014419e /build/src/bitcoind
358-
stap net outbound_message 0x0000000000107c05 /build/src/bitcoind
359-
stap validation block_connected 0x00000000002fb10c /build/src/bitcoind
377+
stap net inbound_message 0x000000000014419e 0x0000000000d29bd2 /build/src/bitcoind
378+
stap net outbound_message 0x0000000000107c05 0x0000000000d29bd0 /build/src/bitcoind
379+
stap validation block_connected 0x00000000002fb10c 0x0000000000d29bd8 /build/src/bitcoind
360380
361381
```
362382

@@ -372,7 +392,7 @@ Displaying notes found in: .note.stapsdt
372392
stapsdt 0x0000005d NT_STAPSDT (SystemTap probe descriptors)
373393
Provider: net
374394
Name: outbound_message
375-
Location: 0x0000000000107c05, Base: 0x0000000000579c90, Semaphore: 0x0000000000000000
395+
Location: 0x0000000000107c05, Base: 0x0000000000579c90, Semaphore: 0x0000000000d29bd0
376396
Arguments: -8@%r12 8@%rbx 8@%rdi 8@192(%rsp) 8@%rax 8@%rdx
377397
378398
```
@@ -391,7 +411,7 @@ between distributions. For example, on
391411

392412
```
393413
$ tplist -l ./build/src/bitcoind -v
394-
b'net':b'outbound_message' [sema 0x0]
414+
b'net':b'outbound_message' [sema 0xd29bd0]
395415
1 location(s)
396416
6 argument(s)
397417

src/coins.cpp

+4
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,10 @@
99
#include <random.h>
1010
#include <util/trace.h>
1111

12+
TRACEPOINT_SEMAPHORE(utxocache, add);
13+
TRACEPOINT_SEMAPHORE(utxocache, spent);
14+
TRACEPOINT_SEMAPHORE(utxocache, uncache);
15+
1216
std::optional<Coin> CCoinsView::GetCoin(const COutPoint& outpoint) const { return std::nullopt; }
1317
uint256 CCoinsView::GetBestBlock() const { return uint256(); }
1418
std::vector<uint256> CCoinsView::GetHeadBlocks() const { return std::vector<uint256>(); }

src/net.cpp

+2
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,8 @@
5353
#include <optional>
5454
#include <unordered_map>
5555

56+
TRACEPOINT_SEMAPHORE(net, outbound_message);
57+
5658
/** Maximum number of block-relay-only anchor connections */
5759
static constexpr size_t MAX_BLOCK_RELAY_ONLY_ANCHORS = 2;
5860
static_assert (MAX_BLOCK_RELAY_ONLY_ANCHORS <= static_cast<size_t>(MAX_BLOCK_RELAY_ONLY_CONNECTIONS), "MAX_BLOCK_RELAY_ONLY_ANCHORS must not exceed MAX_BLOCK_RELAY_ONLY_CONNECTIONS.");

src/net_processing.cpp

+2
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,8 @@
5656

5757
using namespace util::hex_literals;
5858

59+
TRACEPOINT_SEMAPHORE(net, inbound_message);
60+
5961
/** Headers download timeout.
6062
* Timeout = base + per_header * (expected number of headers) */
6163
static constexpr auto HEADERS_DOWNLOAD_TIMEOUT_BASE = 15min;

src/test/util_trace_tests.cpp

+36-1
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,13 @@
88

99
#include <util/trace.h>
1010

11+
TRACEPOINT_SEMAPHORE(test, zero_args);
12+
TRACEPOINT_SEMAPHORE(test, one_arg);
13+
TRACEPOINT_SEMAPHORE(test, six_args);
14+
TRACEPOINT_SEMAPHORE(test, twelve_args);
15+
TRACEPOINT_SEMAPHORE(test, check_if_attached);
16+
TRACEPOINT_SEMAPHORE(test, expensive_section);
17+
1118
BOOST_FIXTURE_TEST_SUITE(util_trace_tests, BasicTestingSetup)
1219

1320
// Tests the TRACEPOINT macro and that we can compile tracepoints with 0 to 12 args.
@@ -20,4 +27,32 @@ BOOST_AUTO_TEST_CASE(test_tracepoints)
2027
BOOST_CHECK(true);
2128
}
2229

23-
BOOST_AUTO_TEST_SUITE_END()
30+
int fail_test_if_executed()
31+
{
32+
BOOST_CHECK(false);
33+
return 0;
34+
}
35+
36+
BOOST_AUTO_TEST_CASE(test_tracepoint_check_if_attached)
37+
{
38+
// TRACEPOINT should check if we are attaching to the tracepoint and only then
39+
// process arguments. This means, only if we are attached to the
40+
// `test:check_if_attached` tracepoint, fail_test_if_executed() is executed.
41+
// Since we don't attach to the tracepoint when running the test, it succeeds.
42+
TRACEPOINT(test, check_if_attached, fail_test_if_executed());
43+
BOOST_CHECK(true);
44+
}
45+
46+
BOOST_AUTO_TEST_CASE(test_tracepoint_manual_tracepoint_active_check)
47+
{
48+
// We should be able to use the TRACEPOINT_ACTIVE() macro to only
49+
// execute an 'expensive' code section if we are attached to the
50+
// tracepoint.
51+
if (TRACEPOINT_ACTIVE(test, expensive_section)) {
52+
BOOST_CHECK(false); // expensive_function()
53+
TRACEPOINT(test, expensive_section);
54+
}
55+
BOOST_CHECK(true);
56+
}
57+
58+
BOOST_AUTO_TEST_SUITE_END()

src/txmempool.cpp

+3
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,9 @@
3434
#include <string_view>
3535
#include <utility>
3636

37+
TRACEPOINT_SEMAPHORE(mempool, added);
38+
TRACEPOINT_SEMAPHORE(mempool, removed);
39+
3740
bool TestLockPointValidity(CChain& active_chain, const LockPoints& lp)
3841
{
3942
AssertLockHeld(cs_main);

src/util/trace.h

+30-4
Original file line numberDiff line numberDiff line change
@@ -13,16 +13,42 @@
1313
// the optional variadic macros to define tracepoints.
1414
#define SDT_USE_VARIADIC 1
1515

16+
// Setting _SDT_HAS_SEMAPHORES let's systemtap (sys/sdt.h) know that we want to
17+
// use the optional semaphore feature for our tracepoints. This feature allows
18+
// us to check if something is attached to a tracepoint. We only want to prepare
19+
// some potentially expensive tracepoint arguments, if the tracepoint is being
20+
// used. Here, an expensive argument preparation could, for example, be
21+
// calculating a hash or serialization of a data structure.
22+
#define _SDT_HAS_SEMAPHORES 1
23+
24+
// Used to define a counting semaphore for a tracepoint. This semaphore is
25+
// automatically incremented by tracing frameworks (bpftrace, bcc, libbpf, ...)
26+
// upon attaching to the tracepoint and decremented when detaching. This needs
27+
// to be a global variable. It's placed in the '.probes' ELF section.
28+
#define TRACEPOINT_SEMAPHORE(context, event) \
29+
unsigned short context##_##event##_semaphore __attribute__((section(".probes")))
30+
1631
#include <sys/sdt.h>
1732

18-
// A USDT tracepoint with zero to twelve arguments.
19-
#define TRACEPOINT(context, ...) \
20-
STAP_PROBEV(context, __VA_ARGS__);
33+
// Returns true if something is attached to the tracepoint.
34+
#define TRACEPOINT_ACTIVE(context, event) (context##_##event##_semaphore > 0)
35+
36+
// A USDT tracepoint with one to twelve arguments. It's checked that the
37+
// tracepoint is active before preparing its arguments.
38+
#define TRACEPOINT(context, event, ...) \
39+
do { \
40+
if (TRACEPOINT_ACTIVE(context, event)) { \
41+
STAP_PROBEV(context, event __VA_OPT__(, ) __VA_ARGS__); \
42+
} \
43+
} while(0)
44+
2145
#else
2246

47+
#define TRACEPOINT_SEMAPHORE(context, event)
48+
#define TRACEPOINT_ACTIVE(context, event) false
2349
#define TRACEPOINT(context, ...)
2450

25-
#endif
51+
#endif // ENABLE_TRACING
2652

2753

2854
#endif // BITCOIN_UTIL_TRACE_H

src/validation.cpp

+5
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,11 @@ const std::vector<std::string> CHECKLEVEL_DOC {
108108
* */
109109
static constexpr int PRUNE_LOCK_BUFFER{10};
110110

111+
TRACEPOINT_SEMAPHORE(validation, block_connected);
112+
TRACEPOINT_SEMAPHORE(utxocache, flush);
113+
TRACEPOINT_SEMAPHORE(mempool, replaced);
114+
TRACEPOINT_SEMAPHORE(mempool, rejected);
115+
111116
const CBlockIndex* Chainstate::FindForkInGlobalIndex(const CBlockLocator& locator) const
112117
{
113118
AssertLockHeld(cs_main);

src/wallet/spend.cpp

+5
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,11 @@ using common::TransactionErrorString;
3535
using interfaces::FoundBlock;
3636
using node::TransactionError;
3737

38+
TRACEPOINT_SEMAPHORE(coin_selection, selected_coins);
39+
TRACEPOINT_SEMAPHORE(coin_selection, normal_create_tx_internal);
40+
TRACEPOINT_SEMAPHORE(coin_selection, attempting_aps_create_tx);
41+
TRACEPOINT_SEMAPHORE(coin_selection, aps_create_tx_internal);
42+
3843
namespace wallet {
3944
static constexpr size_t OUTPUT_GROUP_MAX_ENTRIES{100};
4045

0 commit comments

Comments
 (0)