Skip to content
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

GH-43693: [C++][Acero] Support AVX2 swiss join decoding #43832

Open
wants to merge 29 commits into
base: main
Choose a base branch
from

Conversation

zanmato1984
Copy link
Collaborator

@zanmato1984 zanmato1984 commented Aug 26, 2024

Rationale for this change

You can find the background in #43693.

By looking at how Visit_avx2/VisitNulls_avx2's non-simd counterparts (Visit/VisitNulls) are used, I found they are solely for decoding rows from the build side of the join. So I added AVX2 versions for those decoding methods and wired Visit_avx2/VisitNulls_avx2.

What changes are included in this PR?

  1. Split the decoding methods into smaller pieces to make each of them able to cooperate with the AVX2 version.
  2. Concrete AVX2 specialized functions utilizing the Visit*_avx2 functions to decode fixed-length/offsets/var-length/nulls of the row table.
  3. Fix some bugs in the original Visit*_avx2 functions.
  4. Related benchmarks.

Are these changes tested?

No new tests needed.

The benchmarking result is a bit complicated, I put them in comment #43832 (comment).

Are there any user-facing changes?

No changes other than positive performance improvement. Users can expect such improvement for hash joins related workload. Nevertheless the improvement degree highly depends on not only the workload, but also the CPU models. For Intel CPUs from Skylake to Icelake/Tigerlake, which suffer the performance degradation of AVX2 gather because of an vulnerability mitigation of Intel's (detailed in #43832 (comment)), the improvement is less significant - single digit percent. Other models, e.g. AMD, and the most recent Intel, can achieve better improvement up to 30%.

@zanmato1984
Copy link
Collaborator Author

@github-actions crossbow submit -g cpp

@zanmato1984
Copy link
Collaborator Author

@ursabot please benchmark

@ursabot
Copy link

ursabot commented Aug 26, 2024

Benchmark runs are scheduled for commit e2af277. Watch https://buildkite.com/apache-arrow and https://conbench.ursa.dev for updates. A comment will be posted here when the runs are complete.

Copy link

Revision: e2af277

Submitted crossbow builds: ursacomputing/crossbow @ actions-1db8e52faf

Task Status
test-alpine-linux-cpp GitHub Actions
test-build-cpp-fuzz GitHub Actions
test-conda-cpp GitHub Actions
test-conda-cpp-valgrind GitHub Actions
test-cuda-cpp GitHub Actions
test-debian-12-cpp-amd64 GitHub Actions
test-debian-12-cpp-i386 GitHub Actions
test-fedora-39-cpp GitHub Actions
test-ubuntu-20.04-cpp GitHub Actions
test-ubuntu-20.04-cpp-bundled GitHub Actions
test-ubuntu-20.04-cpp-minimal-with-formats GitHub Actions
test-ubuntu-20.04-cpp-thread-sanitizer GitHub Actions
test-ubuntu-22.04-cpp GitHub Actions
test-ubuntu-22.04-cpp-20 GitHub Actions
test-ubuntu-22.04-cpp-emscripten GitHub Actions
test-ubuntu-22.04-cpp-no-threading GitHub Actions
test-ubuntu-24.04-cpp GitHub Actions
test-ubuntu-24.04-cpp-gcc-13-bundled GitHub Actions
test-ubuntu-24.04-cpp-gcc-14 GitHub Actions

Copy link

Thanks for your patience. Conbench analyzed the 2 benchmarking runs that have been run so far on PR commit e2af277.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details.

@zanmato1984 zanmato1984 changed the title GH-43693: [C++][Acero] Support AVX swiss join decoding GH-43693: [C++][Acero] Support AVX2 swiss join decoding Aug 29, 2024
@lidavidm
Copy link
Member

lidavidm commented Sep 6, 2024

@zanmato1984 might wanna take a second look at that merge/rebase

@lidavidm lidavidm removed their request for review September 6, 2024 06:19
@zanmato1984
Copy link
Collaborator Author

Apologies to whom are mis-involved by my careless merge/rebase. I'm cleaning my branch now. Sorry :(

@zanmato1984
Copy link
Collaborator Author

The merge/rebase has now been fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants