Skip to content
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.

Sign up for GitHub

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

Adding set_stream for column and column_buffer #12100

Open
wants to merge 5 commits into
base: branch-23.08
Choose a base branch
from
Open

Adding set_stream for column and column_buffer #12100

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
937b798
adding set_stream for column and column_buffer
hyperbolic2346 Nov 9, 2022
6434bf9
Merge remote-tracking branch 'upstream/branch-23.08' into mwilson/add…
hyperbolic2346 Jul 3, 2023
9a1c5f9
updating from strings change
hyperbolic2346 Jul 3, 2023
1316ff0
pulling body from header
hyperbolic2346 Jul 3, 2023
34fbefc
adding parameter documentation
hyperbolic2346 Jul 4, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions cpp/include/cudf/column/column.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -347,6 +347,18 @@ class column {
*/
operator mutable_column_view() { return this->mutable_view(); };

/**
* @brief Sets the stream to be used for deallocation of internal buffers
*/
void set_stream(rmm::cuda_stream_view stream) noexcept
{
_data.set_stream(stream);
_null_mask.set_stream(stream);
for (auto& child : _children) {
child->set_stream(stream);
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would prefer the function definition be put into cpp/src/column/column.cu instead of in the header.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has changed to a templated function, but I pulled out the child functions.


private:
cudf::data_type _type{type_id::EMPTY}; ///< Logical type of elements in the column
cudf::size_type _size{}; ///< The number of elements in the column
Expand Down
13 changes: 13 additions & 0 deletions cpp/src/io/utilities/column_buffer.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,19 @@ struct column_buffer {

auto& null_count() { return _null_count; }

/**
* @brief Sets the stream to be used for deallocation of internal buffers
*/
void set_stream(rmm::cuda_stream_view stream) noexcept
{
if (_strings) _strings->set_stream(stream);
_data.set_stream(stream);
_null_mask.set_stream(stream);
for (auto& child : children) {
child.set_stream(stream);
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here. Move the function definition into cpp/src/io/utilities/column_buffer.cpp if possible.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

set_stream() still seems to be defined in the header column_buffer.hpp?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure how that happened. VS Code shows the file without the definition, but the local filesystem still had it. Updated via my old friend, vi.


std::unique_ptr<rmm::device_uvector<string_index_pair>> _strings;
rmm::device_buffer _data{};
rmm::device_buffer _null_mask{};
Expand Down