bench: add window aggregate filter benchmarks by lyne7-sc · Pull Request #24589 · apache/datafusion

lyne7-sc · 2026-08-23T05:34:30Z

Which issue does this PR close?

Refs fix: evaluate window aggregate arguments after FILTER #24508.

Rationale for this change

Add benchmark coverage for evaluating the performance impact of the window aggregate filter argument evaluation changes in #24508.

What changes are included in this PR?

Adds a Criterion benchmark for window aggregates with FILTER over pre-ordered input.

It covers:

Cumulative and sliding frames through BoundedWindowAggExec.
Whole-partition frames through WindowAggExec.
Column, division, and power arguments.
Different representative FILTER selectivities.

This PR only adds benchmark coverage and does not change behavior.

Are these changes tested?

Yes.

Are there any user-facing changes?

No.

codecov-commenter · 2026-08-23T05:57:59Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.53%. Comparing base (dbdc627) to head (28606e8).
⚠️ Report is 120 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #24589      +/-   ##
==========================================
+ Coverage   81.31%   81.53%   +0.22%     
==========================================
  Files        1117     1123       +6     
  Lines      395911   406102   +10191     
  Branches   395911   406102   +10191     
==========================================
+ Hits       321918   331104    +9186     
- Misses      55177    55632     +455     
- Partials    18816    19366     +550

☔ 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.

kumarUjjawal

Thank you @lyne7-sc for working on this.

Overall looks good, I had two questions which I mentioned. Please take a look.

kumarUjjawal · 2026-08-24T07:37:27Z

+    argument_kinds: &[ArgumentKind],
+    filter_percents: &[usize],
+) {
+    let mut group = c.benchmark_group(format!("window_aggregate_filter/{name}"));


bounded_window sets sample_size(10) on its group, should we set it here too, per group so the microsecond-scale whole-partition cases can keep a higher count?

Thanks. I've reduced the sample size to 10.

What I had in mind was setting this per group: 10 for the millisecond-scale bounded frames, and leaving whole-partition at the default.

Updated. the bounded groups use a sample size of 10, while the whole-partition group uses default of 100.

kumarUjjawal · 2026-08-24T07:39:34Z

+        &[ArgumentKind::Column, ArgumentKind::Power],
+        &[30],
+    );
+    benchmark_window_case(


A whole-partition frame is constant in partition and does a single update_batch, while the cumulative and sliding frames walk row by row and spend their time in range calculation rather than argument evaluation. Would it be worth giving this group all three argument kinds and all three selectivities and trimming the cumulative group back?

Thanks for the suggestion. I think this is a reasonable balance. I've updated the benchmark coverage.

kumarUjjawal · 2026-08-25T08:22:06Z

+    argument_kinds: &[ArgumentKind],
+    filter_percents: &[usize],
+) {
+    let mut group = c.benchmark_group(format!("window_aggregate_filter/{name}"));


What I had in mind was setting this per group: 10 for the millisecond-scale bounded frames, and leaving whole-partition at the default.

kumarUjjawal · 2026-08-25T08:30:16Z

+        &[ArgumentKind::Column, ArgumentKind::Power],
+        &[30],


Could we restore divide here, and the 10 and 50 percent points with it?

kumarUjjawal · 2026-08-27T17:23:35Z

+        Arc::clone(&schema),
+        false,
+        false,
+        Some(col("include", &schema).unwrap()),


Every case here passes a filter, so the run has no baseline without FILTER at the same shape. That makes it hard to read the cost of the filter machinery apart from the base window cost. Would it work to add one case per group that passes None here?

Makes sense. added no-filter case per group as a baseline

kumarUjjawal · 2026-08-27T17:23:53Z

+            let value = Float64Array::from_iter_values((start..end).map(|i| i as f64));
+            let include = BooleanArray::from(
+                (start..end)
+                    .map(|i| (i * filter_percent) % 100 < filter_percent)


At 10, 30 and 50 percent this expression never selects two rows in a row, so every selected run is one row long. The whole-partition case filters the full batch, and that shape is the most
expensive one for arrow::compute::filter. i % 100 < filter_percent gives the same selectivity with clustered runs. Would it be worth a second pattern with clustered runs, so the numbers show
the range rather than one end of it?

Thanks again for the feedback. I took a closer look, and whether the selected rows are clustered or scattered does not seem to change the window-specific evaluation or accumulator work. The layout can still affect Arrow's filter kernel, but that feels like a separate dimension from what this benchmark is intended to isolate.

Should we keep this benchmark focused on the window FILTER path? I think we can adjust the existing include distribution so that it has a mix of isolated and consecutive selected rows. Would that address your concern?

yes, keep the benchmark on the window FILTER path, and the mixed distribution addresses my concern.

Updated. The filter mask now uses a fixed-seed shuffle to mix isolated and consecutive selected rows.

kumarUjjawal · 2026-08-27T17:24:44Z

+        c,
+        &runtime,
+        "bounded_sliding_10_rows",
+        &sliding_frame(),


The sliding frame filters twice per row, once for the update slice and once for the retract slice. That makes it the heaviest filter path of the three groups, but it skips divide. Would you add it here too?

Added Divide and aligned the sliding filter selectivities with the other groups.

kumarUjjawal · 2026-08-30T04:02:06Z

+    let mut group = c.benchmark_group(format!("window_aggregate_filter/{name}"));
+    group.sample_size(sample_size);
+
+    let plan = make_window_plan(None, ArgumentKind::Column, window_frame.clone());


The baseline is fixed to ArgumentKind::Column, so divide and power_udf have no unfiltered run at their own shape. The plan evaluates the argument expression once per batch before the filter runs, so power_udf carries a base cost that column/no_filter does not show. Would it work to loop the baseline over argument_kinds?

Updated. Each argument kind now has its own unfiltered baseline.

kumarUjjawal

Thank you @lyne7-sc for the iterations.

Looks good 👍

lyne7-sc · 2026-08-31T11:23:54Z

Thank you @kumarUjjawal

bench: add window aggregate filter benchmarks

9e87eaa

github-actions Bot added the physical-plan Changes to the physical-plan crate label Aug 23, 2026

lyne7-sc mentioned this pull request Aug 23, 2026

fix: evaluate window aggregate arguments after FILTER #24508

Draft

neilconway self-requested a review August 23, 2026 09:12

kumarUjjawal reviewed Aug 24, 2026

View reviewed changes

refine window aggregate filter coverage

8f67b26

kumarUjjawal reviewed Aug 25, 2026

View reviewed changes

refine window filter benchmark sampling and coverage

9425746

kumarUjjawal reviewed Aug 27, 2026

View reviewed changes

add unfiltered baselines and expand sliding window coverage

4b107d4

kumarUjjawal reviewed Aug 30, 2026

View reviewed changes

lyne7-sc added 2 commits August 30, 2026 21:48

refine window filter benchmark baselines and distribution

c9fe2e5

simplify benchmark setup

28606e8

kumarUjjawal approved these changes Aug 31, 2026

View reviewed changes

kumarUjjawal added this pull request to the merge queue Aug 31, 2026

Merged via the queue into apache:main with commit a274959 Aug 31, 2026
42 checks passed

nuno-faria added the v56.0.0 label Oct 3, 2026

Conversation

lyne7-sc commented Aug 23, 2026

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

Uh oh!

codecov-commenter commented Aug 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

kumarUjjawal left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kumarUjjawal left a comment

Choose a reason for hiding this comment

Uh oh!

lyne7-sc commented Aug 31, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov-commenter commented Aug 23, 2026 •

edited

Loading