-
Notifications
You must be signed in to change notification settings - Fork 1.1k
[cudf] Thread memory resources through equality/hash row preprocessing #23665
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 all commits
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 |
|---|---|---|
| @@ -1,5 +1,5 @@ | ||
| /* | ||
| * SPDX-FileCopyrightText: Copyright (c) 2022-2026, NVIDIA CORPORATION. | ||
| * SPDX-FileCopyrightText: Copyright (c) 2022-2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved. | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
|
|
@@ -146,7 +146,8 @@ void apply_struct_equality_op(mutable_column_view& out, | |
|
|
||
| auto tlhs = table_view{{lhs}}; | ||
| auto trhs = table_view{{rhs}}; | ||
| auto table_comparator = cudf::detail::row::equality::two_table_comparator{tlhs, trhs, stream}; | ||
| auto table_comparator = cudf::detail::row::equality::two_table_comparator{ | ||
| tlhs, trhs, stream, cudf::get_current_device_resource_ref()}; | ||
|
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 used more than once in the function — worth extracting a local? |
||
|
|
||
| auto outd = column_device_view::create(out, stream); | ||
| auto optional_iter = | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -41,8 +41,9 @@ std::unique_ptr<column> decode(dictionary_column_view const& source, | |
| if (source.is_empty()) return make_empty_column(type_id::EMPTY); | ||
|
|
||
| // annotated indices include the offset, size and bitmask from it's parent | ||
| auto const indices = source.get_indices_annotated(); | ||
| auto const d_indices = column_device_view::create(indices, stream); | ||
| auto const indices = source.get_indices_annotated(); | ||
| auto const d_indices = | ||
| column_device_view::create(indices, stream, cudf::get_current_device_resource_ref()); | ||
|
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. Nit: you're explicitly replacing the parameter by its default value… How is this related to the purposes of this PR? |
||
| auto const d_iterator = cudf::detail::indexalator_factory::make_input_iterator(indices); | ||
| auto const indices_begin = cudf::detail::make_counting_transform_iterator( | ||
| 0, indices_handler_fn{d_iterator, *d_indices, source.keys().size()}); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -187,8 +187,9 @@ std::unique_ptr<column> concatenate(host_span<column_view const> columns, | |
| cudf::detail::row::hash::device_row_hasher<cudf::hashing::detail::default_hash, | ||
| cudf::nullate::NO>>; | ||
| auto const tv = cudf::table_view({all_keys->view()}); | ||
| auto const row_hash = cudf::detail::row::hash::row_hasher(tv, stream); | ||
| auto const row_equal = cudf::detail::row::equality::self_comparator(tv, stream); | ||
| auto const temp_mr = cudf::get_current_device_resource_ref(); | ||
|
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. Propagate into other uses in this function (e.g., lines 198, 204, 208, 213, etc)? |
||
| auto const row_hash = cudf::detail::row::hash::row_hasher(tv, stream, temp_mr); | ||
| auto const row_equal = cudf::detail::row::equality::self_comparator(tv, stream, temp_mr); | ||
| auto const comparator = cudf::detail::row::equality::nan_equal_physical_equality_comparator{}; | ||
| auto const d_equal = | ||
| row_equal.equal_to<false>(cudf::nullate::NO{}, null_equality::EQUAL, comparator); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -50,8 +50,9 @@ struct unique_keys_dispatch_fn { | |
|
|
||
| auto const has_nulls = nullate::DYNAMIC{false}; | ||
| auto const keys_tv = table_view({all_keys}); | ||
| auto const row_hash = cudf::detail::row::hash::row_hasher(keys_tv, stream); | ||
| auto const row_equal = cudf::detail::row::equality::self_comparator(keys_tv, stream); | ||
| auto const temp_mr = cudf::get_current_device_resource_ref(); | ||
|
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. Should this also flow into the |
||
| auto const row_hash = cudf::detail::row::hash::row_hasher(keys_tv, stream, temp_mr); | ||
| auto const row_equal = cudf::detail::row::equality::self_comparator(keys_tv, stream, temp_mr); | ||
| auto const comparator = cudf::detail::row::equality::nan_equal_physical_equality_comparator{}; | ||
| auto const d_equal = row_equal.equal_to<false>(has_nulls, null_equality::EQUAL, comparator); | ||
| auto const empty_key = cuco::empty_key{cudf::detail::CUDF_SIZE_TYPE_SENTINEL}; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,5 @@ | ||
| /* | ||
| * SPDX-FileCopyrightText: Copyright (c) 2020-2026, NVIDIA CORPORATION. | ||
| * SPDX-FileCopyrightText: Copyright (c) 2020-2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved. | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
|
|
@@ -79,7 +79,8 @@ std::unique_ptr<column> group_nunique(column_view const& values, | |
| if (num_groups == 0) { return result; } | ||
|
|
||
| auto const values_view = table_view{{values}}; | ||
| auto const comparator = cudf::detail::row::equality::self_comparator{values_view, stream}; | ||
| auto const comparator = cudf::detail::row::equality::self_comparator{ | ||
| values_view, stream, cudf::get_current_device_resource_ref()}; | ||
|
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 used more than once in the function — worth extracting a local? |
||
|
|
||
| auto const d_values_view = column_device_view::create(values, stream); | ||
|
|
||
|
|
||
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.
[Optional] Worth adding a default value, so code could be migrated gradually (aligned with #20780 (comment))? Also in
two_table_comparator, androw_hasher/preprocessed_table::createoutside this file…