bench: Fix negative values and zero for -evals flag #17267

pull nijynot wants to merge 1 commits into bitcoin:master from nijynot:fix/eval changing 1 files +7 −0

nijynot commented at 6:52 PM on October 26, 2019: contributor
This PR makes bench_bitcoin -evals=0 evaluate at once and throws when -evals is a negative integer.

Currently when you run bench_bitcoin -evals=0, it'll get stuck at
```
# Benchmark, evals, iterations, total, min, max, median
```
. This is not intuitively expected and should instead evaluate instantly as it's set to zero. Negative integers for -evals does not make sense either and should throw if set.
fanquake added the label Tests on Oct 26, 2019
promag commented at 3:48 PM on October 27, 2019: member

ACK dd5028512340d07b4bbe1c19fca9fa3dcd864412.
practicalswift commented at 10:00 PM on October 27, 2019: contributor

Nice first-time contribution @nijynot! Hope to see more contributions from you!

ACK dd5028512340d07b4bbe1c19fca9fa3dcd864412 -- diff looks correct!

in src/bench/bench_bitcoin.cpp:57 in dd50285123 outdated

  50 | @@ -51,6 +51,13 @@ int main(int argc, char** argv)
  51 |      std::string scaling_str = gArgs.GetArg("-scaling", DEFAULT_BENCH_SCALING);
  52 |      bool is_list_only = gArgs.GetBoolArg("-list", false);
  53 |  
  54 | +    if (evaluations == 0) {
  55 | +        return EXIT_SUCCESS;
  56 | +    } else if (evaluations < 0) {
  57 | +        tfm::format(std::cerr, "Error parsing evaluations argument: %s\n", std::to_string(evaluations).c_str());

laanwj commented at 11:57 AM on October 28, 2019:

the .c_str() should not be needed here

laanwj commented at 11:58 AM on October 28, 2019:

also, casting evaluations from an integer to a string is wrong, it will do some pointer arithmethic, I think?

Why not

tfm::format(std::cerr, "Error parsing evaluations argument: %d\n", evaluations));

nijynot commented at 12:04 PM on October 28, 2019:

Ah, ofc. Fixed :+1:

fanquake added the label Waiting for author on Oct 28, 2019
fanquake commented at 12:05 PM on October 28, 2019: member

@nijynot Please squash your commits.
bench: Fix negative values and zero for -evals flag 3bb0a4674f
nijynot force-pushed on Oct 28, 2019
nijynot commented at 12:11 PM on October 28, 2019: contributor

@fanquake Done!
fanquake removed the label Waiting for author on Oct 28, 2019
laanwj commented at 1:13 PM on October 28, 2019: member

ACK 3bb0a4674f74d22043c7911ea76ab8a4d93fed62
laanwj referenced this in commit f8cc2b967b on Oct 28, 2019
laanwj merged this on Oct 28, 2019
laanwj closed this on Oct 28, 2019
nijynot deleted the branch on Oct 28, 2019
MarkLTZ referenced this in commit 8823f65197 on Apr 7, 2020
MarkLTZ referenced this in commit 0a1571d657 on Apr 7, 2020
deadalnix referenced this in commit 8dab59074c on Nov 2, 2020
MarcoFalke locked this on Dec 16, 2021

Contributors

Labels