Skip to content

Commit fca8073

Browse files
Remove pimpl to hide reference_wrapper<nvbench::benchmake_base>, use plain pointer
Since benchmark_base instance lifetime is guaranteed while state instance exists, one can just use plain const nvbench::benchmark_base * instead of std::reference_wrapper<const nvbench::benchmark_base> while allowing for nvbench::benchmark_base to be incomplete type.
1 parent e13e9aa commit fca8073

File tree

2 files changed

+8
-40
lines changed

2 files changed

+8
-40
lines changed

nvbench/state.cuh

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@
2626
#include <nvbench/summary.cuh>
2727
#include <nvbench/types.cuh>
2828

29-
#include <memory>
3029
#include <optional>
3130
#include <string>
3231
#include <vector>
@@ -60,13 +59,9 @@ struct state
6059
{
6160
// move-only
6261
state(const state &) = delete;
62+
state(state &&) = default;
6363
state &operator=(const state &) = delete;
64-
state(state &&);
65-
state &operator=(state &&);
66-
67-
// Destructor must be defined in C++ where complete definitions of
68-
// all types for member variables are available. See #235
69-
~state();
64+
state &operator=(state &&) = default;
7065

7166
/// If a stream exists, return that. Otherwise, create a new stream using the current
7267
/// device (or the current device if none is set), save it, and return it.
@@ -312,11 +307,7 @@ private:
312307

313308
[[nodiscard]] bool skip_hot_measurement() const { return get_run_once() || get_skip_batched(); }
314309

315-
// PImpl to structure holding reference to benchmark, needed to
316-
// work around -Wsfinae-incomplete.
317-
// See https://github.com/NVIDIA/nvbench/issues/235
318-
struct bench_ref_impl;
319-
std::unique_ptr<bench_ref_impl> m_benchmark_wrapper;
310+
const nvbench::benchmark_base *m_benchmark_ptr;
320311

321312
nvbench::named_values m_axis_values;
322313
std::optional<nvbench::device_info> m_device;

nvbench/state.cxx

Lines changed: 5 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -25,25 +25,14 @@
2525
#include <fmt/format.h>
2626

2727
#include <algorithm>
28-
#include <functional>
29-
#include <memory>
3028
#include <stdexcept>
3129
#include <string>
3230

3331
namespace nvbench
3432
{
3533

36-
struct state::bench_ref_impl
37-
{
38-
explicit bench_ref_impl(const benchmark_base &bench)
39-
: m_benchmark{bench}
40-
{}
41-
42-
std::reference_wrapper<const benchmark_base> m_benchmark;
43-
};
44-
4534
state::state(const benchmark_base &bench)
46-
: m_benchmark_wrapper{std::make_unique<bench_ref_impl>(bench)}
35+
: m_benchmark_ptr{&bench}
4736
, m_is_cpu_only(bench.get_is_cpu_only())
4837
, m_run_once{bench.get_run_once()}
4938
, m_disable_blocking_kernel{bench.get_disable_blocking_kernel()}
@@ -61,7 +50,7 @@ state::state(const benchmark_base &bench,
6150
nvbench::named_values values,
6251
std::optional<nvbench::device_info> device,
6352
std::size_t type_config_index)
64-
: m_benchmark_wrapper{std::make_unique<bench_ref_impl>(bench)}
53+
: m_benchmark_ptr{&bench}
6554
, m_axis_values{std::move(values)}
6655
, m_device{std::move(device)}
6756
, m_type_config_index{type_config_index}
@@ -79,17 +68,7 @@ state::state(const benchmark_base &bench,
7968
, m_cuda_stream{std::nullopt}
8069
{}
8170

82-
// Destructor and move-constructor, move-assignment must be defined in C++ file
83-
// where benchmark_base type definition is known to be complete. See #235
84-
state::state(state &&) = default;
85-
state &state::operator=(state &&) = default;
86-
87-
state::~state() = default;
88-
89-
const nvbench::benchmark_base &state::get_benchmark() const
90-
{
91-
return m_benchmark_wrapper->m_benchmark.get();
92-
}
71+
const nvbench::benchmark_base &state::get_benchmark() const { return *m_benchmark_ptr; }
9372

9473
nvbench::int64_t state::get_int64(const std::string &axis_name) const
9574
{
@@ -228,7 +207,7 @@ std::string state::get_axis_values_as_string(bool color) const
228207
append_key_value("Device", m_device->get_id());
229208
}
230209

231-
const axes_metadata &axes = m_benchmark_wrapper->m_benchmark.get().get_axes();
210+
const axes_metadata &axes = m_benchmark_ptr->get_axes();
232211
for (const auto &name : m_axis_values.get_names())
233212
{
234213
const auto axis_type = m_axis_values.get_type(name);
@@ -265,9 +244,7 @@ std::string state::get_short_description(bool color) const
265244
};
266245

267246
return fmt::format("{} [{}]",
268-
fmt::format(style(fmt::emphasis::bold),
269-
"{}",
270-
m_benchmark_wrapper->m_benchmark.get().get_name()),
247+
fmt::format(style(fmt::emphasis::bold), "{}", m_benchmark_ptr->get_name()),
271248
this->get_axis_values_as_string(color));
272249
}
273250

0 commit comments

Comments
 (0)