Fix temporary host source lifetime with a separate copy API - #23561
Fix temporary host source lifetime with a separate copy API#23561PointKernel wants to merge 10 commits into
Conversation
|
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. |
|
/ok to test |
|
/ok to test 210be0d |
| .flags = cudaMemcpyFlagPreferOverlapWithCompute}; | ||
| CUDF_CUDA_TRY( | ||
| cudaMemcpyBatchAsync(dsts, srcs, sizes, 1, &attrs, &attrs_idxs, 1, stream.value())); | ||
| return; |
There was a problem hiding this comment.
needs a sync before return in this case?
There was a problem hiding this comment.
ah no because of the memcpy attributes?
There was a problem hiding this comment.
I would prefer a solution with explicit syncs.
There was a problem hiding this comment.
I also don't like that we're doing an explicit call inside an async API. Let me rework this.
| std::size_t const* sizes, | ||
| std::size_t count, | ||
| rmm::cuda_stream_view stream, | ||
| host_source_access_order source_access_order = host_source_access_order::STREAM); |
There was a problem hiding this comment.
testing only, not the final solution
|
/ok to test eeadee3 |
|
/ok to test |
|
/ok to test df79dac |
|
/ok to test fb5ea3f |
|
/ok to test |
Adds the *_async_consume_source copy APIs (cudaMemcpySrcAccessOrderDuringApiCall on CUDA 13+) and converts the call sites that pass short-lived host buffers. Deferred pageable reads on coherent-memory systems otherwise become use-after-free once the caller frees the source (observed on GB10 as intermittent sort-order corruption under runtime PTDS).
…urces Covers the call sites NVIDIA#23561 does not convert (row operators, ORC/Parquet writers, contiguous_split, row_bit_count and tests) with explicit synchronization before the host buffers go out of scope. Where the two backports overlap, the NVIDIA#23561 form is kept.
|
Closing this as #23517 is the right solution to go. |
## Description Fix asynchronous host-to-device copies whose host source could be destroyed or mutated before the copy completed. The CUDA 13 `cudaMemcpyBatchAsync` changes in rapidsai/rmm#2511 exposed these invalid lifetime assumptions as nondeterministic failures in pylibcudf, cudf-polars, and hybrid scan tests. Synchronize affected copies at the ownership boundary, and preserve backing storage for Python buffer slices until queued copies can consume them. Also recognize `cudaMemcpyBatchAsync` in the stream-usage checker. This does not introduce a new API or change source ownership semantics. This borrows some lifetime fixes from #23517 and #23561 that we observed were necessary on GB300 but haven't been merged upstream yet. ## Checklist - [x] I am familiar with the [Contributing Guidelines](https://github.com/NVIDIA/cudf/blob/HEAD/CONTRIBUTING.md). - [x] New or existing tests cover these changes. - [x] The documentation is up to date with these changes. --------- Co-authored-by: Nghia Truong <7416935+ttnghia@users.noreply.github.com>
The upstream-selected resolution of the deferred pageable host-to-device copy hazards (supersedes the in-review NVIDIA#23561/NVIDIA#23517 forms shipped in build 4): synchronize affected copies at the ownership boundary instead of adding a separate copy API. Adapted to 26.08 types (rmm::cuda_stream_view / synchronize()); the Python-side buffer preservation is out of scope for the libcudf package.
The upstream-selected resolution of the deferred pageable host-to-device copy hazards (supersedes the in-review NVIDIA#23561/NVIDIA#23517 forms shipped in build 4): synchronize affected copies at the ownership boundary instead of adding a separate copy API. Adapted to 26.08 types (rmm::cuda_stream_view::synchronize()); the Python-side buffer preservation is out of scope for the libcudf package.
The upstream-selected resolution of the deferred pageable host-to-device copy hazards (supersedes the in-review NVIDIA#23561/NVIDIA#23517 forms shipped in build 4): synchronize affected copies at the ownership boundary instead of adding a separate copy API. Adapted to 26.08 types (rmm::cuda_stream_view::synchronize()); the Python-side buffer preservation is out of scope for the libcudf package. (cherry picked from commit 3d94601)
Summary
*_async_consume_sourceAPIs for host-to-device copies whose CPU source may disappear when the function returnscudaMemcpyBatchAsyncwithcudaMemcpySrcAccessOrderDuringApiCallon CUDA 13+; use a stream synchronization on older CUDA/default-stream paths to provide the same source-lifetime promisecudaMemcpyBatchAsyncand its per-thread-default-stream entry point on CUDA 13+Root cause
This is a cuDF caller lifetime bug, not a bug in the normal async-copy API.
Some functions gave CUDA a pointer to a local CPU vector, queued an asynchronous copy, and then destroyed the vector when the function returned. CUDA is allowed to read that source later. GB300's timing exposed the race: the GPU sometimes read memory after the CPU vector was gone.
The first real hybrid-scan failure was
HybridScanFiltersTest.FilterRowGroupsWithDictionary. Its temporary list of dictionary-result pointers had this lifetime problem. The later hybrid-scan failures happened after the first illegal access had already damaged the CUDA context.The new API gives callers an explicit choice:
The later
STREAM_MERGE_TESTwrong-stream exception was emitted by the test memory-resource guard, not the CUDA API callback checker. The same GB300 job passed all 120 C++ test binaries when rerun without changing the commit, and the exact merge case did not fail in 4,000 local stress runs. That exception is therefore intermittent and is not evidence that the new copy API selected the wrong stream. During the audit, however, merge was found to have two real host-source lifetime gaps; both now use the consume-source API.Validation
Built and installed latest
mainplus this patch in the CUDA 13.3 devcontainer for one local GH200-compatible architecture (sm90), with sccache enabled.STREAM_MERGE_TEST: 49/49 passed; exactMergeTest.KeysWithNullspassed 100 additional post-fix repetitionsHYBRID_SCAN_TEST: 85/85 passedmainRelated: #23498 and #23517.