diff --git a/bindings/python/google_benchmark/benchmark.cc b/bindings/python/google_benchmark/benchmark.cc index cf2da2e8..f4447690 100644 --- a/bindings/python/google_benchmark/benchmark.cc +++ b/bindings/python/google_benchmark/benchmark.cc @@ -34,7 +34,7 @@ std::vector Initialize(const std::vector& argv) { return remaining_argv; } -benchmark::internal::Benchmark* RegisterBenchmark(const char* name, +benchmark::internal::Benchmark* RegisterBenchmark(const std::string& name, nb::callable f) { return benchmark::RegisterBenchmark( name, [f](benchmark::State& state) { f(&state); }); @@ -165,8 +165,8 @@ NB_MODULE(_benchmark, m) { .def_prop_rw("complexity_n", &State::complexity_length_n, &State::SetComplexityN) .def_prop_rw("items_processed", &State::items_processed, - &State::SetItemsProcessed) - .def("set_label", (void (State::*)(const char*)) & State::SetLabel) + &State::SetItemsProcessed) + .def("set_label", &State::SetLabel) .def("range", &State::range, nb::arg("pos") = 0) .def_prop_ro("iterations", &State::iterations) .def_prop_ro("name", &State::name) diff --git a/docs/user_guide.md b/docs/user_guide.md index 7fffb603..133fca5b 100644 --- a/docs/user_guide.md +++ b/docs/user_guide.md @@ -56,7 +56,7 @@ [Exiting with an Error](#exiting-with-an-error) -[A Faster KeepRunning Loop](#a-faster-keep-running-loop) +[A Faster `KeepRunning` Loop](#a-faster-keep-running-loop) ## Benchmarking Tips @@ -271,10 +271,12 @@ information about the machine on which the benchmarks are run. Global setup/teardown specific to each benchmark can be done by passing a callback to Setup/Teardown: -The setup/teardown callbacks will be invoked once for each benchmark. -If the benchmark is multi-threaded (will run in k threads), they will be invoked exactly once before -each run with k threads. -If the benchmark uses different size groups of threads, the above will be true for each size group. +The setup/teardown callbacks will be invoked once for each benchmark. If the +benchmark is multi-threaded (will run in k threads), they will be invoked +exactly once before each run with k threads. + +If the benchmark uses different size groups of threads, the above will be true +for each size group. Eg., @@ -1142,7 +1144,7 @@ int main(int argc, char** argv) { When errors caused by external influences, such as file I/O and network communication, occur within a benchmark the -`State::SkipWithError(const char* msg)` function can be used to skip that run +`State::SkipWithError(const std::string& msg)` function can be used to skip that run of benchmark and report the error. Note that only future iterations of the `KeepRunning()` are skipped. For the ranged-for version of the benchmark loop Users must explicitly exit the loop, otherwise all iterations will be performed. @@ -1253,7 +1255,8 @@ the benchmark loop should be preferred. If you see this error: ``` -***WARNING*** CPU scaling is enabled, the benchmark real time measurements may be noisy and will incur extra overhead. +***WARNING*** CPU scaling is enabled, the benchmark real time measurements may +be noisy and will incur extra overhead. ``` you might want to disable the CPU frequency scaling while running the diff --git a/include/benchmark/benchmark.h b/include/benchmark/benchmark.h index 153cbba2..e44d5341 100644 --- a/include/benchmark/benchmark.h +++ b/include/benchmark/benchmark.h @@ -865,11 +865,7 @@ class BENCHMARK_EXPORT State { // BM_Compress 50 50 14115038 compress:27.3% // // REQUIRES: a benchmark has exited its benchmarking loop. - void SetLabel(const char* label); - - void BENCHMARK_ALWAYS_INLINE SetLabel(const std::string& str) { - this->SetLabel(str.c_str()); - } + void SetLabel(const std::string& label); // Range arguments for this run. CHECKs if the argument has been set. BENCHMARK_ALWAYS_INLINE @@ -1249,8 +1245,8 @@ class BENCHMARK_EXPORT Benchmark { TimeUnit GetTimeUnit() const; protected: - explicit Benchmark(const char* name); - void SetName(const char* name); + explicit Benchmark(const std::string& name); + void SetName(const std::string& name); public: const char* GetName() const; @@ -1305,12 +1301,12 @@ class BENCHMARK_EXPORT Benchmark { // the specified functor 'fn'. // // RETURNS: A pointer to the registered benchmark. -internal::Benchmark* RegisterBenchmark(const char* name, +internal::Benchmark* RegisterBenchmark(const std::string& name, internal::Function* fn); #if defined(BENCHMARK_HAS_CXX11) template -internal::Benchmark* RegisterBenchmark(const char* name, Lambda&& fn); +internal::Benchmark* RegisterBenchmark(const std::string& name, Lambda&& fn); #endif // Remove all registered benchmarks. All pointers to previously registered @@ -1322,7 +1318,7 @@ namespace internal { // (ie those created using the BENCHMARK(...) macros. class BENCHMARK_EXPORT FunctionBenchmark : public Benchmark { public: - FunctionBenchmark(const char* name, Function* func) + FunctionBenchmark(const std::string& name, Function* func) : Benchmark(name), func_(func) {} void Run(State& st) BENCHMARK_OVERRIDE; @@ -1339,20 +1335,20 @@ class LambdaBenchmark : public Benchmark { private: template - LambdaBenchmark(const char* name, OLambda&& lam) + LambdaBenchmark(const std::string& name, OLambda&& lam) : Benchmark(name), lambda_(std::forward(lam)) {} LambdaBenchmark(LambdaBenchmark const&) = delete; template // NOLINTNEXTLINE(readability-redundant-declaration) - friend Benchmark* ::benchmark::RegisterBenchmark(const char*, Lam&&); + friend Benchmark* ::benchmark::RegisterBenchmark(const std::string&, Lam&&); Lambda lambda_; }; #endif } // namespace internal -inline internal::Benchmark* RegisterBenchmark(const char* name, +inline internal::Benchmark* RegisterBenchmark(const std::string& name, internal::Function* fn) { return internal::RegisterBenchmarkInternal( ::new internal::FunctionBenchmark(name, fn)); @@ -1360,7 +1356,7 @@ inline internal::Benchmark* RegisterBenchmark(const char* name, #ifdef BENCHMARK_HAS_CXX11 template -internal::Benchmark* RegisterBenchmark(const char* name, Lambda&& fn) { +internal::Benchmark* RegisterBenchmark(const std::string& name, Lambda&& fn) { using BenchType = internal::LambdaBenchmark::type>; return internal::RegisterBenchmarkInternal( @@ -1371,7 +1367,7 @@ internal::Benchmark* RegisterBenchmark(const char* name, Lambda&& fn) { #if defined(BENCHMARK_HAS_CXX11) && \ (!defined(BENCHMARK_GCC_VERSION) || BENCHMARK_GCC_VERSION >= 409) template -internal::Benchmark* RegisterBenchmark(const char* name, Lambda&& fn, +internal::Benchmark* RegisterBenchmark(const std::string& name, Lambda&& fn, Args&&... args) { return benchmark::RegisterBenchmark( name, [=](benchmark::State& st) { fn(st, args...); }); diff --git a/src/benchmark.cc b/src/benchmark.cc index 14088402..f1633b70 100644 --- a/src/benchmark.cc +++ b/src/benchmark.cc @@ -273,7 +273,7 @@ void State::SetIterationTime(double seconds) { timer_->SetIterationTime(seconds); } -void State::SetLabel(const char* label) { +void State::SetLabel(const std::string& label) { MutexLock l(manager_->GetBenchmarkMutex()); manager_->results.report_label_ = label; } diff --git a/src/benchmark_register.cc b/src/benchmark_register.cc index 4503dd1d..e447c9a2 100644 --- a/src/benchmark_register.cc +++ b/src/benchmark_register.cc @@ -204,7 +204,7 @@ bool FindBenchmarksInternal(const std::string& re, // Benchmark //=============================================================================// -Benchmark::Benchmark(const char* name) +Benchmark::Benchmark(const std::string& name) : name_(name), aggregation_report_mode_(ARM_Unspecified), time_unit_(GetDefaultTimeUnit()), @@ -230,7 +230,7 @@ Benchmark::Benchmark(const char* name) Benchmark::~Benchmark() {} Benchmark* Benchmark::Name(const std::string& name) { - SetName(name.c_str()); + SetName(name); return this; } @@ -468,7 +468,7 @@ Benchmark* Benchmark::ThreadPerCpu() { return this; } -void Benchmark::SetName(const char* name) { name_ = name; } +void Benchmark::SetName(const std::string& name) { name_ = name; } const char* Benchmark::GetName() const { return name_.c_str(); } diff --git a/src/benchmark_runner.cc b/src/benchmark_runner.cc index 62383ea8..f7ae4243 100644 --- a/src/benchmark_runner.cc +++ b/src/benchmark_runner.cc @@ -177,11 +177,10 @@ BenchTimeType ParseBenchMinTime(const std::string& value) { } if (value.back() == 'x') { - const char* iters_str = value.c_str(); char* p_end; // Reset errno before it's changed by strtol. errno = 0; - IterationCount num_iters = std::strtol(iters_str, &p_end, 10); + IterationCount num_iters = std::strtol(value.c_str(), &p_end, 10); // After a valid parse, p_end should have been set to // point to the 'x' suffix. @@ -194,7 +193,6 @@ BenchTimeType ParseBenchMinTime(const std::string& value) { return ret; } - const char* time_str = value.c_str(); bool has_suffix = value.back() == 's'; if (!has_suffix) { BM_VLOG(0) << "Value passed to --benchmark_min_time should have a suffix. " @@ -204,7 +202,7 @@ BenchTimeType ParseBenchMinTime(const std::string& value) { char* p_end; // Reset errno before it's changed by strtod. errno = 0; - double min_time = std::strtod(time_str, &p_end); + double min_time = std::strtod(value.c_str(), &p_end); // After a successful parse, p_end should point to the suffix 's', // or the end of the string if the suffix was omitted. diff --git a/src/json_reporter.cc b/src/json_reporter.cc index 36efbf0b..6559dfd5 100644 --- a/src/json_reporter.cc +++ b/src/json_reporter.cc @@ -297,7 +297,8 @@ void JSONReporter::PrintRunData(Run const& run) { out << ",\n" << indent << FormatKV("max_bytes_used", memory_result.max_bytes_used); - auto report_if_present = [&out, &indent](const char* label, int64_t val) { + auto report_if_present = [&out, &indent](const std::string& label, + int64_t val) { if (val != MemoryManager::TombstoneValue) out << ",\n" << indent << FormatKV(label, val); }; diff --git a/test/output_test.h b/test/output_test.h index c6ff8ef2..c08fe1d8 100644 --- a/test/output_test.h +++ b/test/output_test.h @@ -85,7 +85,7 @@ std::string GetFileReporterOutput(int argc, char* argv[]); struct Results; typedef std::function ResultsCheckFn; -size_t AddChecker(const char* bm_name_pattern, const ResultsCheckFn& fn); +size_t AddChecker(const std::string& bm_name_pattern, const ResultsCheckFn& fn); // Class holding the results of a benchmark. // It is passed in calls to checker functions. @@ -117,7 +117,7 @@ struct Results { // get the string for a result by name, or nullptr if the name // is not found - const std::string* Get(const char* entry_name) const { + const std::string* Get(const std::string& entry_name) const { auto it = values.find(entry_name); if (it == values.end()) return nullptr; return &it->second; @@ -126,12 +126,12 @@ struct Results { // get a result by name, parsed as a specific type. // NOTE: for counters, use GetCounterAs instead. template - T GetAs(const char* entry_name) const; + T GetAs(const std::string& entry_name) const; // counters are written as doubles, so they have to be read first // as a double, and only then converted to the asked type. template - T GetCounterAs(const char* entry_name) const { + T GetCounterAs(const std::string& entry_name) const { double dval = GetAs(entry_name); T tval = static_cast(dval); return tval; @@ -139,7 +139,7 @@ struct Results { }; template -T Results::GetAs(const char* entry_name) const { +T Results::GetAs(const std::string& entry_name) const { auto* sv = Get(entry_name); BM_CHECK(sv != nullptr && !sv->empty()); std::stringstream ss; diff --git a/test/output_test_helper.cc b/test/output_test_helper.cc index 986c4adb..241af5c9 100644 --- a/test/output_test_helper.cc +++ b/test/output_test_helper.cc @@ -299,7 +299,7 @@ std::vector ResultsChecker::SplitCsv_(const std::string& line) { } // end namespace internal -size_t AddChecker(const char* bm_name, const ResultsCheckFn& fn) { +size_t AddChecker(const std::string& bm_name, const ResultsCheckFn& fn) { auto& rc = internal::GetResultsChecker(); rc.Add(bm_name, fn); return rc.results.size(); @@ -394,14 +394,14 @@ void RunOutputTests(int argc, char* argv[]) { benchmark::JSONReporter JR; benchmark::CSVReporter CSVR; struct ReporterTest { - const char* name; + std::string name; std::vector& output_cases; std::vector& error_cases; benchmark::BenchmarkReporter& reporter; std::stringstream out_stream; std::stringstream err_stream; - ReporterTest(const char* n, std::vector& out_tc, + ReporterTest(const std::string& n, std::vector& out_tc, std::vector& err_tc, benchmark::BenchmarkReporter& br) : name(n), output_cases(out_tc), error_cases(err_tc), reporter(br) { @@ -409,12 +409,12 @@ void RunOutputTests(int argc, char* argv[]) { reporter.SetErrorStream(&err_stream); } } TestCases[] = { - {"ConsoleReporter", GetTestCaseList(TC_ConsoleOut), + {std::string("ConsoleReporter"), GetTestCaseList(TC_ConsoleOut), GetTestCaseList(TC_ConsoleErr), CR}, - {"JSONReporter", GetTestCaseList(TC_JSONOut), GetTestCaseList(TC_JSONErr), - JR}, - {"CSVReporter", GetTestCaseList(TC_CSVOut), GetTestCaseList(TC_CSVErr), - CSVR}, + {std::string("JSONReporter"), GetTestCaseList(TC_JSONOut), + GetTestCaseList(TC_JSONErr), JR}, + {std::string("CSVReporter"), GetTestCaseList(TC_CSVOut), + GetTestCaseList(TC_CSVErr), CSVR}, }; // Create the test reporter and run the benchmarks. @@ -423,7 +423,8 @@ void RunOutputTests(int argc, char* argv[]) { benchmark::RunSpecifiedBenchmarks(&test_rep); for (auto& rep_test : TestCases) { - std::string msg = std::string("\nTesting ") + rep_test.name + " Output\n"; + std::string msg = + std::string("\nTesting ") + rep_test.name + std::string(" Output\n"); std::string banner(msg.size() - 1, '-'); std::cout << banner << msg << banner << "\n"; @@ -440,7 +441,7 @@ void RunOutputTests(int argc, char* argv[]) { // the checks to subscribees. auto& csv = TestCases[2]; // would use == but gcc spits a warning - BM_CHECK(std::strcmp(csv.name, "CSVReporter") == 0); + BM_CHECK(csv.name == std::string("CSVReporter")); internal::GetResultsChecker().CheckResults(csv.out_stream); } diff --git a/test/register_benchmark_test.cc b/test/register_benchmark_test.cc index 240c8c24..d69d144a 100644 --- a/test/register_benchmark_test.cc +++ b/test/register_benchmark_test.cc @@ -19,11 +19,11 @@ class TestReporter : public benchmark::ConsoleReporter { }; struct TestCase { - std::string name; - const char* label; + const std::string name; + const std::string label; // Note: not explicit as we rely on it being converted through ADD_CASES. - TestCase(const char* xname) : TestCase(xname, nullptr) {} - TestCase(const char* xname, const char* xlabel) + TestCase(const std::string& xname) : TestCase(xname, "") {} + TestCase(const std::string& xname, const std::string& xlabel) : name(xname), label(xlabel) {} typedef benchmark::BenchmarkReporter::Run Run; @@ -32,7 +32,7 @@ struct TestCase { // clang-format off BM_CHECK(name == run.benchmark_name()) << "expected " << name << " got " << run.benchmark_name(); - if (label) { + if (!label.empty()) { BM_CHECK(run.report_label == label) << "expected " << label << " got " << run.report_label; } else { @@ -123,7 +123,7 @@ void TestRegistrationAtRuntime() { { CustomFixture fx; benchmark::RegisterBenchmark("custom_fixture", fx); - AddCases({"custom_fixture"}); + AddCases({std::string("custom_fixture")}); } #endif #ifndef BENCHMARK_HAS_NO_VARIADIC_REGISTER_BENCHMARK diff --git a/test/skip_with_error_test.cc b/test/skip_with_error_test.cc index 2dd222a2..b4c5e154 100644 --- a/test/skip_with_error_test.cc +++ b/test/skip_with_error_test.cc @@ -48,7 +48,8 @@ struct TestCase { std::vector ExpectedResults; -int AddCases(const char* base_name, std::initializer_list const& v) { +int AddCases(const std::string& base_name, + std::initializer_list const& v) { for (auto TC : v) { TC.name = base_name + TC.name; ExpectedResults.push_back(std::move(TC));