Use cuda::stream_ref for core libcudf APIs #23645
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -70,7 +70,7 @@ namespace CUDF_EXPORT cudf { | |
| std::vector<packed_table> contiguous_split( | ||
| cudf::table_view const& input, | ||
| std::vector<size_type> const& splits, | ||
| cuda::stream_ref stream = cudf::get_default_stream(), | ||
| rmm::device_async_resource_ref mr = cudf::get_current_device_resource_ref()); | ||
|
|
||
| namespace detail { | ||
|
|
@@ -155,7 +155,7 @@ class chunked_pack { | |
| explicit chunked_pack( | ||
| cudf::table_view const& input, | ||
| std::size_t user_buffer_size, | ||
| cuda::stream_ref stream = cudf::get_default_stream(), | ||
| rmm::device_async_resource_ref temp_mr = cudf::get_current_device_resource_ref()); | ||
|
Comment on lines
+158
to
159
There was a problem hiding this comment. 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -e
file=$(git ls-files 'cpp/include/cudf/contiguous_split.hpp')
printf '%s\n' "$file"
ast-grep outline "$file"
printf '\n--- relevant documentation and declarations ---\n'
rg -n -C 12 'chunked_pack|cudaMemcpyAsync|contiguous_split|create\(' "$file"
printf '\n--- related declarations/usages ---\n'
rg -n -C 8 'chunked_pack::create|class chunked_pack|struct chunked_pack|create\(.*user_buffer_size|cudaMemcpyAsync' cpp/include cpp/src tests 2>/dev/null | head -n 240Repository: NVIDIA/cudf Length of output: 26307 🏁 Script executed: #!/bin/bash
set -e
printf '%s\n' '--- stream_ref definition and handle accessors ---'
rg -n -C 10 'class stream_ref|struct stream_ref|stream_ref::get|get\(\).*cudaStream_t|cudaStream_t.*get\(\)' cpp/include cpp/src 2>/dev/null | head -n 220
printf '%s\n' '--- all stream declarations in the example/header ---'
rg -n -C 4 '\bstream\b|cuda::stream_ref' cpp/include/cudf/contiguous_split.hpp
printf '%s\n' '--- exact API argument types ---'
sed -n '143,228p' cpp/include/cudf/contiguous_split.hpp
printf '%s\n' '--- implementation forwarding ---'
sed -n '2197,2210p' cpp/src/copying/contiguous_split.cuRepository: NVIDIA/cudf Length of output: 8995 🏁 Script executed: #!/bin/bash
set -e
python3 - <<'PY'
from pathlib import Path
import re
p = Path("cpp/include/cudf/contiguous_split.hpp")
s = p.read_text()
decl = re.search(
r'static std::unique_ptr<chunked_pack> create\(\s*'
r'cudf::table_view const& input,\s*'
r'std::size_t user_buffer_size,\s*'
r'cuda::stream_ref stream\s*=.*?,\s*'
r'rmm::device_async_resource_ref temp_mr\s*=.*?\);',
s,
re.S,
)
assert decl, "chunked_pack::create declaration was not found"
print("create declaration orders arguments as: input, user_buffer_size, stream, temp_mr")
example = s[s.index(" * `@code`{.pseudo}", s.index("class chunked_pack")):
s.index(" * `@endcode`", s.index("class chunked_pack"))]
assert "auto mr = cudf::get_current_device_resource_ref();" in example
assert "auto chunked_packer = cudf::chunked_pack::create(tv, user_buffer_size, mr);" in example
assert "cudaMemcpyDefault,\n * stream);" in example
print("example passes mr as the third create argument and stream as cudaMemcpyAsync's final argument")
print("example declares stream:", bool(re.search(r'\b(stream|auto const stream)\b', example)))
PY
printf '%s\n' '--- repository references to stream_ref handle access ---'
rg -n -C 4 'cuda::stream_ref|stream\.get\(\)|stream\.value\(\)' cpp include 2>/dev/null | head -n 260Repository: NVIDIA/cudf Length of output: 336 🌐 Web query:
💡 Result: In the CUDA Core Compute Libraries (CCCL), cuda::stream_ref is a non-owning wrapper designed to interoperate seamlessly with native CUDA Runtime (cudart) handles [1][2]. The get member function is the standard mechanism for retrieving the underlying native cudaStream_t handle from a cuda::stream_ref object [3][2]. Key details regarding this interoperability include: 1. Retrieval: Calling.get on a cuda::stream_ref returns the managed cudaStream_t handle, allowing you to pass the underlying stream to traditional CUDA Runtime APIs [1][3]. 2. Construction: You can construct a cuda::stream_ref directly from a native cudaStream_t handle [3][2]. Furthermore, cudaStream_t supports implicit conversion to cuda::stream_ref, facilitating easier integration into C++ codebases [3][4]. 3. Lifetime Management: Because cuda::stream_ref is non-owning, it does not manage the lifetime of the underlying stream; the caller must ensure that the stream handle remains valid for the duration of the cuda::stream_ref's usage [3][4]. 4. Ownership Transfer: For scenarios requiring ownership (where the C++ object should manage the stream's lifetime, such as destroying it upon destruction), use the owning counterpart, cuda::stream [1][2]. The owning type supports from_native_handle to take control of a raw handle and release to relinquish that control back to the native CUDA Runtime [1][5][4]. This pattern is consistent across other CCCL runtime types, such as event_ref, providing a unified approach to bridging modern C++ interfaces with legacy CUDA Runtime code [1][4]. Citations:
🏁 Script executed: #!/bin/bash
set -e
python3 - <<'PY'
from pathlib import Path
import re
s = Path("cpp/include/cudf/contiguous_split.hpp").read_text()
start = s.index(" * `@code`{.pseudo}", s.index("class chunked_pack"))
end = s.index("`@endcode`", start)
example = s[start:end]
assert "auto mr = cudf::get_current_device_resource_ref();" in example
assert "auto chunked_packer = cudf::chunked_pack::create(tv, user_buffer_size, mr);" in example
assert "cudaMemcpyDefault,\n * stream);" in example
assert not re.search(r"auto\s+(?:const\s+)?stream\s*=", example)
print("The example has no stream declaration.")
print("The example passes mr as create's third argument.")
print("The example passes stream directly as cudaMemcpyAsync's final argument.")
create = re.search(
r'static std::unique_ptr<chunked_pack> create\(\s*'
r'cudf::table_view const& input,\s*'
r'std::size_t user_buffer_size,\s*'
r'cuda::stream_ref stream\s*=.*?,\s*'
r'rmm::device_async_resource_ref temp_mr\s*=.*?\);',
s,
re.S,
)
assert create
print("The declaration requires the third argument to be cuda::stream_ref and the fourth to be temp_mr.")
PYRepository: NVIDIA/cudf Length of output: 254 🏁 Script executed: #!/bin/bash
set -e
python3 - <<'PY'
from pathlib import Path
import re
s = Path("cpp/include/cudf/contiguous_split.hpp").read_text()
start = s.index(" * `@code`{.pseudo}")
end = s.index("`@endcode`", start)
example = s[start:end]
assert "auto mr = cudf::get_current_device_resource_ref();" in example
assert "auto chunked_packer = cudf::chunked_pack::create(tv, user_buffer_size, mr);" in example
assert "cudaMemcpyDefault,\n * stream);" in example
assert not re.search(r"auto\s+(?:const\s+)?stream\s*=", example)
print("The example has no stream declaration.")
print("The example passes mr as create's third argument.")
print("The example passes stream directly as cudaMemcpyAsync's final argument.")
decl_start = s.index("static std::unique_ptr<chunked_pack> create(")
decl_end = s.index(");", decl_start)
decl = s[decl_start:decl_end]
assert re.search(r"cuda::stream_ref\s+stream", decl)
assert re.search(r"rmm::device_async_resource_ref\s+temp_mr", decl)
print("The declaration requires stream before temp_mr.")
PYRepository: NVIDIA/cudf Length of output: 237 🏁 Script executed: #!/bin/bash
set -e
python3 - <<'PY'
from pathlib import Path
lines = Path("cpp/include/cudf/contiguous_split.hpp").read_text().splitlines()
start = next(i for i, line in enumerate(lines) if "`@code`{.pseudo}" in line)
end = next(i for i in range(start + 1, len(lines)) if "`@endcode`" in lines[i])
print("\n".join(f"{i+1}: {lines[i]}" for i in range(start, end + 1)))
print("--- checks ---")
checks = {
"mr declaration": any("auto mr = cudf::get_current_device_resource_ref();" in x for x in lines[start:end]),
"create call": any("create(tv, user_buffer_size, mr);" in x for x in lines[start:end]),
"cudaMemcpyDefault": any("cudaMemcpyDefault," in x for x in lines[start:end]),
"direct stream": any("stream);" in x for x in lines[start:end]),
"stream declaration": any("auto const stream" in x or "auto stream" in x for x in lines[start:end]),
}
for name, value in checks.items():
print(f"{name}: {value}")
PYRepository: NVIDIA/cudf Length of output: 609 🏁 Script executed: #!/bin/bash
set -e
python3 - <<'PY'
from pathlib import Path
lines = Path("cpp/include/cudf/contiguous_split.hpp").read_text().splitlines()
marker = next(i for i, line in enumerate(lines) if "Create a table_view" in line)
start = max(i for i in range(marker + 1) if "`@code`{.pseudo}" in lines[i])
end = next(i for i in range(marker, len(lines)) if "`@endcode`" in lines[i])
example = lines[start:end]
assert any("auto mr = cudf::get_current_device_resource_ref();" in x for x in example)
assert any("create(tv, user_buffer_size, mr);" in x for x in example)
assert any("cudaMemcpyDefault," in x for x in example)
assert any("stream);" in x for x in example)
assert not any("auto const stream" in x or "auto stream" in x for x in example)
decl_start = next(i for i, line in enumerate(lines) if "static std::unique_ptr<chunked_pack> create(" in line)
decl = "\n".join(lines[decl_start:decl_start + 6])
stream_pos = decl.index("cuda::stream_ref stream")
mr_pos = decl.index("rmm::device_async_resource_ref temp_mr")
assert stream_pos < mr_pos
print("The chunked_pack example has no stream declaration.")
print("The example passes mr as create's third argument.")
print("The example passes stream directly to cudaMemcpyAsync.")
print("chunked_pack::create declares stream before temp_mr.")
PYRepository: NVIDIA/cudf Length of output: 359 Correct the Declare 🤖 Prompt for AI AgentsSource: Coding guidelines |
||
|
|
||
| /** | ||
|
|
@@ -222,7 +222,7 @@ class chunked_pack { | |
| [[nodiscard]] static std::unique_ptr<chunked_pack> create( | ||
| cudf::table_view const& input, | ||
| std::size_t user_buffer_size, | ||
| cuda::stream_ref stream = cudf::get_default_stream(), | ||
| rmm::device_async_resource_ref temp_mr = cudf::get_current_device_resource_ref()); | ||
|
|
||
| private: | ||
|
|
@@ -244,7 +244,7 @@ class chunked_pack { | |
| * and device memory respectively | ||
| */ | ||
| packed_columns pack(cudf::table_view const& input, | ||
| cuda::stream_ref stream = cudf::get_default_stream(), | ||
| rmm::device_async_resource_ref mr = cudf::get_current_device_resource_ref()); | ||
|
|
||
| /** | ||
|
|
@@ -261,7 +261,7 @@ packed_columns pack(cudf::table_view const& input, | |
| */ | ||
| std::size_t packed_size( | ||
| cudf::table_view const& input, | ||
| cuda::stream_ref stream = cudf::get_default_stream(), | ||
| rmm::device_async_resource_ref temp_mr = cudf::get_current_device_resource_ref()); | ||
|
|
||
| /** | ||
|
|
||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
Repository: NVIDIA/cudf
Length of output: 22503
🏁 Script executed:
Repository: NVIDIA/cudf
Length of output: 50367
🏁 Script executed:
Repository: NVIDIA/cudf
Length of output: 21596
🏁 Script executed:
Repository: NVIDIA/cudf
Length of output: 26743
🏁 Script executed:
Repository: NVIDIA/cudf
Length of output: 50367
🏁 Script executed:
Repository: NVIDIA/cudf
Length of output: 5932
Migrate the remaining stream-bearing factory APIs.
Use
cuda::stream_refin the declarations and definitions ofmake_strings_column,make_strings_column_batch,make_structs_column, andcreate_structs_hierarchy. Add non-default-stream coverage to the related tests and benchmarks.🤖 Prompt for AI Agents
Source: Coding guidelines