From 17e1c405dd67858ca47b53b5968e564895dba965 Mon Sep 17 00:00:00 2001 From: Marek Kurdej Date: Mon, 24 Oct 2016 09:49:36 +0200 Subject: [PATCH 1/4] Add ArgName() and ArgNames() methods to name arguments/ranges. --- include/benchmark/benchmark_api.h | 11 +++++++-- src/benchmark_register.cc | 18 ++++++++++++++ src/string_util.cc | 2 +- test/reporter_output_test.cc | 40 +++++++++++++++++++++++++++++++ 4 files changed, 68 insertions(+), 3 deletions(-) diff --git a/include/benchmark/benchmark_api.h b/include/benchmark/benchmark_api.h index 39edcff4..8292f14f 100644 --- a/include/benchmark/benchmark_api.h +++ b/include/benchmark/benchmark_api.h @@ -510,6 +510,13 @@ class Benchmark { // REQUIRES: The function passed to the constructor must accept arg1, arg2 ... Benchmark* Ranges(const std::vector >& ranges); + // Equivalent to ArgNames({name}) + Benchmark* ArgName(const std::string& name); + + // Set the argument names to display in the benchmark name. If not called, + // only argument values will be shown. + Benchmark* ArgNames(const std::vector& names); + // Equivalent to Ranges({{lo1, hi1}, {lo2, hi2}}). // NOTE: This is a legacy C++03 interface provided for compatibility only. // New code should use 'Ranges'. @@ -526,8 +533,7 @@ class Benchmark { Benchmark* Apply(void (*func)(Benchmark* benchmark)); // Set the range multiplier for non-dense range. If not called, the range - // multiplier - // kRangeMultiplier will be used. + // multiplier kRangeMultiplier will be used. Benchmark* RangeMultiplier(int multiplier); // Set the minimum amount of time to use when running this benchmark. This @@ -618,6 +624,7 @@ class Benchmark { std::string name_; ReportMode report_mode_; + std::vector arg_names_; // Args for all benchmark runs std::vector > args_; // Args for all benchmark runs TimeUnit time_unit_; int range_multiplier_; diff --git a/src/benchmark_register.cc b/src/benchmark_register.cc index 2657d4aa..ae109f85 100644 --- a/src/benchmark_register.cc +++ b/src/benchmark_register.cc @@ -151,8 +151,16 @@ bool BenchmarkFamilies::FindBenchmarks( instance.threads = num_threads; // Add arguments to instance name + size_t arg_i = 0; for (auto const& arg : args) { + if (arg_i < family->arg_names_.size()) { + instance.name += + StringPrintF("/%s:", family->arg_names_[arg_i].c_str()); + } else { + instance.name += "/"; + } AppendHumanReadable(arg, &instance.name); + ++arg_i; } if (!IsZero(family->min_time_)) { @@ -293,6 +301,16 @@ Benchmark* Benchmark::Ranges(const std::vector>& ranges) { return this; } +Benchmark* Benchmark::ArgName(const std::string& name) { + arg_names_ = {name}; + return this; +} + +Benchmark* Benchmark::ArgNames(const std::vector& names) { + arg_names_ = names; + return this; +} + Benchmark* Benchmark::DenseRange(int start, int limit, int step) { CHECK(ArgsCnt() == -1 || ArgsCnt() == 1); CHECK_GE(start, 0); diff --git a/src/string_util.cc b/src/string_util.cc index e073b811..4cefbfba 100644 --- a/src/string_util.cc +++ b/src/string_util.cc @@ -107,7 +107,7 @@ std::string ToBinaryStringFullySpecified(double value, double threshold, void AppendHumanReadable(int n, std::string* str) { std::stringstream ss; // Round down to the nearest SI prefix. - ss << "/" << ToBinaryStringFullySpecified(n, 1.0, 0); + ss << ToBinaryStringFullySpecified(n, 1.0, 0); *str += ss.str(); } diff --git a/test/reporter_output_test.cc b/test/reporter_output_test.cc index 59a0a39f..2bd36764 100644 --- a/test/reporter_output_test.cc +++ b/test/reporter_output_test.cc @@ -51,6 +51,46 @@ ADD_CASES(TC_JSONOut, {{"\"name\": \"BM_error\",$"}, ADD_CASES(TC_CSVOut, {{"^\"BM_error\",,,,,,,,true,\"message\"$"}}); +// ========================================================================= // +// ------------------------ Testing No Arg Name Output ----------------------- +// // +// ========================================================================= // + +void BM_no_arg_name(benchmark::State& state) { + while (state.KeepRunning()) { + } +} +BENCHMARK(BM_no_arg_name)->Arg(3); +ADD_CASES(TC_ConsoleOut, {{"^BM_no_arg_name/3 %console_report$"}}); +ADD_CASES(TC_JSONOut, {{"\"name\": \"BM_no_arg_name/3\",$"}}); +ADD_CASES(TC_CSVOut, {{"^\"BM_no_arg_name/3\",%csv_report$"}}); + +// ========================================================================= // +// ------------------------ Testing Arg Name Output ----------------------- // +// ========================================================================= // + +void BM_arg_name(benchmark::State& state) { + while (state.KeepRunning()) { + } +} +BENCHMARK(BM_arg_name)->Arg(3)->ArgName("first"); +ADD_CASES(TC_ConsoleOut, {{"^BM_arg_name/first:3 %console_report$"}}); +ADD_CASES(TC_JSONOut, {{"\"name\": \"BM_arg_name/first:3\",$"}}); +ADD_CASES(TC_CSVOut, {{"^\"BM_arg_name/first:3\",%csv_report$"}}); + +// ========================================================================= // +// ------------------------ Testing Arg Names Output ----------------------- // +// ========================================================================= // + +void BM_arg_names(benchmark::State& state) { + while (state.KeepRunning()) { + } +} +BENCHMARK(BM_arg_names)->Args({2, 4})->ArgNames({"first", "second"}); +ADD_CASES(TC_ConsoleOut, {{"^BM_arg_names/first:2/second:4 %console_report$"}}); +ADD_CASES(TC_JSONOut, {{"\"name\": \"BM_arg_names/first:2/second:4\",$"}}); +ADD_CASES(TC_CSVOut, {{"^\"BM_arg_names/first:2/second:4\",%csv_report$"}}); + // ========================================================================= // // ----------------------- Testing Complexity Output ----------------------- // // ========================================================================= // From c1c01b2cd303826cab1f419f1a1582d18d5905df Mon Sep 17 00:00:00 2001 From: Marek Kurdej Date: Tue, 25 Oct 2016 09:45:35 +0200 Subject: [PATCH 2/4] Handle the case when the argument name is an empty string. --- src/benchmark_register.cc | 12 ++++++++---- test/reporter_output_test.cc | 9 +++++---- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/src/benchmark_register.cc b/src/benchmark_register.cc index ae109f85..02aa298b 100644 --- a/src/benchmark_register.cc +++ b/src/benchmark_register.cc @@ -153,12 +153,16 @@ bool BenchmarkFamilies::FindBenchmarks( // Add arguments to instance name size_t arg_i = 0; for (auto const& arg : args) { + instance.name += "/"; + if (arg_i < family->arg_names_.size()) { - instance.name += - StringPrintF("/%s:", family->arg_names_[arg_i].c_str()); - } else { - instance.name += "/"; + const auto& arg_name = family->arg_names_[arg_i]; + if (!arg_name.empty()) { + instance.name += + StringPrintF("%s:", family->arg_names_[arg_i].c_str()); + } } + AppendHumanReadable(arg, &instance.name); ++arg_i; } diff --git a/test/reporter_output_test.cc b/test/reporter_output_test.cc index 2bd36764..f80d6b61 100644 --- a/test/reporter_output_test.cc +++ b/test/reporter_output_test.cc @@ -86,10 +86,11 @@ void BM_arg_names(benchmark::State& state) { while (state.KeepRunning()) { } } -BENCHMARK(BM_arg_names)->Args({2, 4})->ArgNames({"first", "second"}); -ADD_CASES(TC_ConsoleOut, {{"^BM_arg_names/first:2/second:4 %console_report$"}}); -ADD_CASES(TC_JSONOut, {{"\"name\": \"BM_arg_names/first:2/second:4\",$"}}); -ADD_CASES(TC_CSVOut, {{"^\"BM_arg_names/first:2/second:4\",%csv_report$"}}); +BENCHMARK(BM_arg_names)->Args({2, 5, 4})->ArgNames({"first", "", "third"}); +ADD_CASES(TC_ConsoleOut, + {{"^BM_arg_names/first:2/5/third:4 %console_report$"}}); +ADD_CASES(TC_JSONOut, {{"\"name\": \"BM_arg_names/first:2/5/third:4\",$"}}); +ADD_CASES(TC_CSVOut, {{"^\"BM_arg_names/first:2/5/third:4\",%csv_report$"}}); // ========================================================================= // // ----------------------- Testing Complexity Output ----------------------- // From cfee1a54e4d4dcc455ee077092aa5e2d054899c7 Mon Sep 17 00:00:00 2001 From: Marek Kurdej Date: Wed, 26 Oct 2016 09:29:28 +0200 Subject: [PATCH 3/4] Check argument count in `ArgName` and `ArgNames`. --- src/benchmark_register.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/benchmark_register.cc b/src/benchmark_register.cc index 02aa298b..2c49b45b 100644 --- a/src/benchmark_register.cc +++ b/src/benchmark_register.cc @@ -306,11 +306,13 @@ Benchmark* Benchmark::Ranges(const std::vector>& ranges) { } Benchmark* Benchmark::ArgName(const std::string& name) { + CHECK(ArgsCnt() == -1 || ArgsCnt() == 1); arg_names_ = {name}; return this; } Benchmark* Benchmark::ArgNames(const std::vector& names) { + CHECK(ArgsCnt() == -1 || ArgsCnt() == static_cast(names.size())); arg_names_ = names; return this; } From 3f23832a097db2da0d9823987dc574be3cefa1f3 Mon Sep 17 00:00:00 2001 From: Marek Kurdej Date: Wed, 26 Oct 2016 09:36:39 +0200 Subject: [PATCH 4/4] Allow calling Args and ArgNames in any order. --- src/benchmark_register.cc | 6 +++++- test/reporter_output_test.cc | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/benchmark_register.cc b/src/benchmark_register.cc index 2c49b45b..4e580d8e 100644 --- a/src/benchmark_register.cc +++ b/src/benchmark_register.cc @@ -422,7 +422,11 @@ Benchmark* Benchmark::ThreadPerCpu() { void Benchmark::SetName(const char* name) { name_ = name; } int Benchmark::ArgsCnt() const { - return args_.empty() ? -1 : static_cast(args_.front().size()); + if (args_.empty()) { + if (arg_names_.empty()) return -1; + return static_cast(arg_names_.size()); + } + return static_cast(args_.front().size()); } //=============================================================================// diff --git a/test/reporter_output_test.cc b/test/reporter_output_test.cc index f80d6b61..cd6fa7c6 100644 --- a/test/reporter_output_test.cc +++ b/test/reporter_output_test.cc @@ -73,7 +73,7 @@ void BM_arg_name(benchmark::State& state) { while (state.KeepRunning()) { } } -BENCHMARK(BM_arg_name)->Arg(3)->ArgName("first"); +BENCHMARK(BM_arg_name)->ArgName("first")->Arg(3); ADD_CASES(TC_ConsoleOut, {{"^BM_arg_name/first:3 %console_report$"}}); ADD_CASES(TC_JSONOut, {{"\"name\": \"BM_arg_name/first:3\",$"}}); ADD_CASES(TC_CSVOut, {{"^\"BM_arg_name/first:3\",%csv_report$"}});