build: add systemtap’s sys/sdt.h as depends for GUIX builds with USDT tracepoints #23724

pull 0xB10C wants to merge 2 commits into bitcoin:master from 0xB10C:2021-12-usdt-depends changing 7 files +64 −13
  1. 0xB10C commented at 3:15 pm on December 9, 2021: member

    There has been light conceptual agreement on including the Userspace, Statically Defined Tracing tracepoints in Bitcoin Core release builds. This, for example, enables user to hook into production deployments, if they need to. Binaries don’t have to be switched out. This is possible because we don’t do expensive computations only needed for the tracepoints. The tracepoints are NOPs when not used.

    Systemtap’s sys/sdt.h header is required to build Bitcoin Core with USDT support. The header file defines the DTRACE_PROBE macros used in src/util/trace.h. This PR adds Systemtap 4.5 (May 2021) as dependency. GUIX builds for Linux hosts now include the tracepoints.

    Closes #23297.

  2. 0xB10C commented at 3:17 pm on December 9, 2021: member

    Additional information that may be relevant for reviewers:

    • I’ve documented my progress on including sys/sdt.h in #23297
    • Build support for the USDT was initially added in #19866. Some initial discussion on tracepoints in release builds took place in this thread #19866 (review). Some more discussion in a bitcoin-core-dev meeting January 2021.
    • There was a PR review club on #22006
    • We have documentation listing tracepoints in a binary in doc/tracing.md.
    • The tracepoints are currently not automatically/CI tested (#23296). Merging this now could leave us with broken tracepoints in a release build.

    I’ve tested the GUIX binaries on aarch64-linux-gnu, arm-linux-gnueabihf, x86_64-linux-gnu and on x86_64-w64-mingw32. I don’t have hardware to test on x86_64-apple-darwin19, powerpc64le-linux-gnu, powerpc64-linux-gnu, or powerpc64-linux-gnu (though I might try running the binaries in qemu). Any help testing on these platforms would be appreciated. Of course, testing and review on the already tested platforms is welcome too!

    platform binary contains tracepoints synced signet tracepoints tested and working
    aarch64-linux-gnu :heavy_check_mark: :heavy_check_mark: :heavy_check_mark:
    arm-linux-gnueabihf :heavy_check_mark: :heavy_check_mark: :grey_exclamation:¹
    powerpc64-linux-gnu :heavy_check_mark:
    powerpc64le-linux-gnu :heavy_check_mark:
    riscv64-linux-gnu :heavy_check_mark: [:heavy_check_mark: (laanwj)][laanwj-riscv] [:grey_exclamation: (laanwj)][laanwj-riscv]
    x86_64-linux-gnu :heavy_check_mark: :heavy_check_mark: :heavy_check_mark:
    x86_64-apple-darwin19 - ² -
    x86_64-w64-mingw32 - :heavy_check_mark: -

    [laanwj-riscv]: #23724 (comment)

    ¹ bpftrace isn’t available on Ubuntu 21.10 for armhf. BCC is available, but compiling the C code didn’t work. Presumably it only worrks on 64-bit? Needs further testing. ² ~Maybe the x86_64-apple-darwin19 binaries contain tracepoints that can be listed with e.g. dtrace on macOS?~ No, tracepoints aren’t compiled in as this check fails. This is OK, we currently only support tracepoints on Linux.

    @DrahtBot !request_GUIX_build

  3. laanwj commented at 3:20 pm on December 9, 2021: member

    There has been light conceptual agreement on including the Userspace, Statically Defined Tracing tracepoints in Bitcoin Core release builds. This, for example, enables user to hook into production deployments, if they need to

    Big concept ACK

  4. DrahtBot added the label Build system on Dec 9, 2021
  5. DrahtBot commented at 5:57 pm on December 10, 2021: member

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

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #22708 (build, qt: Add Wayland support for Linux builds with depends by hebasto)
    • #22555 (build: Fix make apk for Android w/ non-default SOURCES_PATH in depends by hebasto)
    • #22552 (build: Improve depends build system robustness by hebasto)

    If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

  6. laanwj commented at 4:24 pm on December 13, 2021: member

    Any help testing on these platforms would be appreciated.

    As I have a running RV64 system (SiFive Unmatched) I can give it a try on riscv64-linux-gnu. Which steps did you follow for “synced signet” and “tracepoints tested”? Literally just sync signet, and try the tracepoint examples?

  7. jb55 commented at 4:45 pm on December 13, 2021: member
    Concept ACK, will test soon
  8. 0xB10C commented at 5:25 pm on December 13, 2021: member

    I can give it a try on riscv64-linux-gnu.

    Thanks @laanwj! That would be awesome!

    Which steps did you follow for “synced signet” and “tracepoints tested”? Literally just sync signet, and try the tracepoint examples?

    Yes. By syncing signet I just wanted to make sure nothing is horribly broken on that arch (I see no reason why it should be). Running the tests suits should work as well. I ran the p2p_monitor.py and a bpftrace example to test the tracepoints. Happy to do further testing if needed!

  9. 0xB10C commented at 5:28 pm on December 13, 2021: member
    We’ll be discussing this PR in the BItcoin Core PR review club this Wednesday at 17:00 UTC: https://bitcoincore.reviews/23724
  10. laanwj commented at 6:13 pm on December 14, 2021: member

    Tested on riscv64-linux-gnu:

    • :heavy_check_mark: Tracepoints are present in the binary (checked only bitcoind and bitcoin-qt)
    0$ gdb bitcoind
1GNU gdb (Debian 10.1-2) 10.1.90.20210103-git
2(gdb) info probes
3Type Provider   Name             Where              Semaphore Object
4stap net        inbound_message  0x00000000000ea198           /…/bitcoin-070570806404/bin/bitcoind
5…
    • :heavy_check_mark: Signet sync from scratch to successful to height=68616
    • :woman_shrugging: Tracepoints tested

    No bcc or bpftrace packages exist for RISC-V debian, so I had to build them from source (against clang13).

    Bcc builds successfully (including python module), unfortunately, bpftrace doesn’t support riscv64 at the moment:

    0CMake Error at src/arch/CMakeLists.txt:14 (message):
1  Unsupported architecture: riscv64

    Trying to run the python scripts runs into parse errors:

     0# ./log_raw_p2p_msgs.py …/bitcoin-070570806404/bin/bitcoind
 1Parse error:
 2    -8@s2 8@s3 8@a0 8@24(s1) 8@a5 8@a4
 3---------^
 4Parse error:
 5    -8@s2 8@s3 8@a0 8@24(s1) 8@a5 8@a4
 6--------------^
 7Parse error:
 8    -8@s2 8@s3 8@a0 8@24(s1) 8@a5 8@a4
 9-------------------^
10Parse error:
11    -8@s2 8@s3 8@a0 8@24(s1) 8@a5 8@a4

    So I think this is a no-go for now.

  11. 0xB10C commented at 7:30 pm on December 14, 2021: member

    Thanks @laanwj!! I’ve updated the table in #23724 (comment).

    From the parse error it seems like bcc isn’t able to parse the register names in the systemtap ELF notes in bitcoind. There was some RISC-V work in https://github.com/iovisor/bcc/pull/3678 which might have touched this. Not sure though. I’m getting a similar error on arm-linux-gnueabihf:

     0$ python3 log_raw_p2p_msgs.py ../../../bitcoin-0729f577f0e2/bin/bitcoind
 1Parse error:
 2    -8@r8 4@r6 4@r0 4@[r5, [#12](/bitcoin-bitcoin/12/)] 4@r3 4@r2
 3---------^
 4Parse error:
 5    -8@r8 4@r6 4@r0 4@[r5, [#12](/bitcoin-bitcoin/12/)] 4@r3 4@r2
 6--------------^
 7Parse error:
 8    -8@r8 4@r6 4@r0 4@[r5, [#12](/bitcoin-bitcoin/12/)] 4@r3 4@r2
 9-------------------^
10Parse error:
11    -8@r8 4@r6 4@r0 4@[r5, [#12](/bitcoin-bitcoin/12/)] 4@r3 4@r2
12--------------------------^
13[..]

    I haven’t tested the tracepoints with the Systemtap tools at all. Maybe this can be used on arm-linux-gnueabihf, and riscv64-linux-gnu? If not, we might not want to build binaries with tracepoints for these platforms. Only supporting x86_64-linux-gnu and aarch64-linux-gnu in release builds for now.

  12. 0xB10C commented at 7:40 pm on December 14, 2021: member

    I’ve also checked that this check fails for x86_64-apple-darwin19 and x86_64-w64-mingw32. Also USDT tracing = no is being printed during configure. This means we don’t need to copy the new systemtap depends package for these two platforms. No tracepoints on macOS and Windows.

    I’ve update the table in #23724 (comment).

  13. laanwj commented at 10:07 am on December 15, 2021: member

    From the parse error it seems like bcc isn’t able to parse the register names in the systemtap ELF notes in bitcoind. There was some RISC-V work in iovisor/bcc#3678

    Thanks, I’ll look into that patch. Edit: I am running the version from git, so should have that merged PR already, so unfortunately it doesn’t fix it.

    If not, we might not want to build binaries with tracepoints for these platforms. Only supporting x86_64-linux-gnu and aarch64-linux-gnu in release builds for now.

    It depends, if compiler support is working, and it’s purely an issue with the tooling (bcc, bpftrace) I’d argue there is no reason for disabling it on those platforms. It will work as soon as the tools are updated to support the architectures (especially for RISC-V I expect this to be happening soon). I guess the problem is that we can’t really test that the tracepoints are inserted correctly in the binary?

    Or maybe we can through gdb? E.g. put a breakpoint at a tracepoint and then inspect the arguments manually.

    Edit: I checked bcc (as of commit 52900a435de85e706f5280f4a184374dd729c58e, current HEAD) and FWIW, specialized argumentparsers only exist for the following platforms:

    0class ArgumentParser_aarch64 : public ArgumentParser {
1class ArgumentParser_powerpc64 : public ArgumentParser {
2class ArgumentParser_s390x : public ArgumentParser {
3class ArgumentParser_x64 : public ArgumentParser {

    There is no general behavior: the base class is a pure virtual base class. So while I haven’t checked which one it’s instantiating for riscv64-linux-gnu and arm-linux-gnueabihf, it can’t actually be correct :smile:

    Edit.2: On unsupported architectures, you get the _x64 one for free !!!

    0#ifdef __aarch64__
1  ArgumentParser_aarch64 parser(arg_fmt);
2#elif __powerpc64__
3  ArgumentParser_powerpc64 parser(arg_fmt);
4#elif __s390x__
5  ArgumentParser_s390x parser(arg_fmt);
6#else
7  ArgumentParser_x64 parser(arg_fmt);
8#endif
  14. in depends/packages/systemtap.mk:8 in 0705708064 outdated 
    0@@ -0,0 +1,15 @@
1+package=systemtap
2+$(package)_version=4.5
3+$(package)_download_path=https://sourceware.org/systemtap/ftp/releases/
4+$(package)_file_name=$(package)-$($(package)_version).tar.gz
5+$(package)_sha256_hash=75078ed37e0dd2a769c9d1f9394170b2d9f4d7daa425f43ca80c13bad6cfc925
6+$(package)_patches=remove_SDT_ASM_SECTION_AUTOGROUP_SUPPORT_check.patch
7+
8+# Only the includes/sys/sdt.h header is needed from Systemtap.
    fanquake commented at 12:38 pm on December 15, 2021:
    I don’t think we need these comments. If anything, a comment in regards to why you’re using 4.5 rather than 4.6 (GCC issues) could be useful, and/or noting that 4.7 should be safe to use.
  15. ccdle12 commented at 1:26 pm on December 15, 2021: contributor
    Concept ACK, running the guix build
  16. ccdle12 commented at 10:08 pm on December 15, 2021: contributor

    Testing on Ubuntu 18, was getting segfaults for log_utxos.bt and connectblock_benchmark.bt. Had to build bpftrace from source with the following versions:

    • bpftrace v0.12.1
    • bcc v0.8.0 (required to build bpftrace from source)
  17. in depends/packages/packages.mk:22 in 0705708064 outdated 
    18@@ -19,6 +19,8 @@ natpmp_packages=libnatpmp
19 multiprocess_packages = libmultiprocess capnp
20 multiprocess_native_packages = native_libmultiprocess native_capnp
21 
22+usdt_packages=systemtap
    fanquake commented at 3:06 am on December 16, 2021:

    I think this should be Linux only for now (will solve not copying the headers for darwin and win).

    0linux_usdt_packages=systemtap
    0xB10C commented at 5:58 pm on December 16, 2021:

    Agree this should only be Linux only for now. However, the suggested change doesn’t seem to do the trick.

    The systemtap package isn’t copied:

    0copying packages: native_b2 boost libevent qt expat libxcb xcb_proto libXau xproto freetype fontconfig libxkbcommon libxcb_util libxcb_util_render libxcb_util_keysyms libx
1to: /bitcoin/depends/x86_64-linux-gnu

    Probably needs changes in depends/Makefile too (grep for usdt_packages). I’ll have a look and see if I can figure it out.

    fanquake commented at 3:18 am on December 17, 2021:
    Sorry, here’s a working diff. See https://github.com/fanquake/bitcoin/commit/4c29ad41210ba542716da476f2c596929664270c in the 23724_linux_only branch.
  18. in depends/packages/systemtap.mk:14 in 0705708064 outdated 
     9+# The configure and make steps and not required.
10+
11+define $(package)_preprocess_cmds
12+  patch -p1 < $($(package)_patch_dir)/remove_SDT_ASM_SECTION_AUTOGROUP_SUPPORT_check.patch && \
13+  mkdir -p $($(package)_staging_prefix_dir)/include/sys && \
14+  cp -r includes/sys $($(package)_staging_prefix_dir)/include
    fanquake commented at 4:36 am on December 16, 2021:

    Given we aren’t using sdt-config.h, there’s no reason to copy sdt-config.h.in into our includes dir. This could be simplified to:

    0  cp includes/sys/sdt.h $($(package)_staging_prefix_dir)/include/sys/sdt.h

    This would also make it clear that the only thing we care about is the sdt header.

  19. in depends/patches/systemtap/remove_SDT_ASM_SECTION_AUTOGROUP_SUPPORT_check.patch:7 in 0705708064 outdated 
    0@@ -0,0 +1,31 @@
1+commit b92d4c121486f3c6e8a2cea537c53eb09894479a
2+Author: 0xb10c <0xb10c@gmail.com>
3+Date:   Tue Dec 7 11:02:07 2021 +0100
4+
5+    Remove _SDT_ASM_SECTION_AUTOGROUP_SUPPORT check
6+    
7+    We assume that the assembler supports "?" in .pushsection directives.
    fanquake commented at 4:41 am on December 16, 2021:

    After looking at this again, I think assuming the assembler supports ? is ok, assuming we make this Linux only (see other review comments).

    pushsection is an ELF specific directive. If you try to compile the assembly used in the configure check which would define _SDT_ASM_SECTION_AUTOGROUP_SUPPORT, with mingw32-gcc, it won’t compile:

     0int main() {
 1
 2asm(".section .note.foo,\"?\",\"note\"\n"
 3     ".byte 1, 2, 3\n"
 4     ".previous\n"
 5     ".section .text,\"axG\",\"progbits\",foogroup,comdat\n"
 6     ".byte 1\n"
 7     ".pushsection .note.foo,\"?\",\"note\"\n"
 8     ".byte 4, 5, 6\n"
 9     ".popsection\n"
10     ".byte 2\n");
11return 0;
12}

    0x86_64-w64-mingw32-gcc push.c 
1/tmp/ccDQdocw.s: Assembler messages:
2/tmp/ccDQdocw.s:18: Warning: unknown section attribute '?'
3/tmp/ccDQdocw.s:18: Error: junk at end of line, first unrecognized character is `,'
4/tmp/ccDQdocw.s:20: Error: unknown pseudo-op: `.previous'
5/tmp/ccDQdocw.s:21: Warning: unknown section attribute 'G'
6/tmp/ccDQdocw.s:21: Warning: Ignoring changed section attributes for .text
7/tmp/ccDQdocw.s:21: Error: junk at end of line, first unrecognized character is `,'
8/tmp/ccDQdocw.s:23: Error: unknown pseudo-op: `.pushsection'
9/tmp/ccDQdocw.s:25: Error: unknown pseudo-op: `.popsection'

    I think making the systemtap package Linux only for now is the way to go.

  20. fanquake commented at 4:46 am on December 16, 2021: member

    Concept ACK on adding tracepoints to the release builds, assuming there is 0 overhead for users who don’t care about them. I think they should only be use for Linux for now. macOS support isn’t as straightforward as Linux, and it’s not clear to me if these are even a thing on Windows.

    Guix build:

     0bash-5.1# find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
 134bbefa86359e60b815552bb61d5e947eb6eaff5ab942b0415c9cdc0dad187b6  guix-build-070570806404/output/aarch64-linux-gnu/SHA256SUMS.part
 248d6dcde292965cf81aa52d6f55c45e5b73281b863382c1f0aeae23ab4c10394  guix-build-070570806404/output/aarch64-linux-gnu/bitcoin-070570806404-aarch64-linux-gnu-debug.tar.gz
 3c72b18b090fc72d7fbf50bb41d6289ec71d234513102a96ecfaa8aa6003f7855  guix-build-070570806404/output/aarch64-linux-gnu/bitcoin-070570806404-aarch64-linux-gnu.tar.gz
 4c98a0b60b26fae5cf8b57212a70a160c8e9f8fc1756039bcaabeb833e921a414  guix-build-070570806404/output/arm-linux-gnueabihf/SHA256SUMS.part
 5cbddd5bdcbc9eebab5aafdf3267c89badc1d3acf19f8e9cb1fb0dba5a83a86c9  guix-build-070570806404/output/arm-linux-gnueabihf/bitcoin-070570806404-arm-linux-gnueabihf-debug.tar.gz
 6b465dffd7149f9f02c9255a0a5befec005c0b552bddbee40fbddfd2b0ca9d4e6  guix-build-070570806404/output/arm-linux-gnueabihf/bitcoin-070570806404-arm-linux-gnueabihf.tar.gz
 7e7a68186bf3d37e2c2134ae11a465f21f0b9d5d3233380e85bfd5c9eda043992  guix-build-070570806404/output/dist-archive/bitcoin-070570806404.tar.gz
 817b0190a61345c7b6c7fd058bdf13ec9f50bab6eb4c0302c131a9b07a5c4433a  guix-build-070570806404/output/powerpc64-linux-gnu/SHA256SUMS.part
 99d2c24eddceb5a88caa8372dfd1c306bde0266edaa01aa101da4dcd089ce9d41  guix-build-070570806404/output/powerpc64-linux-gnu/bitcoin-070570806404-powerpc64-linux-gnu-debug.tar.gz
10cf0d5cd27c514b639eb22e577863a0ecf8bd45336522936eabfc9fa3963bab65  guix-build-070570806404/output/powerpc64-linux-gnu/bitcoin-070570806404-powerpc64-linux-gnu.tar.gz
11af2930854be42d18f52787091fe4b796995b2cadec27a11262bbf5bcc284a869  guix-build-070570806404/output/powerpc64le-linux-gnu/SHA256SUMS.part
125b307c9cf5032ded0c11586f0533ce404dda93f7edc136260f5e4fdeeabd5442  guix-build-070570806404/output/powerpc64le-linux-gnu/bitcoin-070570806404-powerpc64le-linux-gnu-debug.tar.gz
136fe051d7b241b3c3d57f4c4ad40290b79eb4561d2abd4af110d1caefb5d0e3d3  guix-build-070570806404/output/powerpc64le-linux-gnu/bitcoin-070570806404-powerpc64le-linux-gnu.tar.gz
144b1c765f7a0c0110f51eb2ea80f974c0803654440f4dd22f5b5dee69f4429e89  guix-build-070570806404/output/riscv64-linux-gnu/SHA256SUMS.part
1565454fe4048692e9eec35d06b6809a6f28fef9dbb9b1fc6eed734f6077c260ef  guix-build-070570806404/output/riscv64-linux-gnu/bitcoin-070570806404-riscv64-linux-gnu-debug.tar.gz
1687c08928fd01eed62b4d68a1309cbb8c0943a655b169451cfb97e4911532270a  guix-build-070570806404/output/riscv64-linux-gnu/bitcoin-070570806404-riscv64-linux-gnu.tar.gz
1705a3dd5516e4fc744b07d3342bc385ad0197ea5e7028783e6b156fddf0defebc  guix-build-070570806404/output/x86_64-apple-darwin/SHA256SUMS.part
185683bec557166c36d44dfccdf7c627952bde300a4a35ec4f63197513bffc0b9e  guix-build-070570806404/output/x86_64-apple-darwin/bitcoin-070570806404-osx-unsigned.dmg
196cb290ddf4e2b15404a2da9af160523c72d74e428ccc367e73c8ccbf111a0fe0  guix-build-070570806404/output/x86_64-apple-darwin/bitcoin-070570806404-osx-unsigned.tar.gz
205a7d3ad1643789dcda1329440238d236bf497599ba423ca686ab08fd63149c62  guix-build-070570806404/output/x86_64-apple-darwin/bitcoin-070570806404-osx64.tar.gz
21b03af7a9654d852a3d65e0c4968dd6ebcc16587f10ac4c6bc59d6ee1ae9763b0  guix-build-070570806404/output/x86_64-linux-gnu/SHA256SUMS.part
22fe9ecd853fa7bbe9d9896bf4b66c451eb47aa6b11d37643d52ea529e44007b3c  guix-build-070570806404/output/x86_64-linux-gnu/bitcoin-070570806404-x86_64-linux-gnu-debug.tar.gz
233bbf38305cfff63765f39675cea5e27c2dc3590ec6f54e07e18372e8953468c8  guix-build-070570806404/output/x86_64-linux-gnu/bitcoin-070570806404-x86_64-linux-gnu.tar.gz
247959f8df5702f5d68a57acdbd8b51c49fe9adc2c2a85fec0d8f10e3b9c78b7a9  guix-build-070570806404/output/x86_64-w64-mingw32/SHA256SUMS.part
2528a7a7d8253f07d4109a33e5d7eb8725be50d6255ab42fc398b239b655efc463  guix-build-070570806404/output/x86_64-w64-mingw32/bitcoin-070570806404-win-unsigned.tar.gz
26805150f60152a39220f7c8c4ce0f2bd3950d03c0c9320ff931b3ba1e807f8be7  guix-build-070570806404/output/x86_64-w64-mingw32/bitcoin-070570806404-win64-debug.zip
2772b5edabde57d23dde1be60fbc95b3c35df0788bab11583002233c39953500f4  guix-build-070570806404/output/x86_64-w64-mingw32/bitcoin-070570806404-win64-setup-unsigned.exe
2895d631b9bf70c45374f1a756257ef676ceec8553906a2876cd3e2c004d6e3a39  guix-build-070570806404/output/x86_64-w64-mingw32/bitcoin-070570806404-win64.zip
  21. 0xB10C force-pushed on Dec 17, 2021
  22. 0xB10C commented at 6:09 pm on December 17, 2021: member

    Thanks for your comments @fanquake. I’ve addressed them in my last push.

    Concept ACK on adding tracepoints to the release builds, assuming there is 0 overhead for users who don’t care about them.

    This isn’t the case. There is bit of overhead, even if it’s only slightly more than zero overhead. When we compile a binary without tracepoints, the arguments (e.g. function calls) passed to the TRACEx macros are optimized out. When tracepoints are enabled, these arguments and executed functions cause additional instructions to be executed.The tracepoint it self is a single NOP instruction. No additional instructions are executed for the actual tracepoint when not used (the arguments are still processed).

    In terms of overhead:

    0binary without tracepoints < binary with tracepoints < binary with tracepoints and tracepoints being used

    As an example, I’m looking at the net:inbound_message tracepoint in net_processing.cpp at commit 767c012665953557ad02f731a84c6d10907a46af. This is the relevant code section:

    https://github.com/bitcoin/bitcoin/blob/767c012665953557ad02f731a84c6d10907a46af/src/net_processing.cpp#L4141-L4152

    To count the number of executed instructions I’m using gdb and set two breakpoints. The first breakpoint in line 4141 and the second breakpoint in line 4152. When hitting the first breakpoint, I run record btrace (see GDB Process Record and Replay) and continue to the second breakpoint. There I print information about the recording with info record and record end. This prints the number of executed instructions between the two breakpoints. I do this for a binary with and a binary without breakpoints.

    To automate this process you can use the following gdb script:

     0b net_processing.cpp:4141
 1commands 1
 2  record btrace
 3  continue
 4end
 5
 6b net_processing.cpp:4152
 7commands 2
 8  info record
 9  record stop
10  continue
11end
12
13run

    0$ gdb --batch --command=net_inboundmsg_ins.gdb --args bitcoind -debug=net | grep -e 'received\:\|instructions in'

    This interlaces bitcoind’s net logging for received messages with the number of instructions being executed. Here is the output it produced for me on x86_64-linux-gnu (Intel CPU):

    (I’m not sure why the first record includes 469 instructions.)

     0Recorded 469 instructions in 28 functions (0 gaps) for thread 21 (Thread 0x7fff38868640 (LWP 60195)).
 12021-12-17T17:24:53Z received: version (102 bytes) peer=0
 2Recorded 278 instructions in 26 functions (0 gaps) for thread 21 (Thread 0x7fff38868640 (LWP 60195)).
 32021-12-17T17:24:53Z received: verack (0 bytes) peer=0
 4Recorded 278 instructions in 26 functions (0 gaps) for thread 21 (Thread 0x7fff38868640 (LWP 60195)).
 52021-12-17T17:24:53Z received: sendheaders (0 bytes) peer=0
 6Recorded 278 instructions in 26 functions (0 gaps) for thread 21 (Thread 0x7fff38868640 (LWP 60195)).
 72021-12-17T17:24:53Z received: sendcmpct (9 bytes) peer=0
 8Recorded 278 instructions in 26 functions (0 gaps) for thread 21 (Thread 0x7fff38868640 (LWP 60195)).
 92021-12-17T17:24:53Z received: sendcmpct (9 bytes) peer=0
10Recorded 278 instructions in 26 functions (0 gaps) for thread 21 (Thread 0x7fff38868640 (LWP 60195)).
112021-12-17T17:24:53Z received: ping (8 bytes) peer=0
12Recorded 278 instructions in 26 functions (0 gaps) for thread 21 (Thread 0x7fff38868640 (LWP 60195)).
132021-12-17T17:24:53Z received: getheaders (1029 bytes) peer=0
14Recorded 278 instructions in 26 functions (0 gaps) for thread 21 (Thread 0x7fff38868640 (LWP 60195)).
152021-12-17T17:24:53Z received: feefilter (8 bytes) peer=0
162021-12-17T17:24:53Z received: feefilter of 0.00001000 BTC/kvB from peer=0
17Recorded 278 instructions in 26 functions (0 gaps) for thread 21 (Thread 0x7fff38868640 (LWP 60195)).
182021-12-17T17:24:53Z received: pong (8 bytes) peer=0
19Recorded 278 instructions in 26 functions (0 gaps) for thread 21 (Thread 0x7fff38868640 (LWP 60195)).
202021-12-17T17:24:54Z received: headers (22683 bytes) peer=0
21Recorded 278 instructions in 26 functions (0 gaps) for thread 21 (Thread 0x7fff38868640 (LWP 60195)).
222021-12-17T17:24:54Z received: block (1512354 bytes) peer=0
23Recorded 278 instructions in 26 functions (0 gaps) for thread 21 (Thread 0x7fff38868640 (LWP 60195)).
242021-12-17T17:24:55Z received: block (1439515 bytes) peer=0
25Recorded 275 instructions in 26 functions (0 gaps) for thread 21 (Thread 0x7fff38868640 (LWP 60195)).
262021-12-17T17:24:55Z received: block (1511944 bytes) peer=0
27Recorded 278 instructions in 26 functions (0 gaps) for thread 21 (Thread 0x7fff38868640 (LWP 60195)).
282021-12-17T17:24:56Z received: block (1015177 bytes) peer=0
29Recorded 275 instructions in 26 functions (0 gaps) for thread 21 (Thread 0x7fff38868640 (LWP 60195)).
302021-12-17T17:24:56Z received: block (743309 bytes) peer=0
31Recorded 278 instructions in 26 functions (0 gaps) for thread 21 (Thread 0x7fff38868640 (LWP 60195)).
322021-12-17T17:24:58Z received: block (1448974 bytes) peer=0
33Recorded 278 instructions in 26 functions (0 gaps) for thread 21 (Thread 0x7fff38868640 (LWP 60195)).
342021-12-17T17:24:59Z received: block (324652 bytes) peer=0
35Recorded 278 instructions in 26 functions (0 gaps) for thread 21 (Thread 0x7fff38868640 (LWP 60195)).
362021-12-17T17:25:00Z received: block (1478186 bytes) peer=0
37Recorded 278 instructions in 26 functions (0 gaps) for thread 21 (Thread 0x7fff38868640 (LWP 60195)).
382021-12-17T17:25:04Z received: block (1547585 bytes) peer=0
39Recorded 278 instructions in 26 functions (0 gaps) for thread 21 (Thread 0x7fff38868640 (LWP 60195)).
402021-12-17T17:25:13Z received: version (104 bytes) peer=3
41Recorded 275 instructions in 26 functions (0 gaps) for thread 21 (Thread 0x7fff38868640 (LWP 60195)).
422021-12-17T17:25:13Z received: block (57225 bytes) peer=0
43Recorded 275 instructions in 26 functions (0 gaps) for thread 21 (Thread 0x7fff38868640 (LWP 60195)).
442021-12-17T17:25:13Z received: version (102 bytes) peer=4
45Recorded 275 instructions in 26 functions (0 gaps) for thread 21 (Thread 0x7fff38868640 (LWP 60195)).
462021-12-17T17:25:13Z received: verack (0 bytes) peer=3
47Recorded 275 instructions in 26 functions (0 gaps) for thread 21 (Thread 0x7fff38868640 (LWP 60195)).
482021-12-17T17:25:13Z received: block (1692970 bytes) peer=0
49Recorded 275 instructions in 26 functions (0 gaps) for thread 21 (Thread 0x7fff38868640 (LWP 60195)).
502021-12-17T17:25:18Z received: sendheaders (0 bytes) peer=3
51Recorded 275 instructions in 26 functions (0 gaps) for thread 21 (Thread 0x7fff38868640 (LWP 60195)).
522021-12-17T17:25:18Z received: verack (0 bytes) peer=4
53Recorded 275 instructions in 26 functions (0 gaps) for thread 21 (Thread 0x7fff38868640 (LWP 60195)).
542021-12-17T17:25:18Z received: sendcmpct (9 bytes) peer=3
55Recorded 278 instructions in 26 functions (0 gaps) for thread 21 (Thread 0x7fff38868640 (LWP 60195)).
562021-12-17T17:25:18Z received: block (909231 bytes) peer=0

     0Recorded 4 instructions in 1 functions (0 gaps) for thread 21 (Thread 0x7fff2a7fc640 (LWP 85161)).
 12021-12-17T17:49:51Z received: version (102 bytes) peer=0
 2Recorded 4 instructions in 1 functions (0 gaps) for thread 21 (Thread 0x7fff2a7fc640 (LWP 85161)).
 32021-12-17T17:50:18Z received: version (102 bytes) peer=2
 4Recorded 4 instructions in 1 functions (0 gaps) for thread 21 (Thread 0x7fff2a7fc640 (LWP 85161)).
 52021-12-17T17:50:18Z received: verack (0 bytes) peer=2
 6Recorded 4 instructions in 1 functions (0 gaps) for thread 21 (Thread 0x7fff2a7fc640 (LWP 85161)).
 72021-12-17T17:50:18Z received: sendheaders (0 bytes) peer=2
 8Recorded 4 instructions in 1 functions (0 gaps) for thread 21 (Thread 0x7fff2a7fc640 (LWP 85161)).
 92021-12-17T17:50:18Z received: sendcmpct (9 bytes) peer=2
10Recorded 4 instructions in 1 functions (0 gaps) for thread 21 (Thread 0x7fff2a7fc640 (LWP 85161)).
112021-12-17T17:50:18Z received: sendcmpct (9 bytes) peer=2
12Recorded 4 instructions in 1 functions (0 gaps) for thread 21 (Thread 0x7fff2a7fc640 (LWP 85161)).
132021-12-17T17:50:18Z received: ping (8 bytes) peer=2
14Recorded 4 instructions in 1 functions (0 gaps) for thread 21 (Thread 0x7fff2a7fc640 (LWP 85161)).
152021-12-17T17:50:18Z received: getheaders (1029 bytes) peer=2
16Recorded 4 instructions in 1 functions (0 gaps) for thread 21 (Thread 0x7fff2a7fc640 (LWP 85161)).
172021-12-17T17:50:18Z received: feefilter (8 bytes) peer=2
182021-12-17T17:50:18Z received: feefilter of 0.00001000 BTC/kvB from peer=2
19Recorded 4 instructions in 1 functions (0 gaps) for thread 21 (Thread 0x7fff2a7fc640 (LWP 85161)).
202021-12-17T17:50:18Z received: pong (8 bytes) peer=2
21Recorded 4 instructions in 1 functions (0 gaps) for thread 21 (Thread 0x7fff2a7fc640 (LWP 85161)).
222021-12-17T17:50:18Z received: headers (23007 bytes) peer=2
23Recorded 4 instructions in 1 functions (0 gaps) for thread 21 (Thread 0x7fff2a7fc640 (LWP 85161)).
242021-12-17T17:50:19Z received: block (1512354 bytes) peer=2
25Recorded 4 instructions in 1 functions (0 gaps) for thread 21 (Thread 0x7fff2a7fc640 (LWP 85161)).
262021-12-17T17:50:19Z received: block (1439515 bytes) peer=2
27Recorded 4 instructions in 1 functions (0 gaps) for thread 21 (Thread 0x7fff2a7fc640 (LWP 85161)).
282021-12-17T17:50:20Z received: block (1511944 bytes) peer=2
29Recorded 4 instructions in 1 functions (0 gaps) for thread 21 (Thread 0x7fff2a7fc640 (LWP 85161)).
302021-12-17T17:50:20Z received: block (1015177 bytes) peer=2
31Recorded 4 instructions in 1 functions (0 gaps) for thread 21 (Thread 0x7fff2a7fc640 (LWP 85161)).
322021-12-17T17:50:21Z received: block (743309 bytes) peer=2
33Recorded 4 instructions in 1 functions (0 gaps) for thread 21 (Thread 0x7fff2a7fc640 (LWP 85161)).
342021-12-17T17:50:21Z received: block (1448974 bytes) peer=2
35Recorded 4 instructions in 1 functions (0 gaps) for thread 21 (Thread 0x7fff2a7fc640 (LWP 85161)).
362021-12-17T17:50:21Z received: block (324652 bytes) peer=2
37Recorded 4 instructions in 1 functions (0 gaps) for thread 21 (Thread 0x7fff2a7fc640 (LWP 85161)).
382021-12-17T17:50:22Z received: block (1478186 bytes) peer=2
39Recorded 4 instructions in 1 functions (0 gaps) for thread 21 (Thread 0x7fff2a7fc640 (LWP 85161)).
402021-12-17T17:50:22Z received: block (1547585 bytes) peer=2
41Recorded 4 instructions in 1 functions (0 gaps) for thread 21 (Thread 0x7fff2a7fc640 (LWP 85161)).
422021-12-17T17:50:25Z received: block (57225 bytes) peer=2
43Recorded 4 instructions in 1 functions (0 gaps) for thread 21 (Thread 0x7fff2a7fc640 (LWP 85161)).
442021-12-17T17:50:25Z received: block (1692970 bytes) peer=2
45Recorded 4 instructions in 1 functions (0 gaps) for thread 21 (Thread 0x7fff2a7fc640 (LWP 85161)).
462021-12-17T17:50:28Z received: block (909231 bytes) peer=2
47Recorded 4 instructions in 1 functions (0 gaps) for thread 21 (Thread 0x7fff2a7fc640 (LWP 85161)).
482021-12-17T17:50:29Z received: version (120 bytes) peer=3
49Recorded 4 instructions in 1 functions (0 gaps) for thread 21 (Thread 0x7fff2a7fc640 (LWP 85161)).
502021-12-17T17:50:29Z received: block (1487873 bytes) peer=2
51Recorded 4 instructions in 1 functions (0 gaps) for thread 21 (Thread 0x7fff2a7fc640 (LWP 85161)).
522021-12-17T17:50:31Z received: block (1597437 bytes) peer=2
53Recorded 4 instructions in 1 functions (0 gaps) for thread 21 (Thread 0x7fff2a7fc640 (LWP 85161)).
542021-12-17T17:50:32Z received: wtxidrelay (0 bytes) peer=3
55Recorded 4 instructions in 1 functions (0 gaps) for thread 21 (Thread 0x7fff2a7fc640 (LWP 85161)).
562021-12-17T17:50:32Z received: block (1482700 bytes) peer=2
57Recorded 4 instructions in 1 functions (0 gaps) for thread 21 (Thread 0x7fff2a7fc640 (LWP 85161)).
582021-12-17T17:50:33Z received: sendaddrv2 (0 bytes) peer=3
59Recorded 4 instructions in 1 functions (0 gaps) for thread 21 (Thread 0x7fff2a7fc640 (LWP 85161)).
602021-12-17T17:50:33Z received: verack (0 bytes) peer=3
61Recorded 4 instructions in 1 functions (0 gaps) for thread 21 (Thread 0x7fff2a7fc640 (LWP 85161)).
622021-12-17T17:50:33Z received: block (1537427 bytes) peer=2
63Recorded 4 instructions in 1 functions (0 gaps) for thread 21 (Thread 0x7fff2a7fc640 (LWP 85161)).
642021-12-17T17:50:33Z received: block (1560096 bytes) peer=2
65Recorded 4 instructions in 1 functions (0 gaps) for thread 21 (Thread 0x7fff2a7fc640 (LWP 85161)).
662021-12-17T17:50:34Z received: sendheaders (0 bytes) peer=3
67Recorded 4 instructions in 1 functions (0 gaps) for thread 21 (Thread 0x7fff2a7fc640 (LWP 85161)).
682021-12-17T17:50:34Z received: block (755937 bytes) peer=2
69Recorded 4 instructions in 1 functions (0 gaps) for thread 21 (Thread 0x7fff2a7fc640 (LWP 85161)).
702021-12-17T17:50:34Z received: sendcmpct (9 bytes) peer=3
71Recorded 4 instructions in 1 functions (0 gaps) for thread 21 (Thread 0x7fff2a7fc640 (LWP 85161)).
722021-12-17T17:50:34Z received: sendcmpct (9 bytes) peer=3
73Recorded 4 instructions in 1 functions (0 gaps) for thread 21 (Thread 0x7fff2a7fc640 (LWP 85161)).
742021-12-17T17:50:34Z received: block (822822 bytes) peer=2

    When using the binary with tracepoints, ~274 more instructions are executed between the two breakpoints compared to the binary without breakpoints. That’s why, when adding tracepoints, we don’t do expensive computations only needed for the tracepoints and only use data that’s already present. I do think 274 additional instructions (for this tracepoint) isn’t a substantial price to pay and is close to zero overhead. I haven’t done these measurements for the other tracepoints though (probably makes sense to to them).

  23. jb55 commented at 7:02 pm on December 17, 2021: member
    @0xB10C why does the one with tracepoints say instructions in 26 functions and the one without tracepoints says: instructions in 1 functions? is this counting the same things?
  24. 0xB10C commented at 9:00 pm on December 17, 2021: member

    @jb55 If ENABLE_TRACING is defined, the TRACE6(context, event, a, b, c, d, e, f) macro is replaced by the DTRACE_PROBE6(context, event, a, b, c, d, e, f) macro. If the arguments a, b, c, d, e, f are the return values of functions (that’s the case for all net:inbound_message arguments), these get compiled in, executed, and counted by gdb. You can probably step into the functions and count them manually too with gdb.

    If ENABLE_TRACING is not defined, TRACE6(context, event, a, b, c, d, e, f) macro is an empty macro. There is no code being generated. None of the argument functions are compiled in and executed.

    On that note and if we think e.g. ~274 instructions are still too much: We might be able branch before we execute the tracepoint arguments based on e.g. a -tracing command line argument. You’d need to specifically enable tracing for the tracepoints to be reached. However, at the moment I prefer it as is.

    0if (gArgs.GetBoolArg("-tracing", false)) {
1    TRACE6(..)
2}

    (or put this check in the macros too)

  25. jb55 commented at 9:30 pm on December 17, 2021: member
    I guess I’m surprised that those calls that simply return pointers aren’t inlined, but I’m guessing that’s because of some vtable indirection or something. It looks like ConnectionTypeAsString does some work, doesn’t this violate our recommendation of doing no work in tracepoints? It’s out of scope for this PR of course, but it would be interesting to see the instruction diff by just returning the enum instead of the enum string (I’ll go ahead and try this to test out your gdb script).
  26. jb55 commented at 11:15 pm on December 17, 2021: member

    @0xB10C

    before:

    0Recorded 293 instructions in 27 functions

    after with this patch:

     0diff --git a/src/net.h b/src/net.h
 1index 9623a630e7..6f99d56661 100644
 2--- a/src/net.h
 3+++ b/src/net.h
 4@@ -660,6 +660,8 @@ public:
 5 
 6     std::string ConnectionTypeAsString() const { return ::ConnectionTypeAsString(m_conn_type); }
 7 
 8+    ConnectionType GetConnectionType() const { return m_conn_type; }
 9+
10     /** A ping-pong round trip has completed successfully. Update latest and minimum ping times. */
11     void PongReceived(std::chrono::microseconds ping_time) {
12         m_last_ping_time = ping_time;
13diff --git a/src/net_processing.cpp b/src/net_processing.cpp
14index d67ced99ea..6f8209d21c 100644
15--- a/src/net_processing.cpp
16+++ b/src/net_processing.cpp
17@@ -4146,7 +4146,7 @@ bool PeerManagerImpl::ProcessMessages(CNode* pfrom, std::atomic<bool>& interrupt
18     TRACE6(net, inbound_message,
19         pfrom->GetId(),
20         pfrom->m_addr_name.c_str(),
21-        pfrom->ConnectionTypeAsString().c_str(),
22+        pfrom->GetConnectionType(),
23         msg.m_command.c_str(),
24         msg.m_recv.size(),
25         msg.m_recv.data()

    the number of instructions becomes trivial:

    0Recorded 9 instructions in 1 functions

    I wasn’t able to do this same test on my AMD because gdb doesn’t support branch tracing on AMDs apparently :(

  27. 0xB10C commented at 0:04 am on December 18, 2021: member

    @jb55 Oh wouldn’t have thought that ConnectionTypeAsString() was responsible for the majority of these instructions. Thanks for looking into this! I get the same results on Intel:

    0Recorded 9 instructions in 1 functions

    However, I think as soon as the ConnectionType enum is changed, the tracing scripts might break/will confuse the connection types and the tracepoint documentation will be outdated. Additionally, we might want to cast here as we implicitly convert a scoped enum to an int (?). Diving deeper into this here is probably a bit off-topic, but I think it shows that we might want to iterate before having the tracepoints enabled in a release.

    GUIX build hashes for c1df11eafc8c:

     0$ find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
 1b613183271f4b940348c02d98da60123a0d7ae42a79ab06f83c5d23617f8951b  guix-build-c1df11eafc8c/output/aarch64-linux-gnu/SHA256SUMS.part
 2275d2c82c7680220331f08451143deb4856c17fd03c549b15eb3446cfd4e6342  guix-build-c1df11eafc8c/output/aarch64-linux-gnu/bitcoin-c1df11eafc8c-aarch64-linux-gnu-debug.tar.gz
 3479767ad6e6dca437a58f93a0148d9c3d97374a5d70978801bd898b8c03f7cf4  guix-build-c1df11eafc8c/output/aarch64-linux-gnu/bitcoin-c1df11eafc8c-aarch64-linux-gnu.tar.gz
 482c5dfdfe2551590b57b004f4944dba39d359baa13ced6344ef1e2ce940d36ed  guix-build-c1df11eafc8c/output/arm-linux-gnueabihf/SHA256SUMS.part
 589471c634ba5ef3c5e0465c2f2f3386cf66c3ff1b2eafaf6dff0faf9e4781af7  guix-build-c1df11eafc8c/output/arm-linux-gnueabihf/bitcoin-c1df11eafc8c-arm-linux-gnueabihf-debug.tar.gz
 69c94535ff1cb7dc3c8359dd13312f2b9353ed94a4b2569bea9f4e0f0869e0418  guix-build-c1df11eafc8c/output/arm-linux-gnueabihf/bitcoin-c1df11eafc8c-arm-linux-gnueabihf.tar.gz
 76182d4b3a6cf03029b1d85df6a5f16db5929dd8b165066be807d328da995719c  guix-build-c1df11eafc8c/output/dist-archive/bitcoin-c1df11eafc8c.tar.gz
 8f6893a6aa46afe1846c97862367e8c3426a13f5a09f342cfc6f0a74d95eb25e7  guix-build-c1df11eafc8c/output/powerpc64-linux-gnu/SHA256SUMS.part
 9ca591256fe08a17991230ba82ba9ef9ddccb4d73ad7f781d3c2e1665818d7387  guix-build-c1df11eafc8c/output/powerpc64-linux-gnu/bitcoin-c1df11eafc8c-powerpc64-linux-gnu-debug.tar.gz
107930a0806c053741b2bc14f2f738f7d2a5c04a46f4a1dd8b0b01614cd28c26f5  guix-build-c1df11eafc8c/output/powerpc64-linux-gnu/bitcoin-c1df11eafc8c-powerpc64-linux-gnu.tar.gz
112e6865291ca5b4f6e1b4dddb2c5f6f266a77a22990abf0dc01c128625924443e  guix-build-c1df11eafc8c/output/powerpc64le-linux-gnu/SHA256SUMS.part
12909310911698d40e85078f5d54030e1f7273f2d88b8a472f9bbbfe0d74ee5487  guix-build-c1df11eafc8c/output/powerpc64le-linux-gnu/bitcoin-c1df11eafc8c-powerpc64le-linux-gnu-debug.tar.gz
13bd78a8468c899078b55fa9626791f7ea60e319f0fa7c31ddd8b3118e60d76945  guix-build-c1df11eafc8c/output/powerpc64le-linux-gnu/bitcoin-c1df11eafc8c-powerpc64le-linux-gnu.tar.gz
1444bad54bf4aab886c919c96bfef6e82f0d5e0ad094ab571c81c617e6d434fad7  guix-build-c1df11eafc8c/output/riscv64-linux-gnu/SHA256SUMS.part
15d69d02cf9fbb1124ec858dcc0e8f3f345031fde6a677eb720a6d6d92580cc11e  guix-build-c1df11eafc8c/output/riscv64-linux-gnu/bitcoin-c1df11eafc8c-riscv64-linux-gnu-debug.tar.gz
1686a8ad2bc5bda6ff9e6a502e2e05f32e410778dbcb61c3e96ff0cf2e784c1720  guix-build-c1df11eafc8c/output/riscv64-linux-gnu/bitcoin-c1df11eafc8c-riscv64-linux-gnu.tar.gz
17b0c70a0fb9f165a74d47672775007a74298c27f3ee1834b9cb2f3767fc75e8ed  guix-build-c1df11eafc8c/output/x86_64-apple-darwin/SHA256SUMS.part
1866cca84f5cb254654f4cd482c9c3364c97f754475cf4850a1d6226d0c419d65f  guix-build-c1df11eafc8c/output/x86_64-apple-darwin/bitcoin-c1df11eafc8c-osx-unsigned.dmg
19d7dee56cea21d26ef8da8ae74d5741448713131827fa4e988c58a4694a1ca14d  guix-build-c1df11eafc8c/output/x86_64-apple-darwin/bitcoin-c1df11eafc8c-osx-unsigned.tar.gz
2037ff6d2dca19df4697f4dd352953ee1392b4b857cd92b54ba81b93019deedb7c  guix-build-c1df11eafc8c/output/x86_64-apple-darwin/bitcoin-c1df11eafc8c-osx64.tar.gz
217adcff8cd68fcca18f91c5ddc2a274558cbf844b9538ade395167a70625ce04a  guix-build-c1df11eafc8c/output/x86_64-linux-gnu/SHA256SUMS.part
22fd28efd1e6bd7e71ff0dd13480ce6e992be062f95a975157204ab206886ebf9e  guix-build-c1df11eafc8c/output/x86_64-linux-gnu/bitcoin-c1df11eafc8c-x86_64-linux-gnu-debug.tar.gz
23349a67997624bec4b5909378a32a32f522d2343af3131d2214150f0436203501  guix-build-c1df11eafc8c/output/x86_64-linux-gnu/bitcoin-c1df11eafc8c-x86_64-linux-gnu.tar.gz
24e5b3f76e29b3d3f378ec152fe7c4253e38b79eee3347c16f6dcab250e43355fe  guix-build-c1df11eafc8c/output/x86_64-w64-mingw32/SHA256SUMS.part
25b24d134d0c59010df891032713479fb3d66efb2753537a60e8d44e5aae816de6  guix-build-c1df11eafc8c/output/x86_64-w64-mingw32/bitcoin-c1df11eafc8c-win-unsigned.tar.gz
266d003e820bc05f7e366fcc839670d9872e4087ce1d8acda23990c1b9f025274d  guix-build-c1df11eafc8c/output/x86_64-w64-mingw32/bitcoin-c1df11eafc8c-win64-debug.zip
2737a35e793c7fb8800f2f48afac3d9a314c63938e071a704fcde5f4d7f54f8512  guix-build-c1df11eafc8c/output/x86_64-w64-mingw32/bitcoin-c1df11eafc8c-win64-setup-unsigned.exe
287273cde4f8155f247715da7d6e06b90eda69605b4436e5d35a09ded6b3c18fde  guix-build-c1df11eafc8c/output/x86_64-w64-mingw32/bitcoin-c1df11eafc8c-win64.zip

    At first I wondered why these changed from #23724#pullrequestreview-832761102, but I remembered that I did rebase before pushing. My hashes for 070570806404 matched with fanquake’s.

  28. jb55 referenced this in commit a715f5d5f4 on Dec 18, 2021
  29. jb55 referenced this in commit c68c226f76 on Dec 18, 2021
  30. jb55 referenced this in commit 467bef6a3c on Dec 18, 2021
  31. jb55 commented at 2:10 am on December 18, 2021: member

    @0xB10C

    However, I think as soon as the ConnectionType enum is changed, the tracing scripts might break/will confuse the connection types and the tracepoint documentation will be outdated. Additionally, we might want to cast here as we implicitly convert a scoped enum to an int (?).

    All these issues should be solved by https://github.com/bitcoin/bitcoin/pull/23809

  32. jb55 referenced this in commit 08efc0da99 on Dec 18, 2021
  33. laanwj commented at 11:18 am on December 18, 2021: member 
    0-        pfrom->ConnectionTypeAsString().c_str(),
1+        pfrom->GetConnectionType(),

    This keeps coming back: please don’t pass strings that are generated on the fly to tracepoints. We should add a rule about this.

  34. 0xB10C commented at 3:10 pm on December 18, 2021: member

    I’ve checked the utxocache:add and the utxocache:spent tracepoints. Both only add an extra 4 executed instructions. I’m assuming utxocache:uncache behaves similarly as the same arguments are passed.

    The validation:block_connected tracepoint also seems to be more on the expensive side with “Recorded 8229 instructions in 80 functions”. This includes, e.g. calculating the hash of the block and GetTimeMicros(). I’d argue that this is however still inexpensive compared to the total cost of validating a block.

    https://github.com/bitcoin/bitcoin/blob/cb27d60f966ad20c465e5db05ba1a2253bd4dab3/src/validation.cpp#L2160-L2167

    For the utxocache:flush tracepoint, gdb reports “Recorded 171 instructions in 19 functions”. These are mainly from GetTimeMicros(). If needed, dropping GetTimeMicros() could be a possibility here. Could be done in the follow-up to #22902.

    https://github.com/bitcoin/bitcoin/blob/cb27d60f966ad20c465e5db05ba1a2253bd4dab3/src/validation.cpp#L2328-L2334

  35. jb55 commented at 7:04 pm on December 18, 2021: member

    On Sat, Dec 18, 2021 at 03:19:07AM -0800, W. J. van der Laan wrote:

    0-        pfrom->ConnectionTypeAsString().c_str(),
1+        pfrom->GetConnectionType(),

    This keeps coming back: please don’t pass strings that are generated on the fly to tracepoints. We should add a rule about this.

    To be fair in this case it’s not really generated, the switch returns static strings, but it does take a surprising amount of instructions for what should be a simple lookup in a switch jump table. Perhaps the switch was not big enough for g++ to optimize it into a jump table, instead opting for a chain of conditional checks. 100+ still is a lot, I’ll look into prematurely optimizing the other tracepoints as well, especially the one that is taking 8000 instructions (wtf?)

  36. laanwj commented at 10:53 am on December 19, 2021: member

    To be fair in this case it’s not really generated, the switch returns static strings

    I’m fairly sure the way c++ strings work, even though it returns static strings, it still does an allocation and copy (from the C string in .rodata) every time (then deallocating it again afterwards).

    It’s possible I’m wrong and modern compilers are smart enough to avoid it (can it see through std::string("static c-string").c_str() ?, but wouldn’t bet on it.

    I’ll look into prematurely optimizing the other tracepoints as well, especially the one that is taking 8000 instructions (wtf?)

    In any case I’m happy that we have a way to measure and quantify the number of instructions generated by a tracepoint now!

  37. jb55 commented at 5:33 pm on December 19, 2021: member

    I’m fairly sure the way c++ strings work, even though it returns static strings, it still does an allocation and copy every time.

    yes you may be right due to the number of instructions here. I was hoping the compiler would be smarter about this…

  38. jb55 commented at 8:24 pm on December 19, 2021: member

    On Sat, Dec 18, 2021 at 07:10:24AM -0800, 0xB10C wrote:

    The validation:block_connected tracepoint also seems to be more on the expensive side with “Recorded 8229 instructions in 80 functions”. This includes, e.g. calculating the hash of the block and GetTimeMicros(). I’d argue that this is however still inexpensive compared to the total cost of validating a block.

    Some data:

    0instrs    delta    description
1
28276      0        baseline
38167      -109     no micros
453        -8114    no GetHash()

    It looks like GetHash is the culprit here.

    I’ll spend some time today looking at alternate solutions.

  39. fanquake commented at 7:06 am on December 30, 2021: member
    This change is looking good now. I don’t think it necessarily has to wait for #23809 or #23819 to be merged first, but we’d want those (and any other changes that reduce tracepoint overheads) merged for 23.0. If you’d like to have reviewers perform a Guix build of this PR, I’d suggest rebasing on master, so we don’t have to worry about mismatches from a recent determinism issue in Qt.
  40. build: add systemtap's sys/sdt.h as depends 
    The sys/sdt.h header is required to build Bitcoin Core with Userspace
Statically Defined Tracing support. Systemtap version 4.5 (May 2021)
is used as the most recent version 4.6 (Nov 2021) fails to build.
See e.g. https://sourceware.org/git/?p=systemtap.git;a=commit;h=1d3653936fc1fd13135a723a27e6c7e959793ad0

As Systemtap itself is not needed, the build steps (configure and
make) are skipped. We require fewer build dependecies and don't
waste time building depends we don't end up using. However, the
configure step would normally processes sys/sdt-config.h.in. The
resulting sdt-config.h defines _SDT_ASM_SECTION_AUTOGROUP_SUPPORT
(either 0 or 1 to indicate whether the assembler supports "?" in
.pushsection directives). For now, we assume all currently used
assemblers supports this feature and remove the check from the
sys/sdt.h header file in a patch.

Co-authored-by: Michael Ford <fanquake@gmail.com>
    e158a2a7aa
  41. 0xB10C force-pushed on Jan 4, 2022
  42. 0xB10C commented at 9:22 am on January 4, 2022: member

    Thanks, rebased.

    Agree that we want to have these before 23.0:

  43. fanquake commented at 6:54 am on January 6, 2022: member

    Thanks, rebased.

    Could you also integrate this change, https://github.com/fanquake/bitcoin/commit/02dbe046c6c864b998b686eb19b3966dfe410aa5, into your second commit.

  44. build: rename --enable-ebpf to --enable-usdt 
    eBPF is a Linux kernel technology used to "extend the capabilities
of the kernel without requiring to change kernel source code or
load kernel modules". While Userspace, Statically Defined Tracing
(USDT) uses eBPF under the hood, --enable-usdt better resembles that
support for USDT is enabled, and tracepoints will be included in the
binary.
    6200fbf54f
  45. 0xB10C force-pushed on Jan 6, 2022
  46. 0xB10C commented at 10:05 am on January 6, 2022: member

    Could you also integrate this change, fanquake@02dbe04, into your second commit.

    Done, thanks!

    Running a GUIX build now.

  47. 0xB10C commented at 2:39 pm on January 9, 2022: member

    GUIX builds for 6200fbf54fa9:

     0$ find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
 192da8c535d4b1f07c04fb235a6c5806ae46f5abeed9c123785a6be96911dc831  guix-build-6200fbf54fa9/output/aarch64-linux-gnu/SHA256SUMS.part
 20dd21a5a9c8d0d8cb3f537359a722d0431d235e37cc4a2d38c1e87f025406f29  guix-build-6200fbf54fa9/output/aarch64-linux-gnu/bitcoin-6200fbf54fa9-aarch64-linux-gnu-debug.tar.gz
 38085b6034858d6d59f8223dc06b659549cf05021f057580b24958acba5d3fae7  guix-build-6200fbf54fa9/output/aarch64-linux-gnu/bitcoin-6200fbf54fa9-aarch64-linux-gnu.tar.gz
 4e8537078c389e1374d49f43e8e52512030089e397d1a667e78df3530de1bf2d9  guix-build-6200fbf54fa9/output/arm-linux-gnueabihf/SHA256SUMS.part
 5148c1574b8edbd6baeaa77c807f6d3a17159b49193c777978fe99279b42732bd  guix-build-6200fbf54fa9/output/arm-linux-gnueabihf/bitcoin-6200fbf54fa9-arm-linux-gnueabihf-debug.tar.gz
 64a6b2162c08d986ae4df147f9e50f90c039e7ca62b283da9d7fc4308e0757f00  guix-build-6200fbf54fa9/output/arm-linux-gnueabihf/bitcoin-6200fbf54fa9-arm-linux-gnueabihf.tar.gz
 71b150b40541260de2f4f905a68db7b86339d792f404ee4c275a57d5b45f3ce1a  guix-build-6200fbf54fa9/output/dist-archive/bitcoin-6200fbf54fa9.tar.gz
 81f6f3ec14f6ad98391746cd73c7aabc722ddf66d720c46fbdc9c2aebbb1d0232  guix-build-6200fbf54fa9/output/powerpc64-linux-gnu/SHA256SUMS.part
 9869cd22a0f1eb6721f8549ca690cec1a1d54bb8305e1f363ce0b6d2a6d654277  guix-build-6200fbf54fa9/output/powerpc64-linux-gnu/bitcoin-6200fbf54fa9-powerpc64-linux-gnu-debug.tar.gz
1008288ab32d123dc7872dd9fcc2af24695004da171109eec9ccc9600d8efba8b3  guix-build-6200fbf54fa9/output/powerpc64-linux-gnu/bitcoin-6200fbf54fa9-powerpc64-linux-gnu.tar.gz
115e783d7c7b0ca80e682be432c81ae1758561ec9273f20b521653417ae73e2e9a  guix-build-6200fbf54fa9/output/powerpc64le-linux-gnu/SHA256SUMS.part
1272daa19fe9a2724f8cb30faedb99feac633c1456d551d81c820a6226b54fb4a8  guix-build-6200fbf54fa9/output/powerpc64le-linux-gnu/bitcoin-6200fbf54fa9-powerpc64le-linux-gnu-debug.tar.gz
13721344bd6a6e0a0cbb90c769e949f59449e6a90bc75bc4ade487664d29459685  guix-build-6200fbf54fa9/output/powerpc64le-linux-gnu/bitcoin-6200fbf54fa9-powerpc64le-linux-gnu.tar.gz
142996e463e4296c783a0cf733a84a1e426d2ca393da99f2d42168969402ecca7e  guix-build-6200fbf54fa9/output/riscv64-linux-gnu/SHA256SUMS.part
15168ddd7ff09980c70002279e391450d80d0e49a4828a6a2647693d386068b6d0  guix-build-6200fbf54fa9/output/riscv64-linux-gnu/bitcoin-6200fbf54fa9-riscv64-linux-gnu-debug.tar.gz
16a1adf8450dc4a5ca72b588081c355f9400286bd059664b1d6facb0ef33523b43  guix-build-6200fbf54fa9/output/riscv64-linux-gnu/bitcoin-6200fbf54fa9-riscv64-linux-gnu.tar.gz
1778a663bbd7618b47dd84d0a729de55fab758246de4d400827450a3897df78a85  guix-build-6200fbf54fa9/output/x86_64-apple-darwin/SHA256SUMS.part
18b7edf07f67492dde52b2b0da1e5e200d6f56ba91168b4e377f0c8f6589eb4358  guix-build-6200fbf54fa9/output/x86_64-apple-darwin/bitcoin-6200fbf54fa9-osx-unsigned.dmg
190f5dfab2ed141bc35f8d19e1c8832399695a122e9339637e40924c5847e05bab  guix-build-6200fbf54fa9/output/x86_64-apple-darwin/bitcoin-6200fbf54fa9-osx-unsigned.tar.gz
20fe5aa2d9ce7b5f80b5a624b6613c577c8fd4d116bfccde8e0a00bd52e19db57e  guix-build-6200fbf54fa9/output/x86_64-apple-darwin/bitcoin-6200fbf54fa9-osx64.tar.gz
21abedd8376c4fd0086f1cc699610d2adf926f991ad6fba437bb24949df3ae1320  guix-build-6200fbf54fa9/output/x86_64-linux-gnu/SHA256SUMS.part
2252ccacc7e5a035355d253940aa3ea9e478b59178a2fcac2e0217028c0b817e3d  guix-build-6200fbf54fa9/output/x86_64-linux-gnu/bitcoin-6200fbf54fa9-x86_64-linux-gnu-debug.tar.gz
23446ac535e043fb56164711f42ab78117713a901ce7a46a866b0f4f9da43f9b88  guix-build-6200fbf54fa9/output/x86_64-linux-gnu/bitcoin-6200fbf54fa9-x86_64-linux-gnu.tar.gz
24e6a3980492b714430479d7803765294544999b50ef65df37d8648b240dc4bc26  guix-build-6200fbf54fa9/output/x86_64-w64-mingw32/SHA256SUMS.part
259c1c3652170a6217d8bea4b9241e8921f980c86ce85e3389371d5278612fb7c8  guix-build-6200fbf54fa9/output/x86_64-w64-mingw32/bitcoin-6200fbf54fa9-win-unsigned.tar.gz
2642bfe4ba6570f8f524f0d6ae45f91cf18ef04e77fa19e0c07abd5ea7ce43b5e6  guix-build-6200fbf54fa9/output/x86_64-w64-mingw32/bitcoin-6200fbf54fa9-win64-debug.zip
27225e5de0e161424da9506ba89b63aaaab2b5b7fe535aca3def68bebd1962e297  guix-build-6200fbf54fa9/output/x86_64-w64-mingw32/bitcoin-6200fbf54fa9-win64-setup-unsigned.exe
28922cd070d5db7b83d81d9e2fb14248f555bc8dde917dde9b555f0cae9faa6c1c  guix-build-6200fbf54fa9/output/x86_64-w64-mingw32/bitcoin-6200fbf54fa9-win64.zip
  48. fanquake approved
  49. fanquake commented at 2:59 am on January 10, 2022: member

    ACK 6200fbf54fa919899d99f1cdd5ef88ec8b074cd6 - tested enabling / disabling and with/without SDT from depends. We can follow up with #23819, #23907 and #23296, and if any serious issues arise before feature freeze, it is easy for us to flip depends such that USDT becomes opt-in, rather than opt-out, and thus, releases would be tracepoint free.

    Guix build also matched:

     092da8c535d4b1f07c04fb235a6c5806ae46f5abeed9c123785a6be96911dc831  guix-build-6200fbf54fa9/output/aarch64-linux-gnu/SHA256SUMS.part
 10dd21a5a9c8d0d8cb3f537359a722d0431d235e37cc4a2d38c1e87f025406f29  guix-build-6200fbf54fa9/output/aarch64-linux-gnu/bitcoin-6200fbf54fa9-aarch64-linux-gnu-debug.tar.gz
 28085b6034858d6d59f8223dc06b659549cf05021f057580b24958acba5d3fae7  guix-build-6200fbf54fa9/output/aarch64-linux-gnu/bitcoin-6200fbf54fa9-aarch64-linux-gnu.tar.gz
 3e8537078c389e1374d49f43e8e52512030089e397d1a667e78df3530de1bf2d9  guix-build-6200fbf54fa9/output/arm-linux-gnueabihf/SHA256SUMS.part
 4148c1574b8edbd6baeaa77c807f6d3a17159b49193c777978fe99279b42732bd  guix-build-6200fbf54fa9/output/arm-linux-gnueabihf/bitcoin-6200fbf54fa9-arm-linux-gnueabihf-debug.tar.gz
 54a6b2162c08d986ae4df147f9e50f90c039e7ca62b283da9d7fc4308e0757f00  guix-build-6200fbf54fa9/output/arm-linux-gnueabihf/bitcoin-6200fbf54fa9-arm-linux-gnueabihf.tar.gz
 61b150b40541260de2f4f905a68db7b86339d792f404ee4c275a57d5b45f3ce1a  guix-build-6200fbf54fa9/output/dist-archive/bitcoin-6200fbf54fa9.tar.gz
 71f6f3ec14f6ad98391746cd73c7aabc722ddf66d720c46fbdc9c2aebbb1d0232  guix-build-6200fbf54fa9/output/powerpc64-linux-gnu/SHA256SUMS.part
 8869cd22a0f1eb6721f8549ca690cec1a1d54bb8305e1f363ce0b6d2a6d654277  guix-build-6200fbf54fa9/output/powerpc64-linux-gnu/bitcoin-6200fbf54fa9-powerpc64-linux-gnu-debug.tar.gz
 908288ab32d123dc7872dd9fcc2af24695004da171109eec9ccc9600d8efba8b3  guix-build-6200fbf54fa9/output/powerpc64-linux-gnu/bitcoin-6200fbf54fa9-powerpc64-linux-gnu.tar.gz
105e783d7c7b0ca80e682be432c81ae1758561ec9273f20b521653417ae73e2e9a  guix-build-6200fbf54fa9/output/powerpc64le-linux-gnu/SHA256SUMS.part
1172daa19fe9a2724f8cb30faedb99feac633c1456d551d81c820a6226b54fb4a8  guix-build-6200fbf54fa9/output/powerpc64le-linux-gnu/bitcoin-6200fbf54fa9-powerpc64le-linux-gnu-debug.tar.gz
12721344bd6a6e0a0cbb90c769e949f59449e6a90bc75bc4ade487664d29459685  guix-build-6200fbf54fa9/output/powerpc64le-linux-gnu/bitcoin-6200fbf54fa9-powerpc64le-linux-gnu.tar.gz
132996e463e4296c783a0cf733a84a1e426d2ca393da99f2d42168969402ecca7e  guix-build-6200fbf54fa9/output/riscv64-linux-gnu/SHA256SUMS.part
14168ddd7ff09980c70002279e391450d80d0e49a4828a6a2647693d386068b6d0  guix-build-6200fbf54fa9/output/riscv64-linux-gnu/bitcoin-6200fbf54fa9-riscv64-linux-gnu-debug.tar.gz
15a1adf8450dc4a5ca72b588081c355f9400286bd059664b1d6facb0ef33523b43  guix-build-6200fbf54fa9/output/riscv64-linux-gnu/bitcoin-6200fbf54fa9-riscv64-linux-gnu.tar.gz
1678a663bbd7618b47dd84d0a729de55fab758246de4d400827450a3897df78a85  guix-build-6200fbf54fa9/output/x86_64-apple-darwin/SHA256SUMS.part
17b7edf07f67492dde52b2b0da1e5e200d6f56ba91168b4e377f0c8f6589eb4358  guix-build-6200fbf54fa9/output/x86_64-apple-darwin/bitcoin-6200fbf54fa9-osx-unsigned.dmg
180f5dfab2ed141bc35f8d19e1c8832399695a122e9339637e40924c5847e05bab  guix-build-6200fbf54fa9/output/x86_64-apple-darwin/bitcoin-6200fbf54fa9-osx-unsigned.tar.gz
19fe5aa2d9ce7b5f80b5a624b6613c577c8fd4d116bfccde8e0a00bd52e19db57e  guix-build-6200fbf54fa9/output/x86_64-apple-darwin/bitcoin-6200fbf54fa9-osx64.tar.gz
20abedd8376c4fd0086f1cc699610d2adf926f991ad6fba437bb24949df3ae1320  guix-build-6200fbf54fa9/output/x86_64-linux-gnu/SHA256SUMS.part
2152ccacc7e5a035355d253940aa3ea9e478b59178a2fcac2e0217028c0b817e3d  guix-build-6200fbf54fa9/output/x86_64-linux-gnu/bitcoin-6200fbf54fa9-x86_64-linux-gnu-debug.tar.gz
22446ac535e043fb56164711f42ab78117713a901ce7a46a866b0f4f9da43f9b88  guix-build-6200fbf54fa9/output/x86_64-linux-gnu/bitcoin-6200fbf54fa9-x86_64-linux-gnu.tar.gz
23e6a3980492b714430479d7803765294544999b50ef65df37d8648b240dc4bc26  guix-build-6200fbf54fa9/output/x86_64-w64-mingw32/SHA256SUMS.part
249c1c3652170a6217d8bea4b9241e8921f980c86ce85e3389371d5278612fb7c8  guix-build-6200fbf54fa9/output/x86_64-w64-mingw32/bitcoin-6200fbf54fa9-win-unsigned.tar.gz
2542bfe4ba6570f8f524f0d6ae45f91cf18ef04e77fa19e0c07abd5ea7ce43b5e6  guix-build-6200fbf54fa9/output/x86_64-w64-mingw32/bitcoin-6200fbf54fa9-win64-debug.zip
26225e5de0e161424da9506ba89b63aaaab2b5b7fe535aca3def68bebd1962e297  guix-build-6200fbf54fa9/output/x86_64-w64-mingw32/bitcoin-6200fbf54fa9-win64-setup-unsigned.exe
27922cd070d5db7b83d81d9e2fb14248f555bc8dde917dde9b555f0cae9faa6c1c  guix-build-6200fbf54fa9/output/x86_64-w64-mingw32/bitcoin-6200fbf54fa9-win64.zip
  50. fanquake merged this on Jan 10, 2022
  51. fanquake closed this on Jan 10, 2022
  52. sidhujag referenced this in commit 5ca60a8fdc on Jan 10, 2022
  53. 0xB10C deleted the branch on Jan 17, 2022
  54. DrahtBot locked this on Jan 17, 2023


0xB10C laanwj DrahtBot jb55 fanquake ccdle12

Labels
Build system

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-07-03 07:12 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me