Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mark copy events as nodiscard #286

Merged
merged 1 commit into from
Oct 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions benchmarks/core/benchmark_copy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,11 @@ void jaggedVectorUnknownHtoDCopy(::benchmark::State& state) {
const data::jagged_vector_data<int> source_data = get_data(source);
// Create the "destination buffer".
data::jagged_vector_buffer<int> dest(sizes, host_mr);
host_copy.setup(dest);
host_copy.setup(dest)->wait();

// Perform the copy benchmark.
for (auto _ : state) {
host_copy(source_data, dest);
host_copy(source_data, dest)->wait();
}
}
// Set up the benchmark.
Expand Down Expand Up @@ -82,11 +82,11 @@ void jaggedVectorKnownHtoDCopy(::benchmark::State& state) {
const data::jagged_vector_data<int> source_data = get_data(source);
// Create the "destination buffer".
data::jagged_vector_buffer<int> dest(sizes, host_mr);
host_copy.setup(dest);
host_copy.setup(dest)->wait();

// Perform the copy benchmark.
for (auto _ : state) {
host_copy(source_data, dest, copy::type::host_to_device);
host_copy(source_data, dest, copy::type::host_to_device)->wait();
}
}
// Set up the benchmark.
Expand All @@ -111,14 +111,14 @@ void jaggedVectorUnknownDtoHCopy(::benchmark::State& state) {

// Create the "source buffer".
data::jagged_vector_buffer<int> source(sizes, host_mr);
host_copy.setup(source);
host_copy.setup(source)->wait();
// Create the "destination vector".
jagged_vector<int> dest = make_jagged_vector(sizes, host_mr);
data::jagged_vector_data<int> dest_data = get_data(dest);

// Perform the copy benchmark.
for (auto _ : state) {
host_copy(source, dest_data);
host_copy(source, dest_data)->wait();
}
}
// Set up the benchmark.
Expand All @@ -143,14 +143,14 @@ void jaggedVectorKnownDtoHCopy(::benchmark::State& state) {

// Create the "source buffer".
data::jagged_vector_buffer<int> source(sizes, host_mr);
host_copy.setup(source);
host_copy.setup(source)->wait();
// Create the "destination vector".
jagged_vector<int> dest = make_jagged_vector(sizes, host_mr);
data::jagged_vector_data<int> dest_data = get_data(dest);

// Perform the copy benchmark.
for (auto _ : state) {
host_copy(source, dest_data, copy::type::device_to_host);
host_copy(source, dest_data, copy::type::device_to_host)->wait();
}
}
// Set up the benchmark.
Expand Down
16 changes: 8 additions & 8 deletions benchmarks/cuda/benchmark_copy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,11 @@ void jaggedVectorUnknownHtoDCopy(::benchmark::State& state) {
const data::jagged_vector_data<int> source_data = get_data(source);
// Create the "destination buffer".
data::jagged_vector_buffer<int> dest(sizes, device_mr, &host_mr);
cuda_copy.setup(dest);
cuda_copy.setup(dest)->wait();

// Perform the copy benchmark.
for (auto _ : state) {
cuda_copy(source_data, dest);
cuda_copy(source_data, dest)->wait();
}
}
// Set up the benchmark.
Expand Down Expand Up @@ -87,11 +87,11 @@ void jaggedVectorKnownHtoDCopy(::benchmark::State& state) {
const data::jagged_vector_data<int> source_data = get_data(source);
// Create the "destination buffer".
data::jagged_vector_buffer<int> dest(sizes, device_mr, &host_mr);
cuda_copy.setup(dest);
cuda_copy.setup(dest)->wait();

// Perform the copy benchmark.
for (auto _ : state) {
cuda_copy(source_data, dest, copy::type::host_to_device);
cuda_copy(source_data, dest, copy::type::host_to_device)->wait();
}
}
// Set up the benchmark.
Expand All @@ -116,15 +116,15 @@ void jaggedVectorUnknownDtoHCopy(::benchmark::State& state) {

// Create the "source buffer".
data::jagged_vector_buffer<int> source(sizes, device_mr, &host_mr);
cuda_copy.setup(source);
cuda_copy.setup(source)->wait();
// Create the "destination vector".
jagged_vector<int> dest =
vecmem::benchmark::make_jagged_vector(sizes, host_mr);
data::jagged_vector_data<int> dest_data = get_data(dest);

// Perform the copy benchmark.
for (auto _ : state) {
cuda_copy(source, dest_data);
cuda_copy(source, dest_data)->wait();
}
}
// Set up the benchmark.
Expand All @@ -149,15 +149,15 @@ void jaggedVectorKnownDtoHCopy(::benchmark::State& state) {

// Create the "source buffer".
data::jagged_vector_buffer<int> source(sizes, device_mr, &host_mr);
cuda_copy.setup(source);
cuda_copy.setup(source)->wait();
// Create the "destination vector".
jagged_vector<int> dest =
vecmem::benchmark::make_jagged_vector(sizes, host_mr);
data::jagged_vector_data<int> dest_data = get_data(dest);

// Perform the copy benchmark.
for (auto _ : state) {
cuda_copy(source, dest_data, copy::type::device_to_host);
cuda_copy(source, dest_data, copy::type::device_to_host)->wait();
}
}
// Set up the benchmark.
Expand Down
16 changes: 8 additions & 8 deletions benchmarks/sycl/benchmark_copy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -57,11 +57,11 @@ void jaggedVectorUnknownHtoDCopy(::benchmark::State& state) {
const data::jagged_vector_data<int> source_data = get_data(source);
// Create the "destination buffer".
data::jagged_vector_buffer<int> dest(sizes, device_mr, &host_mr);
sycl_copy.setup(dest);
sycl_copy.setup(dest)->wait();

// Perform the copy benchmark.
for (auto _ : state) {
sycl_copy(source_data, dest);
sycl_copy(source_data, dest)->wait();
}
}
// Set up the benchmark.
Expand Down Expand Up @@ -90,11 +90,11 @@ void jaggedVectorKnownHtoDCopy(::benchmark::State& state) {
const data::jagged_vector_data<int> source_data = get_data(source);
// Create the "destination buffer".
data::jagged_vector_buffer<int> dest(sizes, device_mr, &host_mr);
sycl_copy.setup(dest);
sycl_copy.setup(dest)->wait();

// Perform the copy benchmark.
for (auto _ : state) {
sycl_copy(source_data, dest, copy::type::host_to_device);
sycl_copy(source_data, dest, copy::type::host_to_device)->wait();
}
}
// Set up the benchmark.
Expand All @@ -119,15 +119,15 @@ void jaggedVectorUnknownDtoHCopy(::benchmark::State& state) {

// Create the "source buffer".
data::jagged_vector_buffer<int> source(sizes, device_mr, &host_mr);
sycl_copy.setup(source);
sycl_copy.setup(source)->wait();
// Create the "destination vector".
jagged_vector<int> dest =
vecmem::benchmark::make_jagged_vector(sizes, host_mr);
data::jagged_vector_data<int> dest_data = get_data(dest);

// Perform the copy benchmark.
for (auto _ : state) {
sycl_copy(source, dest_data);
sycl_copy(source, dest_data)->wait();
}
}
// Set up the benchmark.
Expand All @@ -152,15 +152,15 @@ void jaggedVectorKnownDtoHCopy(::benchmark::State& state) {

// Create the "source buffer".
data::jagged_vector_buffer<int> source(sizes, device_mr, &host_mr);
sycl_copy.setup(source);
sycl_copy.setup(source)->wait();
// Create the "destination vector".
jagged_vector<int> dest =
vecmem::benchmark::make_jagged_vector(sizes, host_mr);
data::jagged_vector_data<int> dest_data = get_data(dest);

// Perform the copy benchmark.
for (auto _ : state) {
sycl_copy(source, dest_data, copy::type::device_to_host);
sycl_copy(source, dest_data, copy::type::device_to_host)->wait();
}
}
// Set up the benchmark.
Expand Down
19 changes: 19 additions & 0 deletions core/include/vecmem/utils/attributes.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
/*
* VecMem project, part of the ACTS project (R&D line)
*
* (c) 2021-2023 CERN for the benefit of the ACTS project
*
* Mozilla Public License Version 2.0
*/

