From 32601f3594c1f8cc33659111eedc3ed946c6f1cc Mon Sep 17 00:00:00 2001 From: Guillaume Lagrange Date: Wed, 25 Jun 2025 18:24:25 +0200 Subject: [PATCH 1/5] fix(core): use proper type for pid on windows --- core/src/walltime.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/walltime.cpp b/core/src/walltime.cpp index 23b8c4b..7f12cdf 100644 --- a/core/src/walltime.cpp +++ b/core/src/walltime.cpp @@ -108,7 +108,7 @@ void write_codspeed_benchmarks_to_json( std::string creator_name = "codspeed-cpp"; std::string creator_version = CODSPEED_VERSION; #ifdef _WIN32 - pid_t creator_pid = _getpid(); + auto creator_pid = _getpid(); #else pid_t creator_pid = getpid(); #endif From adda9432242eca6f919cc6c6a2166fcc085b58df Mon Sep 17 00:00:00 2001 From: Guillaume Lagrange Date: Wed, 25 Jun 2025 22:27:07 +0200 Subject: [PATCH 2/5] feat(core): add utils function to get env var in a safe way --- core/BUILD | 2 +- core/src/utils.h | 13 +++++++++++++ core/src/walltime.cpp | 7 ++++--- core/src/workspace.cpp | 23 +++++++++++++++++++++-- 4 files changed, 39 insertions(+), 6 deletions(-) create mode 100644 core/src/utils.h diff --git a/core/BUILD b/core/BUILD index f4fc270..aa3ee27 100644 --- a/core/BUILD +++ b/core/BUILD @@ -6,7 +6,7 @@ CODSPEED_VERSION = "1.1.1" # Define the codspeed library cc_library( name = "codspeed", - srcs = glob(["src/**/*.cpp"]), + srcs = glob(["src/**/*.cpp"] + ["src/**/*.h"]), hdrs = glob(["include/**/*.h"] + ["include/**/*.hpp"]), includes = ["include"], defines = [ diff --git a/core/src/utils.h b/core/src/utils.h new file mode 100644 index 0000000..42fefc2 --- /dev/null +++ b/core/src/utils.h @@ -0,0 +1,13 @@ +#ifndef CODSPEED_UTILS_H +#define CODSPEED_UTILS_H + +#include + +namespace codspeed { + +// Cross-platform getenv wrapper +std::string safe_getenv(const char* var_name); + +} // namespace codspeed + +#endif // CODSPEED_UTILS_H \ No newline at end of file diff --git a/core/src/walltime.cpp b/core/src/walltime.cpp index 7f12cdf..320774d 100644 --- a/core/src/walltime.cpp +++ b/core/src/walltime.cpp @@ -9,6 +9,7 @@ #include #include "codspeed.h" +#include "utils.h" #ifdef _WIN32 #include #else @@ -31,7 +32,7 @@ struct BenchmarkStats { double total_time; uint64_t iqr_outlier_rounds; uint64_t stdev_outlier_rounds; - long iter_per_round; + uint64_t iter_per_round; uint64_t warmup_iters; }; @@ -169,8 +170,8 @@ void write_codspeed_benchmarks_to_json( oss << "}"; // Determine the directory path - const char *profile_folder = std::getenv("CODSPEED_PROFILE_FOLDER"); - std::string directory = profile_folder ? profile_folder : "."; + std::string profile_folder = safe_getenv("CODSPEED_PROFILE_FOLDER"); + std::string directory = profile_folder.empty() ? "." : profile_folder; // Create the results directory if it does not exist std::filesystem::path results_path = directory + "/results"; diff --git a/core/src/workspace.cpp b/core/src/workspace.cpp index 466e510..d2e6bcd 100644 --- a/core/src/workspace.cpp +++ b/core/src/workspace.cpp @@ -3,13 +3,32 @@ #include #include "codspeed.h" +#include "utils.h" namespace codspeed { -std::string get_path_relative_to_workspace(const std::string &path) { +std::string safe_getenv(const char* var_name) { +#ifdef _WIN32 + // On Windows, use _dupenv_s instead of std::getenv for thread safety + char* value; + size_t len; + errno_t err = _dupenv_s(&value, &len, var_name); + if (err == 0 && value) { + std::string result(value); + free(value); + return result; + } + return ""; +#else + const char* value = std::getenv(var_name); + return value ? value : ""; +#endif +} + +std::string get_path_relative_to_workspace(const std::string& path) { // 1. Check for bazel usage, through the BUILD_WORKSPACE_DIRECTORY env var // If so, __FILE__ will already be relative to the bazel workspace root - if (std::getenv("BUILD_WORKSPACE_DIRECTORY") != NULL) { + if (!safe_getenv("BUILD_WORKSPACE_DIRECTORY").empty()) { return path; } From e2ba701f7946a94ff4e29a28b987e2babe42f9f9 Mon Sep 17 00:00:00 2001 From: Guillaume Lagrange Date: Wed, 25 Jun 2025 22:27:52 +0200 Subject: [PATCH 3/5] fix: allow project to build on windows --- core/BUILD | 9 +++++++++ core/CMakeLists.txt | 6 +++--- core/include/codspeed.h | 3 ++- core/include/measurement.hpp | 10 ++++++++++ core/src/codspeed.cpp | 2 +- core/src/uri.cpp | 4 ++++ .../google_benchmark_cmake/CMakeLists.txt | 19 ++++++++++++++++++- .../include/benchmark/benchmark.h | 14 ++++++++------ 8 files changed, 55 insertions(+), 12 deletions(-) diff --git a/core/BUILD b/core/BUILD index aa3ee27..e179255 100644 --- a/core/BUILD +++ b/core/BUILD @@ -3,12 +3,21 @@ load("@rules_cc//cc:defs.bzl", "cc_library") CODSPEED_VERSION = "1.1.1" +config_setting( + name = "windows", + constraint_values = ["@platforms//os:windows"], +) + # Define the codspeed library cc_library( name = "codspeed", srcs = glob(["src/**/*.cpp"] + ["src/**/*.h"]), hdrs = glob(["include/**/*.h"] + ["include/**/*.hpp"]), includes = ["include"], + copts = select({ + ":windows": ["/std:c++17"], + "//conditions:default": [], + }), defines = [ "CODSPEED_VERSION=\\\"{}\\\"".format(CODSPEED_VERSION), ] + select({ diff --git a/core/CMakeLists.txt b/core/CMakeLists.txt index 270dc04..cf0d1a0 100644 --- a/core/CMakeLists.txt +++ b/core/CMakeLists.txt @@ -77,7 +77,7 @@ set_property( ) if(NOT CODSPEED_MODE STREQUAL "off") - target_compile_definitions(codspeed INTERFACE -DCODSPEED_ENABLED) + target_compile_definitions(codspeed PUBLIC -DCODSPEED_ENABLED) if(NOT CMAKE_BUILD_TYPE) message( @@ -95,10 +95,10 @@ if(NOT CODSPEED_MODE STREQUAL "off") if(CODSPEED_MODE STREQUAL "instrumentation") target_compile_definitions( codspeed - INTERFACE -DCODSPEED_INSTRUMENTATION + PUBLIC -DCODSPEED_INSTRUMENTATION ) elseif(CODSPEED_MODE STREQUAL "walltime") - target_compile_definitions(codspeed INTERFACE -DCODSPEED_WALLTIME) + target_compile_definitions(codspeed PUBLIC -DCODSPEED_WALLTIME) else() message( FATAL_ERROR diff --git a/core/include/codspeed.h b/core/include/codspeed.h index dce56dd..e46184b 100644 --- a/core/include/codspeed.h +++ b/core/include/codspeed.h @@ -1,6 +1,7 @@ #ifndef CODSPEED_H #define CODSPEED_H +#include #include #include @@ -33,7 +34,7 @@ class CodSpeed { struct RawWalltimeBenchmark { std::string name; std::string uri; - long iter_per_round; + uint64_t iter_per_round; double mean_ns; double median_ns; double stdev_ns; diff --git a/core/include/measurement.hpp b/core/include/measurement.hpp index e34ab62..4604d32 100644 --- a/core/include/measurement.hpp +++ b/core/include/measurement.hpp @@ -3,7 +3,9 @@ #include +#ifdef CODSPEED_INSTRUMENTATION #include "callgrind.h" +#endif inline std::string get_version() { #ifdef CODSPEED_VERSION @@ -13,6 +15,7 @@ inline std::string get_version() { #endif } +#ifdef CODSPEED_INSTRUMENTATION inline bool measurement_is_instrumented() { return RUNNING_ON_VALGRIND; } inline void measurement_set_metadata() { @@ -30,5 +33,12 @@ __attribute__((always_inline)) inline void measurement_stop( CALLGRIND_STOP_INSTRUMENTATION; CALLGRIND_DUMP_STATS_AT(name.c_str()); }; +#else +// Stub implementations for non-instrumentation builds +inline bool measurement_is_instrumented() { return false; } +inline void measurement_set_metadata() {} +inline void measurement_start() {} +inline void measurement_stop(const std::string &name) { (void)name; } +#endif #endif // MEASUREMENT_H diff --git a/core/src/codspeed.cpp b/core/src/codspeed.cpp index 72ef873..5143d67 100644 --- a/core/src/codspeed.cpp +++ b/core/src/codspeed.cpp @@ -76,7 +76,7 @@ void CodSpeed::start_benchmark(const std::string &name) { // Sanity check URI and add a placeholder if format is wrong if (name.find("::") == std::string::npos) { - std::string uri = "unknown_file::" + name; + uri = "unknown_file::" + name; std::cout << "WARNING: Benchmark name does not contain '::'. Using URI: " << uri << std::endl; } diff --git a/core/src/uri.cpp b/core/src/uri.cpp index ff58d21..efb4e58 100644 --- a/core/src/uri.cpp +++ b/core/src/uri.cpp @@ -46,6 +46,10 @@ std::string extract_lambda_namespace(const std::string &pretty_func) { return extract_namespace_clang(pretty_func); #elif __GNUC__ return extract_namespace_gcc(pretty_func); +#elif _MSC_VER + // MSVC doesn't support __PRETTY_FUNCTION__ in the same way + // Return empty string as fallback for Windows + return {}; #else #error "Unsupported compiler" #endif diff --git a/examples/google_benchmark_cmake/CMakeLists.txt b/examples/google_benchmark_cmake/CMakeLists.txt index 3d3067a..92ea7ff 100644 --- a/examples/google_benchmark_cmake/CMakeLists.txt +++ b/examples/google_benchmark_cmake/CMakeLists.txt @@ -6,12 +6,29 @@ project(codspeed_picobench_compat VERSION 0.0.0 LANGUAGES CXX) # Treat warnings as errors if(CMAKE_CXX_COMPILER_ID MATCHES "GNU|Clang") add_compile_options(-Wall -Wextra -Werror) +elseif(MSVC) + # Set C++17 standard + set(CMAKE_CXX_STANDARD 17) + set(CMAKE_CXX_STANDARD_REQUIRED ON) + + add_compile_options( + /W4 + /WX + /wd5051 # Ignore [[maybe_unused]] C++17 warnings from Google Benchmark + /wd4038 # Ignore STL C++17 warnings + ) endif() # On the small benchmarks of this repo, most of the benches will be optimized out, but this is expected. set(CMAKE_BUILD_TYPE RelWithDebInfo) -option(BENCHMARK_ENABLE_GTEST_TESTS OFF) +# Disable Google Benchmark tests for examples +set(BENCHMARK_ENABLE_GTEST_TESTS + OFF + CACHE BOOL + "Enable testing of the benchmark library." + FORCE +) FetchContent_Declare( google_benchmark diff --git a/google_benchmark/include/benchmark/benchmark.h b/google_benchmark/include/benchmark/benchmark.h index 0bca99f..a97761e 100644 --- a/google_benchmark/include/benchmark/benchmark.h +++ b/google_benchmark/include/benchmark/benchmark.h @@ -1459,14 +1459,16 @@ class Fixture : public internal::Benchmark { #define BENCHMARK_PRIVATE_CONCAT_NAME(BaseClass, Method) \ BaseClass##_##Method##_Benchmark -#define BENCHMARK_PRIVATE_DECLARE(n) \ - /* NOLINTNEXTLINE(misc-use-anonymous-namespace) */ \ - static ::benchmark::internal::Benchmark const* const BENCHMARK_PRIVATE_NAME( \ - n) [[maybe_unused]] +#define BENCHMARK_PRIVATE_DECLARE(n) \ + /* NOLINTNEXTLINE(misc-use-anonymous-namespace) */ \ + static ::benchmark::internal::Benchmark const* const BENCHMARK_PRIVATE_NAME(n) #ifdef CODSPEED_ENABLED -#define CUR_FILE \ - codspeed::get_path_relative_to_workspace(__FILE__) + "::" +#define CUR_FILE codspeed::get_path_relative_to_workspace(__FILE__) + "::" +#ifdef _MSC_VER +// TODO: __PRETTY_FUNCTION__ is not available in MSVC and we dont support +#define __PRETTY_FUNCTION__ "unsupported" +#endif #define NAMESPACE \ (([]() { return codspeed::extract_lambda_namespace(__PRETTY_FUNCTION__); })()) #define STATIC_NAMESPACE_STRING(name) static std::string name = NAMESPACE; From 8219fe671843d4175607266d8b2b00ad28b31219 Mon Sep 17 00:00:00 2001 From: Guillaume Lagrange Date: Wed, 25 Jun 2025 18:48:42 +0200 Subject: [PATCH 4/5] chore(core): prevent building tests if there are warnings This way we can get early warnings in our CI without disrupting downstream users by adding -Wall -Werror etc if they do not wish to do so. --- core/test/CMakeLists.txt | 7 +++++++ examples/google_benchmark_bazel/BUILD.bazel | 10 +++++++++- .../google_benchmark_cmake/CMakeLists.txt | 2 +- .../include/benchmark/benchmark.h | 20 +++++++++---------- 4 files changed, 27 insertions(+), 12 deletions(-) diff --git a/core/test/CMakeLists.txt b/core/test/CMakeLists.txt index 32524bf..9e2f220 100644 --- a/core/test/CMakeLists.txt +++ b/core/test/CMakeLists.txt @@ -18,5 +18,12 @@ target_link_libraries(unit_tests GTest::gtest_main ) +# Treat warnings as errors for tests to catch issues early +if(MSVC) + target_compile_options(unit_tests PRIVATE /W4 /WX) +elseif(CMAKE_CXX_COMPILER_ID MATCHES "GNU|Clang") + target_compile_options(unit_tests PRIVATE -Wall -Wextra -Werror) +endif() + include(GoogleTest) gtest_discover_tests(unit_tests) diff --git a/examples/google_benchmark_bazel/BUILD.bazel b/examples/google_benchmark_bazel/BUILD.bazel index 444a4dc..d18290d 100644 --- a/examples/google_benchmark_bazel/BUILD.bazel +++ b/examples/google_benchmark_bazel/BUILD.bazel @@ -1,7 +1,15 @@ +config_setting( + name = "windows", + constraint_values = ["@platforms//os:windows"], +) + cc_binary( name = "my_benchmark", srcs = glob(["*.cpp", "*.hpp"]), - copts = ["-Wall", "-Wextra", "-Werror"], + copts = select({ + ":windows": ["/std:c++17", "/W4", "/WX"], + "//conditions:default": ["-Wall", "-Wextra", "-Werror"], + }), deps = [ "//google_benchmark:benchmark", ], diff --git a/examples/google_benchmark_cmake/CMakeLists.txt b/examples/google_benchmark_cmake/CMakeLists.txt index 92ea7ff..ddfe611 100644 --- a/examples/google_benchmark_cmake/CMakeLists.txt +++ b/examples/google_benchmark_cmake/CMakeLists.txt @@ -1,7 +1,7 @@ cmake_minimum_required(VERSION 3.12) include(FetchContent) -project(codspeed_picobench_compat VERSION 0.0.0 LANGUAGES CXX) +project(codspeed_google_benchmark_example VERSION 0.0.0 LANGUAGES CXX) # Treat warnings as errors if(CMAKE_CXX_COMPILER_ID MATCHES "GNU|Clang") diff --git a/google_benchmark/include/benchmark/benchmark.h b/google_benchmark/include/benchmark/benchmark.h index a97761e..a8cc6e6 100644 --- a/google_benchmark/include/benchmark/benchmark.h +++ b/google_benchmark/include/benchmark/benchmark.h @@ -185,10 +185,10 @@ BENCHMARK(BM_test)->Unit(benchmark::kMillisecond); #ifdef CODSPEED_ENABLED #include -#include #include -#endif // CODSPEED_ENABLED +#include +#endif // CODSPEED_ENABLED #if defined(_MSC_VER) #include // for _ReadWriteBarrier @@ -947,7 +947,7 @@ class BENCHMARK_EXPORT BENCHMARK_INTERNAL_CACHELINE_ALIGNED State { public: const IterationCount max_iterations; #ifdef CODSPEED_INSTRUMENTATION - codspeed::CodSpeed *codspeed_; + codspeed::CodSpeed* codspeed_; #endif private: @@ -970,11 +970,11 @@ class BENCHMARK_EXPORT BENCHMARK_INTERNAL_CACHELINE_ALIGNED State { internal::ThreadTimer* timer, internal::ThreadManager* manager, internal::PerfCountersMeasurement* perf_counters_measurement, ProfilerManager* profiler_manager - #ifdef CODSPEED_INSTRUMENTATION +#ifdef CODSPEED_INSTRUMENTATION , - codspeed::CodSpeed *codspeed = NULL - #endif - ); + codspeed::CodSpeed* codspeed = NULL +#endif + ); void StartKeepRunning(); // Implementation of KeepRunning() and KeepRunningBatch(). @@ -1609,12 +1609,12 @@ class Fixture : public internal::Benchmark { #ifdef CODSPEED_ENABLED #define BENCHMARK_PRIVATE_DECLARE_F(BaseClass, Method) \ - STATIC_NAMESPACE_STRING(ns_##BaseClass##_##Method); \ + STATIC_NAMESPACE_STRING(ns_##BaseClass##_##Method); \ class BaseClass##_##Method##_Benchmark : public BaseClass { \ public: \ BaseClass##_##Method##_Benchmark() { \ this->SetName(CUR_FILE + ns_##BaseClass##_##Method + \ - #Method "[" #BaseClass "]"); \ + #Method "[" #BaseClass "]"); \ } \ \ protected: \ @@ -1660,7 +1660,7 @@ class Fixture : public internal::Benchmark { void BenchmarkCase(::benchmark::State&) override; \ }; -#else // CODSPEED_ENABLED undefined: +#else // CODSPEED_ENABLED undefined: #define BENCHMARK_PRIVATE_DECLARE_F(BaseClass, Method) \ class BaseClass##_##Method##_Benchmark : public BaseClass { \ From 7da6671bcc4b1d1726585ad03da43f997afa2003 Mon Sep 17 00:00:00 2001 From: Guillaume Lagrange Date: Wed, 25 Jun 2025 18:39:18 +0200 Subject: [PATCH 5/5] ci: add windows minimal compilation test --- .github/workflows/ci.yml | 52 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1d87aa2..2f2ff69 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -111,3 +111,55 @@ jobs: with: run: bazel run //examples/google_benchmark_bazel:my_benchmark --//core:codspeed_mode=${{ matrix.codspeed-mode }} token: ${{ secrets.CODSPEED_TOKEN }} + + windows-cmake-build: + strategy: + matrix: + codspeed-mode: ["off", "walltime"] + runs-on: windows-latest + steps: + - name: Checkout code + uses: actions/checkout@v3 + + - name: Cache build + uses: actions/cache@v3 + with: + path: examples/google_benchmark_cmake/build + key: ${{ runner.os }}-build-${{ matrix.codspeed-mode }}-${{ hashFiles('**/CMakeLists.txt') }} + + - name: Create build directory + run: | + if (-not (Test-Path examples\google_benchmark_cmake\build)) { + mkdir examples\google_benchmark_cmake\build + } + shell: pwsh + + - name: Build benchmark example + run: | + cd examples\google_benchmark_cmake\build + cmake -DCODSPEED_MODE=${{ matrix.codspeed-mode }} .. + cmake --build . --config Release + shell: pwsh + + windows-bazel-build: + strategy: + matrix: + codspeed-mode: ["off", "walltime"] + runs-on: windows-latest + steps: + - uses: actions/checkout@v4 + + - name: Set up Bazel + uses: bazel-contrib/setup-bazel@0.14.0 + with: + # Avoid downloading Bazel every time. + bazelisk-cache: true + # Store build cache per workflow. + disk-cache: ${{ github.workflow }} + # Share repository cache between workflows. + repository-cache: true + + - name: Build benchmark example + run: | + bazel build //examples/google_benchmark_bazel:my_benchmark --//core:codspeed_mode=${{ matrix.codspeed-mode }} + shell: pwsh