Skip to content

Commit

Permalink
GH-43211: [C++] Fix decimal benchmarks to avoid out-of-bounds accesses (
Browse files Browse the repository at this point in the history
#43212)

### Rationale for this change

Some of the decimal benchmarks access their benchmark data in strides, without checking that the accesses fall within bounds.

A side effect is that this will break benchmark history because the iterations/s calculation was wrong, even though actual performance is unchanged.

### Are these changes tested?

By the continuous benchmarking jobs.

### Are there any user-facing changes?

No.
* GitHub Issue: #43211

Authored-by: Antoine Pitrou <[email protected]>
Signed-off-by: Antoine Pitrou <[email protected]>
  • Loading branch information
pitrou authored Jul 15, 2024
1 parent 57ac40c commit d21761e
Showing 1 changed file with 10 additions and 2 deletions.
12 changes: 10 additions & 2 deletions cpp/src/arrow/util/decimal_benchmark.cc
Original file line number Diff line number Diff line change
Expand Up @@ -77,14 +77,16 @@ static void ToString(benchmark::State& state) { // NOLINT non-const reference
state.SetItemsProcessed(state.iterations() * values.size());
}

constexpr int32_t kValueSize = 10;
constexpr int32_t kValueSize = 12;

static void BinaryCompareOp(benchmark::State& state) { // NOLINT non-const reference
std::vector<BasicDecimal128> v1, v2;
for (int x = 0; x < kValueSize; x++) {
v1.emplace_back(100 + x, 100 + x);
v2.emplace_back(200 + x, 200 + x);
}
static_assert(kValueSize % 4 == 0,
"kValueSize needs to be a multiple of 4 to avoid out-of-bounds accesses");
for (auto _ : state) {
for (int x = 0; x < kValueSize; x += 4) {
auto equal = v1[x] == v2[x];
Expand All @@ -93,7 +95,7 @@ static void BinaryCompareOp(benchmark::State& state) { // NOLINT non-const refe
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];
auto greater_than_or_equal2 = v1[x + 3] >= v2[x + 3];
benchmark::DoNotOptimize(greater_than_or_equal2);
}
}
Expand All @@ -106,6 +108,8 @@ static void BinaryCompareOpConstant(
for (int x = 0; x < kValueSize; x++) {
v1.emplace_back(100 + x, 100 + x);
}
static_assert(kValueSize % 4 == 0,
"kValueSize needs to be a multiple of 4 to avoid out-of-bounds accesses");
BasicDecimal128 constant(313, 212);
for (auto _ : state) {
for (int x = 0; x < kValueSize; x += 4) {
Expand Down Expand Up @@ -245,6 +249,8 @@ static void UnaryOp(benchmark::State& state) { // NOLINT non-const reference
v.emplace_back(100 + x, 100 + x);
}

static_assert(kValueSize % 2 == 0,
"kValueSize needs to be a multiple of 2 to avoid out-of-bounds accesses");
for (auto _ : state) {
for (int x = 0; x < kValueSize; x += 2) {
auto abs = v[x].Abs();
Expand Down Expand Up @@ -274,6 +280,8 @@ static void BinaryBitOp(benchmark::State& state) { // NOLINT non-const referenc
v2.emplace_back(200 + x, 200 + x);
}

static_assert(kValueSize % 2 == 0,
"kValueSize needs to be a multiple of 2 to avoid out-of-bounds accesses");
for (auto _ : state) {
for (int x = 0; x < kValueSize; x += 2) {
benchmark::DoNotOptimize(v1[x] |= v2[x]);
Expand Down

0 comments on commit d21761e

Please sign in to comment.