Skip to content

Commit

Permalink
apacheGH-35458: [C++][Benchmarking] Require Google Benchmark 1.6.1 or…
Browse files Browse the repository at this point in the history 
… later (apache#35459)

### Rationale for this change

Google Benchmark 1.6.1 added `benchmark::MemoryManager::Stop(Result&)` and deprecated `benchmark::MemoryManager::Stop(Result*)`.

Google Benchmark 1.8.0 dropped deprecated
`benchmark::MemoryManager::Stop(Result*)`.

Google Benchmark deprecated `DoNotOptimize(const &)`:
google/benchmark#1493

### What changes are included in this PR?

We can always use `benchmark::MemoryManager::Stop(Result&)` by requiring Google Benchmark 1.6.1 or later.

Don't use deprecated `DoNotOptimize(const &)`.

### Are these changes tested?

Yes.

### Are there any user-facing changes?

No.
* Closes: apache#35458

Authored-by: Sutou Kouhei <kou@clear-code.com>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
  • Loading branch information
@kou @ArgusLi
kou authored and ArgusLi committed May 15, 2023
1 parent 6f67cc8 commit 2f707e2
Show file tree
Hide file tree
Showing 6 changed files with 60 additions and 29 deletions.
2 changes: 1 addition & 1 deletion cpp/cmake_modules/ThirdpartyToolchain.cmake
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2325,7 +2325,7 @@ macro(build_benchmark)
endmacro()

if(ARROW_BUILD_BENCHMARKS)
set(BENCHMARK_REQUIRED_VERSION 1.6.0)
set(BENCHMARK_REQUIRED_VERSION 1.6.1)
resolve_dependency(benchmark
REQUIRED_VERSION
${BENCHMARK_REQUIRED_VERSION}
Expand Down
3 changes: 2 additions & 1 deletion cpp/src/arrow/csv/parser_benchmark.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,8 @@ static void BenchmarkCSVChunking(benchmark::State& state, // NOLINT non-const r
while (state.KeepRunning()) {
std::shared_ptr<Buffer> whole, partial;
ABORT_NOT_OK(chunker->Process(block, &whole, &partial));
benchmark::DoNotOptimize(whole->size());
auto size = whole->size();
benchmark::DoNotOptimize(size);
}
state.SetBytesProcessed(state.iterations() * csv.length());
Expand Down
6 changes: 4 additions & 2 deletions cpp/src/arrow/io/memory_benchmark.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,8 @@ static void Read(void* src, void* dst, size_t size) {
memset(&c, 0, sizeof(c));
memset(&d, 0, sizeof(d));

benchmark::DoNotOptimize(a + b + c + d);
auto result = a + b + c + d;
benchmark::DoNotOptimize(result);
}

// See http://codearcana.com/posts/2013/05/18/achieving-maximum-memory-bandwidth.html
Expand All @@ -124,7 +125,8 @@ static void StreamRead(void* src, void* dst, size_t size) {
VectorStreamLoadAsm(simd[i + 3], d);
}

benchmark::DoNotOptimize(a + b + c + d);
auto result = a + b + c + d;
benchmark::DoNotOptimize(result);
}

static void StreamWrite(void* src, void* dst, size_t size) {
Expand Down
15 changes: 11 additions & 4 deletions cpp/src/arrow/util/benchmark_util.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,14 @@ class MemoryPoolMemoryManager : public benchmark::MemoryManager {
global_allocations_start = default_pool->num_allocations();
}

void Stop(benchmark::MemoryManager::Result* result) override {
// BENCHMARK_DONT_OPTIMIZE is used here to detect Google Benchmark
// 1.8.0. We can remove this Stop(Result*) when we require Google
// Benchmark 1.8.0 or later.
#ifndef BENCHMARK_DONT_OPTIMIZE
void Stop(Result* result) override { Stop(*result); }
#endif

void Stop(benchmark::MemoryManager::Result& result) override {
// If num_allocations is still zero, we assume that the memory pool wasn't passed down
// so we should record them.
MemoryPool* default_pool = default_memory_pool();
Expand All @@ -166,9 +173,9 @@ class MemoryPoolMemoryManager : public benchmark::MemoryManager {
<< " allocations.\n";
}

result->max_bytes_used = memory_pool->max_memory();
result->total_allocated_bytes = memory_pool->total_bytes_allocated();
result->num_allocs = memory_pool->num_allocations();
result.max_bytes_used = memory_pool->max_memory();
result.total_allocated_bytes = memory_pool->total_bytes_allocated();
result.num_allocs = memory_pool->num_allocations();
}
}

Expand Down
60 changes: 40 additions & 20 deletions cpp/src/arrow/util/decimal_benchmark.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,8 @@ static void FromString(benchmark::State& state) { // NOLINT non-const reference
for (const auto& value : values) {
Decimal128 dec;
int32_t scale, precision;
benchmark::DoNotOptimize(Decimal128::FromString(value, &dec, &scale, &precision));
auto status = Decimal128::FromString(value, &dec, &scale, &precision);
benchmark::DoNotOptimize(status);
}
}
state.SetItemsProcessed(state.iterations() * values.size());
Expand All @@ -69,7 +70,8 @@ static void ToString(benchmark::State& state) { // NOLINT non-const reference
static const std::vector<DecimalValueAndScale> values = GetDecimalValuesAndScales();
for (auto _ : state) {
for (const DecimalValueAndScale& item : values) {
benchmark::DoNotOptimize(item.decimal.ToString(item.scale));
auto string = item.decimal.ToString(item.scale);
benchmark::DoNotOptimize(string);
}
}
state.SetItemsProcessed(state.iterations() * values.size());
Expand All @@ -85,10 +87,14 @@ static void BinaryCompareOp(benchmark::State& state) { // NOLINT non-const refe
}
for (auto _ : state) {
for (int x = 0; x < kValueSize; x += 4) {
benchmark::DoNotOptimize(v1[x] == v2[x]);
benchmark::DoNotOptimize(v1[x + 1] <= v2[x + 1]);
benchmark::DoNotOptimize(v1[x + 2] >= v2[x + 2]);
benchmark::DoNotOptimize(v1[x + 3] >= v1[x + 3]);
auto equal = v1[x] == v2[x];
benchmark::DoNotOptimize(equal);
auto less_than_or_equal = v1[x + 1] <= v2[x + 1];
benchmark::DoNotOptimize(less_than_or_equal);
auto greater_than_or_equal1 = v1[x + 2] >= v2[x + 2];
benchmark::DoNotOptimize(greater_than_or_equal1);
auto greater_than_or_equal2 = v1[x + 3] >= v1[x + 3];
benchmark::DoNotOptimize(greater_than_or_equal2);
}
}
state.SetItemsProcessed(state.iterations() * kValueSize);
Expand All @@ -103,10 +109,14 @@ static void BinaryCompareOpConstant(
BasicDecimal128 constant(313, 212);
for (auto _ : state) {
for (int x = 0; x < kValueSize; x += 4) {
benchmark::DoNotOptimize(v1[x] == constant);
benchmark::DoNotOptimize(v1[x + 1] <= constant);
benchmark::DoNotOptimize(v1[x + 2] >= constant);
benchmark::DoNotOptimize(v1[x + 3] != constant);
auto equal = v1[x] == constant;
benchmark::DoNotOptimize(equal);
auto less_than_or_equal = v1[x + 1] <= constant;
benchmark::DoNotOptimize(less_than_or_equal);
auto greater_than_or_equal = v1[x + 2] >= constant;
benchmark::DoNotOptimize(greater_than_or_equal);
auto not_equal = v1[x + 3] != constant;
benchmark::DoNotOptimize(not_equal);
}
}
state.SetItemsProcessed(state.iterations() * kValueSize);
Expand Down Expand Up @@ -138,7 +148,8 @@ static void BinaryMathOpAdd128(benchmark::State& state) { // NOLINT non-const r

for (auto _ : state) {
for (int x = 0; x < kValueSize; ++x) {
benchmark::DoNotOptimize(v1[x] + v2[x]);
auto add = v1[x] + v2[x];
benchmark::DoNotOptimize(add);
}
}
state.SetItemsProcessed(state.iterations() * kValueSize);
Expand All @@ -154,7 +165,8 @@ static void BinaryMathOpMultiply128(

for (auto _ : state) {
for (int x = 0; x < kValueSize; ++x) {
benchmark::DoNotOptimize(v1[x] * v2[x]);
auto multiply = v1[x] * v2[x];
benchmark::DoNotOptimize(multiply);
}
}
state.SetItemsProcessed(state.iterations() * kValueSize);
Expand All @@ -170,7 +182,8 @@ static void BinaryMathOpDivide128(

for (auto _ : state) {
for (int x = 0; x < kValueSize; ++x) {
benchmark::DoNotOptimize(v1[x] / v2[x]);
auto divide = v1[x] / v2[x];
benchmark::DoNotOptimize(divide);
}
}
state.SetItemsProcessed(state.iterations() * kValueSize);
Expand All @@ -185,7 +198,8 @@ static void BinaryMathOpAdd256(benchmark::State& state) { // NOLINT non-const r

for (auto _ : state) {
for (int x = 0; x < kValueSize; ++x) {
benchmark::DoNotOptimize(v1[x] + v2[x]);
auto add = v1[x] + v2[x];
benchmark::DoNotOptimize(add);
}
}
state.SetItemsProcessed(state.iterations() * kValueSize);
Expand All @@ -201,7 +215,8 @@ static void BinaryMathOpMultiply256(

for (auto _ : state) {
for (int x = 0; x < kValueSize; ++x) {
benchmark::DoNotOptimize(v1[x] * v2[x]);
auto multiply = v1[x] * v2[x];
benchmark::DoNotOptimize(multiply);
}
}
state.SetItemsProcessed(state.iterations() * kValueSize);
Expand All @@ -217,7 +232,8 @@ static void BinaryMathOpDivide256(

for (auto _ : state) {
for (int x = 0; x < kValueSize; ++x) {
benchmark::DoNotOptimize(v1[x] / v2[x]);
auto divide = v1[x] / v2[x];
benchmark::DoNotOptimize(divide);
}
}
state.SetItemsProcessed(state.iterations() * kValueSize);
Expand All @@ -231,8 +247,10 @@ static void UnaryOp(benchmark::State& state) { // NOLINT non-const reference

for (auto _ : state) {
for (int x = 0; x < kValueSize; x += 2) {
benchmark::DoNotOptimize(v[x].Abs());
benchmark::DoNotOptimize(v[x + 1].Negate());
auto abs = v[x].Abs();
benchmark::DoNotOptimize(abs);
auto negate = v[x + 1].Negate();
benchmark::DoNotOptimize(negate);
}
}
state.SetItemsProcessed(state.iterations() * kValueSize);
Expand All @@ -241,8 +259,10 @@ static void UnaryOp(benchmark::State& state) { // NOLINT non-const reference
static void Constants(benchmark::State& state) { // NOLINT non-const reference
BasicDecimal128 d1(-546, 123), d2(-123, 456);
for (auto _ : state) {
benchmark::DoNotOptimize(BasicDecimal128::GetMaxValue() - d1);
benchmark::DoNotOptimize(BasicDecimal128::GetScaleMultiplier(3) + d2);
auto sub = BasicDecimal128::GetMaxValue() - d1;
benchmark::DoNotOptimize(sub);
auto add = BasicDecimal128::GetScaleMultiplier(3) + d2;
benchmark::DoNotOptimize(add);
}
state.SetItemsProcessed(state.iterations() * 2);
}
Expand Down
3 changes: 2 additions & 1 deletion cpp/src/arrow/util/tdigest_benchmark.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ static void BenchmarkTDigest(benchmark::State& state) {
for (double value : values) {
td.Add(value);
}
benchmark::DoNotOptimize(td.Quantile(0));
auto quantile = td.Quantile(0);
benchmark::DoNotOptimize(quantile);
}
state.SetItemsProcessed(state.iterations() * items);
}
Expand Down

0 comments on commit 2f707e2

Please sign in to comment.