Skip to content

Commit

Permalink
Removed mr parameter from inplace bitmask operations (NVIDIA#10805)
Browse files Browse the repository at this point in the history
Closes NVIDIA#10763

This PR removed `mr` parameter from inplace bitmask operations by following [guidance for temporary memory allocation](https://github.com/rapidsai/cudf/blob/branch-22.06/cpp/docs/DEVELOPER_GUIDE.md#temporary-memory).

Authors:
  - Tim (https://github.com/AtlantaPepsi)
  - Karthikeyan (https://github.com/karthikeyann)
  - Yunsong Wang (https://github.com/PointKernel)

Approvers:
  - Yunsong Wang (https://github.com/PointKernel)
  - Karthikeyan (https://github.com/karthikeyann)

URL: rapidsai/cudf#10805
  • Loading branch information
AtlantaPepsi authored May 25, 2022
1 parent dbd2b08 commit df5dc08
Show file tree
Hide file tree
Showing 5 changed files with 18 additions and 28 deletions.
19 changes: 8 additions & 11 deletions cpp/include/cudf/detail/null_mask.cuh
Original file line number Diff line number Diff line change
Expand Up @@ -130,8 +130,7 @@ std::pair<rmm::device_buffer, size_type> bitmask_binop(
masks,
masks_begin_bits,
mask_size_bits,
stream,
mr);
stream);

return std::pair(std::move(dest_mask), null_count);
}
Expand All @@ -146,18 +145,15 @@ std::pair<rmm::device_buffer, size_type> bitmask_binop(
* @param[in] masks_begin_bits The bit offsets from which each mask is to be merged
* @param[in] mask_size_bits The number of bits to be ANDed in each mask
* @param[in] stream CUDA stream used for device memory operations and kernel launches
* @param[in] mr Device memory resource used to allocate the returned device_buffer
* @return size_type Count of set bits
*/
template <typename Binop>
size_type inplace_bitmask_binop(
Binop op,
device_span<bitmask_type> dest_mask,
host_span<bitmask_type const*> masks,
host_span<size_type const> masks_begin_bits,
size_type mask_size_bits,
rmm::cuda_stream_view stream,
rmm::mr::device_memory_resource* mr = rmm::mr::get_current_device_resource())
size_type inplace_bitmask_binop(Binop op,
device_span<bitmask_type> dest_mask,
host_span<bitmask_type const*> masks,
host_span<size_type const> masks_begin_bits,
size_type mask_size_bits,
rmm::cuda_stream_view stream)
{
CUDF_EXPECTS(
std::all_of(masks_begin_bits.begin(), masks_begin_bits.end(), [](auto b) { return b >= 0; }),
Expand All @@ -166,6 +162,7 @@ size_type inplace_bitmask_binop(
CUDF_EXPECTS(std::all_of(masks.begin(), masks.end(), [](auto p) { return p != nullptr; }),
"Mask pointer cannot be null");

rmm::mr::device_memory_resource* mr = rmm::mr::get_current_device_resource();
rmm::device_scalar<size_type> d_counter{0, stream, mr};
rmm::device_uvector<bitmask_type const*> d_masks(masks.size(), stream, mr);
rmm::device_uvector<size_type> d_begin_bits(masks_begin_bits.size(), stream, mr);
Expand Down
15 changes: 6 additions & 9 deletions cpp/include/cudf/detail/null_mask.hpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2019-2021, NVIDIA CORPORATION.
* Copyright (c) 2019-2022, NVIDIA CORPORATION.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -267,16 +267,13 @@ std::pair<rmm::device_buffer, size_type> bitmask_or(
* @param masks_begin_bits The bit offsets from which each mask is to be ANDed
* @param mask_size_bits The number of bits to be ANDed in each mask
* @param stream CUDA stream used for device memory operations and kernel launches
* @param mr Device memory resource used to allocate the returned device_buffer
* @return Count of set bits
*/
cudf::size_type inplace_bitmask_and(
device_span<bitmask_type> dest_mask,
host_span<bitmask_type const*> masks,
host_span<size_type const> masks_begin_bits,
size_type mask_size_bits,
rmm::cuda_stream_view stream,
rmm::mr::device_memory_resource* mr = rmm::mr::get_current_device_resource());
cudf::size_type inplace_bitmask_and(device_span<bitmask_type> dest_mask,
host_span<bitmask_type const*> masks,
host_span<size_type const> masks_begin_bits,
size_type mask_size_bits,
rmm::cuda_stream_view stream);

} // namespace detail

Expand Down
6 changes: 2 additions & 4 deletions cpp/src/bitmask/null_mask.cu
Original file line number Diff line number Diff line change
Expand Up @@ -408,17 +408,15 @@ cudf::size_type inplace_bitmask_and(device_span<bitmask_type> dest_mask,
host_span<bitmask_type const*> masks,
host_span<size_type const> begin_bits,
size_type mask_size,
rmm::cuda_stream_view stream,
rmm::mr::device_memory_resource* mr)
rmm::cuda_stream_view stream)
{
return inplace_bitmask_binop(
[] __device__(bitmask_type left, bitmask_type right) { return left & right; },
dest_mask,
masks,
begin_bits,
mask_size,
stream,
mr);
stream);
}

// Bitwise AND of the masks
Expand Down
3 changes: 1 addition & 2 deletions cpp/src/reductions/simple_segmented.cuh
Original file line number Diff line number Diff line change
Expand Up @@ -200,8 +200,7 @@ std::unique_ptr<column> string_segmented_reduction(column_view const& col,
masks,
begin_bits,
result->size(),
stream,
mr);
stream);
result->set_null_count(result->size() - valid_count);
}
}
Expand Down
3 changes: 1 addition & 2 deletions cpp/src/structs/utilities.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -333,8 +333,7 @@ void superimpose_parent_nulls(bitmask_type const* parent_null_mask,
masks,
begin_bits,
child.size(),
stream,
mr);
stream);
auto const null_count = child.size() - valid_count;
child.set_null_count(null_count);
}
Expand Down

0 comments on commit df5dc08

Please sign in to comment.