Skip to content

perf: gather Linear-mode window input more efficiently - #24034

Open
neilconway wants to merge 2 commits into
apache:mainfrom
neilconway:neilc/perf-window-ingest-gather
Open

perf: gather Linear-mode window input more efficiently#24034
neilconway wants to merge 2 commits into
apache:mainfrom
neilconway:neilc/perf-window-ingest-gather

Conversation

@neilconway

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Rationale for this change

LinearSearch::evaluate_partition_batches issued one take_record_batch call per partition present in the input batch. For batches with many partitions, this is inefficient.

Instead, we can build a Vec of the batch's row indices that groups the rows by partition, gather all rows with a single take_record_batch call, and hand each partition a slice of the result. Batches that contain a single partition skip the gather entirely.

Memory caveat: the emitted slices share the gathered batch's buffers. A partition that never receives rows again retains its slice and therefore pins the gathered batch's buffers (up to one input batch worth of memory per input batch in the worst case). This could be addressed, e.g., with a compaction pass to copy long-lived slices into owned buffers, but I have omitted that for now.

Benchmarks: (using #24032)

  • linear 100 partitions: 44.3 ms -> 44.1 ms (within noise)
  • linear 10000 partitions: 199.8 ms -> 170.1 ms (-14.9%)
  • linear sparse 32768 partitions: 224.6 ms -> 205.3 ms (-8.6%)
  • linear rows 10000 partitions: 169.0 ms -> 142.0 ms (-15.9%)
  • linear multi 10000 partitions: 295.9 ms -> 268.2 ms (-9.4%)
  • sorted 10000 partitions: 34.0 ms -> 34.5 ms (+1.3%; unchanged code path)

What changes are included in this PR?

  • Rewrite get_per_partition_indices and rename to compute_partition_permutation
  • Add focused unit test for computing partition permutations correctly

Are these changes tested?

Yes, new test added.

Are there any user-facing changes?

No.

@github-actions github-actions Bot added the physical-plan Changes to the physical-plan crate label Jul 31, 2026
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 89.52381% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.85%. Comparing base (dbcb5c0) to head (f12b81a).

Files with missing lines Patch % Lines
...ysical-plan/src/windows/bounded_window_agg_exec.rs 89.52% 4 Missing and 7 partials ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main   #24034   +/-   ##
=======================================
  Coverage   80.85%   80.85%           
=======================================
  Files        1101     1101           
  Lines      374933   375018   +85     
  Branches   374933   375018   +85     
=======================================
+ Hits       303166   303237   +71     
- Misses      53671    53679    +8     
- Partials    18096    18102    +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.

@alamb

alamb commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

FYI @avantgardnerio / @2010YOUY01 who were perhaps interested in window function performance

Comment thread datafusion/physical-plan/src/windows/bounded_window_agg_exec.rs Outdated

@Dandandan Dandandan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice result for a (relatively) small change!

LinearSearch::evaluate_partition_batches issued one take_record_batch
call per partition present in the input batch. For batches with many
partitions, this is inefficient.

Instead, we can build a Vec of the batch's row indices that groups the
rows by partition, gather all rows with a single `take_record_batch`
call, and hand each partition a slice of the result. Batches that
contain a single partition skip the gather entirely.

Sliced partition batches already flow through this path: the Sorted
searcher returns record_batch.slice(..), and downstream code
(PartitionBatchState::extend, the new-partition store in
update_partition_batch, prune_partition_batches) handles non-zero
offsets.

Memory caveat: the emitted slices share the gathered batch's buffers.  A
partition that never receives rows again retains its slice and therefore
pins the gathered batch's buffers (up to one input batch worth of memory
per input batch in the worst case).  This could be addressed, e.g., with
a compaction pass to copy long-lived slices into owned buffers, but I
have omitted that for now.

Benchmarks:

- linear 100 partitions:           44.3 ms -> 44.1 ms (within noise)
- linear 10000 partitions:        199.8 ms -> 170.1 ms (-14.9%)
- linear sparse 32768 partitions: 224.6 ms -> 205.3 ms (-8.6%)
- linear rows 10000 partitions:   169.0 ms -> 142.0 ms (-15.9%)
- linear multi 10000 partitions:  295.9 ms -> 268.2 ms (-9.4%)
- sorted 10000 partitions:         34.0 ms -> 34.5 ms (+1.3%; the
  Sorted path is untouched -- binary layout / session noise)
@neilconway
neilconway force-pushed the neilc/perf-window-ingest-gather branch from f12b81a to 01de892 Compare August 4, 2026 14:43
@neilconway

Copy link
Copy Markdown
Contributor Author

@Dandandan Thanks for the reviews!

@neilconway
neilconway enabled auto-merge August 4, 2026 14:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

physical-plan Changes to the physical-plan crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants