From 498714357f0a64f4f56523182cf75acd785f2d9d Mon Sep 17 00:00:00 2001 From: dominic <510002+dmah42@users.noreply.github.com> Date: Wed, 23 Oct 2024 10:27:18 +0100 Subject: [PATCH 1/6] upgrade bazel mods. requires c++14 for tests (#1867) --- MODULE.bazel | 8 ++++---- test/BUILD | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/MODULE.bazel b/MODULE.bazel index e4f170c83d..4147925742 100644 --- a/MODULE.bazel +++ b/MODULE.bazel @@ -3,13 +3,13 @@ module( version = "1.9.0", ) -bazel_dep(name = "bazel_skylib", version = "1.5.0") -bazel_dep(name = "platforms", version = "0.0.8") +bazel_dep(name = "bazel_skylib", version = "1.7.1") +bazel_dep(name = "platforms", version = "0.0.10") bazel_dep(name = "rules_foreign_cc", version = "0.10.1") bazel_dep(name = "rules_cc", version = "0.0.9") -bazel_dep(name = "rules_python", version = "0.31.0", dev_dependency = True) -bazel_dep(name = "googletest", version = "1.12.1", dev_dependency = True, repo_name = "com_google_googletest") +bazel_dep(name = "rules_python", version = "0.37.0", dev_dependency = True) +bazel_dep(name = "googletest", version = "1.14.0", dev_dependency = True, repo_name = "com_google_googletest") bazel_dep(name = "libpfm", version = "4.11.0") diff --git a/test/BUILD b/test/BUILD index b245fa7622..f2179f61c1 100644 --- a/test/BUILD +++ b/test/BUILD @@ -10,7 +10,7 @@ platform( TEST_COPTS = [ "-pedantic", "-pedantic-errors", - "-std=c++11", + "-std=c++14", "-Wall", "-Wconversion", "-Wextra", From be2134584d6ee4f8c160c413d4df8e4c5db17d54 Mon Sep 17 00:00:00 2001 From: Nicholas Junge Date: Wed, 23 Oct 2024 11:38:53 +0200 Subject: [PATCH 2/6] Update nanobind_bazel to v2.2.0 (#1866) Adds support for free-threaded nanobind extension builds, though we don't currently build a free-threaded wheel. Co-authored-by: dominic <510002+dmah42@users.noreply.github.com> --- MODULE.bazel | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/MODULE.bazel b/MODULE.bazel index 4147925742..40e306fafa 100644 --- a/MODULE.bazel +++ b/MODULE.bazel @@ -38,4 +38,4 @@ use_repo(pip, "tools_pip_deps") # -- bazel_dep definitions -- # -bazel_dep(name = "nanobind_bazel", version = "2.1.0", dev_dependency = True) +bazel_dep(name = "nanobind_bazel", version = "2.2.0", dev_dependency = True) From c45d9c4c2fa077b7bb2018f1c52b0668b65d4b22 Mon Sep 17 00:00:00 2001 From: dominic <510002+dmah42@users.noreply.github.com> Date: Thu, 24 Oct 2024 09:46:02 +0100 Subject: [PATCH 3/6] bump googletest version to match bazel (#1868) * bump googletest version to match bazel * bump minimum cmake to 3.13 per supported versions --- .github/workflows/build-and-test-min-cmake.yml | 2 +- CMakeLists.txt | 2 +- cmake/GoogleTest.cmake.in | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/build-and-test-min-cmake.yml b/.github/workflows/build-and-test-min-cmake.yml index b49800629b..2509984204 100644 --- a/.github/workflows/build-and-test-min-cmake.yml +++ b/.github/workflows/build-and-test-min-cmake.yml @@ -20,7 +20,7 @@ jobs: - uses: lukka/get-cmake@latest with: - cmakeVersion: 3.10.0 + cmakeVersion: 3.13.0 - name: create build environment run: cmake -E make_directory ${{ runner.workspace }}/_build diff --git a/CMakeLists.txt b/CMakeLists.txt index a86a5686ed..3aac35fe69 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1,5 +1,5 @@ # Require CMake 3.10. If available, use the policies up to CMake 3.22. -cmake_minimum_required (VERSION 3.10...3.22) +cmake_minimum_required (VERSION 3.13...3.22) project (benchmark VERSION 1.9.0 LANGUAGES CXX) diff --git a/cmake/GoogleTest.cmake.in b/cmake/GoogleTest.cmake.in index ce653ac375..c791446754 100644 --- a/cmake/GoogleTest.cmake.in +++ b/cmake/GoogleTest.cmake.in @@ -1,4 +1,4 @@ -cmake_minimum_required(VERSION 2.8.12) +cmake_minimum_required (VERSION 3.13...3.22) project(googletest-download NONE) @@ -38,7 +38,7 @@ else() ExternalProject_Add( googletest GIT_REPOSITORY https://github.com/google/googletest.git - GIT_TAG "release-1.11.0" + GIT_TAG "v1.14.0" PREFIX "${CMAKE_BINARY_DIR}" STAMP_DIR "${CMAKE_BINARY_DIR}/stamp" DOWNLOAD_DIR "${CMAKE_BINARY_DIR}/download" From ffc727a859d9ae7631fbc7647392efa05032211a Mon Sep 17 00:00:00 2001 From: xdje42 Date: Thu, 24 Oct 2024 02:22:58 -0700 Subject: [PATCH 4/6] Verify RegisterProfilerManager doesn't overwrite an existing registration (#1837) * Verify RegisterProfilerManager doesn't overwrite an existing registration Tested: Add a second registration to test/profiler_manager_test.cc and verify the test crashes as expected. * Verify RegisterProfilerManager doesn't overwrite an existing registration Tested: Configure with: cmake -GNinja -DCMAKE_BUILD_TYPE=Debug -DBENCHMARK_DOWNLOAD_DEPENDENCIES=on Then run: ctest -R profiler_manager_gtest Before change test fails (expected), after change test passes (expected) --------- Co-authored-by: dominic <510002+dmah42@users.noreply.github.com> --- src/benchmark.cc | 4 ++++ test/CMakeLists.txt | 1 + test/profiler_manager_gtest.cc | 42 ++++++++++++++++++++++++++++++++++ 3 files changed, 47 insertions(+) create mode 100644 test/profiler_manager_gtest.cc diff --git a/src/benchmark.cc b/src/benchmark.cc index 2605077444..0ea90aeb6a 100644 --- a/src/benchmark.cc +++ b/src/benchmark.cc @@ -668,6 +668,10 @@ void RegisterMemoryManager(MemoryManager* manager) { } void RegisterProfilerManager(ProfilerManager* manager) { + // Don't allow overwriting an existing manager. + if (manager != nullptr) { + BM_CHECK_EQ(internal::profiler_manager, nullptr); + } internal::profiler_manager = manager; } diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 815b581889..321e24d94b 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -254,6 +254,7 @@ if (BENCHMARK_ENABLE_GTEST_TESTS) add_gtest(perf_counters_gtest) add_gtest(time_unit_gtest) add_gtest(min_time_parse_gtest) + add_gtest(profiler_manager_gtest) endif(BENCHMARK_ENABLE_GTEST_TESTS) ############################################################################### diff --git a/test/profiler_manager_gtest.cc b/test/profiler_manager_gtest.cc new file mode 100644 index 0000000000..434e4ecadf --- /dev/null +++ b/test/profiler_manager_gtest.cc @@ -0,0 +1,42 @@ +#include + +#include "benchmark/benchmark.h" +#include "gtest/gtest.h" + +namespace { + +class TestProfilerManager : public benchmark::ProfilerManager { + public: + void AfterSetupStart() override { ++start_called; } + void BeforeTeardownStop() override { ++stop_called; } + + int start_called = 0; + int stop_called = 0; +}; + +void BM_empty(benchmark::State& state) { + for (auto _ : state) { + auto iterations = state.iterations(); + benchmark::DoNotOptimize(iterations); + } +} +BENCHMARK(BM_empty); + +TEST(ProfilerManager, ReregisterManager) { +#if GTEST_HAS_DEATH_TEST + // Tests only runnable in debug mode (when BM_CHECK is enabled). +#ifndef NDEBUG +#ifndef TEST_BENCHMARK_LIBRARY_HAS_NO_ASSERTIONS + ASSERT_DEATH_IF_SUPPORTED( + { + std::unique_ptr pm(new TestProfilerManager()); + benchmark::RegisterProfilerManager(pm.get()); + benchmark::RegisterProfilerManager(pm.get()); + }, + "RegisterProfilerManager"); +#endif +#endif +#endif +} + +} // namespace From 4e3f2d8b6728d628b3baa77a8d2359dd8e35bab5 Mon Sep 17 00:00:00 2001 From: Richard Cole Date: Thu, 24 Oct 2024 12:31:06 +0100 Subject: [PATCH 5/6] [#1487] ensure that when printing color text the background color of the terminal on windows is preserved (#1865) Co-authored-by: dominic <510002+dmah42@users.noreply.github.com> --- src/colorprint.cc | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/colorprint.cc b/src/colorprint.cc index abc71492f7..fd1971ad3c 100644 --- a/src/colorprint.cc +++ b/src/colorprint.cc @@ -135,19 +135,25 @@ void ColorPrintf(std::ostream& out, LogColor color, const char* fmt, // Gets the current text color. CONSOLE_SCREEN_BUFFER_INFO buffer_info; GetConsoleScreenBufferInfo(stdout_handle, &buffer_info); - const WORD old_color_attrs = buffer_info.wAttributes; + const WORD original_color_attrs = buffer_info.wAttributes; // We need to flush the stream buffers into the console before each // SetConsoleTextAttribute call lest it affect the text that is already // printed but has not yet reached the console. out.flush(); - SetConsoleTextAttribute(stdout_handle, - GetPlatformColorCode(color) | FOREGROUND_INTENSITY); + + const WORD original_background_attrs = + original_color_attrs & (BACKGROUND_RED | BACKGROUND_GREEN | + BACKGROUND_BLUE | BACKGROUND_INTENSITY); + + SetConsoleTextAttribute(stdout_handle, GetPlatformColorCode(color) | + FOREGROUND_INTENSITY | + original_background_attrs); out << FormatString(fmt, args); out.flush(); - // Restores the text color. - SetConsoleTextAttribute(stdout_handle, old_color_attrs); + // Restores the text and background color. + SetConsoleTextAttribute(stdout_handle, original_color_attrs); #else const char* color_code = GetPlatformColorCode(color); if (color_code) out << FormatString("\033[0;3%sm", color_code); From d99cdd7356de97b3056684d6b511189778d8a247 Mon Sep 17 00:00:00 2001 From: Nicholas Junge Date: Mon, 28 Oct 2024 19:18:40 +0100 Subject: [PATCH 6/6] Add `nb::is_flag()` annotation to Counter::Flags (#1870) This saves us the definition of `__or__`, because we can just use the one from `enum.IntFlag`. --- bindings/python/google_benchmark/benchmark.cc | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/bindings/python/google_benchmark/benchmark.cc b/bindings/python/google_benchmark/benchmark.cc index 64ffb92b48..a935822536 100644 --- a/bindings/python/google_benchmark/benchmark.cc +++ b/bindings/python/google_benchmark/benchmark.cc @@ -118,7 +118,7 @@ NB_MODULE(_benchmark, m) { using benchmark::Counter; nb::class_ py_counter(m, "Counter"); - nb::enum_(py_counter, "Flags", nb::is_arithmetic()) + nb::enum_(py_counter, "Flags", nb::is_arithmetic(), nb::is_flag()) .value("kDefaults", Counter::Flags::kDefaults) .value("kIsRate", Counter::Flags::kIsRate) .value("kAvgThreads", Counter::Flags::kAvgThreads) @@ -129,10 +129,7 @@ NB_MODULE(_benchmark, m) { .value("kAvgIterations", Counter::Flags::kAvgIterations) .value("kAvgIterationsRate", Counter::Flags::kAvgIterationsRate) .value("kInvert", Counter::Flags::kInvert) - .export_values() - .def("__or__", [](Counter::Flags a, Counter::Flags b) { - return static_cast(a) | static_cast(b); - }); + .export_values(); nb::enum_(py_counter, "OneK") .value("kIs1000", Counter::OneK::kIs1000) @@ -140,13 +137,9 @@ NB_MODULE(_benchmark, m) { .export_values(); py_counter - .def( - "__init__", - [](Counter* c, double value, int flags, Counter::OneK oneK) { - new (c) Counter(value, static_cast(flags), oneK); - }, - nb::arg("value") = 0., nb::arg("flags") = Counter::kDefaults, - nb::arg("k") = Counter::kIs1000) + .def(nb::init(), + nb::arg("value") = 0., nb::arg("flags") = Counter::kDefaults, + nb::arg("k") = Counter::kIs1000) .def("__init__", ([](Counter* c, double value) { new (c) Counter(value); })) .def_rw("value", &Counter::value)