Merge bitcoin/bitcoin#26158: bench: add "priority level" to the benchmark framework

3e9d0be build: only run high priority benchmarks in 'make check' (furszy)
466b54b bench: surround main() execution with try/catch (furszy)
3da7cd2 bench: explicitly make all current benchmarks "high" priority (furszy)
05b8c76 bench: add "priority level" to the benchmark framework (furszy)
f159378 bench: place benchmark implementation inside benchmark namespace (furszy)

Pull request description:

  This is from today's meeting, a simple "priority level" for the benchmark framework.

  Will allow us to run certain benchmarks while skip non-prioritized ones in `make check`.

  By default, `bench_bitcoin` will run all the benchmarks. `make check`will only run the high priority ones,
  and have marked all the existent benchmarks as "high priority" to retain the current behavior.

  Could test it by modifying any benchmark priority to something different from "high", and
  run `bench_bitcoin -priority-level=high` and/or `bench_bitcoin -priority-level=medium,low`
  (the first command will skip the modified bench while the second one will include it).

  Note: the second commit could be avoided by having a default arg value for the priority
  level but.. an explicit set in every `BENCHMARK` macro call makes it less error-prone.

ACKs for top commit:
  kouloumos:
    re-ACK 3e9d0be
  achow101:
    ACK 3e9d0be
  theStack:
    re-ACK 3e9d0be
  stickies-v:
    re-ACK bitcoin/bitcoin@3e9d0be

Tree-SHA512: ece59bf424c5fc1db335f84caa507476fb8ad8c6151880f1f8289562e17023aae5b5e7de03e8cbba6337bf09215f9be331e9ef51c791c43bce43f7446813b054

Author: achow101

src/Makefile.test.include

@@ -376,8 +376,8 @@ endif
 if TARGET_WINDOWS
 else
 if ENABLE_BENCH
-	@echo "Running bench/bench_bitcoin (one iteration sanity check)..."
-	$(BENCH_BINARY) --sanity-check > /dev/null
+	@echo "Running bench/bench_bitcoin (one iteration sanity check, only high priority)..."
+	$(BENCH_BINARY) -sanity-check -priority-level=high > /dev/null
 endif
 endif
 	$(AM_V_at)$(MAKE) $(AM_MAKEFLAGS) -C secp256k1 check

src/bench/addrman.cpp

@@ -133,7 +133,7 @@ static void AddrManAddThenGood(benchmark::Bench& bench)
     });
 }
 
-BENCHMARK(AddrManAdd);
-BENCHMARK(AddrManSelect);
-BENCHMARK(AddrManGetAddr);
-BENCHMARK(AddrManAddThenGood);
+BENCHMARK(AddrManAdd, benchmark::PriorityLevel::HIGH);
+BENCHMARK(AddrManSelect, benchmark::PriorityLevel::HIGH);
+BENCHMARK(AddrManGetAddr, benchmark::PriorityLevel::HIGH);
+BENCHMARK(AddrManAddThenGood, benchmark::PriorityLevel::HIGH);

src/bench/base58.cpp

@@ -50,6 +50,6 @@ static void Base58Decode(benchmark::Bench& bench)
 }
 
 
-BENCHMARK(Base58Encode);
-BENCHMARK(Base58CheckEncode);
-BENCHMARK(Base58Decode);
+BENCHMARK(Base58Encode, benchmark::PriorityLevel::HIGH);
+BENCHMARK(Base58CheckEncode, benchmark::PriorityLevel::HIGH);
+BENCHMARK(Base58Decode, benchmark::PriorityLevel::HIGH);

src/bench/bech32.cpp

@@ -32,5 +32,5 @@ static void Bech32Decode(benchmark::Bench& bench)
 }
 
 
-BENCHMARK(Bech32Encode);
-BENCHMARK(Bech32Decode);
+BENCHMARK(Bech32Encode, benchmark::PriorityLevel::HIGH);
+BENCHMARK(Bech32Decode, benchmark::PriorityLevel::HIGH);

src/bench/bench.cpp

@@ -6,6 +6,7 @@
 
 #include <fs.h>
 #include <test/util/setup_common.h>
+#include <util/string.h>
 
 #include <chrono>
 #include <fstream>
@@ -41,18 +42,42 @@ void GenerateTemplateResults(const std::vector<ankerl::nanobench::Result>& bench
 
 } // namespace
 
