Build: Add build support for profiling. #12373

murrayn commented at 5:27 AM on February 7, 2018: contributor

Support for profiling build: ./configure --enable-profiling

fanquake added the label Build system on Feb 7, 2018

laanwj requested review from theuni on Feb 15, 2018

laanwj commented at 4:12 PM on February 15, 2018: member

Should this mention in the help message that this is specifically for profiling with gprof? E.g. Valgrind is also used quite frequently for profiling.

Added @theuni as reviewer because of build system change.

murrayn commented at 11:58 PM on February 15, 2018: contributor

On 2/15/2018 8:14 AM, Wladimir J. van der Laan wrote:

Should this mention in the help message that this is specifically for profiling with |gprof|? E.g. Valgrind is also used quite frequently for profiling.

Yes, it should. I will make the change.

Murray

MarcoFalke added the label Tests on Feb 16, 2018

in configure.ac:1351 in 851b9e0cc7 outdated

1347 | @@ -1330,6 +1348,7 @@ echo "  with bench    = $use_bench"
1348 |  echo "  with upnp     = $use_upnp"
1349 |  echo "  use asm       = $use_asm"
1350 |  echo "  debug enabled = $enable_debug"
1351 | +echo "  profiling enabled = $enable_profiling"

jonasschnelli commented at 10:43 AM on February 17, 2018:

nit: alignment

in configure.ac:235 in 20032d8400 outdated

 230 | @@ -224,6 +231,17 @@ AC_ARG_ENABLE([werror],
 231 |  AC_LANG_PUSH([C++])
 232 |  AX_CHECK_COMPILE_FLAG([-Werror],[CXXFLAG_WERROR="-Werror"],[CXXFLAG_WERROR=""])
 233 |  
 234 | +if test "x$enable_gprof" = xyes; then
 235 | +    LDFLAGS="$LDFLAGS -pg"

theuni commented at 2:46 PM on February 20, 2018:

Please use AX_CHECK_LINK_FLAG here to make sure that it actually works. Also, let's stash -pg in PROFILE_LDFLAGS rather than LDFLAGS. Otherwise, the rest of the configure checks will be linked with -pg. So something like:

Something like:

AX_CHECK_LINK_FLAG([[-pg]],[PROFILE_LDFLAGS="$PROFILE_LDFLAGS -pg"],[AC_MSG_ERROR("profiling requested but not available"])

Then the same for CXXFLAGS.

MarcoFalke deleted a comment on Feb 20, 2018

jamesob commented at 5:32 PM on February 20, 2018: member

Unsure if this is working with clang; getting some warnings:

 $ ./configure LDFLAGS="-L${BDB_PREFIX}/lib/" CPPFLAGS="-I${BDB_PREFIX}/include/" CXXFLAGS="${CXXFLAGS}" --enable-wallet --enable-gprof --with-daemon
[...]
Options used to compile and link:
  with wallet   = yes
  with gui / qt = yes
    qt version  = 5
    with qr     = auto
  with zmq      = yes
  with test     = yes
  with bench    = yes
  with upnp     = auto
  use asm       = yes
  debug enabled = no
  gprof enabled = yes
  werror        = no

  target os     = linux
  build os      =

  CC            = /usr/bin/clang-4.0
  CFLAGS        = -g -O2 -pg
  CPPFLAGS      = -I/home/james/tmp/bitcoin/db4/include/ -DHAVE_BUILD_INFO -D__STDC_FORMAT_MACROS
  CXX           = /usr/bin/clang++-4.0 -std=c++11
  CXXFLAGS      = -std=c++11  -pg
  LDFLAGS       = -L/home/james/tmp/bitcoin/db4/lib/ -pg
  ARFLAGS       = cr

$ make -j 3
[...]
  CXX      bench/bench_bench_bitcoin-base58.o
  CXX      bench/bench_bench_bitcoin-coin_selection.o
  CXX      qt/qt_bitcoin_qt-bitcoin.o
  CXX      qt/qt_libbitcoinqt_a-bantablemodel.o
  CXX      qt/qt_libbitcoinqt_a-bitcoinaddressvalidator.o
  CXXLD    libbitcoinconsensus.la
clang: warning: argument unused during compilation: '-pg' [-Wunused-command-line-argument]
clang: warning: argument unused during compilation: '-pthread' [-Wunused-command-line-argument]
clang: warning: argument unused during compilation: '-pg' [-Wunused-command-line-argument]
clang: warning: argument unused during compilation: '-pthread' [-Wunused-command-line-argument]
/usr/bin/ar: `u' modifier ignored since `D' is the default (see `U')
[...]

Edit: my mistake, clang doesn't accept -pg and (apparently?) doesn't work with gprof. Worth adding an explicit warning about this for clang users?

theuni commented at 6:04 PM on February 20, 2018: member

@jamesob The suggestion above should take care of it.

murrayn commented at 11:38 AM on February 22, 2018: contributor

@theuni If this latest change (or any related assignment to LDFLAGS as was done with CXXFLAGS) occurs later in configure.ac, gprof profiling fails. Perhaps there is some order of arguments issue with ld.

theuni commented at 9:41 PM on February 22, 2018: member

@murrayn Please try this on top of yours: https://github.com/theuni/bitcoin/commit/bb91da4122852c7ac3a054d383e9bbb7a1139d4a. Works for me.

murrayn commented at 12:23 AM on February 23, 2018: contributor

@theuni This doesn't work for me. It compiles, and the binaries run, but if you look at the gprof output, it is empty. My test case:

$ make $ cd src/qt $ ./bitcoin-qt --help $ gprof bitcoin-qt

theuni commented at 11:30 PM on February 23, 2018: member

@murrayn turns out the problem is that -pg is incompatible with -pie. Could you please give https://github.com/theuni/bitcoin/commit/38450db901ce21a2e3c595026583033bb3bf6d78 a try on top of yours? I believe that now accounts for everything.

murrayn commented at 3:29 AM on February 24, 2018: contributor

Works for me now.

theuni commented at 10:55 PM on February 26, 2018: member

@murrayn Could you please squash all of our changes into your first commit?

Add build support for 'gprof' profiling. cfaac2a60f

murrayn force-pushed on Feb 27, 2018

theuni commented at 8:39 PM on March 5, 2018: member

Thanks for sticking with this! utACK cfaac2a60f3ac63ae8deccb03d88bd559449b78c

laanwj merged this on Mar 6, 2018

laanwj closed this on Mar 6, 2018

laanwj referenced this in commit f13d756cdd on Mar 6, 2018

murrayn deleted the branch on Mar 7, 2018

murrayn restored the branch on Mar 7, 2018

zkbot referenced this in commit 8e8a9350c3 on Jan 30, 2020

PastaPastaPasta referenced this in commit 94c47e5f16 on Jun 10, 2020

PastaPastaPasta referenced this in commit bc3751be97 on Jun 12, 2020

PastaPastaPasta referenced this in commit fa50d7b852 on Jun 13, 2020

PastaPastaPasta referenced this in commit 0849ffc00e on Jun 14, 2020

PastaPastaPasta referenced this in commit 78ef3fdd3a on Jun 14, 2020

gades referenced this in commit 1f7a3a7295 on Jun 26, 2021

MarcoFalke locked this on Sep 8, 2021