Skip to content

Commit 9d2b8fd

Browse files
achow101achow101
committed
Merge bitcoin#34210: bench: Remove -priority-level= option
fa3df52 bench: Require semicolon after BENCHMARK(foo) (MarcoFalke) fa8938f bench: Remove incorrect __LINE__ in BENCHMARK macro (MarcoFalke) fa51a28 scripted-diff: Remove priority_level from BENCHMARK macro (MarcoFalke) fa790c3 bench: Remove -priority-level= option (MarcoFalke) Pull request description: The option was added in bitcoin#26158, when the project was using an autotools-based build system. However, in the meantime this option is unused: * First, commit 27f1121 removed the option from one CI task * Then bitcoin#32310 removed the option from CMakeList.txt, because: * they only run as a sanity check (fastest version) * no one otherwise runs them, not even CI * issues have been missed due to this Finally, after commit 0ad4376, I don't see a single reason to keep this option, so remove it. Also, there is a commit to turn a silent ignore of duplicate bench names into an error. ACKs for top commit: achow101: ACK fa3df52 l0rinc: ACK fa3df52 hebasto: re-ACK fa3df52, only suggested changes since my recent [review](bitcoin#34210 (review)). Tree-SHA512: 68a314bff551fa878196d5a615d41d71e1c8c504135e6fc555659aa9f0c8786957d49ba038448e933554a8bc54caea2ddd7d628042c5627bf3bf37628210f8fb
2 parents ae3b5a9 + fa3df52 commit 9d2b8fd

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

57 files changed

+178
-238
lines changed

‎src/bench/addrman.cpp‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -175,9 +175,9 @@ static void AddrManAddThenGood(benchmark::Bench& bench)
175175
});
176176
}
177177

178-
BENCHMARK(AddrManAdd, benchmark::PriorityLevel::HIGH);
179-
BENCHMARK(AddrManSelect, benchmark::PriorityLevel::HIGH);
180-
BENCHMARK(AddrManSelectFromAlmostEmpty, benchmark::PriorityLevel::HIGH);
181-
BENCHMARK(AddrManSelectByNetwork, benchmark::PriorityLevel::HIGH);
182-
BENCHMARK(AddrManGetAddr, benchmark::PriorityLevel::HIGH);
183-
BENCHMARK(AddrManAddThenGood, benchmark::PriorityLevel::HIGH);
178+
BENCHMARK(AddrManAdd);
179+
BENCHMARK(AddrManSelect);
180+
BENCHMARK(AddrManSelectFromAlmostEmpty);
181+
BENCHMARK(AddrManSelectByNetwork);
182+
BENCHMARK(AddrManGetAddr);
183+
BENCHMARK(AddrManAddThenGood);

‎src/bench/base58.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,6 @@ static void Base58Decode(benchmark::Bench& bench)
5151
}
5252

5353

54-
BENCHMARK(Base58Encode, benchmark::PriorityLevel::HIGH);
55-
BENCHMARK(Base58CheckEncode, benchmark::PriorityLevel::HIGH);
56-
BENCHMARK(Base58Decode, benchmark::PriorityLevel::HIGH);
54+
BENCHMARK(Base58Encode);
55+
BENCHMARK(Base58CheckEncode);
56+
BENCHMARK(Base58Decode);

‎src/bench/bech32.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,5 +31,5 @@ static void Bech32Decode(benchmark::Bench& bench)
3131
}
3232

3333

34-
BENCHMARK(Bech32Encode, benchmark::PriorityLevel::HIGH);
35-
BENCHMARK(Bech32Decode, benchmark::PriorityLevel::HIGH);
34+
BENCHMARK(Bech32Encode);
35+
BENCHMARK(Bech32Decode);

‎src/bench/bench.cpp‎

Lines changed: 5 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -5,25 +5,20 @@
55
#include <bench/bench.h>
66

77
#include <test/util/setup_common.h> // IWYU pragma: keep
8-
#include <tinyformat.h>
8+
#include <util/check.h>
99
#include <util/fs.h>
10-
#include <util/string.h>
1110

1211
#include <chrono>
1312
#include <compare>
1413
#include <fstream>
1514
#include <functional>
1615
#include <iostream>
17-
#include <map>
18-
#include <ratio>
1916
#include <regex>
20-
#include <set>
21-
#include <stdexcept>
2217
#include <string>
18+
#include <utility>
2319
#include <vector>
2420

2521
using namespace std::chrono_literals;
26-
using util::Join;
2722

2823
const std::function<void(const std::string&)> G_TEST_LOG_FUN{};
2924

