Repository navigation
chore: add benches for remaining comet native kernels - #6648
Merged
coderfender merged 3 commits intoOct 6, 2026
Merged
Conversation
sunchao
approved these changes
Oct 5, 2026
sunchao
left a comment
sunchao
left a comment
Member
There was a problem hiding this comment.
Summary
- Prior state and problem: Eight native expression benchmark targets were absent at the branch point.
- Design approach: Add standalone Criterion benchmarks using existing public kernel APIs and aggregate harness patterns.
- Correctness / compatibility analysis: Checked input construction, API contracts, surrounding implementations, and relevant Spark sources across supported versions. No introduced P1/P2 issues found within this review.
- Key design decisions: Deterministic inputs, setup outside timed loops, real serialized sketches, and scalar/null cases where included.
- Implementation sketch: Eight benchmark files plus Cargo registrations cover Bloom probes, nested casts, concatenation, HLL aggregates/scalars, list positions, map construction, and square roots.
- Behavioral changes worth calling out: Production execution and Spark routing are unchanged. The additions introduce no query runtime overhead or production abstractions.
- Suggested improvements: None meeting the P1/P2 reporting threshold.
Reviewed SHA: adcd3a97bd71b5d59d1240c919986406f9d91741. Reviewed the entire nine-file PR diff against base 33f21da181c14df7fa3e6a3104c3d7215810147e, using merge base 02df8133aa82823065d8e2959cf979f17d97f4ea. The PR remains non-draft. No existing review concerns were present.
Routed skills: review-comet-pr and review-comet-expression-pr.
Exact-head CI: Only label reported success. No build/test CI verdict was available.
Validation: All eight targets passed locked, offline cargo check. All 19 Criterion cases passed using cargo bench --profile dev --config profile.dev.debug=0 -- --test with those targets selected. No release performance measurements or Spark JVM suites were run. GitHub reports merge conflicts with the current base, so conflict-resolution changes will require fresh validation.
coderfender
force-pushed
the
bench_remaining_comet_native_kernels
branch
from
October 5, 2026 02:27
adcd3a9 to
84420d8
Compare
github-merge-queue
Bot
removed this pull request from the merge queue due to a conflict with the base branch
Oct 5, 2026
…et_native_kernels
coderfender
force-pushed
the
bench_remaining_comet_native_kernels
branch
from
October 6, 2026 06:57
e229f8f to
edceefe
Compare
andygrove
added a commit
to sam-1112/datafusion-comet
that referenced
this pull request
Oct 10, 2026
apache#6648 added this bench after the last merge of main into this branch, and it still passed the third argument that this PR removes from SparkCastOptions::new.
viirya
pushed a commit
to viirya/arrow-datafusion-comet
that referenced
this pull request
Oct 10, 2026
* refactor: remove unused cast compatibility flag * refactor: drop unused parquet allow_incompat option The flag has no remaining reader after the cast-option cleanup, and the unused CometConf import fails scalafix. * fix: preserve DPP filters when reverting native scans * Fix SparkCastOptions constructor calls * refactor: drop the DPP scan reversion from the cast cleanup * fix: drop the removed cast-option argument from case-when calls * fix: drop the removed parquet-option argument from the page-index test * fix: drop the removed cast-option argument from the cast_nested bench apache#6648 added this bench after the last merge of main into this branch, and it still passed the third argument that this PR removes from SparkCastOptions::new. --------- Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: Andy Grove <agrove@apache.org>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Which issue does this PR close?
Closes #5396
Closes #.
Rationale for this change
What changes are included in this PR?
How are these changes tested?