GH-41720: [C++][Acero] Remove an useless parameter for QueryContext::Init called in hash_join_benchmark

[ZhangHuiGui]

Rationale for this change

My local compilation parameters will include the compilation of some basic benchmarks. I discovered this compilation problem today. It seems that #41334 of QueryContext::Init is not synchronized to hash_join_benchmark.cc, and CI has not found this problem. .

What changes are included in this PR?

Remove the first arg .

Are these changes tested?

Needn't

Are there any user-facing changes?

No

• GitHub Issue: [C++][Acero] An useless parameter for QueryContext::Init called in hash_join_benchmark #41720

[ZhangHuiGui]

@zanmato1984 PTAL this?
@kou Should our CI include full benchmark compilation? Seems current hash_join_benchmark compiled by ARROW_BUILD_OPENMP_BENCHMARKS not included in CI?

[zanmato1984]

+1. Thanks for catching this!

[zanmato1984]

Though the fix is small enough, I don't think it is a minor change as there is code modification. Could you open an issue and subject this PR to it? Thanks. @ZhangHuiGui

[kou]

Can we measure how much -DARROW_BUILD_OPENMP_BENCHMARKS=ON increases CI time?

The following change will enable the benchmark on "C++ / AMD64 Conda C++ AVX2":

diff --git a/docker-compose.yml b/docker-compose.yml
index a1d8f60a26..0c9784cc0d 100644
--- a/docker-compose.yml
+++ b/docker-compose.yml
@@ -285,6 +285,7 @@ services:
     environment:
       <<: [*common, *ccache, *sccache, *cpp]
       ARROW_BUILD_BENCHMARKS: "ON"
+      ARROW_BUILD_OPENMP_BENCHMARKS: "ON"
       ARROW_BUILD_EXAMPLES: "ON"
       ARROW_ENABLE_TIMING_TESTS:  # inherit
       ARROW_EXTRA_ERROR_CONTEXT: "ON"

[github-actions]

⚠️ GitHub issue #41720 has been automatically assigned in GitHub to PR creator.

[ZhangHuiGui]

Can we measure how much -DARROW_BUILD_OPENMP_BENCHMARKS=ON increases CI time?

The following change will enable the benchmark on "C++ / AMD64 Conda C++ AVX2":

diff --git a/docker-compose.yml b/docker-compose.yml
index a1d8f60a26..0c9784cc0d 100644
--- a/docker-compose.yml
+++ b/docker-compose.yml
@@ -285,6 +285,7 @@ services:
     environment:
       <<: [*common, *ccache, *sccache, *cpp]
       ARROW_BUILD_BENCHMARKS: "ON"
+      ARROW_BUILD_OPENMP_BENCHMARKS: "ON"
       ARROW_BUILD_EXAMPLES: "ON"
       ARROW_ENABLE_TIMING_TESTS:  # inherit
       ARROW_EXTRA_ERROR_CONTEXT: "ON"

Thanks, added this to current PR, but I’m not sure how to compare it with the previous CI compilation time.

[kou]

We can CI logs for it.

Anyway... The parameter wasn't used...: https://github.com/apache/arrow/actions/runs/9152098620/job/25159059866?pr=41716#step:6:864

Could you also add this?

diff --git a/ci/scripts/cpp_build.sh b/ci/scripts/cpp_build.sh
index a1f40fc360..6a3a53f253 100755
--- a/ci/scripts/cpp_build.sh
+++ b/ci/scripts/cpp_build.sh
@@ -120,6 +120,7 @@ else
     -DARROW_BUILD_BENCHMARKS=${ARROW_BUILD_BENCHMARKS:-OFF} \
     -DARROW_BUILD_EXAMPLES=${ARROW_BUILD_EXAMPLES:-OFF} \
     -DARROW_BUILD_INTEGRATION=${ARROW_BUILD_INTEGRATION:-OFF} \
+    -DARROW_BUILD_OPENMP_BENCHMARKS=${ARROW_BUILD_OPENMP_BENCHMARKS:-OFF} \
     -DARROW_BUILD_SHARED=${ARROW_BUILD_SHARED:-ON} \
     -DARROW_BUILD_STATIC=${ARROW_BUILD_STATIC:-ON} \
     -DARROW_BUILD_TESTS=${ARROW_BUILD_TESTS:-OFF} \

[ZhangHuiGui]

off:
AMD64 Conda C++ AVX2
succeeded 7 hours ago in 26m 43s

on:
AMD64 Conda C++ AVX2
succeeded 4 hours ago in 5m 42s

It seems to have decreased a lot...

[kou]

It's caused by ccache.

The "off" case wasn't cached: https://github.com/apache/arrow/actions/runs/9152098620/job/25159059866?pr=41716#step:6:2944

   Hits:               14 / 882 ( 1.59%)
  Misses:            868 / 882 (98.41%)

The "on" case cached: https://github.com/apache/arrow/actions/runs/9154084580/job/25164035145?pr=41716#step:6:2956

  Hits:              895 / 1765 (50.71%)
  Misses:            870 / 1765 (49.29%)

I think that about 6min with cache isn't a problem.

[conbench-apache-arrow]

After merging your PR, Conbench analyzed the 7 benchmarking runs that have been run so far on merge-commit c8f89d0.

There were 8 benchmark results indicating a performance regression:

• Commit Run on ec2-m5-4xlarge-us-east-2 at 2024-05-23 18:29:27Z
  • dataset-serialize (Python) with dataset=nyctaxi_multi_ipc_s3, format=arrow, selectivity=100pc
  • file-write (Python) with compression=lz4, dataset=fanniemae_2016Q4, file_type=feather, input_type=table
• and 6 more (see the report linked below)

The full Conbench report has more details. It also includes information about 47 possible false positives for unstable benchmarks that are known to sometimes produce them.