#pragma once

#ifdef __has_cpp_attribute
#if __has_cpp_attribute(nodiscard) >= 201603L
#define VECMEM_NODISCARD [[nodiscard]]
#else
#define VECMEM_NODISCARD
#endif
#else
#define VECMEM_NODISCARD
#endif
krasznaa marked this conversation as resolved.
Show resolved Hide resolved
56 changes: 31 additions & 25 deletions core/include/vecmem/utils/copy.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include "vecmem/edm/view.hpp"
#include "vecmem/memory/memory_resource.hpp"
#include "vecmem/utils/abstract_event.hpp"
#include "vecmem/utils/attributes.hpp"
#include "vecmem/vecmem_core_export.hpp"

// System include(s).
Expand Down Expand Up @@ -75,11 +76,12 @@ class VECMEM_CORE_EXPORT copy {

/// Set up the internal state of a vector buffer correctly on a device
template <typename TYPE>
event_type setup(data::vector_view<TYPE> data) const;
VECMEM_NODISCARD event_type setup(data::vector_view<TYPE> data) const;

/// Set all bytes of the vector to some value
template <typename TYPE>
event_type memset(data::vector_view<TYPE> data, int value) const;
VECMEM_NODISCARD event_type memset(data::vector_view<TYPE> data,
int value) const;

/// Copy a 1-dimensional vector to the specified memory resource
template <typename TYPE>
Expand All @@ -89,15 +91,17 @@ class VECMEM_CORE_EXPORT copy {

/// Copy a 1-dimensional vector's data between two existing memory blocks
template <typename TYPE>
event_type operator()(const data::vector_view<std::add_const_t<TYPE>>& from,
data::vector_view<TYPE> to,
type::copy_type cptype = type::unknown) const;
VECMEM_NODISCARD event_type
operator()(const data::vector_view<std::add_const_t<TYPE>>& from,
data::vector_view<TYPE> to,
type::copy_type cptype = type::unknown) const;

/// Copy a 1-dimensional vector's data into a vector object
template <typename TYPE, typename ALLOC>
event_type operator()(const data::vector_view<std::add_const_t<TYPE>>& from,
std::vector<TYPE, ALLOC>& to,
type::copy_type cptype = type::unknown) const;
VECMEM_NODISCARD event_type
operator()(const data::vector_view<std::add_const_t<TYPE>>& from,
std::vector<TYPE, ALLOC>& to,
type::copy_type cptype = type::unknown) const;

/// Helper function for getting the size of a resizable 1D buffer
template <typename TYPE>
Expand All @@ -111,11 +115,13 @@ class VECMEM_CORE_EXPORT copy {

/// Copy the internal state of a jagged vector buffer to the target device
template <typename TYPE>
event_type setup(data::jagged_vector_view<TYPE> data) const;
VECMEM_NODISCARD event_type
setup(data::jagged_vector_view<TYPE> data) const;

/// Set all bytes of the jagged vector to some value
template <typename TYPE>
event_type memset(data::jagged_vector_view<TYPE> data, int value) const;
VECMEM_NODISCARD event_type memset(data::jagged_vector_view<TYPE> data,
int value) const;

/// Copy a jagged vector to the specified memory resource
template <typename TYPE>
Expand All @@ -126,17 +132,17 @@ class VECMEM_CORE_EXPORT copy {

/// Copy a jagged vector's data between two existing allocations
template <typename TYPE>
event_type operator()(
const data::jagged_vector_view<std::add_const_t<TYPE>>& from,
data::jagged_vector_view<TYPE> to,
type::copy_type cptype = type::unknown) const;
VECMEM_NODISCARD event_type
operator()(const data::jagged_vector_view<std::add_const_t<TYPE>>& from,
data::jagged_vector_view<TYPE> to,
type::copy_type cptype = type::unknown) const;

/// Copy a jagged vector's data into a vector object
template <typename TYPE, typename ALLOC1, typename ALLOC2>
event_type operator()(
const data::jagged_vector_view<std::add_const_t<TYPE>>& from,
std::vector<std::vector<TYPE, ALLOC2>, ALLOC1>& to,
type::copy_type cptype = type::unknown) const;
VECMEM_NODISCARD event_type
operator()(const data::jagged_vector_view<std::add_const_t<TYPE>>& from,
std::vector<std::vector<TYPE, ALLOC2>, ALLOC1>& to,
type::copy_type cptype = type::unknown) const;

/// Helper function for getting the sizes of a resizable jagged vector
template <typename TYPE>
Expand All @@ -145,7 +151,7 @@ class VECMEM_CORE_EXPORT copy {

/// Helper function for setting the sizes of a resizable jagged vector
template <typename TYPE>
event_type set_sizes(
VECMEM_NODISCARD event_type set_sizes(
const std::vector<typename data::vector_view<TYPE>::size_type>& sizes,
data::jagged_vector_view<TYPE> data) const;

Expand All @@ -156,24 +162,24 @@ class VECMEM_CORE_EXPORT copy {

/// Set up the internal state of a buffer correctly on a device
template <typename SCHEMA>
event_type setup(edm::view<SCHEMA> data) const;
VECMEM_NODISCARD event_type setup(edm::view<SCHEMA> data) const;

/// Set all bytes of the container to some value
template <typename... VARTYPES>
event_type memset(edm::view<edm::schema<VARTYPES...>> data,
int value) const;
VECMEM_NODISCARD event_type memset(edm::view<edm::schema<VARTYPES...>> data,
int value) const;

/// Copy between two views
template <typename... VARTYPES>
event_type operator()(
VECMEM_NODISCARD event_type operator()(
const edm::view<edm::details::add_const_t<edm::schema<VARTYPES...>>>&
from,
edm::view<edm::schema<VARTYPES...>> to,
type::copy_type cptype = type::unknown) const;

/// Copy from a view, into a host container
template <typename... VARTYPES, template <typename> class INTERFACE>
event_type operator()(
VECMEM_NODISCARD event_type operator()(
const edm::view<edm::details::add_const_t<edm::schema<VARTYPES...>>>&
from,
edm::host<edm::schema<VARTYPES...>, INTERFACE>& to,
Expand All @@ -193,7 +199,7 @@ class VECMEM_CORE_EXPORT copy {
/// Perform a "low level" memory filling operation
virtual void do_memset(std::size_t size, void* ptr, int value) const;
/// Create an event for synchronization
virtual event_type create_event() const;
VECMEM_NODISCARD virtual event_type create_event() const;

private:
/// Implementation for the 1D vector copy operator
Expand Down
18 changes: 12 additions & 6 deletions tests/common/copy_tests.ipp
Original file line number Diff line number Diff line change
Expand Up @@ -260,10 +260,12 @@ TEST_P(copy_tests, mismatched_vector_buffer) {

// Verify that data cannot be copied around like this.
EXPECT_THROW(main_copy()(host_buffer1, device_buffer2,
vecmem::copy::type::host_to_device),
vecmem::copy::type::host_to_device)
->wait(),
std::exception);
EXPECT_THROW(main_copy()(device_buffer2, host_buffer3,
vecmem::copy::type::device_to_host),
vecmem::copy::type::device_to_host)
->wait(),
std::exception);

// Create resizable device and host buffers, which are (progressively)
Expand Down Expand Up @@ -297,7 +299,8 @@ TEST_P(copy_tests, mismatched_vector_buffer) {

// Verify that data cannot be copied around like this.
EXPECT_THROW(main_copy()(host_buffer1, device_buffer4,
vecmem::copy::type::host_to_device),
vecmem::copy::type::host_to_device)
->wait(),
std::exception);
}

Expand Down Expand Up @@ -467,10 +470,12 @@ TEST_P(copy_tests, mismatched_jagged_vector_buffer) {

// Verify that data cannot be copied around like this.
EXPECT_THROW(main_copy()(host_buffer1, device_buffer2,
vecmem::copy::type::host_to_device),
vecmem::copy::type::host_to_device)
->wait(),
std::exception);
EXPECT_THROW(main_copy()(device_buffer2, host_buffer3,
vecmem::copy::type::device_to_host),
vecmem::copy::type::device_to_host)
->wait(),
std::exception);

// Create resizable device and host buffers, which are (progressively)
Expand Down Expand Up @@ -506,7 +511,8 @@ TEST_P(copy_tests, mismatched_jagged_vector_buffer) {

// Verify that data cannot be copied around like this.
EXPECT_THROW(main_copy()(host_buffer1, device_buffer4,
vecmem::copy::type::host_to_device),
vecmem::copy::type::host_to_device)
->wait(),
std::exception);
}

Expand Down
Loading
Loading