Skip to content

Commit

Permalink
address review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
tgujar committed Aug 1, 2024
1 parent 77be694 commit 19944cf
Show file tree
Hide file tree
Showing 8 changed files with 28 additions and 45 deletions.
40 changes: 13 additions & 27 deletions cpp/include/cudf/detail/distinct_hash_join.cuh
Original file line number Diff line number Diff line change
Expand Up @@ -109,33 +109,19 @@ struct distinct_hash_join {
using cuco_storage_type = cuco::storage<1>;

/// Hash table type
using hash_table_type = std::variant<
cuco::static_set<cuco::pair<hash_value_type, rhs_index_type>,
cuco::extent<size_type>,
cuda::thread_scope_device,
comparator_adapter<cudf::experimental::row::equality::
strong_index_comparator_adapter<row_comparator>>,
probing_scheme_type,
cudf::detail::cuco_allocator,
cuco_storage_type>,
cuco::static_set<
cuco::pair<hash_value_type, rhs_index_type>,
cuco::extent<size_type>,
cuda::thread_scope_device,
comparator_adapter<cudf::experimental::row::equality::strong_index_comparator_adapter<
row_comparator_no_nested>>,
probing_scheme_type,
cudf::detail::cuco_allocator,
cuco_storage_type>,
cuco::static_set<
cuco::pair<hash_value_type, rhs_index_type>,
cuco::extent<size_type>,
cuda::thread_scope_device,
comparator_adapter<cudf::experimental::row::equality::strong_index_comparator_adapter<
row_comparator_no_compound>>,
probing_scheme_type,
cudf::detail::cuco_allocator,
cuco_storage_type>>;
template <typename Comparator>
using static_set_with_comparator = cuco::static_set<
cuco::pair<hash_value_type, rhs_index_type>,
cuco::extent<size_type>,
cuda::thread_scope_device,
comparator_adapter<
cudf::experimental::row::equality::strong_index_comparator_adapter<Comparator>>,
probing_scheme_type,
cudf::detail::cuco_allocator,
cuco_storage_type>;
using hash_table_type = std::variant<static_set_with_comparator<row_comparator>,
static_set_with_comparator<row_comparator_no_nested>,
static_set_with_comparator<row_comparator_no_compound>>;

bool _has_nulls; ///< true if nulls are present in either build table or probe table
cudf::null_equality _nulls_equal; ///< whether to consider nulls as equal
Expand Down
2 changes: 1 addition & 1 deletion cpp/include/cudf/table/experimental/row_operators.cuh
Original file line number Diff line number Diff line change
Expand Up @@ -1517,7 +1517,7 @@ class device_row_comparator {
* @brief Compares the specified elements for equality.
*
* is_equality_comparable differs from implementation for std::equality_comparable and considers
* void as and equality comparable type. Thus we need to disable this for when type is void.
* void as an equality comparable type. Thus we need to disable this for when type is void.
*
* @param lhs_element_index The index of the first element
* @param rhs_element_index The index of the second element
Expand Down
17 changes: 7 additions & 10 deletions cpp/src/join/distinct_hash_join.cu
Original file line number Diff line number Diff line change
Expand Up @@ -83,10 +83,9 @@ auto prepare_device_equal(

return std::visit(
[&](auto& comparator) {
return ret_type{
std::in_place_type<
comparator_adapter<typename std::remove_reference<decltype(comparator)>::type>>,
comparator};
return ret_type{std::in_place_type<
comparator_adapter<typename std::remove_reference_t<decltype(comparator)>>>,
comparator};
},
d_comparator);
}
Expand Down Expand Up @@ -157,7 +156,7 @@ distinct_hash_join<HasNested>::distinct_hash_join(cudf::table_view const& build,
cuco::static_set<cuco::pair<hash_value_type, rhs_index_type>,
cuco::extent<size_type>,
cuda::thread_scope_device,
typename std::remove_reference<decltype(comparator_adapter)>::type,
typename std::remove_reference_t<decltype(comparator_adapter)>,
distinct_hash_join::probing_scheme_type,
cudf::detail::cuco_allocator,
distinct_hash_join::cuco_storage_type>;
Expand Down Expand Up @@ -190,8 +189,7 @@ distinct_hash_join<HasNested>::distinct_hash_join(cudf::table_view const& build,
[&](auto&& hasher, auto&& hash_table) {
auto const iter = cudf::detail::make_counting_transform_iterator(
0,
build_keys_fn<typename std::remove_reference<decltype(hasher)>::type, rhs_index_type>{
hasher});
build_keys_fn<typename std::remove_reference_t<decltype(hasher)>, rhs_index_type>{hasher});

size_type const build_table_num_rows{build.num_rows()};
if (this->_nulls_equal == cudf::null_equality::EQUAL or (not cudf::nullable(this->_build))) {
Expand Down Expand Up @@ -253,8 +251,7 @@ distinct_hash_join<HasNested>::inner_join(rmm::cuda_stream_view stream,
[&](auto&& hasher, auto&& hash_table) {
auto const iter = cudf::detail::make_counting_transform_iterator(
0,
build_keys_fn<typename std::remove_reference<decltype(hasher)>::type, lhs_index_type>{
hasher});
build_keys_fn<typename std::remove_reference_t<decltype(hasher)>, lhs_index_type>{hasher});

auto const [probe_indices_end, _] = hash_table.retrieve(iter,
iter + probe_table_num_rows,
Expand Down Expand Up @@ -307,7 +304,7 @@ std::unique_ptr<rmm::device_uvector<size_type>> distinct_hash_join<HasNested>::l
[&](auto&& hasher, auto&& hash_table) {
auto const iter = cudf::detail::make_counting_transform_iterator(
0,
build_keys_fn<typename std::remove_reference<decltype(hasher)>::type, lhs_index_type>{
build_keys_fn<typename std::remove_reference_t<decltype(hasher)>, lhs_index_type>{
hasher});

auto const output_begin =
Expand Down
2 changes: 1 addition & 1 deletion cpp/src/join/mixed_join_kernel_semi_impl.cuh
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
#include "join/join_common_utils.cuh"
#include "join/join_common_utils.hpp"
#include "join/mixed_join_common_utils.cuh"
#include "mixed_join_kernels_semi.cuh"
#include "join/mixed_join_kernels_semi.cuh"

#include <cudf/ast/detail/expression_evaluator.cuh>
#include <cudf/ast/detail/expression_parser.hpp>
Expand Down
2 changes: 1 addition & 1 deletion cpp/src/join/mixed_join_kernels_semi.cu
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
*/

#include "join/mixed_join_common_utils.cuh"
#include "mixed_join_kernel_semi_impl.cuh"
#include "join/mixed_join_kernel_semi_impl.cuh"

namespace cudf {
namespace detail {
Expand Down
2 changes: 1 addition & 1 deletion cpp/src/join/mixed_join_kernels_semi_compound.cu
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
*/

#include "join/mixed_join_common_utils.cuh"
#include "mixed_join_kernel_semi_impl.cuh"
#include "join/mixed_join_kernel_semi_impl.cuh "

namespace cudf {
namespace detail {
Expand Down
2 changes: 1 addition & 1 deletion cpp/src/join/mixed_join_kernels_semi_nested.cu
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
*/

#include "join/mixed_join_common_utils.cuh"
#include "mixed_join_kernel_semi_impl.cuh"
#include "join/mixed_join_kernel_semi_impl.cuh"

namespace cudf {
namespace detail {
Expand Down
6 changes: 3 additions & 3 deletions cpp/src/join/mixed_join_semi.cu
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@
* limitations under the License.
*/

#include "join_common_utils.cuh"
#include "join_common_utils.hpp"
#include "mixed_join_kernels_semi.cuh"
#include "join/join_common_utils.cuh"
#include "join/join_common_utils.hpp"
#include "join/mixed_join_kernels_semi.cuh"

#include <cudf/ast/detail/expression_parser.hpp>
#include <cudf/ast/expressions.hpp>
Expand Down

0 comments on commit 19944cf

Please sign in to comment.