[PERF]: Improve the performance of `plc.io.parquet.read_parquet` with prefetched parquet file metadata. by TomAugspurger · Pull Request #23558 · NVIDIA/cudf

TomAugspurger · 2026-08-05T19:55:36Z

Description

We've observed a performance penalty from enabling metadata prefetching in cudf-polars for data in local storage. We'd like to enable prefetching by default, since it's crucial for good performance with high-latency remote storage systems (S3). Also, you'd naively expect prefetching to help locally since you'd do less work (you avoid re-parsing the footer when reading different row groups out of the same file multiple times).

Over simplifying things, prefetching slowed things down because it's (surprisingly?) expensive to copy a FileMetaData object. This pain was compounded in cudf-polars because we run concurrent read_parquets in a thread pool, and the GIL was held for these expensive copies.

(Note: why are we copying in the first place? IIUC, it's because the parquet reader currently mutates something on the object in read_parquet, so it needs an owned FileMetaData.)

This PR moves FileMetaData footers into Python without holding the GIL. This lets concurrent prefetches happen more... concurrently. See https://github.com/user-attachments/assets/c0f34117-8483-413a-a3de-e101a0093c12 for an nsys profile screenshot showing GIL contention.
After this fix, prefetching is still a clear win on S3 but there's still a ~16% penalty on NVMe. Rather than holding off entirely, this PR enables prefetching by default only for remote URIs (s3://, gs://, etc.). And queries that mix remote and local reads will only prefetch for the remote reads if metedata prefetching is unset (default).

Benchmarks (SF100, all 22 TPC-H queries, 256 kvikio threads):

Storage	prefetch	Total Mean
NVMe	off	9.47s
NVMe	on	10.95s (+15.6%)
S3	off	75.66s
S3	on	54.98s (-27.3%)

Closes Enable cudf-polars parquet metadata prefetching by default on the cloud #23551

copy-pr-bot · 2026-08-05T19:55:39Z

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

TomAugspurger · 2026-08-05T19:56:57Z

/ok to test 39cc045

TomAugspurger · 2026-08-06T12:42:44Z

This change did help with the metadata prefetching. But I still see GIL contention in Scan nodes:

TomAugspurger · 2026-08-06T13:00:04Z

/ok to test b4d6a87

This changes the ownership model of FileMetadata to own the footers via unique_ptr and move them (under nogil) in the pylibcudf wrapper. Additionally, we avoid an unnecessary copy in get_parquet_metadatas().

TomAugspurger · 2026-08-06T13:21:14Z

/ok to test 1ac2985

TomAugspurger · 2026-08-06T14:45:19Z

/ok to test af347e8

When page indexes are present, cloning the parquet footer FileMetaData object can be expensive. This slows down `read_parquet` when the user provides a prefetched metadata object. This PR adds a new `read_page_indexes` parameter to `read_parquet_footers` that controls whether page indexes are materialized. The default is `True` matching the existing behavior.

TomAugspurger · 2026-08-06T19:44:01Z

/ok to test f5525b5

TomAugspurger · 2026-08-06T20:11:38Z


      timer.start();
-      auto const metadatas = cudf::io::read_parquet_footers(sources);
+      auto const metadatas = cudf::io::read_parquet_footers(sources, write_page_index);


I'm not sure this change is needed, but it shouldn't hurt.

This is currently a no-op (you're telling the benchmark to read page indexes only if they are written), but we probably want to benchmark a path where the indexes are present but ignored (write_page_index==true, read_page_indexes==false) at some point…

I don't think we should stop reading page indexes. So I removed that commit. This means that we're not quite at parity on-prem with prefetch metdata turned off vs on--although we are closer than we were before this PR. For that reason, I'm only switching it on by default for cloud. And I've relaxed the garuntee that the metadata will be prefetched and cached if the setting is turned on to handle the case where some scans are reading from remote sources and others are not.

I actually think what may align the on-prem results is when metadata prefetching happens concurrently with scan execution (reading bytes)--see #23131.

#23131 would overlap the footer read with other operations, but the clone happens on the scan critical path, so cost remains an issue. One complementary approach could be to minimize what gets cloned: the reader only mutates the schema (repetition_type promotion and apply_arrow_schema()), so row-group and page-index data could in principle be shared. This would be a larger undertaking, and definitely out of scope for this PR. Worth filing a separate feature request to track, or noting this in #23131?

mhm, lets use #23572 as the issue

TomAugspurger · 2026-08-07T12:37:24Z

/ok to test 0053fb4

TomAugspurger · 2026-08-07T15:44:28Z

These might overlap conceptually with #23546.

Matt711 · 2026-08-12T16:50:11Z

/ok to test 93bccd7

copy-pr-bot · 2026-08-12T22:06:07Z

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.

Matt711 · 2026-08-12T22:08:05Z

/ok to test 8b7563a

igorpeshansky · 2026-08-12T18:49:52Z

    def version(self):
        """Get the file format version."""
-        return self.c_obj.version
+        return dereference(self.c_obj).version


Doesn't __new__ bypass __init__? Wouldn't FileMetaData.__new__(FileMetaData) work outside of from_libcudf?

igorpeshansky · 2026-08-13T04:24:06Z


      timer.start();
-      auto const metadatas = cudf::io::read_parquet_footers(sources);
+      auto const metadatas = cudf::io::read_parquet_footers(sources, write_page_index);


#23131 would overlap the footer read with other operations, but the clone happens on the scan critical path, so cost remains an issue. One complementary approach could be to minimize what gets cloned: the reader only mutates the schema (repetition_type promotion and apply_arrow_schema()), so row-group and page-index data could in principle be shared. This would be a larger undertaking, and definitely out of scope for this PR. Worth filing a separate feature request to track, or noting this in #23131?

Matt711 · 2026-08-13T14:58:35Z

/ok to test b545203

TomAugspurger added Performance Performance related issue improvement Improvement / enhancement to an existing function pylibcudf Issues specific to the pylibcudf package labels Aug 5, 2026

github-project-automation Bot added this to cuDF Python Aug 5, 2026

github-actions Bot assigned TomAugspurger Aug 5, 2026

github-actions Bot added libcudf Affects libcudf (C++/CUDA) code. Python Affects Python cuDF API. labels Aug 5, 2026

GPUtester moved this to In Progress in cuDF Python Aug 5, 2026

TomAugspurger added the non-breaking Non-breaking change label Aug 5, 2026

[PERF]: Avoid deep copies with GIL in pylibcudf read_parquet_footers

17794d7

This changes the ownership model of FileMetadata to own the footers via unique_ptr and move them (under nogil) in the pylibcudf wrapper. Additionally, we avoid an unnecessary copy in get_parquet_metadatas().

TomAugspurger force-pushed the tom/parquet-metadata-gil-perf branch from b4d6a87 to 1ac2985 Compare August 6, 2026 13:20

TomAugspurger added 2 commits August 6, 2026 10:43

Attempt to reduce GIL contention in Scan nodes.

ecfd5b6

TomAugspurger force-pushed the tom/parquet-metadata-gil-perf branch from 3d8de45 to b3775f5 Compare August 6, 2026 17:53

Enable prefetching by default

f5525b5

test style

0053fb4

TomAugspurger changed the title ~~[PERF]: Avoid deep copies with GIL in pylibcudf read_parquet_footers~~ [PERF]: Improve the performance of read_parquet with prefetched parquet file metadata. Aug 6, 2026

TomAugspurger changed the title ~~[PERF]: Improve the performance of read_parquet with prefetched parquet file metadata.~~ [PERF]: Improve the performance of plc.io.parquet.read_parquet with prefetched parquet file metadata. Aug 6, 2026

TomAugspurger commented Aug 6, 2026

View reviewed changes

TomAugspurger mentioned this pull request Aug 6, 2026

Enable parquet metadata prefetching by default #23307

Closed

3 tasks

github-actions Bot added the cudf-polars Issues specific to cudf-polars label Aug 6, 2026

Merge branch 'main' into tom/parquet-metadata-gil-perf

98fbdd8

Matt711 requested review from igorpeshansky and wence- August 12, 2026 16:22

pre-commit

93bccd7

NVIDIA deleted a comment from copy-pr-bot Bot Aug 12, 2026

remove copyright change

df0b4d4

NVIDIA deleted a comment from copy-pr-bot Bot Aug 12, 2026

Matt711 removed request for a team and igorpeshansky August 12, 2026 17:11

Matt711 mentioned this pull request Aug 12, 2026

[PERF]: Use 256 kvikio threads by default in cudf-polars #23633

Open

Merge branch 'main' into tom/parquet-metadata-gil-perf

8b7563a

vuule reviewed Aug 13, 2026

View reviewed changes

Comment thread python/cudf_polars/tests/test_config.py

igorpeshansky reviewed Aug 13, 2026

View reviewed changes

Matt711 added 9 commits August 13, 2026 13:42

default is Unspecified for prefetching with streaming engine

ad20e44

add a test for ParquetOptions w/o setting prefetching default

74cda45

fix tests because we fallback now if metadata is not prefetched

0af3e32

test remote_only=True

4c55108

docs, small fixes

071bd04

revert unnecessary changes

ea351a6

remove copy_parquet_metadatas

d84b230

more doc strings fixes

420469a

trigger github update

813740f

Matt711 mentioned this pull request Aug 13, 2026

[PERF]: Avoid discarding prefetched parquet metadata for scans that mix cached and uncached metadata for paths #23647

Open

Merge branch 'main' into tom/parquet-metadata-gil-perf

b545203

Matt711 requested review from igorpeshansky and vuule August 13, 2026 14:59

Uh oh!

Conversation

TomAugspurger commented Aug 5, 2026 • edited by Matt711 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Uh oh!

copy-pr-bot Bot commented Aug 5, 2026

Uh oh!

TomAugspurger commented Aug 5, 2026

Uh oh!

TomAugspurger commented Aug 6, 2026

Uh oh!

TomAugspurger commented Aug 6, 2026

Uh oh!

TomAugspurger commented Aug 6, 2026

Uh oh!

TomAugspurger commented Aug 6, 2026

Uh oh!

TomAugspurger commented Aug 6, 2026

Uh oh!

TomAugspurger Aug 6, 2026

Choose a reason for hiding this comment

Uh oh!

igorpeshansky Aug 10, 2026

Choose a reason for hiding this comment

Uh oh!

Matt711 Aug 12, 2026

Choose a reason for hiding this comment

Uh oh!

igorpeshansky Aug 13, 2026

Choose a reason for hiding this comment

Uh oh!

Matt711 Aug 13, 2026

Choose a reason for hiding this comment

Uh oh!

TomAugspurger commented Aug 7, 2026

Uh oh!

TomAugspurger commented Aug 7, 2026

Uh oh!

Matt711 commented Aug 12, 2026

Uh oh!

copy-pr-bot Bot commented Aug 12, 2026

Uh oh!

Matt711 commented Aug 12, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

igorpeshansky Aug 12, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

igorpeshansky Aug 13, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Matt711 commented Aug 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

TomAugspurger commented Aug 5, 2026 •

edited by Matt711

Loading