Skip to content

Commit ea48e02

Browse files
krasznaakrasznaa
authored
Merge pull request #286 from stephenswat/feat/nodiscard_copy
 Mark copy events as `nodiscard`
2 parents 4ef9d61 + 1d3b9d7 commit ea48e02

16 files changed

+203
-166
lines changed

‎benchmarks/core/benchmark_copy.cpp

+8-8
Original file line numberDiff line numberDiff line change
@@ -50,11 +50,11 @@ void jaggedVectorUnknownHtoDCopy(::benchmark::State& state) {
5050
const data::jagged_vector_data<int> source_data = get_data(source);
5151
// Create the "destination buffer".
5252
data::jagged_vector_buffer<int> dest(sizes, host_mr);
53-
host_copy.setup(dest);
53+
host_copy.setup(dest)->wait();
5454

5555
// Perform the copy benchmark.
5656
for (auto _ : state) {
57-
host_copy(source_data, dest);
57+
host_copy(source_data, dest)->wait();
5858
}
5959
}
6060
// Set up the benchmark.
@@ -82,11 +82,11 @@ void jaggedVectorKnownHtoDCopy(::benchmark::State& state) {
8282
const data::jagged_vector_data<int> source_data = get_data(source);
8383
// Create the "destination buffer".
8484
data::jagged_vector_buffer<int> dest(sizes, host_mr);
85-
host_copy.setup(dest);
85+
host_copy.setup(dest)->wait();
8686

8787
// Perform the copy benchmark.
8888
for (auto _ : state) {
89-
host_copy(source_data, dest, copy::type::host_to_device);
89+
host_copy(source_data, dest, copy::type::host_to_device)->wait();
9090
}
9191
}
9292
// Set up the benchmark.
@@ -111,14 +111,14 @@ void jaggedVectorUnknownDtoHCopy(::benchmark::State& state) {
111111

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

119119
// Perform the copy benchmark.
120120
for (auto _ : state) {
121-
host_copy(source, dest_data);
121+
host_copy(source, dest_data)->wait();
122122
}
123123
}
124124
// Set up the benchmark.
@@ -143,14 +143,14 @@ void jaggedVectorKnownDtoHCopy(::benchmark::State& state) {
143143

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

151151
// Perform the copy benchmark.
152152
for (auto _ : state) {
153-
host_copy(source, dest_data, copy::type::device_to_host);
153+
host_copy(source, dest_data, copy::type::device_to_host)->wait();
154154
}
155155
}
156156
// Set up the benchmark.

‎benchmarks/cuda/benchmark_copy.cpp

+8-8
Original file line numberDiff line numberDiff line change
@@ -54,11 +54,11 @@ void jaggedVectorUnknownHtoDCopy(::benchmark::State& state) {
5454
const data::jagged_vector_data<int> source_data = get_data(source);
5555
// Create the "destination buffer".
5656
data::jagged_vector_buffer<int> dest(sizes, device_mr, &host_mr);
57-
cuda_copy.setup(dest);
57+
cuda_copy.setup(dest)->wait();
5858

5959
// Perform the copy benchmark.
6060
for (auto _ : state) {
61-
cuda_copy(source_data, dest);
61+
cuda_copy(source_data, dest)->wait();
6262
}
6363
}
6464
// Set up the benchmark.
@@ -87,11 +87,11 @@ void jaggedVectorKnownHtoDCopy(::benchmark::State& state) {
8787
const data::jagged_vector_data<int> source_data = get_data(source);
8888
// Create the "destination buffer".
8989
data::jagged_vector_buffer<int> dest(sizes, device_mr, &host_mr);
90-
cuda_copy.setup(dest);
90+
cuda_copy.setup(dest)->wait();
9191

9292
// Perform the copy benchmark.
9393
for (auto _ : state) {
94-
cuda_copy(source_data, dest, copy::type::host_to_device);
94+
cuda_copy(source_data, dest, copy::type::host_to_device)->wait();
9595
}
9696
}
9797
// Set up the benchmark.
@@ -116,15 +116,15 @@ void jaggedVectorUnknownDtoHCopy(::benchmark::State& state) {
116116

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

125125
// Perform the copy benchmark.
126126
for (auto _ : state) {
127-
cuda_copy(source, dest_data);
127+
cuda_copy(source, dest_data)->wait();
128128
}
129129
}
130130
// Set up the benchmark.
@@ -149,15 +149,15 @@ void jaggedVectorKnownDtoHCopy(::benchmark::State& state) {
149149

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

158158
// Perform the copy benchmark.
159159
for (auto _ : state) {
160-
cuda_copy(source, dest_data, copy::type::device_to_host);
160+
cuda_copy(source, dest_data, copy::type::device_to_host)->wait();
161161
}
162162
}
163163
// Set up the benchmark.