-benchmark::BenchRunner::BenchmarkMap& benchmark::BenchRunner::benchmarks()
+namespace benchmark {
+
+// map a label to one or multiple priority levels
+std::map<std::string, uint8_t> map_label_priority = {
+    {"high", PriorityLevel::HIGH},
+    {"low", PriorityLevel::LOW},
+    {"all", 0xff}
+};
+
+std::string ListPriorities()
 {
-    static std::map<std::string, BenchFunction> benchmarks_map;
+    using item_t = std::pair<std::string, uint8_t>;
+    auto sort_by_priority = [](item_t a, item_t b){ return a.second < b.second; };
+    std::set<item_t, decltype(sort_by_priority)> sorted_priorities(map_label_priority.begin(), map_label_priority.end(), sort_by_priority);
+    return Join(sorted_priorities, ',', [](const auto& entry){ return entry.first; });
+}
+
+uint8_t StringToPriority(const std::string& str)
+{
+    auto it = map_label_priority.find(str);
+    if (it == map_label_priority.end()) throw std::runtime_error(strprintf("Unknown priority level %s", str));
+    return it->second;
+}
+
+BenchRunner::BenchmarkMap& BenchRunner::benchmarks()
+{
+    static BenchmarkMap benchmarks_map;
     return benchmarks_map;
 }
 
-benchmark::BenchRunner::BenchRunner(std::string name, benchmark::BenchFunction func)
+BenchRunner::BenchRunner(std::string name, BenchFunction func, PriorityLevel level)
 {
-    benchmarks().insert(std::make_pair(name, func));
+    benchmarks().insert(std::make_pair(name, std::make_pair(func, level)));
 }
 
-void benchmark::BenchRunner::RunAll(const Args& args)
+void BenchRunner::RunAll(const Args& args)
 {
     std::regex reFilter(args.regex_filter);
     std::smatch baseMatch;
@@ -62,33 +87,39 @@ void benchmark::BenchRunner::RunAll(const Args& args)
     }
 
     std::vector<ankerl::nanobench::Result> benchmarkResults;
-    for (const auto& p : benchmarks()) {
-        if (!std::regex_match(p.first, baseMatch, reFilter)) {
+    for (const auto& [name, bench_func] : benchmarks()) {
+        const auto& [func, priority_level] = bench_func;
+
+        if (!(priority_level & args.priority)) {
+            continue;
+        }
+
+        if (!std::regex_match(name, baseMatch, reFilter)) {
             continue;
         }
 
         if (args.is_list_only) {
-            std::cout << p.first << std::endl;
+            std::cout << name << std::endl;
             continue;
         }
 
         Bench bench;
         if (args.sanity_check) {
             bench.epochs(1).epochIterations(1);
         }
-        bench.name(p.first);
+        bench.name(name);
         if (args.min_time > 0ms) {
             // convert to nanos before dividing to reduce rounding errors
             std::chrono::nanoseconds min_time_ns = args.min_time;
             bench.minEpochTime(min_time_ns / bench.epochs());
         }
 
         if (args.asymptote.empty()) {
-            p.second(bench);
+            func(bench);
         } else {
             for (auto n : args.asymptote) {
                 bench.complexityN(n);
-                p.second(bench);
+                func(bench);
             }
             std::cout << bench.complexityBigO() << std::endl;
         }
@@ -103,3 +134,5 @@ void benchmark::BenchRunner::RunAll(const Args& args)
                                                                "{{/result}}");
     GenerateTemplateResults(benchmarkResults, args.output_json, ankerl::nanobench::templates::json());
 }
+
+} // namespace benchmark

src/bench/bench.h

@@ -41,6 +41,16 @@ using ankerl::nanobench::Bench;
 
 typedef std::function<void(Bench&)> BenchFunction;
 
+enum PriorityLevel : uint8_t
+{
+    LOW = 1 << 0,
+    HIGH = 1 << 2,
+};
+
+// List priority labels, comma-separated and sorted by increasing priority
+std::string ListPriorities();
+uint8_t StringToPriority(const std::string& str);
+
 struct Args {
     bool is_list_only;
     bool sanity_check;
@@ -49,22 +59,24 @@ struct Args {
     fs::path output_csv;
     fs::path output_json;
     std::string regex_filter;
+    uint8_t priority;
 };
 
 class BenchRunner
 {
-    typedef std::map<std::string, BenchFunction> BenchmarkMap;
+    // maps from "name" -> (function, priority_level)
+    typedef std::map<std::string, std::pair<BenchFunction, PriorityLevel>> BenchmarkMap;
     static BenchmarkMap& benchmarks();
 
 public:
-    BenchRunner(std::string name, BenchFunction func);
+    BenchRunner(std::string name, BenchFunction func, PriorityLevel level);
 
     static void RunAll(const Args& args);
 };
 } // namespace benchmark
 
-// BENCHMARK(foo) expands to:  benchmark::BenchRunner bench_11foo("foo", foo);
-#define BENCHMARK(n) \
-    benchmark::BenchRunner PASTE2(bench_, PASTE2(__LINE__, n))(STRINGIZE(n), n);
+// BENCHMARK(foo) expands to:  benchmark::BenchRunner bench_11foo("foo", foo, priority_level);
+#define BENCHMARK(n, priority_level) \
+    benchmark::BenchRunner PASTE2(bench_, PASTE2(__LINE__, n))(STRINGIZE(n), n, priority_level);
 
 #endif // BITCOIN_BENCH_BENCH_H

src/bench/bench_bitcoin.cpp

@@ -18,6 +18,8 @@
 
 static const char* DEFAULT_BENCH_FILTER = ".*";
 static constexpr int64_t DEFAULT_MIN_TIME_MS{10};
+/** Priority level default value, run "all" priority levels */
+static const std::string DEFAULT_PRIORITY{"all"};
 
 static void SetupBenchArgs(ArgsManager& argsman)
 {
@@ -30,6 +32,8 @@ static void SetupBenchArgs(ArgsManager& argsman)
     argsman.AddArg("-output-csv=<output.csv>", "Generate CSV file with the most important benchmark results", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
     argsman.AddArg("-output-json=<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);
+    argsman.AddArg("-priority-level=<l1,l2,l3>", strprintf("Run benchmarks of one or multiple priority level(s) (%s), default: '%s'",
+                                                           benchmark::ListPriorities(), DEFAULT_PRIORITY), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
 }
 
 // parses a comma separated list like "10,20,30,50"
@@ -45,6 +49,14 @@ static std::vector<double> parseAsymptote(const std::string& str) {
     return numbers;
 }
 
+static uint8_t parsePriorityLevel(const std::string& str) {
+    uint8_t levels{0};
+    for (const auto& level: SplitString(str, ',')) {
+        levels |= benchmark::StringToPriority(level);
+    }
+    return levels;
+}
+
 int main(int argc, char** argv)
 {
     ArgsManager argsman;
@@ -106,16 +118,22 @@ int main(int argc, char** argv)
         return EXIT_SUCCESS;
     }
 
-    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.regex_filter = argsman.GetArg("-filter", DEFAULT_BENCH_FILTER);
-    args.sanity_check = argsman.GetBoolArg("-sanity-check", false);
+    try {
+        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.regex_filter = argsman.GetArg("-filter", DEFAULT_BENCH_FILTER);
+        args.sanity_check = argsman.GetBoolArg("-sanity-check", false);
+        args.priority = parsePriorityLevel(argsman.GetArg("-priority-level", DEFAULT_PRIORITY));
 
-    benchmark::BenchRunner::RunAll(args);
+        benchmark::BenchRunner::RunAll(args);
 
-    return EXIT_SUCCESS;
+        return EXIT_SUCCESS;
+    } catch (const std::exception& e) {
+        tfm::format(std::cerr, "Error: %s\n", e.what());
+        return EXIT_FAILURE;
+    }
 }

src/bench/block_assemble.cpp

@@ -47,4 +47,4 @@ static void AssembleBlock(benchmark::Bench& bench)
     });
 }
 
-BENCHMARK(AssembleBlock);
+BENCHMARK(AssembleBlock, benchmark::PriorityLevel::HIGH);

src/bench/ccoins_caching.cpp

@@ -51,4 +51,4 @@ static void CCoinsCaching(benchmark::Bench& bench)
     ECC_Stop();
 }
 
-BENCHMARK(CCoinsCaching);
+BENCHMARK(CCoinsCaching, benchmark::PriorityLevel::HIGH);

src/bench/chacha20.cpp

@@ -39,6 +39,6 @@ static void CHACHA20_1MB(benchmark::Bench& bench)
     CHACHA20(bench, BUFFER_SIZE_LARGE);
 }
 
-BENCHMARK(CHACHA20_64BYTES);
-BENCHMARK(CHACHA20_256BYTES);
-BENCHMARK(CHACHA20_1MB);
+BENCHMARK(CHACHA20_64BYTES, benchmark::PriorityLevel::HIGH);
+BENCHMARK(CHACHA20_256BYTES, benchmark::PriorityLevel::HIGH);
+BENCHMARK(CHACHA20_1MB, benchmark::PriorityLevel::HIGH);