tracing: drop GetHash().ToString() argument from the validation:block_connected tracepoint
#23302
Conversation
The tracepoint `validation:block_connected` was introduced in bitcoin#22006. The first argument was the hash of the connected block as a pointer to a C-like String. The last argument passed the hash of the connected block as a pointer to 32 bytes. The hash was only passed as string to allow `bpftrace` scripts to print the hash. It was (incorrectly) assumed that `bpftrace` cannot hex-format and print the block hash given only the hash as bytes. The block hash can be printed in `bpftrace` by calling `printf("%02x")` for each byte of the hash in an `unroll () {...}`. By starting from the last byte of the hash, it can be printed in big-endian (the block-explorer format). ```C $p = $hash + 31; unroll(32) { $b = *(uint8*)$p; printf("%02x", $b); $p -= 1; } ``` See also: bitcoin#22902 (comment) This is a breaking change to the block_connected tracepoint API, however this tracepoint has not yet been included in a release.
|
Concept and code review ACK 53c9fa9 Trace point invocations should be as light as possible to avoid overhead when they are disabled (the common case). Creating an intermediate string and deallocating it, as well as formatting overhead is something that should be avoided. |
|
Concept ACK |
|
ACK 53c9fa9 this is the way. If anyone is looking at the unroll and thinking that's a funny way to print something, it's because the BPF virtual machine inside the kernel requires guaranteed termination, you can't have infinite loops in BPF programs. I'm not familiar enough with BPF bytecode but I assume there is just no "jump" operation to implement loops, which is why you need to unroll them. |
I would hate to think we are already at a point where we have to worry about backwards compatibility for tracepoints. Consumers should understand that they are still new/experimental and are very likely change over the next few releases, as improvements like this are made. |
… the `validation:block_connected` tracepoint 53c9fa9 tracing: drop block_connected hash.toString() arg (0xb10c) Pull request description: The tracepoint `validation:block_connected` was introduced in bitcoin#22006. The first argument was the hash of the connected block as a pointer to a C-like String. The last argument passed the hash of the connected block as a pointer to 32 bytes. The hash was only passed as string to allow `bpftrace` scripts to print the hash. It was (incorrectly) assumed that `bpftrace` cannot hex-format and print the block hash given only the hash as bytes. The block hash can be printed in `bpftrace` by calling `printf("%02x")` for each byte of the hash in an `unroll () {...}`. By starting from the last byte of the hash, it can be printed in big-endian (the block-explorer format). ```C $p = $hash + 31; unroll(32) { $b = *(uint8*)$p; printf("%02x", $b); $p -= 1; } ``` See also: bitcoin#22902 (comment) This is a breaking change to the block_connected tracepoint API, however this tracepoint has not yet been included in a release. ACKs for top commit: laanwj: Concept and code review ACK 53c9fa9 jb55: ACK 53c9fa9 Tree-SHA512: f1b9e4e0ee45aae892e8bf38e04b5ee5fbc643d6e7e27d011b829ed8701dacf966a99b7c877c46cca8666b894a375633e62582c552c8203614c6f2b9c4087585
The tracepoint
validation:block_connectedwas introduced in #22006.The first argument was the hash of the connected block as a pointer
to a C-like String. The last argument passed the hash of the
connected block as a pointer to 32 bytes. The hash was only passed as
string to allow
bpftracescripts to print the hash. It was(incorrectly) assumed that
bpftracecannot hex-format and print theblock hash given only the hash as bytes.
The block hash can be printed in
bpftraceby callingprintf("%02x")for each byte of the hash in anunroll () {...}.By starting from the last byte of the hash, it can be printed in
big-endian (the block-explorer format).
See also: #22902 (comment)
This is a breaking change to the block_connected tracepoint API, however
this tracepoint has not yet been included in a release.