GH-43211: [C++] Fix decimal benchmarks to avoid out-of-bounds accesses #43212
Conversation
…ccesses A side effect is that this will break benchmark history because the previous iterations/s calculation was wrong, even though actual performance is unchanged.
|
@raulcd @austin3dickey This should fix the recent crashes on the macOS ARM machine. |
|
@ursabot please benchmark lang=C++ |
|
Benchmark runs are scheduled for commit a18bd6d. Watch https://buildkite.com/apache-arrow and https://conbench.ursa.dev for updates. A comment will be posted here when the runs are complete. |
|
Thanks for your patience. Conbench analyzed the 4 benchmarking runs that have been run so far on PR commit a18bd6d. There were 14 benchmark results indicating a performance regression:
The full Conbench report has more details. |
|
This did fix the macOS benchmark run, see https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-test-mac-arm/builds/5582#01909d50-02bc-4b6e-ac5e-885a17eff8ca |
|
@felipecrv This is a small one, would you like to take a quick look? |
| auto greater_than_or_equal2 = v1[x + 3] >= v1[x + 3]; | ||
| auto greater_than_or_equal2 = v1[x + 3] >= v2[x + 3]; |
github-actions
bot
added
awaiting merge
|
After merging your PR, Conbench analyzed the 4 benchmarking runs that have been run so far on merge-commit d21761e. There were no benchmark performance regressions. 🎉 The full Conbench report has more details. It also includes information about 5 possible false positives for unstable benchmarks that are known to sometimes produce them. |
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.