From f119d401484e6ad51da36d46db47990759374a62 Mon Sep 17 00:00:00 2001 From: Attila Krasznahorkay <30694331+krasznaa@users.noreply.github.com> Date: Wed, 29 Sep 2021 10:28:47 +0200 Subject: [PATCH] Made it possible to copy from const vectors into non-const ones. (#110) In the previous version of the code one would always have to start copies from non-const host containers. Which was very far from ideal... Added explicit tests for making sure that these types of copies would be possible from now on. --- core/include/vecmem/utils/copy.hpp | 47 ++++---- core/include/vecmem/utils/impl/copy.ipp | 152 ++++++++++++++---------- tests/core/test_core_copy.cpp | 65 ++++++++++ 3 files changed, 180 insertions(+), 84 deletions(-) diff --git a/core/include/vecmem/utils/copy.hpp b/core/include/vecmem/utils/copy.hpp index 23225199..0019d938 100644 --- a/core/include/vecmem/utils/copy.hpp +++ b/core/include/vecmem/utils/copy.hpp @@ -16,6 +16,7 @@ // System include(s). #include +#include #include namespace vecmem { @@ -66,14 +67,14 @@ class copy { /// Copy a 1-dimensional vector to the specified memory resource template - data::vector_buffer to(const data::vector_view& data, - memory_resource& resource, - type::copy_type cptype = type::unknown); + data::vector_buffer> to( + const data::vector_view& data, memory_resource& resource, + type::copy_type cptype = type::unknown); /// Copy a 1-dimensional vector's data between two existing memory blocks - template - void operator()(const data::vector_view& from, - data::vector_view& to, + template + void operator()(const data::vector_view& from, + data::vector_view& to, type::copy_type cptype = type::unknown); /// Copy a 1-dimensional vector's data into a vector object @@ -98,40 +99,40 @@ class copy { /// Copy a jagged vector to the specified memory resource template - data::jagged_vector_buffer to( + data::jagged_vector_buffer> to( const data::jagged_vector_view& data, memory_resource& resource, memory_resource* host_access_resource = nullptr, type::copy_type cptype = type::unknown); /// Copy a jagged vector to the specified memory resource template - data::jagged_vector_buffer to( + data::jagged_vector_buffer> to( const data::jagged_vector_buffer& data, memory_resource& resource, memory_resource* host_access_resource = nullptr, type::copy_type cptype = type::unknown); /// Copy a jagged vector's data between two existing allocations - template - void operator()(const data::jagged_vector_view& from, - data::jagged_vector_view& to, + template + void operator()(const data::jagged_vector_view& from, + data::jagged_vector_view& to, type::copy_type cptype = type::unknown); /// Copy a jagged vector's data between two existing allocations - template - void operator()(const data::jagged_vector_view& from, - data::jagged_vector_buffer& to, + template + void operator()(const data::jagged_vector_view& from, + data::jagged_vector_buffer& to, type::copy_type cptype = type::unknown); /// Copy a jagged vector's data between two existing allocations - template - void operator()(const data::jagged_vector_buffer& from, - data::jagged_vector_view& to, + template + void operator()(const data::jagged_vector_buffer& from, + data::jagged_vector_view& to, type::copy_type cptype = type::unknown); /// Copy a jagged vector's data between two existing allocations - template - void operator()(const data::jagged_vector_buffer& from, - data::jagged_vector_buffer& to, + template + void operator()(const data::jagged_vector_buffer& from, + data::jagged_vector_buffer& to, type::copy_type cptype = type::unknown); /// Copy a jagged vector's data into a vector object @@ -167,9 +168,9 @@ class copy { private: /// Helper function performing the copy of a jagged array/vector - template - void copy_views(std::size_t size, const data::vector_view* from, - data::vector_view* to, type::copy_type cptype); + template + void copy_views(std::size_t size, const data::vector_view* from, + data::vector_view* to, type::copy_type cptype); /// Helper function for getting the sizes of a jagged vector/buffer template std::vector::size_type> get_sizes( diff --git a/core/include/vecmem/utils/impl/copy.ipp b/core/include/vecmem/utils/impl/copy.ipp index cb76aa9d..b67c8249 100644 --- a/core/include/vecmem/utils/impl/copy.ipp +++ b/core/include/vecmem/utils/impl/copy.ipp @@ -14,7 +14,6 @@ // System include(s). #include -#include namespace vecmem { @@ -36,16 +35,17 @@ void copy::setup(data::vector_view& data) { } template -data::vector_buffer copy::to(const vecmem::data::vector_view& data, - memory_resource& resource, - type::copy_type cptype) { +data::vector_buffer> copy::to( + const vecmem::data::vector_view& data, memory_resource& resource, + type::copy_type cptype) { // Set up the result buffer. - data::vector_buffer result(data.capacity(), get_size(data), resource); + data::vector_buffer> result( + data.capacity(), get_size(data), resource); setup(result); // Copy the payload of the vector. - this->operator()(data, result, cptype); + this->operator()(data, result, cptype); VECMEM_DEBUG_MSG(2, "Created a vector buffer of type \"%s\" with " "capacity %u", @@ -53,26 +53,32 @@ data::vector_buffer copy::to(const vecmem::data::vector_view& data, return result; } -template -void copy::operator()(const data::vector_view& from_view, - data::vector_view& to_view, +template +void copy::operator()(const data::vector_view& from_view, + data::vector_view& to_view, type::copy_type cptype) { + // The input and output types are allowed to be different, but only by + // const-ness. + static_assert(std::is_same::value || + details::is_same_nc::value, + "Can only use compatible types in the copy"); + // Get the size of the source view. - const typename data::vector_view::size_type size = + const typename data::vector_view::size_type size = get_size(from_view); // Make sure that if the target view is resizable, that it would be set up // for the correct size. if (to_view.size_ptr() != 0) { assert(to_view.capacity() >= size); - do_copy(sizeof(typename data::vector_view::size_type), &size, + do_copy(sizeof(typename data::vector_view::size_type), &size, to_view.size_ptr(), cptype); } // Copy the payload. assert(size == get_size(to_view)); - do_copy(size * sizeof(TYPE), from_view.ptr(), to_view.ptr(), cptype); + do_copy(size * sizeof(TYPE1), from_view.ptr(), to_view.ptr(), cptype); } template @@ -83,8 +89,7 @@ void copy::operator()(const data::vector_view& from_view, // The input and output types are allowed to be different, but only by // const-ness. static_assert(std::is_same::value || - details::is_same_nc::value || - details::is_same_nc::value, + details::is_same_nc::value, "Can only use compatible types in the copy"); // Figure out the size of the buffer. @@ -159,13 +164,13 @@ void copy::setup(data::jagged_vector_buffer& data) { } template -data::jagged_vector_buffer copy::to( +data::jagged_vector_buffer> copy::to( const data::jagged_vector_view& data, memory_resource& resource, memory_resource* host_access_resource, type::copy_type cptype) { // Create the result buffer object. - data::jagged_vector_buffer result(data, resource, - host_access_resource); + data::jagged_vector_buffer> result( + data, resource, host_access_resource); assert(result.m_size == data.m_size); // Copy the description of the "inner vectors" if necessary. @@ -179,13 +184,13 @@ data::jagged_vector_buffer copy::to( } template -data::jagged_vector_buffer copy::to( +data::jagged_vector_buffer> copy::to( const data::jagged_vector_buffer& data, memory_resource& resource, memory_resource* host_access_resource, type::copy_type cptype) { // Create the result buffer object. - data::jagged_vector_buffer result(data, resource, - host_access_resource); + data::jagged_vector_buffer> result( + data, resource, host_access_resource); assert(result.m_size == data.m_size); // Copy the description of the "inner vectors" if necessary. @@ -198,11 +203,17 @@ data::jagged_vector_buffer copy::to( return result; } -template -void copy::operator()(const data::jagged_vector_view& from_view, - data::jagged_vector_view& to_view, +template +void copy::operator()(const data::jagged_vector_view& from_view, + data::jagged_vector_view& to_view, type::copy_type cptype) { + // The input and output types are allowed to be different, but only by + // const-ness. + static_assert(std::is_same::value || + details::is_same_nc::value, + "Can only use compatible types in the copy"); + // A sanity check. assert(from_view.m_size == to_view.m_size); @@ -210,11 +221,17 @@ void copy::operator()(const data::jagged_vector_view& from_view, copy_views(from_view.m_size, from_view.m_ptr, to_view.m_ptr, cptype); } -template -void copy::operator()(const data::jagged_vector_view& from_view, - data::jagged_vector_buffer& to_buffer, +template +void copy::operator()(const data::jagged_vector_view& from_view, + data::jagged_vector_buffer& to_buffer, type::copy_type cptype) { + // The input and output types are allowed to be different, but only by + // const-ness. + static_assert(std::is_same::value || + details::is_same_nc::value, + "Can only use compatible types in the copy"); + // A sanity check. assert(from_view.m_size == to_buffer.m_size); @@ -222,11 +239,17 @@ void copy::operator()(const data::jagged_vector_view& from_view, copy_views(from_view.m_size, from_view.m_ptr, to_buffer.host_ptr(), cptype); } -template -void copy::operator()(const data::jagged_vector_buffer& from_buffer, - data::jagged_vector_view& to_view, +template +void copy::operator()(const data::jagged_vector_buffer& from_buffer, + data::jagged_vector_view& to_view, type::copy_type cptype) { + // The input and output types are allowed to be different, but only by + // const-ness. + static_assert(std::is_same::value || + details::is_same_nc::value, + "Can only use compatible types in the copy"); + // A sanity check. assert(from_buffer.m_size == to_view.m_size); @@ -235,11 +258,17 @@ void copy::operator()(const data::jagged_vector_buffer& from_buffer, cptype); } -template -void copy::operator()(const data::jagged_vector_buffer& from_buffer, - data::jagged_vector_buffer& to_buffer, +template +void copy::operator()(const data::jagged_vector_buffer& from_buffer, + data::jagged_vector_buffer& to_buffer, type::copy_type cptype) { + // The input and output types are allowed to be different, but only by + // const-ness. + static_assert(std::is_same::value || + details::is_same_nc::value, + "Can only use compatible types in the copy"); + // A sanity check. assert(from_buffer.m_size == to_buffer.m_size); @@ -256,8 +285,7 @@ void copy::operator()(const data::jagged_vector_view& from_view, // The input and output types are allowed to be different, but only by // const-ness. static_assert(std::is_same::value || - details::is_same_nc::value || - details::is_same_nc::value, + details::is_same_nc::value, "Can only use compatible types in the copy"); // Resize the output object to the correct size. @@ -281,8 +309,7 @@ void copy::operator()(const data::jagged_vector_buffer& from_buffer, // The input and output types are allowed to be different, but only by // const-ness. static_assert(std::is_same::value || - details::is_same_nc::value || - details::is_same_nc::value, + details::is_same_nc::value, "Can only use compatible types in the copy"); // Resize the output object to the correct size. @@ -314,15 +341,21 @@ std::vector::size_type> copy::get_sizes( return get_sizes(data.host_ptr(), data.m_size); } -template +template void copy::copy_views(std::size_t size, - const data::vector_view* from_view, - data::vector_view* to_view, + const data::vector_view* from_view, + data::vector_view* to_view, type::copy_type cptype) { + // The input and output types are allowed to be different, but only by + // const-ness. + static_assert(std::is_same::value || + details::is_same_nc::value, + "Can only use compatible types in the copy"); + // Helper variables used in the copy. - const TYPE* from_ptr = nullptr; - TYPE* to_ptr = nullptr; + const std::remove_cv_t* from_ptr = nullptr; + TYPE2* to_ptr = nullptr; std::size_t copy_size = 0; [[maybe_unused]] std::size_t copy_ops = 0; @@ -332,24 +365,21 @@ void copy::copy_views(std::size_t size, // Helper lambda for figuring out if the next vector element is // connected to the currently processed one or not. - auto next_is_connected = - [size](const data::vector_view* array, - const std::vector::size_type>& - sizes, - std::size_t i) { - // Check if the next non-empty vector element is connected to the - // current one. - std::size_t j = i + 1; - while (j < size) { - if (sizes[j] == 0) { - ++j; - continue; - } - return ((array[i].ptr() + sizes[i]) == array[j].ptr()); + auto next_is_connected = [size](const auto* array, const auto& sizes, + std::size_t index) { + // Check if the next non-empty vector element is connected to the + // current one. + std::size_t j = index + 1; + while (j < size) { + if (sizes[j] == 0) { + ++j; + continue; } - // If we got here, then the answer is no... - return false; - }; + return ((array[index].ptr() + sizes[index]) == array[j].ptr()); + } + // If we got here, then the answer is no... + return false; + }; // Perform the copy in multiple steps. for (std::size_t i = 0; i < size; ++i) { @@ -369,12 +399,12 @@ void copy::copy_views(std::size_t size, if ((from_ptr == nullptr) && (to_ptr == nullptr) && (copy_size == 0)) { from_ptr = from_view[i].ptr(); to_ptr = to_view[i].ptr(); - copy_size = from_sizes[i] * sizeof(TYPE); + copy_size = from_sizes[i] * sizeof(TYPE1); } else { assert(from_ptr != nullptr); assert(to_ptr != nullptr); assert(copy_size != 0); - copy_size += from_sizes[i] * sizeof(TYPE); + copy_size += from_sizes[i] * sizeof(TYPE1); } // Check if the next vector element connects to this one. If not, @@ -397,7 +427,7 @@ void copy::copy_views(std::size_t size, VECMEM_DEBUG_MSG(2, "Copied the payload of a jagged vector of type " "\"%s\" with %lu copy operation(s)", - typeid(TYPE).name(), copy_ops); + typeid(TYPE2).name(), copy_ops); } template diff --git a/tests/core/test_core_copy.cpp b/tests/core/test_core_copy.cpp index d1473725..73b78e4a 100644 --- a/tests/core/test_core_copy.cpp +++ b/tests/core/test_core_copy.cpp @@ -54,6 +54,32 @@ TEST_F(core_copy_test, vector) { } } +/// Tests for copying 1-dimensional constant vectors +TEST_F(core_copy_test, const_vector) { + + // Create a reference vector. + const vecmem::vector reference = {{1, 5, 6, 74, 234, 43, 22}, + &m_resource}; + + // Create a view of its data. + auto reference_data = vecmem::get_data(reference); + + // Make a copy of this reference. + auto copy_data = m_copy.to(reference_data, m_resource); + + // Create device vectors over the two, and compare them. + vecmem::device_vector reference_vector(reference_data); + vecmem::device_vector copy_vector(copy_data); + EXPECT_EQ(reference_vector.size(), copy_vector.size()); + auto reference_itr = reference_vector.begin(); + auto copy_itr = copy_vector.begin(); + for (; (reference_itr != reference_vector.end()) && + (copy_itr != copy_vector.end()); + ++reference_itr, ++copy_itr) { + EXPECT_EQ(*reference_itr, *copy_itr); + } +} + /// Tests for copying jagged vectors TEST_F(core_copy_test, jagged_vector) { @@ -92,3 +118,42 @@ TEST_F(core_copy_test, jagged_vector) { } } } + +/// Tests for copying constant jagged vectors +TEST_F(core_copy_test, const_jagged_vector) { + + // Create a reference vector. + const vecmem::jagged_vector reference = { + {{{1, 2, 3, 4, 5}, &m_resource}, + {{6, 7}, &m_resource}, + {{8, 9, 10, 11}, &m_resource}, + {{12, 13, 14, 15, 16, 17, 18}, &m_resource}, + {{19}, &m_resource}, + {{20}, &m_resource}}, + &m_resource}; + + // Create a view of its data. + auto reference_data = vecmem::get_data(reference); + + // Make a copy of this reference. + auto copy_data = m_copy.to(reference_data, m_resource); + + // Create device vectors over the two, and compare them. + vecmem::jagged_device_vector reference_vector(reference_data); + vecmem::jagged_device_vector copy_vector(copy_data); + EXPECT_EQ(reference_vector.size(), copy_vector.size()); + auto reference_itr = reference_vector.begin(); + auto copy_itr = copy_vector.begin(); + for (; (reference_itr != reference_vector.end()) && + (copy_itr != copy_vector.end()); + ++reference_itr, ++copy_itr) { + EXPECT_EQ(reference_itr->size(), copy_itr->size()); + auto reference_itr2 = reference_itr->begin(); + auto copy_itr2 = copy_itr->begin(); + for (; (reference_itr2 != reference_itr->end()) && + (copy_itr2 != copy_itr->end()); + ++reference_itr2, ++copy_itr2) { + EXPECT_EQ(*reference_itr2, *copy_itr2); + } + } +}