Skip to content

perf: copy bitmaps a word at a time in pre_selection_scatter - #25494

Open
abokhalill wants to merge 1 commit into
apache:mainfrom
abokhalill:perf-pre-selection-scatter-bulk-copy
Open

abokhalill wants to merge 1 commit into
apache:mainfrom
abokhalill:perf-pre-selection-scatter-bulk-copy

Conversation

@abokhalill

@abokhalill abokhalill commented Sep 19, 2026 •

Copy link
Copy Markdown

Which issue does this PR close?

  • N/A

Rationale for this change

pre_selection_scatter puts the short-circuited RHS result back at full batch length. For each run of selected rows it slices the RHS and appends one Option<bool> at a time, even though both sides are bit-packed, and every slice clones and drops an Arc.

Since #25771, AND chains of cheap comparisons no longer reach this function, so TPC-H is unchanged. It still runs for OR, and for AND with a conjunct that is not cheap.

TPC-H SF30 lineitem, 24 threads, l_quantity > K or l_discount > 0.05, by share of rows that reach the RHS:

  • 4%: 86.0ms -> 82.0ms
  • 8%: 96.5ms -> 89.5ms
  • 12%: 107.5ms -> 96.0ms
  • 16%: 115.0ms -> 102.0ms
  • 20%: 102.0ms -> 92.0ms

l_quantity < K and l_comment like '%special%' is 1-2% faster. The 22 TPC-H queries are within noise.

What changes are included in this PR?

Borrow the RHS buffers once, copy each run with append_packed_range, and only build a validity bitmap when the RHS has nulls.

Are these changes tested?

Yes, by the existing tests, including test_pre_selection_scatter.

Are there any user-facing changes?

No.

@github-actions github-actions Bot added the physical-expr Changes to the physical-expr crates label Sep 19, 2026
@github-actions

github-actions Bot commented Oct 7, 2026

Copy link
Copy Markdown

Thank you for opening this pull request!

Reviewer note: cargo-semver-checks reported the current version number is not SemVer-compatible with the changes in this pull request (compared against the base branch).

Details
     Cloning apache/main
    Building datafusion-physical-expr v55.1.0 (current)
       Built [  58.613s] (current)
     Parsing datafusion-physical-expr v55.1.0 (current)
      Parsed [   0.048s] (current)
    Building datafusion-physical-expr v55.1.0 (baseline)
       Built [  30.443s] (baseline)
     Parsing datafusion-physical-expr v55.1.0 (baseline)
      Parsed [   0.050s] (baseline)
    Checking datafusion-physical-expr v55.1.0 -> v55.1.0 (no change; assume patch)
     Checked [   0.363s] 223 checks: 221 pass, 2 fail, 0 warn, 31 skip

--- failure inherent_method_missing: pub method removed or renamed ---

Description:
A publicly-visible method or associated fn is no longer available under its prior name. It may have been renamed or removed entirely.
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#item-remove
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.50.0/src/lints/inherent_method_missing.ron

Failed in:
  RangePartitioning::try_new_with_samples, previously in file /home/runner/work/datafusion/datafusion/target/semver-checks/git-apache_main/ade6fce556fc3cf9ea18014d48fe532012268e7b/datafusion/physical-expr/src/partitioning.rs:277
  RangePartitioning::samples, previously in file /home/runner/work/datafusion/datafusion/target/semver-checks/git-apache_main/ade6fce556fc3cf9ea18014d48fe532012268e7b/datafusion/physical-expr/src/partitioning.rs:305
  RangePartitioning::max_partition_count, previously in file /home/runner/work/datafusion/datafusion/target/semver-checks/git-apache_main/ade6fce556fc3cf9ea18014d48fe532012268e7b/datafusion/physical-expr/src/partitioning.rs:320
  RangePartitioning::has_same_layout, previously in file /home/runner/work/datafusion/datafusion/target/semver-checks/git-apache_main/ade6fce556fc3cf9ea18014d48fe532012268e7b/datafusion/physical-expr/src/partitioning.rs:330
  RangePartitioning::scale, previously in file /home/runner/work/datafusion/datafusion/target/semver-checks/git-apache_main/ade6fce556fc3cf9ea18014d48fe532012268e7b/datafusion/physical-expr/src/partitioning.rs:343
  DynamicFilterPhysicalExpr::is_complete, previously in file /home/runner/work/datafusion/datafusion/target/semver-checks/git-apache_main/ade6fce556fc3cf9ea18014d48fe532012268e7b/datafusion/physical-expr/src/expressions/dynamic_filters/mod.rs:403
  ProjectionExprs::project_statistics_with_input_schema, previously in file /home/runner/work/datafusion/datafusion/target/semver-checks/git-apache_main/ade6fce556fc3cf9ea18014d48fe532012268e7b/datafusion/physical-expr/src/projection.rs:722

--- failure type_method_marked_deprecated: type method #[deprecated] added ---

Description:
A type method is now #[deprecated]. Downstream crates will get a compiler warning when using this method.
        ref: https://doc.rust-lang.org/reference/attributes/diagnostics.html#the-deprecated-attribute
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.50.0/src/lints/type_method_marked_deprecated.ron

Failed in:
  method datafusion_physical_expr::scalar_subquery::ScalarSubqueryExpr::index in /home/runner/work/datafusion/datafusion/datafusion/physical-expr/src/scalar_subquery.rs:103

     Summary semver requires new major version: 1 major and 1 minor checks failed
    Finished [  91.052s] datafusion-physical-expr

@github-actions github-actions Bot added the auto detected api change Auto detected API change label Oct 7, 2026
@kosiew

kosiew commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

@abokhalill
Thanks for your contribution.
Can you resolve the merge conflicts?

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.37%. Comparing base (a522cd5) to head (b1a7913).
⚠️ Report is 256 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #25494      +/-   ##
==========================================
- Coverage   82.37%   82.37%   -0.01%     
==========================================
  Files        1138     1138              
  Lines      433506   433529      +23     
  Branches   433506   433529      +23     
==========================================
+ Hits       357102   357107       +5     
- Misses      54850    54862      +12     
- Partials    21554    21560       +6     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Slicing the RHS array per selected run clones and drops an Arc on every
iteration, and iterating the slice sets one bit per row while both sides
are already bit-packed.

Borrow the RHS buffers once and copy each run with append_packed_range.
Only build a validity bitmap when the RHS has nulls, so a null-free
result does not pay a second pass per run.

TPC-H SF30 on 24 threads: q6 -8.3%, q12 -4.1%, q19 -2.0%, others within
noise.
@abokhalill
abokhalill force-pushed the perf-pre-selection-scatter-bulk-copy branch from b1a7913 to 7a60002 Compare October 7, 2026 17:57
@abokhalill

Copy link
Copy Markdown
Author

@kosiew done. the speedups are actually even more prominent now!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto detected api change Auto detected API change physical-expr Changes to the physical-expr crates

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants