Asynchronously fetch byte ranges from multiple sources - #22586
Conversation
…i-source-parquet-io-utils
|
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. |
|
|
||
| } // namespace detail | ||
|
|
||
| std::tuple<std::vector<rmm::device_buffer>, |
There was a problem hiding this comment.
Moved impl in detail vectorized API. Public APIs just call that wrapped with a CUDF_FUNC_RANGE() like every other libcudf API
Co-authored-by: Bradley Dice <bdice@bradleydice.com>
|
@vuule added you as reviewer as you just reviewed its predecessor PR (same thing here except vectorized and using another mutex to serialize host reads too across threads) |
| // Schedule host reads holding the `host_read_mutex` so that all reads for a caller thread | ||
| // are scheduled without interleaving with reads from other threads yielding better pipelining | ||
| { | ||
| std::scoped_lock<std::mutex> lock(host_read_mutex); |
There was a problem hiding this comment.
if I'm reading this right, we now have a mutex around read_parquet IO as well? Have you checked the impact on the multithreaded benchmark?
There was a problem hiding this comment.
Your understanding is correct. I measured performance of Parquet multithreaded benchmark and saw no difference (See #22586 (comment)). I was expecting a speed improvement though.
There was a problem hiding this comment.
I also ran this experiment #22529 (comment) again with this PR and compared the profiles and saw no difference
Performance Impact to PARQUET_MULTITHREAD_READER_NVBENCHSummary: No impact. Everything within noise threshold.
|
| cardinality | io_type | num_cols | num_iterations | num_threads | run_length | total_data_size | main (ms) | PR (ms) | Δ | noise(main/PR) |
|---|---|---|---|---|---|---|---|---|---|---|
| 1000 | PINNED_BUFFER | 4 | 1 | 1 | 8 | 1073741824 | 14.334 | 14.603 | +1.88% | 0.41%/1.03% |
| 1000 | PINNED_BUFFER | 4 | 1 | 2 | 8 | 1073741824 | 14.002 | 14.246 | +1.74% | 1.68%/1.79% |
| 1000 | PINNED_BUFFER | 4 | 1 | 4 | 8 | 1073741824 | 14.734 | 15.033 | +2.03% | 5.11%/4.68% |
| 1000 | PINNED_BUFFER | 4 | 1 | 8 | 8 | 1073741824 | 16.532 | 16.676 | +0.87% | 7.18%/6.57% |
parquet_multithreaded_read_decode_list
| cardinality | io_type | num_cols | num_iterations | num_threads | run_length | total_data_size | main (ms) | PR (ms) | Δ | noise(main/PR) |
|---|---|---|---|---|---|---|---|---|---|---|
| 1000 | PINNED_BUFFER | 4 | 1 | 1 | 8 | 1073741824 | 29.630 | 29.837 | +0.70% | 0.87%/0.81% |
| 1000 | PINNED_BUFFER | 4 | 1 | 2 | 8 | 1073741824 | 28.109 | 28.047 | -0.22% | 2.70%/4.85% |
| 1000 | PINNED_BUFFER | 4 | 1 | 4 | 8 | 1073741824 | 27.215 | 27.480 | +0.97% | 7.30%/9.90% |
| 1000 | PINNED_BUFFER | 4 | 1 | 8 | 8 | 1073741824 | 35.343 | 35.172 | -0.49% | 14.16%/13.49% |
parquet_multithreaded_read_decode_mixed
| cardinality | io_type | num_cols | num_iterations | num_threads | run_length | total_data_size | main (ms) | PR (ms) | Δ | noise(main/PR) |
|---|---|---|---|---|---|---|---|---|---|---|
| 1000 | PINNED_BUFFER | 4 | 1 | 1 | 8 | 1073741824 | 12.903 | 13.042 | +1.07% | 0.67%/1.61% |
| 1000 | PINNED_BUFFER | 4 | 1 | 2 | 8 | 1073741824 | 13.045 | 13.376 | +2.53% | 6.03%/6.13% |
| 1000 | PINNED_BUFFER | 4 | 1 | 4 | 8 | 1073741824 | 14.900 | 15.040 | +0.94% | 6.66%/8.13% |
| 1000 | PINNED_BUFFER | 4 | 1 | 8 | 8 | 1073741824 | 18.373 | 18.664 | +1.58% | 10.06%/9.21% |
parquet_multithreaded_read_decode_string
| cardinality | io_type | num_cols | num_iterations | num_threads | run_length | total_data_size | main (ms) | PR (ms) | Δ | noise(main/PR) |
|---|---|---|---|---|---|---|---|---|---|---|
| 1000 | PINNED_BUFFER | 4 | 1 | 1 | 8 | 1073741824 | 13.568 | 13.789 | +1.64% | 0.50%/1.22% |
| 1000 | PINNED_BUFFER | 4 | 1 | 2 | 8 | 1073741824 | 14.126 | 14.149 | +0.17% | 6.52%/5.48% |
| 1000 | PINNED_BUFFER | 4 | 1 | 4 | 8 | 1073741824 | 15.821 | 15.937 | +0.73% | 14.11%/14.23% |
| 1000 | PINNED_BUFFER | 4 | 1 | 8 | 8 | 1073741824 | 20.153 | 20.702 | +2.72% | 8.61%/8.47% |
Co-authored-by: Yunsong Wang <12716979+PointKernel@users.noreply.github.com>
PointKernel
left a comment
There was a problem hiding this comment.
One last comment but non blocking.
…e sources (#22613) Contributes to #22583. Follow up #22586 This PR adds new overloads of remaining Parquet IO utility to fetch parquet footers and page index bytes from multiple sources. Authors: - Muhammad Haseeb (https://github.com/mhaseeb123) Approvers: - Qi Chen (https://github.com/qbacpey) - Yunsong Wang (https://github.com/PointKernel) URL: #22613
|
/merge |
vyasr
left a comment
There was a problem hiding this comment.
Wow I'm sorry I started reviewing here and then completely forgot to submit. Looks like almost all of my requests have already been addressed so please proceed with merging. I'll just leave the question.
| { | ||
| static std::mutex mutex; | ||
| static std::mutex host_read_mutex; | ||
| static std::mutex device_read_mutex; |
There was a problem hiding this comment.
This is implementing what we discussed in #22550, right?
Yes :) |
Description
Contributes to #22583. Follow up #22550
This PR adds a new overload of
fetch_byte_ranges_to_device_asyncParquet IO utility that enables asynchronously fetching byte ranges across multiple sources.This new API is now being used to read column chunk data by both Parquet and Hybrid scan readers enabling common (pipelining) behavior for multithreaded reads.
Checklist
PARQUET_READER_NVBENCHagainstmain. No perf improvement or regression.