From 652b54e53220fedf2c342e428617bcbd2022964d Mon Sep 17 00:00:00 2001 From: laanwj <126646+laanwj@users.noreply.github.com> Date: Wed, 11 May 2022 16:28:10 +0200 Subject: [PATCH 1/2] bench: Add `--sanity-check` flag, use it in `make check` The benchmarks are run as part of `make check` for a minimum sanity check. The actual results are being ignored. So only run them for one iteration. This makes the `bench_bitcoin` part take 2m00 instead of 5m20 here. Which is still too long (imo), but this needs to be solved in the `WalletLoading*` benchmarks which take that long per iteration. --- src/Makefile.test.include | 4 ++-- src/bench/bench.cpp | 7 +++++++ src/bench/bench.h | 1 + src/bench/bench_bitcoin.cpp | 2 ++ 4 files changed, 12 insertions(+), 2 deletions(-) diff --git a/src/Makefile.test.include b/src/Makefile.test.include index 02a3f9ae7db..77ff6839747 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -364,8 +364,8 @@ endif if TARGET_WINDOWS else if ENABLE_BENCH - @echo "Running bench/bench_bitcoin ..." - $(BENCH_BINARY) > /dev/null + @echo "Running bench/bench_bitcoin (one iteration sanity check)..." + $(BENCH_BINARY) --sanity-check > /dev/null endif endif $(AM_V_at)$(MAKE) $(AM_MAKEFLAGS) -C secp256k1 check diff --git a/src/bench/bench.cpp b/src/bench/bench.cpp index 033d3197502..d26b52410c1 100644 --- a/src/bench/bench.cpp +++ b/src/bench/bench.cpp @@ -57,6 +57,10 @@ void benchmark::BenchRunner::RunAll(const Args& args) std::regex reFilter(args.regex_filter); std::smatch baseMatch; + if (args.sanity_check) { + std::cout << "Running with --sanity check option, benchmark results will be useless." << std::endl; + } + std::vector benchmarkResults; for (const auto& p : benchmarks()) { if (!std::regex_match(p.first, baseMatch, reFilter)) { @@ -69,6 +73,9 @@ void benchmark::BenchRunner::RunAll(const Args& args) } Bench bench; + if (args.sanity_check) { + bench.epochs(1).epochIterations(1); + } bench.name(p.first); if (args.min_time > 0ms) { // convert to nanos before dividing to reduce rounding errors diff --git a/src/bench/bench.h b/src/bench/bench.h index 6634138beb0..17535e4e81d 100644 --- a/src/bench/bench.h +++ b/src/bench/bench.h @@ -43,6 +43,7 @@ typedef std::function BenchFunction; struct Args { bool is_list_only; + bool sanity_check; std::chrono::milliseconds min_time; std::vector asymptote; fs::path output_csv; diff --git a/src/bench/bench_bitcoin.cpp b/src/bench/bench_bitcoin.cpp index d6f9c0f8b50..f196ff5baea 100644 --- a/src/bench/bench_bitcoin.cpp +++ b/src/bench/bench_bitcoin.cpp @@ -29,6 +29,7 @@ static void SetupBenchArgs(ArgsManager& argsman) argsman.AddArg("-min_time=", strprintf("Minimum runtime per benchmark, in milliseconds (default: %d)", DEFAULT_MIN_TIME_MS), ArgsManager::ALLOW_ANY | ArgsManager::DISALLOW_NEGATION, OptionsCategory::OPTIONS); argsman.AddArg("-output_csv=", "Generate CSV file with the most important benchmark results", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); argsman.AddArg("-output_json=", "Generate JSON file with all benchmark results", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); + argsman.AddArg("-sanity-check", "Run benchmarks for only one iteration", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); } // parses a comma separated list like "10,20,30,50" @@ -112,6 +113,7 @@ int main(int argc, char** argv) args.output_csv = argsman.GetPathArg("-output_csv"); args.output_json = argsman.GetPathArg("-output_json"); args.regex_filter = argsman.GetArg("-filter", DEFAULT_BENCH_FILTER); + args.sanity_check = argsman.GetBoolArg("-sanity-check", false); benchmark::BenchRunner::RunAll(args); From 4f31c21b7f384e50e0af8275e831085d1d69b386 Mon Sep 17 00:00:00 2001 From: laanwj <126646+laanwj@users.noreply.github.com> Date: Thu, 12 May 2022 13:13:12 +0200 Subject: [PATCH 2/2] bench: Make all arguments -kebab-case This is customary for UNIX-style arguments, and more consistent with our other tools --- src/bench/bench_bitcoin.cpp | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/bench/bench_bitcoin.cpp b/src/bench/bench_bitcoin.cpp index f196ff5baea..1bb4d34db9c 100644 --- a/src/bench/bench_bitcoin.cpp +++ b/src/bench/bench_bitcoin.cpp @@ -26,9 +26,9 @@ static void SetupBenchArgs(ArgsManager& argsman) argsman.AddArg("-asymptote=", "Test asymptotic growth of the runtime of an algorithm, if supported by the benchmark", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); argsman.AddArg("-filter=", strprintf("Regular expression filter to select benchmark by name (default: %s)", DEFAULT_BENCH_FILTER), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); argsman.AddArg("-list", "List benchmarks without executing them", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); - argsman.AddArg("-min_time=", strprintf("Minimum runtime per benchmark, in milliseconds (default: %d)", DEFAULT_MIN_TIME_MS), ArgsManager::ALLOW_ANY | ArgsManager::DISALLOW_NEGATION, OptionsCategory::OPTIONS); - argsman.AddArg("-output_csv=", "Generate CSV file with the most important benchmark results", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); - argsman.AddArg("-output_json=", "Generate JSON file with all benchmark results", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); + argsman.AddArg("-min-time=", strprintf("Minimum runtime per benchmark, in milliseconds (default: %d)", DEFAULT_MIN_TIME_MS), ArgsManager::ALLOW_ANY | ArgsManager::DISALLOW_NEGATION, OptionsCategory::OPTIONS); + argsman.AddArg("-output-csv=", "Generate CSV file with the most important benchmark results", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); + argsman.AddArg("-output-json=", "Generate JSON file with all benchmark results", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); argsman.AddArg("-sanity-check", "Run benchmarks for only one iteration", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); } @@ -74,7 +74,7 @@ int main(int argc, char** argv) " sure each run has exactly the same preconditions.\n" "\n" " * If results are still not reliable, increase runtime with e.g.\n" - " -min_time=5000 to let a benchmark run for at least 5 seconds.\n" + " -min-time=5000 to let a benchmark run for at least 5 seconds.\n" "\n" " * bench_bitcoin uses nanobench [3] for which there is extensive\n" " documentation available online.\n" @@ -109,9 +109,9 @@ int main(int argc, char** argv) benchmark::Args args; args.asymptote = parseAsymptote(argsman.GetArg("-asymptote", "")); args.is_list_only = argsman.GetBoolArg("-list", false); - args.min_time = std::chrono::milliseconds(argsman.GetIntArg("-min_time", DEFAULT_MIN_TIME_MS)); - args.output_csv = argsman.GetPathArg("-output_csv"); - args.output_json = argsman.GetPathArg("-output_json"); + args.min_time = std::chrono::milliseconds(argsman.GetIntArg("-min-time", DEFAULT_MIN_TIME_MS)); + args.output_csv = argsman.GetPathArg("-output-csv"); + args.output_json = argsman.GetPathArg("-output-json"); args.regex_filter = argsman.GetArg("-filter", DEFAULT_BENCH_FILTER); args.sanity_check = argsman.GetBoolArg("-sanity-check", false);