-
Notifications
You must be signed in to change notification settings - Fork 4k
GH-45939: [C++][Benchmarking] Fix compilation failures #45942
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
@ursabot please benchmark lang=C++ |
|
Benchmark runs are scheduled for commit 86bafb46438023fc2958e058fc78ae2b5b6df8aa. Watch https://buildkite.com/apache-arrow and https://conbench.ursa.dev for updates. A comment will be posted here when the runs are complete. |
|
|
|
|
86bafb4 to
6ef504f
Compare
|
@ursabot please benchmark lang=C++ |
|
Benchmark runs are scheduled for commit 6ef504fbb8b2874a86150954a165d74a7ed64b33. Watch https://buildkite.com/apache-arrow and https://conbench.ursa.dev for updates. A comment will be posted here when the runs are complete. |
6ef504f to
b8d6cd5
Compare
|
@ursabot please benchmark lang=C++ |
|
Benchmark runs are scheduled for commit b8d6cd572e0106e0d47eb4ff6f6809e8b3d8e431. 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 0 benchmarking runs that have been run so far on PR commit 86bafb46438023fc2958e058fc78ae2b5b6df8aa. None of the specified runs were found on the Conbench server. The full Conbench report has more details. |
|
Thanks for your patience. Conbench analyzed the 0 benchmarking runs that have been run so far on PR commit 6ef504fbb8b2874a86150954a165d74a7ed64b33. None of the specified runs were found on the Conbench server. The full Conbench report has more details. |
|
Thanks for your patience. Conbench analyzed the 0 benchmarking runs that have been run so far on PR commit b8d6cd572e0106e0d47eb4ff6f6809e8b3d8e431. None of the specified runs were found on the Conbench server. The full Conbench report has more details. |
b8d6cd5 to
048aed0
Compare
|
@ursabot please benchmark lang=C++ |
|
Benchmark runs are scheduled for commit 048aed0c2d85cba8b4f17a695a1874b4fdf06c19. Watch https://buildkite.com/apache-arrow and https://conbench.ursa.dev for updates. A comment will be posted here when the runs are complete. |
Benchmarks fail compiling with Google Benchmark 1.9.2. This is because the benchmark function names conflict with Arrow API function names. We don't want to break continuous benchmarking history by changing the benchmark names, so instead move the benchmarks to a dedicated namespace.
048aed0 to
d6fa8e4
Compare
|
@ursabot please benchmark lang=C++ |
|
Benchmark runs are scheduled for commit d6fa8e4. 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 3 benchmarking runs that have been run so far on PR commit 048aed0c2d85cba8b4f17a695a1874b4fdf06c19. There were 60 benchmark results indicating a performance regression:
The full Conbench report has more details. |
|
Thanks for your patience. Conbench analyzed the 4 benchmarking runs that have been run so far on PR commit d6fa8e4. There were 62 benchmark results indicating a performance regression:
The full Conbench report has more details. |
|
The benchmark builds have all passed now. The perf regressions (or pseudo-regressions) are unrelated. |
zanmato1984
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thank you for fixing the failure so quickly!
|
After merging your PR, Conbench analyzed the 4 benchmarking runs that have been run so far on merge-commit 9b7875c. There were no benchmark performance regressions. 🎉 The full Conbench report has more details. It also includes information about 27 possible false positives for unstable benchmarks that are known to sometimes produce them. |
…45942) ### Rationale for this change Benchmarks fail compiling with Google Benchmark 1.9.2. The errors seem triggered by this change upstream: google/benchmark#1948 ### What changes are included in this PR? * Move some benchmark functions to a dedicated namespace so that there is no ambiguity with other functions * Reduce reliance on templating in benchmark functions to avoid resolution ambiguities * Other required fixes Note that we don't rename the benchmark functions, to avoid breaking continuous benchmarking history. ### Are these changes tested? By continuous benchmarking. ### Are there any user-facing changes? No. * GitHub Issue: apache#45939 Authored-by: Antoine Pitrou <[email protected]> Signed-off-by: Antoine Pitrou <[email protected]>
Rationale for this change
Benchmarks fail compiling with Google Benchmark 1.9.2. The errors seem triggered by this change upstream: google/benchmark#1948
What changes are included in this PR?
Note that we don't rename the benchmark functions, to avoid breaking continuous benchmarking history.
Are these changes tested?
By continuous benchmarking.
Are there any user-facing changes?
No.