-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Add python bindings for hybrid scan metadata and release GIL in the reader APIs #23546
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
051e21d
06df8af
9fafd51
5627e40
be697fe
916cd4c
bb57d2f
2dfac40
aeb06b9
2b09a07
d424c6a
0350367
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -32,6 +32,11 @@ namespace cudf::io::parquet::experimental::detail { | |
| * Hybrid Scan operation. | ||
| */ | ||
| class hybrid_scan_reader_impl; | ||
|
|
||
| /** | ||
| * @brief Internal parsed Parquet file metadata for the Hybrid Scan reader. | ||
| */ | ||
| class aggregate_reader_metadata; | ||
| } // namespace cudf::io::parquet::experimental::detail | ||
|
|
||
| //! Using `byte_range_info` from cudf::io::text | ||
|
|
@@ -53,6 +58,70 @@ enum class use_data_page_mask : bool { | |
| NO = false ///< Do not compute or use a data page mask | ||
| }; | ||
|
|
||
| /** | ||
| * @brief Shareable, pre-parsed Parquet file metadata for the Hybrid Scan reader. | ||
| * | ||
| * Parses the Parquet file metadata once so that multiple `hybrid_scan_reader` instances reading | ||
| * the same file can share it rather than each re-parsing and copying the row group metadata. | ||
| * The intended use is to read disjoint row-group ranges of a single file: construct one | ||
| * `hybrid_scan_metadata` per file and pass it to as many readers as there are ranges. | ||
| * | ||
| * @code{.cpp} | ||
| * // Parse the metadata once | ||
| * auto metadata = parquet::experimental::hybrid_scan_metadata{*footer_buffer, options}; | ||
| * // Construct lightweight readers that share it | ||
| * auto reader_a = std::make_unique<parquet::experimental::hybrid_scan_reader>(metadata); | ||
| * auto reader_b = std::make_unique<parquet::experimental::hybrid_scan_reader>(metadata); | ||
| * @endcode | ||
| * | ||
| * @note The metadata is immutable after `setup_page_index()` has been called (or immediately after | ||
| * construction if page index setup is skipped). Concurrent usage by multiple readers is thread | ||
| * safe. This handle does not support multi-source (multi-file) metadata. | ||
| */ | ||
| class hybrid_scan_metadata { | ||
| public: | ||
| /** | ||
| * @brief Parse and own Parquet file metadata from a span of footer bytes | ||
| * | ||
| * @param footer_bytes Host span of Parquet file footer bytes | ||
| * @param options Parquet reader options | ||
| */ | ||
| hybrid_scan_metadata(cudf::host_span<uint8_t const> footer_bytes, | ||
| parquet_reader_options const& options); | ||
|
|
||
| /** | ||
| * @brief Own Parquet file metadata from a pre-populated `FileMetaData` | ||
| * | ||
| * @param parquet_metadata Pre-populated Parquet file metadata | ||
| * @param options Parquet reader options | ||
| */ | ||
| hybrid_scan_metadata(FileMetaData const& parquet_metadata, parquet_reader_options const& options); | ||
|
Comment on lines
+92
to
+98
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. non-blocking as it needs a broader fix in a follow up. This may be misleading as the |
||
|
|
||
| /** | ||
| * @brief Destructor for the shared Parquet metadata | ||
| */ | ||
| ~hybrid_scan_metadata(); | ||
|
|
||
| hybrid_scan_metadata(hybrid_scan_metadata const&) = default; ///< Copy constructor | ||
| hybrid_scan_metadata(hybrid_scan_metadata&&) = default; ///< Move constructor | ||
|
|
||
| /** | ||
| * @brief Copy assignment operator | ||
| * @return Reference to this object | ||
| */ | ||
| hybrid_scan_metadata& operator=(hybrid_scan_metadata const&) = default; | ||
|
|
||
| /** | ||
| * @brief Move assignment operator | ||
| * @return Reference to this object | ||
| */ | ||
| hybrid_scan_metadata& operator=(hybrid_scan_metadata&&) = default; | ||
|
|
||
| private: | ||
| std::shared_ptr<detail::aggregate_reader_metadata> _metadata; | ||
| friend class hybrid_scan_reader; | ||
|
wence- marked this conversation as resolved.
|
||
| }; | ||
|
|
||
| /** | ||
| * @brief The experimental parquet reader class to optimally read parquet files subject to | ||
| * highly selective filters, called a Hybrid Scan operation | ||
|
|
@@ -301,6 +370,15 @@ class hybrid_scan_reader { | |
| explicit hybrid_scan_reader(FileMetaData const& parquet_metadata, | ||
| parquet_reader_options const& options); | ||
|
|
||
| /** | ||
| * @brief Constructor that takes shared ownership of pre-parsed Parquet file metadata | ||
| * | ||
| * Constructs a reader that shares the pre-parsed metadata object. | ||
| * | ||
| * @param metadata Shared, pre-parsed Parquet file metadata | ||
| */ | ||
| explicit hybrid_scan_reader(hybrid_scan_metadata metadata); | ||
|
|
||
| /** | ||
| * @brief Destructor for the experimental parquet reader class | ||
| */ | ||
|
|
||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Based on my other feedback, can you add a test that actually creates multiple readers? That would suss out the kinds of issues I was worried about above. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note:
setup_page_index()may itself only be called from one reader using this. Otherwise, it is not thread-safe.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This docstring feels off. The object itself is not immutable. Immutability is enforced at the level of the reader that takes in and leverages this metadata in a particular way. The
shared_ptrmanagement is all done in the readers, not here. The documentation and management of appropriate usage should be in the place where that control is actually happening.