-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Add Hybrid scan page pruning when offset index is absent #23731
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
base: main
Are you sure you want to change the base?
Changes from 22 commits
e32363f
213f35e
1fd4b66
ccc96b6
4273fa9
2da7634
aa7be76
ff88b49
b267d0b
cc6a199
4f786ef
aac2bbb
eaa6931
5a57a4d
ba04997
e2f376c
ef23068
3f1f806
f2ce44d
91b45dd
db097e7
f36b1f9
3e576ad
966e5ea
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 |
|---|---|---|
|
|
@@ -595,7 +595,7 @@ class hybrid_scan_reader { | |
| * | ||
| * @param row_group_indices Input row groups indices | ||
| * @param column_chunk_data Device spans of column chunk data of filter columns | ||
| * @param[in,out] row_mask Mutable boolean column indicating surviving rows from page pruning | ||
| * @param[in,out] row_mask Mutable boolean column indicating surviving rows | ||
| * @param mask_data_pages Whether to build and use a data page mask using the row mask | ||
| * @param options Parquet reader options | ||
| * @param stream CUDA stream used for device memory operations and kernel launches | ||
|
|
@@ -676,7 +676,7 @@ class hybrid_scan_reader { | |
| * @param pass_read_limit Limit on the memory used for reading and decompressing data. `0` if | ||
| * there is no limit | ||
| * @param row_group_indices Input row groups indices | ||
| * @param row_mask Boolean column indicating which rows need to be read | ||
| * @param[in,out] row_mask Mutable boolean column indicating surviving rows | ||
|
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. is this really an out param, isn't row_mask const?
Contributor
Author
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. Improved doc in 3e576ad |
||
| * @param mask_data_pages Whether to build and use a data page mask using the row mask | ||
| * @param column_chunk_data Device spans of column chunk data of filter columns | ||
| * @param options Parquet reader options | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,6 +9,7 @@ | |
| #include "hybrid_scan_helpers.hpp" | ||
| #include "io/parquet/reader_impl_chunking_utils.cuh" | ||
| #include "io/parquet/synthetic_column_helpers.hpp" | ||
| #include "page_index_filter_utils.hpp" | ||
|
|
||
| #include <cudf/copying.hpp> | ||
| #include <cudf/detail/stream_compaction.hpp> | ||
|
|
@@ -629,8 +630,8 @@ hybrid_scan_reader_impl::payload_pages_byte_ranges( | |
|
|
||
| // Compute the data page mask | ||
| auto const mask_size = mask_offsets.back(); | ||
| auto data_page_mask = _extended_metadata->compute_data_page_mask( | ||
| row_mask, row_group_indices, _input_columns, 0, stream); | ||
| auto data_page_mask = | ||
| _extended_metadata->compute_data_page_mask(row_mask, row_group_indices, _input_columns, stream); | ||
| CUDF_EXPECTS(data_page_mask.empty() or data_page_mask.size() == mask_size, | ||
| "Computed data page mask does not match offset indexes"); | ||
|
|
||
|
|
@@ -731,8 +732,9 @@ table_with_metadata hybrid_scan_reader_impl::materialize_filter_columns( | |
|
|
||
| auto data_page_mask = thrust::host_vector<bool>{}; | ||
| if (mask_data_pages == use_data_page_mask::YES) { | ||
| _row_mask = row_mask; | ||
| data_page_mask = _extended_metadata->compute_data_page_mask( | ||
|
Contributor
Author
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. No need to pass in
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. In the no-offset-index case the work is done twice: each call site invokes
Contributor
Author
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. Moved computation of data_page_mask at a central location inside |
||
| row_mask, row_group_indices, _input_columns, _row_mask_offset, stream); | ||
| _row_mask, row_group_indices, _input_columns, stream); | ||
| } | ||
|
|
||
| prepare_data(read_mode::READ_ALL, row_group_indices, column_chunk_data, data_page_mask); | ||
|
|
@@ -769,8 +771,9 @@ table_with_metadata hybrid_scan_reader_impl::materialize_payload_columns( | |
|
|
||
| auto data_page_mask = thrust::host_vector<bool>{}; | ||
| if (not row_mask.is_empty() and mask_data_pages == use_data_page_mask::YES) { | ||
| _row_mask = row_mask; | ||
| data_page_mask = _extended_metadata->compute_data_page_mask( | ||
| row_mask, row_group_indices, _input_columns, _row_mask_offset, stream); | ||
| _row_mask, row_group_indices, _input_columns, stream); | ||
| } | ||
|
|
||
| prepare_data(read_mode::READ_ALL, row_group_indices, column_chunk_data, data_page_mask); | ||
|
|
@@ -840,8 +843,9 @@ void hybrid_scan_reader_impl::setup_chunking_for_filter_columns( | |
|
|
||
| auto data_page_mask = thrust::host_vector<bool>{}; | ||
| if (mask_data_pages == use_data_page_mask::YES) { | ||
| _row_mask = row_mask; | ||
| data_page_mask = _extended_metadata->compute_data_page_mask( | ||
| row_mask, row_group_indices, _input_columns, _row_mask_offset, stream); | ||
| _row_mask, row_group_indices, _input_columns, stream); | ||
| } | ||
|
|
||
| prepare_data(read_mode::CHUNKED_READ, row_group_indices, column_chunk_data, data_page_mask); | ||
|
|
@@ -900,8 +904,9 @@ void hybrid_scan_reader_impl::setup_chunking_for_payload_columns( | |
|
|
||
| auto data_page_mask = thrust::host_vector<bool>{}; | ||
| if (not row_mask.is_empty() and mask_data_pages == use_data_page_mask::YES) { | ||
| _row_mask = row_mask; | ||
| data_page_mask = _extended_metadata->compute_data_page_mask( | ||
| row_mask, row_group_indices, _input_columns, _row_mask_offset, stream); | ||
| _row_mask, row_group_indices, _input_columns, stream); | ||
| } | ||
|
|
||
| prepare_data(read_mode::CHUNKED_READ, row_group_indices, column_chunk_data, data_page_mask); | ||
|
|
@@ -1109,7 +1114,6 @@ bool hybrid_scan_reader_impl::has_next_table_chunk() | |
|
|
||
| void hybrid_scan_reader_impl::reset_internal_state() | ||
| { | ||
| _row_mask_offset = 0; | ||
| _file_itm_data = file_intermediate_data{}; | ||
| _file_preprocessed = false; | ||
| _has_offset_index = false; | ||
|
|
@@ -1133,6 +1137,10 @@ void hybrid_scan_reader_impl::reset_internal_state() | |
| _output_chunk_read_limit = 0; | ||
| _strings_to_categorical = false; | ||
| _reader_column_schema.reset(); | ||
|
|
||
| _row_mask = column_view{}; | ||
| _row_mask_offset = 0; | ||
|
|
||
| _expr_conv = parquet_filter_normalizer{}; | ||
| _mr = cudf::get_current_device_resource_ref(); | ||
| } | ||
|
|
@@ -1204,6 +1212,9 @@ void hybrid_scan_reader_impl::prepare_data( | |
| if (_file_itm_data._current_input_pass < _file_itm_data.num_passes()) { | ||
| handle_chunking(mode, column_chunk_data, data_page_mask); | ||
| } | ||
|
|
||
| // Clear the cached row mask column view | ||
| _row_mask = cudf::column_view{}; | ||
| } | ||
|
|
||
| template <typename RowMaskView> | ||
|
|
@@ -1354,9 +1365,9 @@ table_with_metadata hybrid_scan_reader_impl::finalize_output( | |
| // Prepend the source and row index columns to filter columns only | ||
| if (read_columns_mode == read_columns_mode::FILTER_COLUMNS) { | ||
| if (_options.prepend_row_index_column) { | ||
| out_columns.emplace( | ||
| out_columns.begin(), | ||
| synthesize_row_index_column(_file_itm_data.row_groups, read_info, _stream, _mr)); | ||
| out_columns.emplace(out_columns.begin(), | ||
| parquet::detail::synthesize_row_index_column( | ||
| _file_itm_data.row_groups, read_info, _stream, _mr)); | ||
| out_metadata.schema_info.emplace(out_metadata.schema_info.begin(), | ||
| column_name_info{.name = "row_index", .is_nullable = false}); | ||
| } | ||
|
|
@@ -1480,6 +1491,73 @@ void hybrid_scan_reader_impl::set_pass_page_mask(std::span<bool const> data_page | |
| mark_buffers_nullable_for_pruned_pages(); | ||
| } | ||
|
|
||
| thrust::host_vector<bool> hybrid_scan_reader_impl::compute_data_page_mask_with_page_headers() | ||
|
vuule marked this conversation as resolved.
Outdated
|
||
| { | ||
| auto const& pass = *_pass_itm_data; | ||
|
|
||
| // Return an empty vector if all rows are required | ||
| if (are_all_rows_retained(_row_mask, _stream)) { return thrust::host_vector<bool>(0); } | ||
|
|
||
| std::vector<cudf::size_type> page_row_offsets; | ||
|
Contributor
Author
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. This function is actually simpler than it looks. We are essentially doing the same thing as in Go over all pages and:
Call the |
||
| page_row_offsets.reserve(pass.pages.size() * 2); | ||
|
|
||
| // Maps each data page to its flat-page range; -1 keeps nested pages enabled. | ||
| std::vector<cudf::size_type> row_range_map; | ||
| row_range_map.reserve(pass.pages.size()); | ||
|
|
||
| cudf::size_type previous_chunk_idx = -1; | ||
| auto max_page_size = cudf::size_type{0}; | ||
|
|
||
| for (auto const& page : pass.pages) { | ||
| // Ignore dictionary pages altogether | ||
| if (page.flags & parquet::detail::PAGEINFO_FLAGS_DICTIONARY) { continue; } | ||
|
|
||
| auto const& chunk = pass.chunks[page.chunk_idx]; | ||
|
|
||
| // Don't prune list column pages as rows may span page boundaries when offset index isn't | ||
| // present. | ||
| if (chunk.max_level[parquet::detail::level_type::REPETITION] > 0) { | ||
| row_range_map.push_back(-1); | ||
| continue; | ||
| } | ||
|
|
||
| auto const page_start = chunk.start_row + page.chunk_row; | ||
| auto const page_end = page_start + page.num_rows; | ||
| max_page_size = std::max<cudf::size_type>(max_page_size, page_end - page_start); | ||
|
|
||
| // Starting a new column chunk. Push page start row | ||
| if (page.chunk_idx != previous_chunk_idx) { | ||
| page_row_offsets.push_back(page_start); | ||
| previous_chunk_idx = page.chunk_idx; | ||
| } | ||
|
|
||
| // Push row range index and page end row | ||
| row_range_map.push_back(page_row_offsets.size() - 1); | ||
|
mhaseeb123 marked this conversation as resolved.
|
||
| page_row_offsets.push_back(page_end); | ||
| } | ||
|
|
||
| auto data_page_mask = thrust::host_vector<bool>{}; | ||
|
|
||
| // Compute the row range mask | ||
| CUDF_EXPECTS(std::cmp_equal(_row_mask.size(), pass.num_rows), | ||
| "Row mask must span across all rows in the pass"); | ||
| auto const row_range_mask = | ||
| compute_row_range_selection_mask(_row_mask, page_row_offsets, max_page_size, _stream); | ||
|
|
||
| if (row_range_mask.empty()) { return data_page_mask; } | ||
|
|
||
| CUDF_EXPECTS(row_range_mask.size() == page_row_offsets.size() - 1, | ||
| "Encountered invalid row range mask size"); | ||
|
|
||
| data_page_mask.reserve(row_range_map.size()); | ||
|
|
||
| // Scatter row range results while retaining list column pages. | ||
| for (auto const range_idx : row_range_map) { | ||
| data_page_mask.push_back(range_idx < 0 ? true : row_range_mask[range_idx]); | ||
| } | ||
| return data_page_mask; | ||
| } | ||
|
|
||
| void hybrid_scan_reader_impl::set_sparse_pass_page_mask( | ||
| std::span<cudf::device_span<uint8_t const> const> page_data) | ||
| { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -387,6 +387,11 @@ class hybrid_scan_reader_impl : public parquet::detail::reader_impl { | |
| */ | ||
| void set_sparse_pass_page_mask(std::span<cudf::device_span<uint8_t const> const> page_data); | ||
|
|
||
| /** | ||
| * @brief Compute a data page mask from the decoded page headers. | ||
| */ | ||
| [[nodiscard]] thrust::host_vector<bool> compute_data_page_mask_with_page_headers(); | ||
|
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. maybe document the precondition?
Contributor
Author
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. I think it's now clear with the call site in 3e576ad |
||
|
|
||
| /** | ||
| * @brief Mark output buffers nullable when page pruning synthesizes null rows | ||
| */ | ||
|
|
@@ -619,6 +624,11 @@ class hybrid_scan_reader_impl : public parquet::detail::reader_impl { | |
|
|
||
| std::optional<std::vector<std::string>> _filter_columns_names; | ||
|
|
||
| // Non-owning view of the caller's row mask, only valid for the duration of a single | ||
| // materialization or chunking setup call, during which the pass page mask is computed. Null | ||
| // entries mean the row could not be pruned and is therefore treated as a surviving row. | ||
| cudf::column_view _row_mask{}; | ||
|
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. this is a non-owning view that persists in error cases, ideally we would clear it. not blocking.
Contributor
Author
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. Removed this altogether in 3e576ad |
||
|
|
||
| std::vector<cudf::io::detail::inline_column_buffer> _original_output_buffers_template; | ||
|
|
||
| cudf::size_type _row_mask_offset{0}; | ||
|
|
||
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 case is now handled so enable