Skip to content

Commit

Permalink
Remove unneeded stream/mr from a cudf::make_strings_column (#9148)
Browse files Browse the repository at this point in the history
During development of the text parser, we found a factory function that accepted a `stream` and `mr` parameters but did not actually need them. This PR removes the parameters and fixes any of the callers that passed these.

Authors:
  - David Wendt (https://github.com/davidwendt)

Approvers:
  - Karthikeyan (https://github.com/karthikeyann)
  - Nghia Truong (https://github.com/ttnghia)

URL: #9148
  • Loading branch information
davidwendt authored Sep 2, 2021
1 parent cc71087 commit f5e870b
Show file tree
Hide file tree
Showing 55 changed files with 91 additions and 268 deletions.
26 changes: 10 additions & 16 deletions cpp/include/cudf/column/column_factories.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -420,27 +420,21 @@ std::unique_ptr<column> make_strings_column(
*
* The columns and mask are moved into the resulting strings column.
*
* @param[in] num_strings The number of strings the column represents.
* @param[in] offsets_column The column of offset values for this column. The number of elements is
* @param num_strings The number of strings the column represents.
* @param offsets_column The column of offset values for this column. The number of elements is
* one more than the total number of strings so the `offset[last] - offset[0]` is the total number
* of bytes in the strings vector.
* @param[in] chars_column The column of char bytes for all the strings for this column. Individual
* @param chars_column The column of char bytes for all the strings for this column. Individual
* strings are identified by the offsets and the nullmask.
* @param[in] null_count The number of null string entries.
* @param[in] null_mask The bits specifying the null strings in device memory. Arrow format for
* @param null_count The number of null string entries.
* @param null_mask The bits specifying the null strings in device memory. Arrow format for
* nulls is used for interpreting this bitmask.
* @param[in] stream CUDA stream used for device memory operations and kernel launches.
* @param[in] mr Device memory resource used for allocation of the column's `null_mask` and children
* columns' device memory.
*/
std::unique_ptr<column> make_strings_column(
size_type num_strings,
std::unique_ptr<column> offsets_column,
std::unique_ptr<column> chars_column,
size_type null_count,
rmm::device_buffer&& null_mask,
rmm::cuda_stream_view stream = rmm::cuda_stream_default,
rmm::mr::device_memory_resource* mr = rmm::mr::get_current_device_resource());
std::unique_ptr<column> make_strings_column(size_type num_strings,
std::unique_ptr<column> offsets_column,
std::unique_ptr<column> chars_column,
size_type null_count,
rmm::device_buffer&& null_mask);

/**
* @brief Construct a STRING type column given offsets, columns, and optional null count and null
Expand Down
4 changes: 1 addition & 3 deletions cpp/include/cudf/strings/detail/copy_if_else.cuh
Original file line number Diff line number Diff line change
Expand Up @@ -111,9 +111,7 @@ std::unique_ptr<cudf::column> copy_if_else(
std::move(offsets_column),
std::move(chars_column),
null_count,
std::move(null_mask),
stream,
mr);
std::move(null_mask));
}

} // namespace detail
Expand Down
4 changes: 1 addition & 3 deletions cpp/include/cudf/strings/detail/copy_range.cuh
Original file line number Diff line number Diff line change
Expand Up @@ -210,9 +210,7 @@ std::unique_ptr<column> copy_range(
std::move(p_offsets_column),
std::move(p_chars_column),
null_count,
std::move(null_mask),
stream,
mr);
std::move(null_mask));
}
}

Expand Down
4 changes: 1 addition & 3 deletions cpp/include/cudf/strings/detail/gather.cuh
Original file line number Diff line number Diff line change
Expand Up @@ -339,9 +339,7 @@ std::unique_ptr<cudf::column> gather(
std::move(out_offsets_column),
std::move(out_chars_column),
0,
rmm::device_buffer{0, stream, mr},
stream,
mr);
rmm::device_buffer{});
}
/**
Expand Down
4 changes: 1 addition & 3 deletions cpp/include/cudf/strings/detail/merge.cuh
Original file line number Diff line number Diff line change
Expand Up @@ -103,9 +103,7 @@ std::unique_ptr<column> merge(strings_column_view const& lhs,
std::move(offsets_column),
std::move(chars_column),
null_count,
std::move(null_mask),
stream,
mr);
std::move(null_mask));
}

} // namespace detail
Expand Down
4 changes: 1 addition & 3 deletions cpp/include/cudf/strings/detail/scatter.cuh
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,7 @@ std::unique_ptr<column> scatter(
std::move(offsets_column),
std::move(chars_column),
UNKNOWN_NULL_COUNT,
cudf::detail::copy_bitmask(target.parent(), stream, mr),
stream,
mr);
cudf::detail::copy_bitmask(target.parent(), stream, mr));
}

} // namespace detail
Expand Down
8 changes: 2 additions & 6 deletions cpp/include/cudf/strings/detail/strings_column_factories.cuh
Original file line number Diff line number Diff line change
Expand Up @@ -131,9 +131,7 @@ std::unique_ptr<column> make_strings_column(IndexPairIterator begin,
std::move(offsets_column),
std::move(chars_column),
null_count,
std::move(null_mask),
stream,
mr);
std::move(null_mask));
}

/**
Expand Down Expand Up @@ -189,9 +187,7 @@ std::unique_ptr<column> make_strings_column(CharIterator chars_begin,
std::move(offsets_column),
std::move(chars_column),
null_count,
std::move(null_mask),
stream,
mr);
std::move(null_mask));
}

} // namespace detail
Expand Down
9 changes: 2 additions & 7 deletions cpp/src/hash/md5_hash.cu
Original file line number Diff line number Diff line change
Expand Up @@ -92,13 +92,8 @@ std::unique_ptr<column> md5_hash(table_view const& input,
hasher.finalize(&hash_state, d_chars + (row_index * 32));
});

return make_strings_column(input.num_rows(),
std::move(offsets_column),
std::move(chars_column),
0,
std::move(null_mask),
stream,
mr);
return make_strings_column(
input.num_rows(), std::move(offsets_column), std::move(chars_column), 0, std::move(null_mask));
}

} // namespace detail
Expand Down
4 changes: 1 addition & 3 deletions cpp/src/interop/from_arrow.cu
Original file line number Diff line number Diff line change
Expand Up @@ -285,9 +285,7 @@ std::unique_ptr<column> dispatch_to_cudf_column::operator()<cudf::string_view>(
std::move(offsets_column),
std::move(chars_column),
UNKNOWN_NULL_COUNT,
std::move(*get_mask_buffer(array, stream, mr)),
stream,
mr);
std::move(*get_mask_buffer(array, stream, mr)));

return num_rows == array.length() ? std::move(out_col)
: std::make_unique<column>(cudf::detail::slice(
Expand Down
4 changes: 1 addition & 3 deletions cpp/src/io/csv/durations.cu
Original file line number Diff line number Diff line change
Expand Up @@ -204,9 +204,7 @@ struct dispatch_from_durations_fn {
std::move(offsets_column),
std::move(chars_column),
durations.null_count(),
std::move(null_mask),
stream,
mr);
std::move(null_mask));
}

// non-duration types throw an exception
Expand Down
4 changes: 1 addition & 3 deletions cpp/src/io/csv/writer_impl.cu
Original file line number Diff line number Diff line change
Expand Up @@ -166,9 +166,7 @@ struct column_to_strings_fn {
std::move(children.first),
std::move(children.second),
column_v.null_count(),
cudf::detail::copy_bitmask(column_v, stream_, mr_),
stream_,
mr_);
cudf::detail::copy_bitmask(column_v, stream_, mr_));
}

// ints:
Expand Down
6 changes: 2 additions & 4 deletions cpp/src/lists/copying/scatter_helper.cu
Original file line number Diff line number Diff line change
Expand Up @@ -279,10 +279,8 @@ struct list_child_constructor {
return cudf::make_strings_column(num_child_rows,
std::move(string_offsets),
std::move(string_chars),
child_null_mask.second, // Null count.
std::move(child_null_mask.first), // Null mask.
stream,
mr);
child_null_mask.second, // Null count.
std::move(child_null_mask.first));
}

/**
Expand Down
4 changes: 1 addition & 3 deletions cpp/src/lists/interleave_columns.cu
Original file line number Diff line number Diff line change
Expand Up @@ -189,9 +189,7 @@ struct interleave_list_entries_fn {
std::move(offsets_column),
std::move(chars_column),
null_count,
std::move(null_mask),
stream,
mr);
std::move(null_mask));
}

template <class T>
Expand Down
4 changes: 1 addition & 3 deletions cpp/src/replace/clamp.cu
Original file line number Diff line number Diff line change
Expand Up @@ -140,9 +140,7 @@ std::unique_ptr<cudf::column> clamp_string_column(strings_column_view const& inp
std::move(offsets_column),
std::move(chars_column),
input.null_count(),
std::move(copy_bitmask(input.parent())),
stream,
mr);
std::move(copy_bitmask(input.parent())));
}

template <typename T, typename OptionalScalarIterator, typename ReplaceScalarIterator>
Expand Down
4 changes: 1 addition & 3 deletions cpp/src/replace/nulls.cu
Original file line number Diff line number Diff line change
Expand Up @@ -270,9 +270,7 @@ std::unique_ptr<cudf::column> replace_nulls_column_kernel_forwarder::operator()<
std::move(offsets),
std::move(output_chars),
input.size() - valid_counter.value(stream),
std::move(valid_bits),
stream,
mr);
std::move(valid_bits));
}

template <>
Expand Down
4 changes: 1 addition & 3 deletions cpp/src/replace/replace.cu
Original file line number Diff line number Diff line change
Expand Up @@ -431,9 +431,7 @@ std::unique_ptr<cudf::column> replace_kernel_forwarder::operator()<cudf::string_
std::move(offsets),
std::move(output_chars),
null_count,
std::move(valid_bits),
stream,
mr);
std::move(valid_bits));
}

template <>
Expand Down
4 changes: 1 addition & 3 deletions cpp/src/reshape/interleave_columns.cu
Original file line number Diff line number Diff line change
Expand Up @@ -207,9 +207,7 @@ struct interleave_columns_impl<T, typename std::enable_if_t<std::is_same_v<T, cu
std::move(offsets_column),
std::move(chars_column),
null_count,
std::move(valid_mask.first),
stream,
mr);
std::move(valid_mask.first));
}
};

Expand Down
4 changes: 1 addition & 3 deletions cpp/src/strings/capitalize.cu
Original file line number Diff line number Diff line change
Expand Up @@ -199,9 +199,7 @@ std::unique_ptr<column> capitalizer(CapitalFn cfn,
std::move(children.first),
std::move(children.second),
input.null_count(),
cudf::detail::copy_bitmask(input.parent(), stream, mr),
stream,
mr);
cudf::detail::copy_bitmask(input.parent(), stream, mr));
}

} // namespace
Expand Down
4 changes: 1 addition & 3 deletions cpp/src/strings/case.cu
Original file line number Diff line number Diff line change
Expand Up @@ -144,9 +144,7 @@ std::unique_ptr<column> convert_case(strings_column_view const& strings,
std::move(children.first),
std::move(children.second),
strings.null_count(),
cudf::detail::copy_bitmask(strings.parent(), stream, mr),
stream,
mr);
cudf::detail::copy_bitmask(strings.parent(), stream, mr));
}

} // namespace
Expand Down
4 changes: 1 addition & 3 deletions cpp/src/strings/char_types/char_types.cu
Original file line number Diff line number Diff line change
Expand Up @@ -182,9 +182,7 @@ std::unique_ptr<column> filter_characters_of_type(strings_column_view const& str
std::move(children.first),
std::move(children.second),
strings.null_count(),
std::move(null_mask),
stream,
mr);
std::move(null_mask));
}

} // namespace detail
Expand Down
8 changes: 2 additions & 6 deletions cpp/src/strings/combine/concatenate.cu
Original file line number Diff line number Diff line change
Expand Up @@ -156,9 +156,7 @@ std::unique_ptr<column> concatenate(table_view const& strings_columns,
std::move(children.first),
std::move(children.second),
null_count,
std::move(null_mask),
stream,
mr);
std::move(null_mask));
}

namespace {
Expand Down Expand Up @@ -254,9 +252,7 @@ std::unique_ptr<column> concatenate(table_view const& strings_columns,
std::move(children.first),
std::move(children.second),
null_count,
std::move(null_mask),
stream,
mr);
std::move(null_mask));
}

} // namespace detail
Expand Down
9 changes: 2 additions & 7 deletions cpp/src/strings/combine/join.cu
Original file line number Diff line number Diff line change
Expand Up @@ -117,13 +117,8 @@ std::unique_ptr<column> join_strings(strings_column_view const& strings,
if ((idx + 1) < d_strings.size()) d_buffer = detail::copy_string(d_buffer, d_separator);
});

return make_strings_column(1,
std::move(offsets_column),
std::move(chars_column),
null_count,
std::move(null_mask),
stream,
mr);
return make_strings_column(
1, std::move(offsets_column), std::move(chars_column), null_count, std::move(null_mask));
}

} // namespace detail
Expand Down
18 changes: 4 additions & 14 deletions cpp/src/strings/combine/join_list_elements.cu
Original file line number Diff line number Diff line change
Expand Up @@ -208,13 +208,8 @@ std::unique_ptr<column> join_list_elements(lists_column_view const& lists_string
stream,
mr);

return make_strings_column(num_rows,
std::move(offsets_column),
std::move(chars_column),
null_count,
std::move(null_mask),
stream,
mr);
return make_strings_column(
num_rows, std::move(offsets_column), std::move(chars_column), null_count, std::move(null_mask));
}

namespace {
Expand Down Expand Up @@ -288,13 +283,8 @@ std::unique_ptr<column> join_list_elements(lists_column_view const& lists_string
stream,
mr);

return make_strings_column(num_rows,
std::move(offsets_column),
std::move(chars_column),
null_count,
std::move(null_mask),
stream,
mr);
return make_strings_column(
num_rows, std::move(offsets_column), std::move(chars_column), null_count, std::move(null_mask));
}

} // namespace detail
Expand Down
4 changes: 1 addition & 3 deletions cpp/src/strings/convert/convert_booleans.cu
Original file line number Diff line number Diff line change
Expand Up @@ -140,9 +140,7 @@ std::unique_ptr<column> from_booleans(column_view const& booleans,
std::move(offsets_column),
std::move(chars_column),
booleans.null_count(),
std::move(null_mask),
stream,
mr);
std::move(null_mask));
}

} // namespace detail
Expand Down
4 changes: 1 addition & 3 deletions cpp/src/strings/convert/convert_datetime.cu
Original file line number Diff line number Diff line change
Expand Up @@ -1069,9 +1069,7 @@ std::unique_ptr<column> from_timestamps(column_view const& timestamps,
std::move(offsets_column),
std::move(chars_column),
timestamps.null_count(),
cudf::detail::copy_bitmask(timestamps, stream, mr),
stream,
mr);
cudf::detail::copy_bitmask(timestamps,stream,mr));
}

} // namespace detail
Expand Down
4 changes: 1 addition & 3 deletions cpp/src/strings/convert/convert_durations.cu
Original file line number Diff line number Diff line change
Expand Up @@ -441,9 +441,7 @@ struct dispatch_from_durations_fn {
std::move(offsets_column),
std::move(chars_column),
durations.null_count(),
std::move(null_mask),
stream,
mr);
std::move(null_mask));
}

// non-duration types throw an exception
Expand Down
4 changes: 1 addition & 3 deletions cpp/src/strings/convert/convert_fixed_point.cu
Original file line number Diff line number Diff line change
Expand Up @@ -296,9 +296,7 @@ struct dispatch_from_fixed_point_fn {
std::move(offsets_column),
std::move(chars_column),
input.null_count(),
cudf::detail::copy_bitmask(input, stream, mr),
stream,
mr);
cudf::detail::copy_bitmask(input, stream, mr));
}

template <typename T, std::enable_if_t<not cudf::is_fixed_point<T>()>* = nullptr>
Expand Down
4 changes: 1 addition & 3 deletions cpp/src/strings/convert/convert_floats.cu
Original file line number Diff line number Diff line change
Expand Up @@ -505,9 +505,7 @@ struct dispatch_from_floats_fn {
std::move(offsets_column),
std::move(chars_column),
floats.null_count(),
std::move(null_mask),
stream,
mr);
std::move(null_mask));
}

// non-float types throw an exception
Expand Down
Loading

0 comments on commit f5e870b

Please sign in to comment.