‎benchmarks/sycl/benchmark_copy.cpp

+8-8
Original file line numberDiff line numberDiff line change
@@ -57,11 +57,11 @@ void jaggedVectorUnknownHtoDCopy(::benchmark::State& state) {
5757
const data::jagged_vector_data<int> source_data = get_data(source);
5858
// Create the "destination buffer".
5959
data::jagged_vector_buffer<int> dest(sizes, device_mr, &host_mr);
60-
sycl_copy.setup(dest);
60+
sycl_copy.setup(dest)->wait();
6161

6262
// Perform the copy benchmark.
6363
for (auto _ : state) {
64-
sycl_copy(source_data, dest);
64+
sycl_copy(source_data, dest)->wait();
6565
}
6666
}
6767
// Set up the benchmark.
@@ -90,11 +90,11 @@ void jaggedVectorKnownHtoDCopy(::benchmark::State& state) {
9090
const data::jagged_vector_data<int> source_data = get_data(source);
9191
// Create the "destination buffer".
9292
data::jagged_vector_buffer<int> dest(sizes, device_mr, &host_mr);
93-
sycl_copy.setup(dest);
93+
sycl_copy.setup(dest)->wait();
9494

9595
// Perform the copy benchmark.
9696
for (auto _ : state) {
97-
sycl_copy(source_data, dest, copy::type::host_to_device);
97+
sycl_copy(source_data, dest, copy::type::host_to_device)->wait();
9898
}
9999
}
100100
// Set up the benchmark.
@@ -119,15 +119,15 @@ void jaggedVectorUnknownDtoHCopy(::benchmark::State& state) {
119119

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

128128
// Perform the copy benchmark.
129129
for (auto _ : state) {
130-
sycl_copy(source, dest_data);
130+
sycl_copy(source, dest_data)->wait();
131131
}
132132
}
133133
// Set up the benchmark.
@@ -152,15 +152,15 @@ void jaggedVectorKnownDtoHCopy(::benchmark::State& state) {
152152

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

161161
// Perform the copy benchmark.
162162
for (auto _ : state) {
163-
sycl_copy(source, dest_data, copy::type::device_to_host);
163+
sycl_copy(source, dest_data, copy::type::device_to_host)->wait();
164164
}
165165
}
166166
// Set up the benchmark.

‎core/include/vecmem/utils/attributes.hpp

+19
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
/*
2+
* VecMem project, part of the ACTS project (R&D line)
3+
*
4+
* (c) 2021-2023 CERN for the benefit of the ACTS project
5+
*
6+
* Mozilla Public License Version 2.0
7+
*/
8+
9+
#pragma once
10+
11+
#ifdef __has_cpp_attribute
12+
#if __has_cpp_attribute(nodiscard) >= 201603L
13+
#define VECMEM_NODISCARD [[nodiscard]]
14+
#else
15+
#define VECMEM_NODISCARD
16+
#endif
17+
#else
18+
#define VECMEM_NODISCARD
19+
#endif

‎core/include/vecmem/utils/copy.hpp

+31-25
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
#include "vecmem/edm/view.hpp"
1818
#include "vecmem/memory/memory_resource.hpp"
1919
#include "vecmem/utils/abstract_event.hpp"
20+
#include "vecmem/utils/attributes.hpp"
2021
#include "vecmem/vecmem_core_export.hpp"
2122

