Skip to content

Conversation

@ldionne
Copy link
Member

@ldionne ldionne commented Dec 19, 2024

Since that benchmark is testing n*n inputs, the batch size reported to GoogleBenchmark should be that amount. Otherwise, GoogleBenchmark reports the timing for calling std::gcd on the whole sequence, which is misleading.

Since that benchmark is testing n*n inputs, the batch size reported
to GoogleBenchmark should be that amount. Otherwise, GoogleBenchmark
reports the timing for calling std::gcd on the whole sequence, which
is misleading.
@ldionne ldionne requested a review from a team as a code owner December 19, 2024 18:26
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Dec 19, 2024
@llvmbot
Copy link
Member

llvmbot commented Dec 19, 2024

@llvm/pr-subscribers-libcxx

Author: Louis Dionne (ldionne)

Changes

Since that benchmark is testing n*n inputs, the batch size reported to GoogleBenchmark should be that amount. Otherwise, GoogleBenchmark reports the timing for calling std::gcd on the whole sequence, which is misleading.


Full diff: https://github.com/llvm/llvm-project/pull/120618.diff

1 Files Affected:

  • (modified) libcxx/test/benchmarks/numeric/gcd.bench.cpp (+1-1)
diff --git a/libcxx/test/benchmarks/numeric/gcd.bench.cpp b/libcxx/test/benchmarks/numeric/gcd.bench.cpp
index abbc7e9dd04f96..ca5fed59463a21 100644
--- a/libcxx/test/benchmarks/numeric/gcd.bench.cpp
+++ b/libcxx/test/benchmarks/numeric/gcd.bench.cpp
@@ -25,7 +25,7 @@ static std::array<T, 1000> generate(std::uniform_int_distribution<T> distributio
 
 static void bm_gcd_random(benchmark::State& state) {
   std::array data = generate<int>();
-  while (state.KeepRunningBatch(data.size()))
+  while (state.KeepRunningBatch(data.size() * data.size()))
     for (auto v0 : data)
       for (auto v1 : data)
         benchmark::DoNotOptimize(std::gcd(v0, v1));

@ldionne ldionne changed the title [libc++] Fix the batch sized used in the std::gcd benchmark [libc++] Fix the batch size used in the std::gcd benchmark Dec 20, 2024
@serge-sans-paille
Copy link
Collaborator

lgtm, the error is unrelated.

@ldionne ldionne merged commit 15f30e7 into llvm:main Jan 6, 2025
18 of 19 checks passed
@ldionne ldionne deleted the review/fix-gcd-benchmark-size branch January 6, 2025 20:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants