Skip to content

Derive per-chunk memory reservation from scan statistics - #22846

Merged
rapids-bot[bot] merged 3 commits into
NVIDIA:mainfrom
Matt711:imp/polars/better-chunk-size-estimate
Jun 11, 2026
Merged

Derive per-chunk memory reservation from scan statistics#22846
rapids-bot[bot] merged 3 commits into
NVIDIA:mainfrom
Matt711:imp/polars/better-chunk-size-estimate

Conversation

@Matt711

@Matt711 Matt711 commented Jun 10, 2026

Copy link
Copy Markdown
Member

Description

Simply use the estimated chunk size (already computed) for the memory reservation. I ran TPC-H locally (single-gpu, sf1k) and didn't notice any notable regressions.

xref #22796

Checklist

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

@Matt711
Matt711 requested a review from a team as a code owner June 10, 2026 23:51
@Matt711
Matt711 requested a review from madsbk June 10, 2026 23:51
@Matt711 Matt711 added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Jun 10, 2026
@github-actions github-actions Bot added Python Affects Python cuDF API. cudf-polars Issues specific to cudf-polars labels Jun 10, 2026
@GPUtester GPUtester moved this to In Progress in cuDF Python Jun 10, 2026
@Matt711 Matt711 changed the title Derive per-chunk memory reservation from partition statistics Derive per-chunk memory reservation from scan statistics Jun 10, 2026
@coderabbitai

coderabbitai Bot commented Jun 10, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: ed37e9e7-0d17-4045-8585-bf7eba61a589

📥 Commits

Reviewing files that changed from the base of the PR and between 6f6b12c and af18f48.

📒 Files selected for processing (3)
  • python/cudf_polars/cudf_polars/streaming/actor_graph/io.py
  • python/cudf_polars/cudf_polars/streaming/base.py
  • python/cudf_polars/cudf_polars/streaming/io.py

📝 Walkthrough

Summary by CodeRabbit

  • Refactor
    • Enhanced memory estimation logic for streaming scan operations.

Walkthrough

The change threads an estimated chunk size estimate through the partition planning pipeline. A new optional field on IOPartitionPlan carries decompressed bytes-per-chunk values computed during Parquet partition strategy selection (split or fused), which is then consumed during actor graph scan node creation as the preferred chunk sizing parameter over a generic executor fallback.

Changes

Partition Chunk Estimation Pipeline

Layer / File(s) Summary
IOPartitionPlan contract extension
python/cudf_polars/cudf_polars/streaming/base.py
IOPartitionPlan adds optional `estimated_chunk_bytes: int
Scan partition plan computation
python/cudf_polars/cudf_polars/streaming/io.py
scan_partition_plan for Parquet scans now computes and returns estimated_chunk_bytes within IOPartitionPlan: file_size // factor for split plans, file_size * factor for fused plans.
Actor graph scan node creation
python/cudf_polars/cudf_polars/streaming/actor_graph/io.py
generate_ir_sub_network for StreamingScan passes plan.estimated_chunk_bytes to scan_node when available, otherwise falls back to executor.target_partition_size.

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the main change: deriving per-chunk memory reservations from scan statistics, which aligns with the core objective of using estimated chunk sizes for memory allocation.
Description check ✅ Passed The description is directly related to the changeset, explaining the rationale (using already-computed estimated chunk size) and mentioning testing confirmation, though some checklist items remain unchecked.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

@madsbk madsbk 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

@Matt711

Matt711 commented Jun 11, 2026

Copy link
Copy Markdown
Member Author

/merge

@rapids-bot
rapids-bot Bot merged commit a643d62 into NVIDIA:main Jun 11, 2026
106 checks passed
@github-project-automation github-project-automation Bot moved this from In Progress to Done in cuDF Python Jun 11, 2026
@Matt711
Matt711 deleted the imp/polars/better-chunk-size-estimate branch June 12, 2026 12:10
abigalekim pushed a commit to abigalekim/cudf that referenced this pull request Jun 12, 2026
Simply use the estimated chunk size (already computed) for the memory reservation. I ran TPC-H locally (single-gpu, sf1k) and didn't notice any notable regressions.

xref NVIDIA#22796

Authors:
  - Matthew Murray (https://github.com/Matt711)

Approvers:
  - Mads R. B. Kristensen (https://github.com/madsbk)

URL: NVIDIA#22846
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cudf-polars Issues specific to cudf-polars improvement Improvement / enhancement to an existing function non-breaking Non-breaking change Python Affects Python cuDF API.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants