[PERF] Avoid deep-cloning FileMetaData on parquet_metadatas reuse
- Dominant language
- C++
- Stars
- 9.8k
- Forks
- 1.1k
- Avg merge
- 3d 6m
- Merged PRs (30d)
- 278
Description
Follow up to #23558. That PR fixed the GIL contention when reusing prefetched `FileMetaData` across concurrent reads. But there's still a cost left on the table, raised in this thread: https://github.com/NVIDIA/cudf/pull/23558#discussion_r3731627553
Every `pylibcudf.io.parquet.read_parquet` / `ChunkedParquetReader` call that passes `parquet_metadatas=` still deep clones the whole `FileMetaData` object before reading. You can see it in `parquet.pyx`:
```cython
with nogil:
c_metadatas.reserve(metadata_ptrs.size())
for i in range(metadata_ptrs.size()):
c_metadatas.push_back(dereference(metadata_ptrs[i])) # deep copy
c_result = move(cpp_read_parquet(move(sources), move(c_metadatas), ...))
```
The GIL isn't held for this anymore, so the copy itself is cheaper to do concurrently, but it's still a full copy on every read, and it's forced by the API: `read_parquet` and `chunked_parquet_reader` take `std::vector&&`, so they need to own what's passed in. For files with lots of row groups and column chunks (especially with page indexes), that's a lot of data being copied just to read from it.
I looked at what `aggregate_reader_metadata` actually mutates once it has the metadata (`reader_impl_helpers.cpp:860-908`), and it's small:
1. `schema[i].repetition_type` on the first file only, promoting `REQUIRED` to `OPTIONAL` when merging sources with mismatched nullability.
2. `apply_arrow_schema()`, also just schema.
3. Erasing `ARROW_SCHEMA_KEY` from each file's `keyval_maps`.
`row_groups`, and everything under it (column chunks, page indexes), is never touched. That's the bulk of the object for wide files, and we're cloning it for no reason.
So the fix is to stop requiring ownership. Change the metadata-taking constructors and readers to accept `host_span` instead of `std::vector&&`. `hybrid_scan` already does something like this, so there's precedent. Inside `aggregate_reader_metadata`, only copy the small mutable part (`schema` and `keyval_maps`) into a local working copy, and just reference `row_groups` from the caller's metadata instead of copying it. Once the C++ side takes a span, we can drop the `c_metadatas` copy loop in `parquet.pyx` entirely and pass pointers straight through.
Contributor guide
Assessment
This issue has not been assessed yet.