Skip to content

GH-49835: [C++] A constexpr dynamic dispatch with static dispatch when possible - #49840

Merged
pitrou merged 5 commits into
apache:mainfrom
AntoinePrv:dyn-dispatch-constexpr
Apr 28, 2026
Merged

GH-49835: [C++] A constexpr dynamic dispatch with static dispatch when possible#49840
pitrou merged 5 commits into
apache:mainfrom
AntoinePrv:dyn-dispatch-constexpr

Conversation

@AntoinePrv

@AntoinePrvAntoinePrv commented Apr 22, 2026

Copy link
Copy Markdown
Collaborator

Rationale for this change

  • Reduce verbosity when using DynamicDispatch
  • Optimizations: when all targets can be statically dispatched, simply pick the best one at compile time, avoiding jump and CpuInfo creation.

What changes are included in this PR?

  • Large refactor of dispatch_internal.h
  • Cleanup of call sites

Are these changes tested?

Via existing tests

Are there any user-facing changes?

No

@AntoinePrv
AntoinePrv requested a review from wgtmac as a code ownerApril 22, 2026 16:11
@github-actionsgithub-actionsBot added the awaiting review Awaiting review label Apr 22, 2026
@github-actions

Copy link
Copy Markdown

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

@pitrou
pitrou self-requested a review April 22, 2026 16:15
@AntoinePrv
AntoinePrvforce-pushed the dyn-dispatch-constexpr branch 2 times, most recently from 1b04a87 to d1dd94dCompareApril 23, 2026 08:14
@AntoinePrv
AntoinePrvforce-pushed the dyn-dispatch-constexpr branch from d1dd94d to 07b0f12CompareApril 28, 2026 09:40
@AntoinePrv
AntoinePrvforce-pushed the dyn-dispatch-constexpr branch from 1b6eb4c to 8398f97CompareApril 28, 2026 11:36
@pitroupitrou added CI: Extra: C++ Run extra C++ CI CI: Extra: R Run extra R CI labels Apr 28, 2026
@pitrou

Copy link
Copy Markdown
Member

@github-actions crossbow submit -g cpp

@pitroupitrou left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1, quite a nice improvement. I'll wait for CI and benchmark results though.

@pitrou

Copy link
Copy Markdown
Member

@ursabot please benchmark lang=C++

@github-actionsgithub-actionsBot added awaiting committer review Awaiting committer review and removed awaiting review Awaiting review labels Apr 28, 2026
@github-actions

Copy link
Copy Markdown

Revision: 8398f97

Submitted crossbow builds: ursacomputing/crossbow @ actions-784cd4e111

TaskStatus
example-cpp-minimal-build-staticGitHub Actions
example-cpp-minimal-build-static-system-dependencyGitHub Actions
example-cpp-tutorialGitHub Actions
test-build-cpp-fuzzGitHub Actions
test-conda-cppGitHub Actions
test-conda-cpp-valgrindGitHub Actions
test-debian-13-cpp-amd64GitHub Actions
test-debian-13-cpp-i386GitHub Actions
test-debian-experimental-cpp-gcc-15GitHub Actions
test-fedora-42-cppGitHub Actions
test-ubuntu-22.04-cppGitHub Actions
test-ubuntu-22.04-cpp-bundledGitHub Actions
test-ubuntu-22.04-cpp-emscriptenGitHub Actions
test-ubuntu-22.04-cpp-no-threadingGitHub Actions
test-ubuntu-24.04-cppGitHub Actions
test-ubuntu-24.04-cpp-bundled-offlineGitHub Actions
test-ubuntu-24.04-cpp-gcc-13-bundledGitHub Actions
test-ubuntu-24.04-cpp-gcc-14GitHub Actions
test-ubuntu-24.04-cpp-minimal-with-formatsGitHub Actions
test-ubuntu-24.04-cpp-thread-sanitizerGitHub Actions

@rok

rok commented Apr 28, 2026

Copy link
Copy Markdown
Member

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

@pitrou

pitrou commented Apr 28, 2026

Copy link
Copy Markdown
Member

This PR gives surprisingly massive improvements on our ARM64 benchmarking machine (with Neon):
https://conbench.arrow-dev.org/compare/runs/596185620b9943f48060d1da301bc537...789520580536479fa686c5ad63873abb/

Up to 500% faster in some Parquet benchmarks. 😲

@cyb70289

@pitrou

Copy link
Copy Markdown
Member

I'll merge now, thanks a lot for this @AntoinePrv !

@pitrou
pitrou merged commit 656e8e6 into apache:mainApr 28, 2026
93 of 109 checks passed
@pitroupitrou removed the awaiting committer review Awaiting committer review label Apr 28, 2026
@AntoinePrv

Copy link
Copy Markdown
CollaboratorAuthor

Perhaps from skipping the dynamic dispatch in level_comparison.cc? I think the other two already skipped it.

@conbench-apache-arrow

Copy link
Copy Markdown

Thanks for your patience. Conbench analyzed the 3 benchmarking runs that have been run so far on PR commit 8398f97.

There were 6 benchmark results indicating a performance regression:

The full Conbench report has more details.

@conbench-apache-arrow

Copy link
Copy Markdown

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

There were no benchmark performance regressions. 🎉

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

@AntoinePrv
AntoinePrv deleted the dyn-dispatch-constexpr branch April 29, 2026 08:31
Mottl pushed a commit to Mottl/arrow that referenced this pull request May 26, 2026
…ch when possible (apache#49840)
### Rationale for this change
- Reduce verbosity when using `DynamicDispatch`
- Optimizations: when all targets can be statically dispatched, simply pick the best one at compile time, avoiding jump and `CpuInfo` creation.
### What changes are included in this PR?
- Large refactor of `dispatch_internal.h`
- Cleanup of call sites
### Are these changes tested?
Via existing tests
### Are there any user-facing changes?
No
* GitHub Issue: apache#49835
Authored-by: AntoinePrv <AntoinePrv@users.noreply.github.com>
Signed-off-by: Antoine Pitrou <antoine@python.org>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@AntoinePrv@pitrou@rok