Add parquet benchmark for output_dict_columns option by y2kiran · Pull Request #23596 · NVIDIA/cudf

y2kiran · 2026-08-07T22:08:19Z

Description

This PR adds benchmarks to test the newly added output_dict_columns options for the Parquet reader, which was introduced in this PR

Checklist

I am familiar with the Contributing Guidelines.
New or existing tests cover these changes.
The documentation is up to date with these changes.

coderabbitai · 2026-08-07T22:10:57Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 1b43a691-c7c2-4853-bff4-8578303e0064

📥 Commits

Reviewing files that changed from the base of the PR and between 6edb425 and a40db45.

📒 Files selected for processing (1)

cpp/benchmarks/io/parquet/parquet_reader_options.cpp

📝 Walkthrough

Summary by CodeRabbit

New Features
- Added benchmarks for Parquet dictionary output during reads.
- Supports comparing dictionary output enabled and disabled across different row-group sizes.
- Covers configurable column selection, row selection, string conversion, metadata, and timestamp scenarios.
- Added validation to confirm that string reads produce the configured dictionary or standard string output.

Walkthrough

Changes

Parquet dictionary output benchmarks

Layer / File(s)	Summary
Dictionary output benchmark contract `cpp/benchmarks/io/nvbench_helpers.hpp`	Adds the public `output_dict` enum with `YES` and `NO` values. Registers string conversions for the enum.
Parquet benchmark integration and coverage `cpp/benchmarks/io/parquet/parquet_reader_options.cpp`	Passes `output_dict` to `output_dict_columns`, validates configured string output, exposes the axis in existing benchmarks, and adds a row-group benchmark. Updates copyright attribution.

Estimated code review effort: 2 (Simple) | ~10 minutes

Mergeability Score: ⚪ Minimal · up to a40db

This PR adds benchmark coverage for Parquet reader options without introducing a user-facing behavior change. No actionable merge-blocking risk remains after normal checks and review.

Suggested reviewers: mattgara, misiugodfrey, vuule

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies the addition of a Parquet benchmark for the output_dict_columns option.
Description check	✅ Passed	The description accurately explains that the pull request adds Parquet reader benchmarks for the output_dict_columns option.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

vuule

Could we add this to parquet_reader_options.cpp and follow the existing benchmark patterns there? Since this measures output_dict_columns, keeping it with the other reader-options benchmarks would make it easier to find and maintain.
Could we also make the PR title more specific, for example: “Add benchmark for Parquet output_dict_columns”?

copy-pr-bot · 2026-08-12T21:43:45Z

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

y2kiran · 2026-08-12T21:46:20Z

@vuule Apologies for the title. Apparently I missed it and it just used the name of the commit.

I've incorporated this into the existing parquet_reader_options benchmark. Thanks for the input!

coderabbitai

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@cpp/benchmarks/io/parquet/parquet_reader_options.cpp`:
- Around line 206-222: The BM_parquet_read_options benchmark currently validates
only column count, so add a non-timed preflight that inspects an eligible flat
STRING column and verifies its type is DICTIONARY32 when output_dict is YES and
STRING when output_dict is NO. Keep the timing path unchanged and use the
existing benchmark setup and output_dict axis symbols.
- Around line 221-222: Update the row_group_size_rows axis in the benchmark
options to use 100'000 instead of 1'000'000, preserving 0 while ensuring the
configured values represent distinct row-group limits.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 5e4f95eb-a035-46c1-a993-59aa00e24692

📥 Commits

Reviewing files that changed from the base of the PR and between 4ec7b3b and 6edb425.

📒 Files selected for processing (2)

cpp/benchmarks/io/nvbench_helpers.hpp
cpp/benchmarks/io/parquet/parquet_reader_options.cpp

vuule · 2026-08-14T02:54:05Z

+  // Non-timed preflight: confirm the reader honors `output_dict_columns` on a flat STRING column --
+  // YES transcodes it to DICTIONARY32, NO leaves it as STRING. Skipped when
+  // `convert_strings_to_categories` is set.
+  if constexpr (not str_to_categories) {
+    auto const preflight_tbl = cudf::io::read_parquet(read_options).tbl;
+    auto const preflight     = preflight_tbl->view();
+    auto const has_type      = [&](cudf::type_id id) {
+      return std::any_of(preflight.begin(), preflight.end(), [id](auto const& col) {
+        return col.type().id() == id;
+      });
+    };
+    if constexpr (output_dict_columns) {
+      CUDF_EXPECTS(has_type(cudf::type_id::DICTIONARY32),
+                   "output_dict_columns=YES must produce a DICTIONARY32 column");
+    } else {
+      CUDF_EXPECTS(has_type(cudf::type_id::STRING),
+                   "output_dict_columns=NO must produce a STRING column");
+    }
+  }
+


I don't think we need this check. Other options don't check if they are applied correctly.

y2kiran requested review from a team as code owners August 7, 2026 22:08

y2kiran requested review from mattgara and misiugodfrey August 7, 2026 22:08

github-actions Bot assigned y2kiran Aug 7, 2026

github-actions Bot added libcudf Affects libcudf (C++/CUDA) code. CMake CMake build issue labels Aug 7, 2026

vuule requested changes Aug 12, 2026

View reviewed changes

vuule added tests Unit testing for project improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Aug 12, 2026

y2kiran changed the title ~~Add parquet benchmark~~ Add parquet benchmark for output_dict_columns option Aug 12, 2026

y2kiran force-pushed the ykiran-dict-transcode-bench branch from 6b75627 to 6edb425 Compare August 12, 2026 21:43

y2kiran requested a review from a team as a code owner August 12, 2026 21:43

coderabbitai Bot reviewed Aug 12, 2026

View reviewed changes

Comment thread cpp/benchmarks/io/parquet/parquet_reader_options.cpp Outdated

Comment thread cpp/benchmarks/io/parquet/parquet_reader_options.cpp Outdated

y2kiran added 3 commits August 13, 2026 11:00

Added Parquet Read benchmark

8c11ddd

Folded into parquet_reader_options.cpp

3e9b8cb

MR feedback

a40db45

y2kiran force-pushed the ykiran-dict-transcode-bench branch from 6edb425 to a40db45 Compare August 13, 2026 19:44

vuule self-requested a review August 14, 2026 02:36

Merge branch 'main' into ykiran-dict-transcode-bench

2473951

github-actions Bot assigned vuule Aug 14, 2026

vuule reviewed Aug 14, 2026

View reviewed changes

Navigation Menu

Uh oh!

Add parquet benchmark for output_dict_columns option - #23596

Add parquet benchmark for output_dict_columns option#23596
y2kiran wants to merge 4 commits into
NVIDIA:mainfrom
y2kiran:ykiran-dict-transcode-bench

y2kiran commented Aug 7, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Aug 7, 2026 •

edited

Loading

Summary by CodeRabbit

Walkthrough

Changes

Uh oh!

vuule left a comment

Uh oh!

copy-pr-bot Bot commented Aug 12, 2026

Uh oh!

y2kiran commented Aug 12, 2026

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

vuule Aug 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

y2kiran commented Aug 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Uh oh!

coderabbitai Bot commented Aug 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Walkthrough

Changes

Uh oh!

vuule left a comment

Choose a reason for hiding this comment

Uh oh!

copy-pr-bot Bot commented Aug 12, 2026

Uh oh!

y2kiran commented Aug 12, 2026

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

vuule Aug 14, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

y2kiran commented Aug 7, 2026 •

edited

Loading

coderabbitai Bot commented Aug 7, 2026 •

edited

Loading