From c9bcbe9535ddf218df8c878c17e18dbe97da0c5c Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Wed, 3 Jul 2024 00:06:22 +0000 Subject: [PATCH] Simplify and centralize the column_view changes --- cpp/include/cudf/column/column_view.hpp | 153 +++++++----------------- cpp/include/cudf/utilities/prefetch.hpp | 5 + cpp/src/column/column_view.cpp | 77 ++++++------ 3 files changed, 87 insertions(+), 148 deletions(-) diff --git a/cpp/include/cudf/column/column_view.hpp b/cpp/include/cudf/column/column_view.hpp index 309ce62045e..8de9af50d52 100644 --- a/cpp/include/cudf/column/column_view.hpp +++ b/cpp/include/cudf/column/column_view.hpp @@ -24,7 +24,6 @@ #include #include -#include #include #include @@ -75,7 +74,7 @@ class column_view_base { CUDF_ENABLE_IF(std::is_same_v or is_rep_layout_compatible())> T const* head() const noexcept { - return static_cast(_data); + return static_cast(get_data()); } /** @@ -228,6 +227,17 @@ class column_view_base { [[nodiscard]] size_type offset() const noexcept { return _offset; } protected: + /** + * @brief Returns pointer to the base device memory allocation. + * + * The primary purpose of this function is to allow derived classes to + * override the fundamental properties of memory accesses without needing to + * change all of the different accessors for the underlying pointer. + * + * @return Typed pointer to underlying data + */ + virtual void const* get_data() const noexcept { return _data; } + data_type _type{type_id::EMPTY}; ///< Element type size_type _size{}; ///< Number of elements void const* _data{}; ///< Pointer to device memory containing elements @@ -239,7 +249,7 @@ class column_view_base { ///< Enables zero-copy slicing column_view_base() = default; - ~column_view_base() = default; + virtual ~column_view_base() = default; column_view_base(column_view_base const&) = default; ///< Copy constructor column_view_base(column_view_base&&) = default; ///< Move constructor /** @@ -288,27 +298,6 @@ class column_view_base { }; } // namespace detail -class column_view; -class mutable_column_view; - -namespace detail { -/** - * @brief Prefetches the specified column view's data. - * - * @param col The column view to prefetch - * @param data_ptr The pointer to the data to prefetch - */ -void prefetch_column_view(column_view const& col, void const* data_ptr); - -/** - * @brief Prefetches the specified mutable column view's data. - * - * @param col The column view to prefetch - * @param data_ptr The pointer to the data to prefetch - */ -void prefetch_column_view(mutable_column_view const& col, void* data_ptr); -} // namespace detail - /** * @brief A non-owning, immutable view of device data as a column of elements, * some of which may be null as indicated by a bitmask. @@ -342,7 +331,7 @@ class column_view : public detail::column_view_base { #ifdef __CUDACC__ #pragma nv_exec_check_disable #endif - ~column_view() = default; + ~column_view() override = default; #ifdef __CUDACC__ #pragma nv_exec_check_disable #endif @@ -466,81 +455,18 @@ class column_view : public detail::column_view_base { return device_span(data(), size()); } - // TODO May have to make head virtual eventually to make this override work - // everywhere, but for now just copy-pasting in the relevant other functions - // (data/begin/end). + protected: + // TODO: Fix noexcept, see notes in prefetch.hpp /** - * @brief Returns pointer to the base device memory allocation casted to - * the specified type. - * - * This function will only participate in overload resolution if `is_rep_layout_compatible()` - * or `std::is_same_v` are true. - * - * @note If `offset() == 0`, then `head() == data()` + * @brief Returns pointer to the base device memory allocation. * - * @note It should be rare to need to access the `head()` allocation of a - * column, and instead, accessing the elements should be done via `data()`. + * The primary purpose of this function is to allow derived classes to + * override the fundamental properties of memory accesses without needing to + * change all of the different accessors for the underlying pointer. * - * @tparam The type to cast to * @return Typed pointer to underlying data */ - template or is_rep_layout_compatible())> - T const* head() const noexcept - { - detail::prefetch_column_view(*this, _data); - return static_cast(_data); - } - - /** - * @brief Returns the underlying data casted to the specified type, plus the - * offset. - * - * @note If `offset() == 0`, then `head() == data()` - * - * This function does not participate in overload resolution if `is_rep_layout_compatible` is - * false. - * - * @tparam T The type to cast to - * @return Typed pointer to underlying data, including the offset - */ - template ())> - T const* data() const noexcept - { - return head() + _offset; - } - - /** - * @brief Return first element (accounting for offset) after underlying data - * is casted to the specified type. - * - * This function does not participate in overload resolution if `is_rep_layout_compatible` is - * false. - * - * @tparam T The desired type - * @return Pointer to the first element after casting - */ - template ())> - T const* begin() const noexcept - { - return data(); - } - - /** - * @brief Return one past the last element after underlying data is casted to - * the specified type. - * - * This function does not participate in overload resolution if `is_rep_layout_compatible` is - * false. - * - * @tparam T The desired type - * @return Pointer to one past the last element after casting - */ - template ())> - T const* end() const noexcept - { - return begin() + size(); - } + void const* get_data() const noexcept override; private: friend column_view bit_cast(column_view const& input, data_type type); @@ -573,7 +499,7 @@ class mutable_column_view : public detail::column_view_base { public: mutable_column_view() = default; - ~mutable_column_view(){ + ~mutable_column_view() override{ // Needed so that the first instance of the implicit destructor for any TU isn't 'constructed' // from a host+device function marking the implicit version also as host+device }; @@ -645,36 +571,30 @@ class mutable_column_view : public detail::column_view_base { CUDF_ENABLE_IF(std::is_same_v or is_rep_layout_compatible())> T* head() const noexcept { - detail::prefetch_column_view(*this, _data); - // Reusing the parent head method is preferred and should be reverted to - // once we standardize on a prefetching strategy. For now the two are - // separated to avoid multiple prefetches when this method is called and to - // allow independent control over prefetching. - // return const_cast(detail::column_view_base::head()); - return const_cast(static_cast(_data)); + return const_cast(detail::column_view_base::head()); } /** * @brief Returns the underlying data casted to the specified type, plus the * offset. * - * @note If `offset() == 0`, then `head() == data()` - * * This function does not participate in overload resolution if `is_rep_layout_compatible` is * false. * + * @note If `offset() == 0`, then `head() == data()` + * * @tparam T The type to cast to * @return Typed pointer to underlying data, including the offset */ template ())> T* data() const noexcept { - return head() + _offset; + return const_cast(detail::column_view_base::data()); } /** - * @brief Return first element (accounting for offset) after underlying data - * is casted to the specified type. + * @brief Return first element (accounting for offset) after underlying data is + * casted to the specified type. * * This function does not participate in overload resolution if `is_rep_layout_compatible` is * false. @@ -685,7 +605,7 @@ class mutable_column_view : public detail::column_view_base { template ())> T* begin() const noexcept { - return data(); + return const_cast(detail::column_view_base::begin()); } /** @@ -701,7 +621,7 @@ class mutable_column_view : public detail::column_view_base { template ())> T* end() const noexcept { - return begin() + size(); + return const_cast(detail::column_view_base::end()); } /** @@ -766,6 +686,19 @@ class mutable_column_view : public detail::column_view_base { */ operator column_view() const; + protected: + // TODO: Fix noexcept, see info in prefetch.hpp + /** + * @brief Returns pointer to the base device memory allocation. + * + * The primary purpose of this function is to allow derived classes to + * override the fundamental properties of memory accesses without needing to + * change all of the different accessors for the underlying pointer. + * + * @return Typed pointer to underlying data + */ + void const* get_data() const noexcept override; + private: friend mutable_column_view bit_cast(mutable_column_view const& input, data_type type); diff --git a/cpp/include/cudf/utilities/prefetch.hpp b/cpp/include/cudf/utilities/prefetch.hpp index 3db52a24a9f..d95649bb267 100644 --- a/cpp/include/cudf/utilities/prefetch.hpp +++ b/cpp/include/cudf/utilities/prefetch.hpp @@ -65,6 +65,11 @@ class PrefetchConfig { std::map config_values; //< Map of configuration keys to values }; +// TODO: This function is currently called in noexcept contexts, so it should +// not throw exceptions, but currently it can. We need to decide what the +// appropriate behavior is: either we make those contexts (e.g. the data +// accessors in column_view) not noexcept, or we make this function noexcept +// and have it silently allow failures to prefetch. /** * @brief Enable prefetching for a particular structure or algorithm. * diff --git a/cpp/src/column/column_view.cpp b/cpp/src/column/column_view.cpp index 153919969d7..96bb18fdf3b 100644 --- a/cpp/src/column/column_view.cpp +++ b/cpp/src/column/column_view.cpp @@ -29,6 +29,7 @@ #include #include #include +#include #include namespace cudf { @@ -129,44 +130,6 @@ bool is_shallow_equivalent(column_view const& lhs, column_view const& rhs) return shallow_equivalent_impl(lhs, rhs); } -void prefetch_column_view(column_view const& col, void const* data_ptr) -{ - std::string key{"column_view::head"}; - if (cudf::experimental::prefetch::detail::PrefetchConfig::instance().get(key)) { - if (cudf::is_fixed_width(col.type())) { - cudf::experimental::prefetch::detail::prefetch( - key, data_ptr, col.size() * size_of(col.type())); - } else if (col.type().id() == type_id::STRING) { - strings_column_view scv{col}; - - cudf::experimental::prefetch::detail::prefetch( - key, data_ptr, scv.chars_size(cudf::get_default_stream()) * sizeof(char)); - } else { - std::cout << "prefetch_column_view: Unsupported type: " - << static_cast(col.type().id()) << std::endl; - } - } -} - -void prefetch_column_view(mutable_column_view const& col, void* data_ptr) -{ - std::string key{"mutable_column_view::head"}; - if (cudf::experimental::prefetch::detail::PrefetchConfig::instance().get(key)) { - if (cudf::is_fixed_width(col.type())) { - cudf::experimental::prefetch::detail::prefetch( - key, data_ptr, col.size() * size_of(col.type())); - } else if (col.type().id() == type_id::STRING) { - strings_column_view scv{col}; - - cudf::experimental::prefetch::detail::prefetch( - key, data_ptr, scv.chars_size(cudf::get_default_stream()) * sizeof(char)); - } else { - std::cout << "prefetch_column_view (mutable): Unsupported type: " - << static_cast(col.type().id()) << std::endl; - } - } -} - } // namespace detail // Immutable view constructor @@ -216,6 +179,44 @@ mutable_column_view::operator column_view() const return column_view{_type, _size, _data, _null_mask, _null_count, _offset, std::move(child_views)}; } +void const* column_view::get_data() const noexcept +{ + std::string key{"column_view::get_data"}; + if (cudf::experimental::prefetch::detail::PrefetchConfig::instance().get(key)) { + if (cudf::is_fixed_width(type())) { + cudf::experimental::prefetch::detail::prefetch(key, _data, size() * size_of(type())); + } else if (type().id() == type_id::STRING) { + strings_column_view scv{*this}; + + cudf::experimental::prefetch::detail::prefetch( + key, _data, scv.chars_size(cudf::get_default_stream()) * sizeof(char)); + } else { + std::cout << "column_view::get_data: Unsupported type: " << static_cast(type().id()) + << std::endl; + } + } + return _data; +} + +void const* mutable_column_view::get_data() const noexcept +{ + std::string key{"mutable_column_view::get_data"}; + if (cudf::experimental::prefetch::detail::PrefetchConfig::instance().get(key)) { + if (cudf::is_fixed_width(type())) { + cudf::experimental::prefetch::detail::prefetch(key, _data, size() * size_of(type())); + } else if (type().id() == type_id::STRING) { + strings_column_view scv{*this}; + + cudf::experimental::prefetch::detail::prefetch( + key, _data, scv.chars_size(cudf::get_default_stream()) * sizeof(char)); + } else { + std::cout << "mutable_column_view::get_data: Unsupported type: " + << static_cast(type().id()) << std::endl; + } + } + return _data; +} + size_type count_descendants(column_view parent) { auto descendants = [](auto const& child) { return count_descendants(child); };