@@ -69,37 +64,15 @@ void GenerateTemplateResults(const std::vector<ankerl::nanobench::Result>& bench
6964

7065
namespace benchmark {
7166

72-
// map a label to one or multiple priority levels
73-
std::map<std::string, uint8_t> map_label_priority = {
74-
{"high", PriorityLevel::HIGH},
75-
{"low", PriorityLevel::LOW},
76-
{"all", 0xff}
77-
};
78-
79-
std::string ListPriorities()
80-
{
81-
using item_t = std::pair<std::string, uint8_t>;
82-
auto sort_by_priority = [](item_t a, item_t b){ return a.second < b.second; };
83-
std::set<item_t, decltype(sort_by_priority)> sorted_priorities(map_label_priority.begin(), map_label_priority.end(), sort_by_priority);
84-
return Join(sorted_priorities, ',', [](const auto& entry){ return entry.first; });
85-
}
86-
87-
uint8_t StringToPriority(const std::string& str)
88-
{
89-
auto it = map_label_priority.find(str);
90-
if (it == map_label_priority.end()) throw std::runtime_error(strprintf("Unknown priority level %s", str));
91-
return it->second;
92-
}
93-
9467
BenchRunner::BenchmarkMap& BenchRunner::benchmarks()
9568
{
9669
static BenchmarkMap benchmarks_map;
9770
return benchmarks_map;
9871
}
9972

100-
BenchRunner::BenchRunner(std::string name, BenchFunction func, PriorityLevel level)
73+
BenchRunner::BenchRunner(std::string name, BenchFunction func)
10174
{
102-
benchmarks().insert(std::make_pair(name, std::make_pair(func, level)));
75+
Assert(benchmarks().try_emplace(std::move(name), std::move(func)).second);
10376
}
10477

10578
void BenchRunner::RunAll(const Args& args)
@@ -120,12 +93,7 @@ void BenchRunner::RunAll(const Args& args)
12093
};
12194

12295
std::vector<ankerl::nanobench::Result> benchmarkResults;
123-
for (const auto& [name, bench_func] : benchmarks()) {
124-
const auto& [func, priority_level] = bench_func;
125-
126-
if (!(priority_level & args.priority)) {
127-
continue;
128-
}
96+
for (const auto& [name, func] : benchmarks()) {
12997

13098
if (!std::regex_match(name, baseMatch, reFilter)) {
13199
continue;

‎src/bench/bench.h‎

Lines changed: 7 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,9 @@
1010
#include <util/macros.h>
1111

1212
#include <chrono>
13-
#include <cstdint>
1413
#include <functional>
1514
#include <map>
1615
#include <string>
17-
#include <utility>
1816
#include <vector>
1917

2018
/*
@@ -40,17 +38,7 @@ namespace benchmark {
4038

4139
using ankerl::nanobench::Bench;
4240

43-
typedef std::function<void(Bench&)> BenchFunction;
44-
45-
enum PriorityLevel : uint8_t
46-
{
47-
LOW = 1 << 0,
48-
HIGH = 1 << 2,
49-
};
50-
51-
// List priority labels, comma-separated and sorted by increasing priority
52-
std::string ListPriorities();
53-
uint8_t StringToPriority(const std::string& str);
41+
using BenchFunction = std::function<void(Bench&)>;
5442

5543
struct Args {
5644
bool is_list_only;
@@ -60,25 +48,24 @@ struct Args {
6048
fs::path output_csv;
6149
fs::path output_json;
6250
std::string regex_filter;
63-
uint8_t priority;
6451
std::vector<std::string> setup_args;
6552
};
6653

6754
class BenchRunner
6855
{
69-
// maps from "name" -> (function, priority_level)
70-
typedef std::map<std::string, std::pair<BenchFunction, PriorityLevel>> BenchmarkMap;
56+
// maps from "name" -> function
57+
using BenchmarkMap = std::map<std::string, BenchFunction>;
7158
static BenchmarkMap& benchmarks();
7259

7360
public:
74-
BenchRunner(std::string name, BenchFunction func, PriorityLevel level);
61+
BenchRunner(std::string name, BenchFunction func);
7562

7663
static void RunAll(const Args& args);
7764
};
7865
} // namespace benchmark
7966

80-
// BENCHMARK(foo) expands to: benchmark::BenchRunner bench_11foo("foo", foo, priority_level);
81-
#define BENCHMARK(n, priority_level) \
82-
benchmark::BenchRunner PASTE2(bench_, PASTE2(__LINE__, n))(STRINGIZE(n), n, priority_level);
67+
// BENCHMARK(foo); expands to: benchmark::BenchRunner bench_runner_foo{"foo", foo};
68+
#define BENCHMARK(n) \
69+
benchmark::BenchRunner PASTE2(bench_runner_, n) { STRINGIZE(n), n }
8370

8471
#endif // BITCOIN_BENCH_BENCH_H

‎src/bench/bench_bitcoin.cpp‎

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,8 @@
1818
#include <sstream>
1919
#include <vector>
2020

21-
using util::SplitString;
22-
2321
static const char* DEFAULT_BENCH_FILTER = ".*";
2422
static constexpr int64_t DEFAULT_MIN_TIME_MS{10};
25-
/** Priority level default value, run "all" priority levels */
26-
static const std::string DEFAULT_PRIORITY{"all"};
2723

2824
static void SetupBenchArgs(ArgsManager& argsman)
2925
{
@@ -37,8 +33,6 @@ static void SetupBenchArgs(ArgsManager& argsman)
3733
argsman.AddArg("-output-csv=<output.csv>", "Generate CSV file with the most important benchmark results", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
3834
argsman.AddArg("-output-json=<output.json>", "Generate JSON file with all benchmark results", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
3935
argsman.AddArg("-sanity-check", "Run benchmarks for only one iteration with no output", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
40-
argsman.AddArg("-priority-level=<l1,l2,l3>", strprintf("Run benchmarks of one or multiple priority level(s) (%s), default: '%s'",
41-
benchmark::ListPriorities(), DEFAULT_PRIORITY), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
4236
}
4337

4438
// parses a comma separated list like "10,20,30,50"
@@ -54,14 +48,6 @@ static std::vector<double> parseAsymptote(const std::string& str) {
5448
return numbers;
5549
}
5650

57-
static uint8_t parsePriorityLevel(const std::string& str) {
58-
uint8_t levels{0};
59-
for (const auto& level: SplitString(str, ',')) {
60-
levels |= benchmark::StringToPriority(level);
61-
}
62-
return levels;
63-
}
64-
6551
static std::vector<std::string> parseTestSetupArgs(const ArgsManager& argsman)
6652
{
6753
// Parses unit test framework arguments supported by the benchmark framework.
@@ -144,7 +130,6 @@ int main(int argc, char** argv)
144130
args.output_json = argsman.GetPathArg("-output-json");
145131
args.regex_filter = argsman.GetArg("-filter", DEFAULT_BENCH_FILTER);
146132
args.sanity_check = argsman.GetBoolArg("-sanity-check", false);
147-
args.priority = parsePriorityLevel(argsman.GetArg("-priority-level", DEFAULT_PRIORITY));
148133
args.setup_args = parseTestSetupArgs(argsman);
149134

150135
benchmark::BenchRunner::RunAll(args);

‎src/bench/bip324_ecdh.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,4 +46,4 @@ static void BIP324_ECDH(benchmark::Bench& bench)
4646
});
4747
}
4848

49-
BENCHMARK(BIP324_ECDH, benchmark::PriorityLevel::HIGH);
49+
BENCHMARK(BIP324_ECDH);

‎src/bench/block_assemble.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,5 +69,5 @@ static void BlockAssemblerAddPackageTxns(benchmark::Bench& bench)
6969
});
7070
}
7171

72-
BENCHMARK(AssembleBlock, benchmark::PriorityLevel::HIGH);
73-
BENCHMARK(BlockAssemblerAddPackageTxns, benchmark::PriorityLevel::LOW);
72+
BENCHMARK(AssembleBlock);
73+
BENCHMARK(BlockAssemblerAddPackageTxns);

‎src/bench/blockencodings.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,6 @@ static void BlockEncodingLargeExtra(benchmark::Bench& bench)
126126
BlockEncodingBench(bench, 50000, 5000);
127127
}
128128

129-
BENCHMARK(BlockEncodingNoExtra, benchmark::PriorityLevel::HIGH);
130-
BENCHMARK(BlockEncodingStdExtra, benchmark::PriorityLevel::HIGH);
131-
BENCHMARK(BlockEncodingLargeExtra, benchmark::PriorityLevel::HIGH);
129+
BENCHMARK(BlockEncodingNoExtra);
130+
BENCHMARK(BlockEncodingStdExtra);
131+
BENCHMARK(BlockEncodingLargeExtra);

‎src/bench/ccoins_caching.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,4 +54,4 @@ static void CCoinsCaching(benchmark::Bench& bench)
5454
});
5555
}
5656

57-
BENCHMARK(CCoinsCaching, benchmark::PriorityLevel::HIGH);
57+
BENCHMARK(CCoinsCaching);

0 commit comments

Comments
 (0)