2223
// System include(s).
@@ -75,11 +76,12 @@ class VECMEM_CORE_EXPORT copy {
7576

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

8081
/// Set all bytes of the vector to some value
8182
template <typename TYPE>
82-
event_type memset(data::vector_view<TYPE> data, int value) const;
83+
VECMEM_NODISCARD event_type memset(data::vector_view<TYPE> data,
84+
int value) const;
8385

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

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

9699
/// Copy a 1-dimensional vector's data into a vector object
97100
template <typename TYPE, typename ALLOC>
98-
event_type operator()(const data::vector_view<std::add_const_t<TYPE>>& from,
99-
std::vector<TYPE, ALLOC>& to,
100-
type::copy_type cptype = type::unknown) const;
101+
VECMEM_NODISCARD event_type
102+
operator()(const data::vector_view<std::add_const_t<TYPE>>& from,
103+
std::vector<TYPE, ALLOC>& to,
104+
type::copy_type cptype = type::unknown) const;
101105

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

112116
/// Copy the internal state of a jagged vector buffer to the target device
113117
template <typename TYPE>
114-
event_type setup(data::jagged_vector_view<TYPE> data) const;
118+
VECMEM_NODISCARD event_type
119+
setup(data::jagged_vector_view<TYPE> data) const;
115120

116121
/// Set all bytes of the jagged vector to some value
117122
template <typename TYPE>
118-
event_type memset(data::jagged_vector_view<TYPE> data, int value) const;
123+
VECMEM_NODISCARD event_type memset(data::jagged_vector_view<TYPE> data,
124+
int value) const;
119125

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

127133
/// Copy a jagged vector's data between two existing allocations
128134
template <typename TYPE>
129-
event_type operator()(
130-
const data::jagged_vector_view<std::add_const_t<TYPE>>& from,
131-
data::jagged_vector_view<TYPE> to,
132-
type::copy_type cptype = type::unknown) const;
135+
VECMEM_NODISCARD event_type
136+
operator()(const data::jagged_vector_view<std::add_const_t<TYPE>>& from,
137+
data::jagged_vector_view<TYPE> to,
138+
type::copy_type cptype = type::unknown) const;
133139

134140
/// Copy a jagged vector's data into a vector object
135141
template <typename TYPE, typename ALLOC1, typename ALLOC2>
136-
event_type operator()(
137-
const data::jagged_vector_view<std::add_const_t<TYPE>>& from,
138-
std::vector<std::vector<TYPE, ALLOC2>, ALLOC1>& to,
139-
type::copy_type cptype = type::unknown) const;
142+
VECMEM_NODISCARD event_type
143+
operator()(const data::jagged_vector_view<std::add_const_t<TYPE>>& from,
144+
std::vector<std::vector<TYPE, ALLOC2>, ALLOC1>& to,
145+
type::copy_type cptype = type::unknown) const;
140146

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

146152
/// Helper function for setting the sizes of a resizable jagged vector
147153
template <typename TYPE>
148-
event_type set_sizes(
154+
VECMEM_NODISCARD event_type set_sizes(
149155
const std::vector<typename data::vector_view<TYPE>::size_type>& sizes,
150156
data::jagged_vector_view<TYPE> data) const;
151157

@@ -156,24 +162,24 @@ class VECMEM_CORE_EXPORT copy {
156162

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

161167
/// Set all bytes of the container to some value
162168
template <typename... VARTYPES>
163-
event_type memset(edm::view<edm::schema<VARTYPES...>> data,
164-
int value) const;
169+
VECMEM_NODISCARD event_type memset(edm::view<edm::schema<VARTYPES...>> data,
170+
int value) const;
165171

166172
/// Copy between two views
167173
template <typename... VARTYPES>
168-
event_type operator()(
174+
VECMEM_NODISCARD event_type operator()(
169175
const edm::view<edm::details::add_const_t<edm::schema<VARTYPES...>>>&
170176
from,
171177
edm::view<edm::schema<VARTYPES...>> to,
172178
type::copy_type cptype = type::unknown) const;
173179

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

198204
private:
199205
/// Implementation for the 1D vector copy operator

‎tests/common/copy_tests.ipp

+12-6
Original file line numberDiff line numberDiff line change
@@ -260,10 +260,12 @@ TEST_P(copy_tests, mismatched_vector_buffer) {
260260

261261
// Verify that data cannot be copied around like this.
262262
EXPECT_THROW(main_copy()(host_buffer1, device_buffer2,
263-
vecmem::copy::type::host_to_device),
263+
vecmem::copy::type::host_to_device)
264+
->wait(),
264265
std::exception);
265266
EXPECT_THROW(main_copy()(device_buffer2, host_buffer3,
266-
vecmem::copy::type::device_to_host),
267+
vecmem::copy::type::device_to_host)
268+
->wait(),
267269
std::exception);
268270

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

298300
// Verify that data cannot be copied around like this.
299301
EXPECT_THROW(main_copy()(host_buffer1, device_buffer4,
300-
vecmem::copy::type::host_to_device),
302+
vecmem::copy::type::host_to_device)
303+
->wait(),
301304
std::exception);
302305
}
303306

@@ -467,10 +470,12 @@ TEST_P(copy_tests, mismatched_jagged_vector_buffer) {
467470

468471
// Verify that data cannot be copied around like this.
469472
EXPECT_THROW(main_copy()(host_buffer1, device_buffer2,
470-
vecmem::copy::type::host_to_device),
473+
vecmem::copy::type::host_to_device)
474+
->wait(),
471475
std::exception);
472476
EXPECT_THROW(main_copy()(device_buffer2, host_buffer3,
473-
vecmem::copy::type::device_to_host),
477+
vecmem::copy::type::device_to_host)
478+
->wait(),
474479
std::exception);
475480

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

507512
// Verify that data cannot be copied around like this.
508513
EXPECT_THROW(main_copy()(host_buffer1, device_buffer4,
509-
vecmem::copy::type::host_to_device),
514+
vecmem::copy::type::host_to_device)
515+
->wait(),
510516
std::exception);
511517
}
512518

0 commit comments

Comments
 (0)