Skip to content

bench(h2o): window Top-N sweep over a declared-sorted input - #24732

Merged
kosiew merged 3 commits into
apache:mainfrom
SubhamSinghal:bench-window-topn-sorted
Sep 3, 2026
Merged

kosiew merged 3 commits into
apache:mainfrom
SubhamSinghal:bench-window-topn-sorted

Conversation

@SubhamSinghal

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Related to #6899 — adds benchmark coverage for the WindowTopN operators.

Rationale for this change

The existing h2o --subgroup window Top-N sweep (q13–q29) registers x with no declared ordering, so output_ordering() is None. Any plan that depends on the input being sorted is unreachable from those queries, however the data happens to be laid out on disk. This adds a window_sorted subgroup that publishes the same sweep over a WITH ORDER table, so ordering-dependent plans can be measured.

What changes are included in this PR?

A window_sorted h2o subgroup: 16 queries covering ROW_NUMBER / RANK / DENSE_RANK × 100 / 1K / 10K / 100K partitions, plus heavy-ties variants.

  • Two load directives: the existing load_window_${SIZE}_${FORMAT}.sql creates x, then a new script writes a sorted copy via COPY (... ORDER BY pk, ob DESC) and registers it WITH ORDER (pk ASC, ob DESC). Reusing the existing loader keeps both the --size and --format axes working with no duplication. load is untimed, so the sort stays out of the measurement.

  • WITH ORDER can only name columns, so the partition key (id3 % N) and the tie expression are materialized as pk and ob.

  • Asserts guard the three ways this could silently measure nothing: both config flags took effect, the sorted copy holds every source row, and ob has the cardinality the query name claims.

  • expect_plan PartitionedTopKExec is deliberately the shared substring, so the same file validates the heap operator and any streaming variant that replaces it.

    No Rust changes; benchmark files only.

    Are these changes tested?

    benchmark_runner h2o --subgroup window_sorted runs all 16 queries green. Row counts are exact: 2 per partition for the distinct-ORDER-BY shapes, and 1,010,812 (RANK) / 2,020,722 (DENSE_RANK) for the tie shapes — the top-1 and top-2 of 10 distinct values over 10M rows.

    Are there any user-facing changes?

    No.

@codecov-commenter

codecov-commenter commented Aug 27, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.60%. Comparing base (da89c7c) to head (80e2265).

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #24732      +/-   ##
==========================================
- Coverage   81.60%   81.60%   -0.01%     
==========================================
  Files        1123     1123              
  Lines      408898   408898              
  Branches   408898   408898              
==========================================
- Hits       333670   333664       -6     
- Misses      55625    55626       +1     
- Partials    19603    19608       +5     

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

@kosiew kosiew 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.

@SubhamSinghal, thanks for adding the sorted-input H2O Window Top-N benchmarks. The overall setup looks good to me. I have one non-blocking suggestion to make sure the benchmark continues to exercise the intended declared-ordering setup as it evolves.

# operator replaces the heap one for declared-sorted input. That is the point of
# the subgroup — the comparison is this file's numbers across two branches, so
# the assertion must not be the thing that differs between them.
expect_plan PartitionedTopKExec

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.

Could we also add a plan-level check that the WITH ORDER declaration actually reaches DataSourceExec, for example by checking the source output_ordering? The current PartitionedTopKExec assertion does not verify this because that operator only requires key distribution and can still appear with an unordered source. Having a separate ordering check would help catch accidental loss of the declared-order setup, especially if an ordering-dependent Top-N implementation uses this benchmark in the future.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Addressed in 550dd1b

@kosiew
kosiew added this pull request to the merge queue Sep 3, 2026
@kosiew

kosiew commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

🚀
@SubhamSinghal
Thank you for your contribution.

Merged via the queue into apache:main with commit 17115f3 Sep 3, 2026
38 checks passed
@SubhamSinghal
SubhamSinghal deleted the bench-window-topn-sorted branch September 18, 2026 07:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants