From 9c3a71bb5ce08987bcade034de2b2fb69aff5f4d Mon Sep 17 00:00:00 2001 From: fis Date: Fri, 18 Feb 2022 04:51:07 +0800 Subject: [PATCH 1/5] Implement span storage optimization. --- cpp/include/raft/detail/span.hpp | 25 +++++++++++++++++++++++++ cpp/include/raft/span.hpp | 16 ++++++++-------- 2 files changed, 33 insertions(+), 8 deletions(-) diff --git a/cpp/include/raft/detail/span.hpp b/cpp/include/raft/detail/span.hpp index aa598caf32..d87c622f19 100644 --- a/cpp/include/raft/detail/span.hpp +++ b/cpp/include/raft/detail/span.hpp @@ -86,5 +86,30 @@ __host__ __device__ constexpr auto lexicographical_compare(InputIt1 first1, } return first1 == last1 && first2 != last2; } + +template +struct span_storage { + private: + T* ptr_{nullptr}; + + public: + constexpr span_storage() = default; + constexpr span_storage(T* ptr, SizeType) : ptr_{ptr} {} + [[nodiscard]] constexpr auto size() const -> std::size_t { return Extent; } + constexpr auto data() const -> T* { return ptr_; } +}; + +template +struct span_storage { + private: + T* ptr_{nullptr}; + SizeType size_{0}; + + public: + constexpr span_storage() = default; + constexpr span_storage(T* ptr, SizeType size) : ptr_{ptr}, size_{size} {} + [[nodiscard]] constexpr auto size() const -> SizeType { return size_; } + constexpr auto data() const -> T* { return ptr_; } +}; } // namespace detail } // namespace raft diff --git a/cpp/include/raft/span.hpp b/cpp/include/raft/span.hpp index 389a6a2177..3173dd2a68 100644 --- a/cpp/include/raft/span.hpp +++ b/cpp/include/raft/span.hpp @@ -59,7 +59,7 @@ class span { /** * @brief Constructs a span that is a view over the range [first, first + count); */ - constexpr span(pointer ptr, size_type count) noexcept : size_(count), data_(ptr) + constexpr span(pointer ptr, size_type count) noexcept : storage_{ptr, count} { assert(!(Extent != dynamic_extent && count != Extent)); assert(ptr || count == 0); @@ -67,7 +67,8 @@ class span { /** * @brief Constructs a span that is a view over the range [first, last) */ - constexpr span(pointer first, pointer last) noexcept : size_(last - first), data_(first) + constexpr span(pointer first, pointer last) noexcept + : storage_{first, static_cast(thrust::distance(first, last))} { assert(data_ || size_ == 0); } @@ -75,7 +76,7 @@ class span { * @brief Constructs a span that is a view over the array arr. */ template - constexpr span(element_type (&arr)[N]) noexcept : size_(N), data_(&arr[0]) + constexpr span(element_type (&arr)[N]) noexcept : storage_{&arr[0], N} { } @@ -89,7 +90,7 @@ class span { detail::is_allowed_element_type_conversion_t::value && detail::is_allowed_extent_conversion_t::value>> constexpr span(const span& other) noexcept - : size_(other.size()), data_(other.data()) + : storage_{other.data(), other.size()} { } @@ -139,10 +140,10 @@ class span { return data()[_idx]; } - constexpr auto data() const noexcept -> pointer { return data_; } + constexpr auto data() const noexcept -> pointer { return storage_.data(); } // Observers - [[nodiscard]] constexpr auto size() const noexcept -> size_type { return size_; } + [[nodiscard]] constexpr auto size() const noexcept -> size_type { return storage_.size(); } [[nodiscard]] constexpr auto size_bytes() const noexcept -> size_type { return size() * sizeof(T); @@ -197,8 +198,7 @@ class span { } private: - size_type size_{0}; - pointer data_{nullptr}; + detail::span_storage storage_; }; /** From cb9da021d3eafe50b8ace681bea105fb3e82b2b4 Mon Sep 17 00:00:00 2001 From: fis Date: Fri, 18 Feb 2022 21:32:05 +0800 Subject: [PATCH 2/5] consistency. --- cpp/include/raft/detail/span.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/include/raft/detail/span.hpp b/cpp/include/raft/detail/span.hpp index d87c622f19..cfb313d77b 100644 --- a/cpp/include/raft/detail/span.hpp +++ b/cpp/include/raft/detail/span.hpp @@ -95,7 +95,7 @@ struct span_storage { public: constexpr span_storage() = default; constexpr span_storage(T* ptr, SizeType) : ptr_{ptr} {} - [[nodiscard]] constexpr auto size() const -> std::size_t { return Extent; } + [[nodiscard]] constexpr auto size() const -> SizeType { return Extent; } constexpr auto data() const -> T* { return ptr_; } }; From 8998252d5548d9dffebce878588ffeb41a47627b Mon Sep 17 00:00:00 2001 From: fis Date: Fri, 18 Feb 2022 23:11:29 +0800 Subject: [PATCH 3/5] Address reviewer's comments. * Delegate the ctor. * Use std::size_t consistently. --- cpp/include/raft/detail/span.hpp | 24 ++++++++++++------------ cpp/include/raft/span.hpp | 8 ++++---- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/cpp/include/raft/detail/span.hpp b/cpp/include/raft/detail/span.hpp index cfb313d77b..4ef2e957ea 100644 --- a/cpp/include/raft/detail/span.hpp +++ b/cpp/include/raft/detail/span.hpp @@ -87,29 +87,29 @@ __host__ __device__ constexpr auto lexicographical_compare(InputIt1 first1, return first1 == last1 && first2 != last2; } -template +template struct span_storage { private: T* ptr_{nullptr}; public: - constexpr span_storage() = default; - constexpr span_storage(T* ptr, SizeType) : ptr_{ptr} {} - [[nodiscard]] constexpr auto size() const -> SizeType { return Extent; } - constexpr auto data() const -> T* { return ptr_; } + constexpr span_storage() noexcept = default; + constexpr span_storage(T* ptr, std::size_t) noexcept : ptr_{ptr} {} + [[nodiscard]] constexpr auto size() const noexcept -> std::size_t { return Extent; } + constexpr auto data() const noexcept -> T* { return ptr_; } }; -template -struct span_storage { +template +struct span_storage { private: T* ptr_{nullptr}; - SizeType size_{0}; + std::size_t size_{0}; public: - constexpr span_storage() = default; - constexpr span_storage(T* ptr, SizeType size) : ptr_{ptr}, size_{size} {} - [[nodiscard]] constexpr auto size() const -> SizeType { return size_; } - constexpr auto data() const -> T* { return ptr_; } + constexpr span_storage() noexcept = default; + constexpr span_storage(T* ptr, std::size_t size) noexcept : ptr_{ptr}, size_{size} {} + [[nodiscard]] constexpr auto size() const noexcept -> std::size_t { return size_; } + constexpr auto data() const noexcept -> T* { return ptr_; } }; } // namespace detail } // namespace raft diff --git a/cpp/include/raft/span.hpp b/cpp/include/raft/span.hpp index 3173dd2a68..d6cd332ee1 100644 --- a/cpp/include/raft/span.hpp +++ b/cpp/include/raft/span.hpp @@ -68,7 +68,7 @@ class span { * @brief Constructs a span that is a view over the range [first, last) */ constexpr span(pointer first, pointer last) noexcept - : storage_{first, static_cast(thrust::distance(first, last))} + : span{first, static_cast(thrust::distance(first, last))} { assert(data_ || size_ == 0); } @@ -76,7 +76,7 @@ class span { * @brief Constructs a span that is a view over the array arr. */ template - constexpr span(element_type (&arr)[N]) noexcept : storage_{&arr[0], N} + constexpr span(element_type (&arr)[N]) noexcept : span{&arr[0], N} { } @@ -90,7 +90,7 @@ class span { detail::is_allowed_element_type_conversion_t::value && detail::is_allowed_extent_conversion_t::value>> constexpr span(const span& other) noexcept - : storage_{other.data(), other.size()} + : span{other.data(), other.size()} { } @@ -198,7 +198,7 @@ class span { } private: - detail::span_storage storage_; + detail::span_storage storage_; }; /** From 444a1d7b144ea3abe3fda798fcb3f84d3e874fb4 Mon Sep 17 00:00:00 2001 From: fis Date: Fri, 18 Feb 2022 23:20:35 +0800 Subject: [PATCH 4/5] No discard for data. --- cpp/include/raft/detail/span.hpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cpp/include/raft/detail/span.hpp b/cpp/include/raft/detail/span.hpp index 4ef2e957ea..8a26a33247 100644 --- a/cpp/include/raft/detail/span.hpp +++ b/cpp/include/raft/detail/span.hpp @@ -96,7 +96,7 @@ struct span_storage { constexpr span_storage() noexcept = default; constexpr span_storage(T* ptr, std::size_t) noexcept : ptr_{ptr} {} [[nodiscard]] constexpr auto size() const noexcept -> std::size_t { return Extent; } - constexpr auto data() const noexcept -> T* { return ptr_; } + [[nodiscard]] constexpr auto data() const noexcept -> T* { return ptr_; } }; template @@ -109,7 +109,7 @@ struct span_storage { constexpr span_storage() noexcept = default; constexpr span_storage(T* ptr, std::size_t size) noexcept : ptr_{ptr}, size_{size} {} [[nodiscard]] constexpr auto size() const noexcept -> std::size_t { return size_; } - constexpr auto data() const noexcept -> T* { return ptr_; } + [[nodiscard]] constexpr auto data() const noexcept -> T* { return ptr_; } }; } // namespace detail } // namespace raft From 164c249aade68accb20e30a3e5ff7871f45c4d1f Mon Sep 17 00:00:00 2001 From: fis Date: Fri, 18 Feb 2022 23:47:15 +0800 Subject: [PATCH 5/5] Remove unneeded assert. --- cpp/include/raft/span.hpp | 1 - 1 file changed, 1 deletion(-) diff --git a/cpp/include/raft/span.hpp b/cpp/include/raft/span.hpp index d6cd332ee1..b4fbf5b63a 100644 --- a/cpp/include/raft/span.hpp +++ b/cpp/include/raft/span.hpp @@ -70,7 +70,6 @@ class span { constexpr span(pointer first, pointer last) noexcept : span{first, static_cast(thrust::distance(first, last))} { - assert(data_ || size_ == 0); } /** * @brief Constructs a span that is a view over the array arr.