Do not evaluate hidden LogPrint arguments #9417

1. 
   sipa commented at 10:25 pm on December 23, 2016: member

   There are a few cases where hashes are computed inside LogPrint arguments - where they usually go unused. As LogPrint statements should never have side effects besides printing something, we can avoid the evaluation in this case.

   Advantage: perhaps a small performance improvement; I haven’t benchmarked.

   Disadvantage: if we would have statements with side-effects, this could make this a bit harder to debug.

2. Do not evaluate hidden LogPrint arguments 407cdd6cb8
3. fanquake added the label Utils and libraries on Dec 23, 2016
4. 
   rebroad commented at 4:40 am on December 24, 2016: contributor

   @sipa Thanks - I was just about to look into coding something similar so you have saved me the job. In the similar way that GetBoolArg() works, I wondered if perhaps we could have a boolean check for whether various debug are set, or perhaps even a scenario where we can say log this if debug=tx OR debug=mempool is set since I have some debug lines from mempool and tx functions merged, perhaps something like LogPrint("mempool", "tx", "received tx %s AcceptedToMemPool bla bla", hash.ToString()), or LogPrint("mempool|tx", "received tx %s AcceptedToMemPool bla bla", hash.ToString())

   Looking at the code it looks like the boolean function exists already, but the 2nd part (the “this category or that category” would still be nice).

5. 
   gmaxwell commented at 8:34 pm on December 25, 2016: contributor

   @rebroad See #9424
6. 
   gmaxwell commented at 8:49 pm on December 25, 2016: contributor

   ACK.

   Additional benefit: if evaluation of an argument would crash the software, e.g. via evaluating vector[0] without checking the size, that won’t happen if the debugging is not actually turned on. Not all side effects are good. :)

7. 
   laanwj commented at 9:47 am on January 5, 2017: member

   Makes sense. It’s unfortunate that this needs a macro solution.

   Does macros with variable arguments work for all (recent) C++ compilers? Apparently we already rely on that.

   utACK 407cdd6

8. laanwj merged this on Jan 5, 2017
9. laanwj closed this on Jan 5, 2017

   ---

10. laanwj referenced this in commit c4b7d4f79c on Jan 5, 2017
11. 
    sipa commented at 1:29 pm on January 5, 2017: member

    I looked this up. I find mentions that C++11 mandates support for vararg macros, however, it’s not even listed in GCC’s C++11 support pages. Perhaps it’s been supported for so long that they didn’t bother listing it? Then I noticed we’re already using vararg templates in the codebase, so I stopped worrying.
12. sickpig referenced this in commit 4f72b6c077 on Jan 11, 2018
13. sickpig referenced this in commit ded146f674 on Jan 11, 2018
14. codablock referenced this in commit a1478e586a on Jan 18, 2018
15. andvgal referenced this in commit aa988d0361 on Jan 6, 2019
16. CryptoCentric referenced this in commit 17609ea2e9 on Feb 26, 2019
17. furszy referenced this in commit 9ef9f5dea1 on Mar 27, 2020
18. akshaynexus referenced this in commit 5c753ce7c2 on Mar 30, 2020
19. MarcoFalke locked this on Sep 8, 2021

Contributors
sipa rebroad gmaxwell laanwj

Labels
Utils/log/libs