Skip to content

Allow benchmark allocator features together#21905

Merged
xudong963 merged 1 commit intoapache:mainfrom
xudong963:fix-benchmark-allocator-features
May 7, 2026
Merged

Allow benchmark allocator features together#21905
xudong963 merged 1 commit intoapache:mainfrom
xudong963:fix-benchmark-allocator-features

Conversation

@xudong963
Copy link
Copy Markdown
Member

@xudong963 xudong963 commented Apr 29, 2026

Which issue does this PR close?

Split from #21637.

Rationale for this change

The benchmark binaries currently reject enabling both snmalloc and mimalloc. However, workspace-wide checks such as cargo clippy --all-targets --all-features enable both feature flags, which makes these binaries fail before clippy can run.

What changes are included in this PR?

This PR removes the explicit compile error from the benchmark binaries and makes allocator selection deterministic: snmalloc is used when enabled, otherwise mimalloc is used when enabled.

Are these changes tested?

Are there any user-facing changes?

No. This only affects benchmark binary feature combinations used by development checks.

Copy link
Copy Markdown
Contributor

@kosiew kosiew left a comment

Choose a reason for hiding this comment

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

@xudong963
Thanks for working on this.

I think you need to amend
benchmarks/benches/sql.rs
too.

cargo check -p datafusion-benchmarks --all-targets --all-features

fails with

error: feature "snmalloc" and feature "mimalloc" cannot be enabled at the same time
  --> benchmarks/benches/sql.rs:47:1
   |
47 | / compile_error!(
48 | |     "feature \"snmalloc\" and feature \"mimalloc\" cannot be enabled at the same time"
49 | | );
   | |_^

error[E0428]: the name `ALLOC` is defined multiple times
  --> benchmarks/benches/sql.rs:57:1
   |
53 | static ALLOC: snmalloc_rs::SnMalloc = snmalloc_rs::SnMalloc;
   | ------------------------------------------------------------ previous definition of the value `ALLOC` here
...
57 | static ALLOC: mimalloc::MiMalloc = mimalloc::MiMalloc;

@xudong963 xudong963 force-pushed the fix-benchmark-allocator-features branch from cbca5bf to 041b5f5 Compare May 7, 2026 02:26
@github-actions github-actions Bot added the physical-plan Changes to the physical-plan crate label May 7, 2026
@xudong963 xudong963 force-pushed the fix-benchmark-allocator-features branch from 041b5f5 to 00bcac1 Compare May 7, 2026 02:35
@github-actions github-actions Bot removed the physical-plan Changes to the physical-plan crate label May 7, 2026
@xudong963
Copy link
Copy Markdown
Member Author

@kosiew nice finding, fixed all

Copy link
Copy Markdown
Contributor

@kosiew kosiew left a comment

Choose a reason for hiding this comment

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

looks 👍 to me

@xudong963 xudong963 added this pull request to the merge queue May 7, 2026
Merged via the queue into apache:main with commit cae03c9 May 7, 2026
35 checks passed
@xudong963 xudong963 deleted the fix-benchmark-allocator-features branch May 7, 2026 03:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants