Skip to content

Commit 540db2c

Browse files
havardpehavardpe
authored
Merge pull request #36601 from vespa-engine/havardpe/invert-typed-data-layout-inheritance
invert inheritance to enable custom data that can be forward declared
2 parents a0be69c + 5fc1e1c commit 540db2c

3 files changed

Lines changed: 77 additions & 46 deletions

File tree

‎vespalib/src/tests/util/typed_data_layout_test.cpp‎

Lines changed: 39 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,26 @@ struct alignas(32) BigObj {
3333
};
3434
int BigObj::live_cnt = 0;
3535

36+
template <domain D>
37+
struct MyData : Data<D> {
38+
int value;
39+
static int live_cnt;
40+
MyData(DataKey key) noexcept : Data<D>(key), value(++live_cnt) {}
41+
~MyData() { --live_cnt; }
42+
};
43+
template <domain D>
44+
int MyData<D>::live_cnt = 0;
45+
46+
template <domain D>
47+
struct alignas(32) MyBigData : Data<D> {
48+
int value;
49+
static int live_cnt;
50+
MyBigData(DataKey key) noexcept : Data<D>(key), value(++live_cnt) {}
51+
~MyBigData() { --live_cnt; }
52+
};
53+
template <domain D>
54+
int MyBigData<D>::live_cnt = 0;
55+
3656
struct Size {
3757
size_t value;
3858
template <typename Data> explicit Size(const Data& d) : value(sizeof(d)) {}
@@ -227,14 +247,15 @@ TEST(TypedDataLayoutTest, multi_reserve_and_resolve) {
227247
}
228248
}
229249

230-
TEST(TypedDataLayoutTest, base_class) {
231-
ASSERT_EQ(MyObj::live_cnt, 0);
250+
TEST(TypedDataLayoutTest, sub_class) {
251+
using D = Domain<int, double>;
252+
EXPECT_EQ(MyData<D>::live_cnt, 0);
232253
{
233-
Layout<Domain<int, double>, MyObj> layout;
234-
auto hi = layout.reserve<int>();
235-
auto hd = layout.reserve<double>();
236-
auto data = layout.create_data();
237-
EXPECT_EQ(MyObj::live_cnt, 1);
254+
Layout<D, MyData<D>> layout;
255+
auto hi = layout.reserve<int>();
256+
auto hd = layout.reserve<double>();
257+
auto data = layout.create_data();
258+
EXPECT_EQ(MyData<D>::live_cnt, 1);
238259
EXPECT_EQ(data->allocated(), Size(*data).add<int>(1).add<double>(1).value);
239260
data->value = 42;
240261
data->resolve<int>(hi) = 10;
@@ -243,7 +264,7 @@ TEST(TypedDataLayoutTest, base_class) {
243264
EXPECT_EQ(data->resolve<int>(hi), 10);
244265
EXPECT_EQ(data->resolve<double>(hd), 3.14);
245266
}
246-
EXPECT_EQ(MyObj::live_cnt, 0);
267+
EXPECT_EQ(MyData<D>::live_cnt, 0);
247268
}
248269

249270
TEST(TypedDataLayoutTest, unused_types_do_not_affect_size) {
@@ -258,19 +279,23 @@ TEST(TypedDataLayoutTest, unused_types_do_not_affect_size) {
258279
}
259280

260281
TEST(TypedDataLayoutTest, alignment) {
282+
using D = Domain<char, int, BigObj>;
261283
ASSERT_EQ(BigObj::live_cnt, 0);
284+
EXPECT_EQ(MyBigData<D>::live_cnt, 0);
262285
{
263-
Layout<Domain<char, int, BigObj>, BigObj> layout;
264-
auto hc = layout.reserve<char>();
265-
auto hi = layout.reserve<int>();
266-
auto hb = layout.reserve<BigObj>();
267-
auto data = layout.create_data();
268-
EXPECT_EQ(BigObj::live_cnt, 2);
286+
Layout<D, MyBigData<D>> layout;
287+
auto hc = layout.reserve<char>();
288+
auto hi = layout.reserve<int>();
289+
auto hb = layout.reserve<BigObj>();
290+
auto data = layout.create_data();
291+
EXPECT_EQ(BigObj::live_cnt, 1);
292+
EXPECT_EQ(MyBigData<D>::live_cnt, 1);
269293
EXPECT_EQ(data->allocated(), Size(*data).add<char>(1).add<int>(1).add<BigObj>(1).value);
270294
EXPECT_TRUE(is_aligned(data.get()));
271295
EXPECT_TRUE(is_aligned(&data->resolve<char>(hc)));
272296
EXPECT_TRUE(is_aligned(&data->resolve<int>(hi)));
273297
EXPECT_TRUE(is_aligned(&data->resolve<BigObj>(hb)));
274298
}
275299
EXPECT_EQ(BigObj::live_cnt, 0);
300+
EXPECT_EQ(MyBigData<D>::live_cnt, 0);
276301
}

‎vespalib/src/vespa/vespalib/util/typed_data_layout.cpp‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,11 +28,12 @@ static_assert(Domain<char>::max_align == 1);
2828
static_assert(Domain<char, uint32_t>::max_align == 4);
2929
static_assert(Domain<char, uint64_t, uint32_t>::max_align == 8);
3030

31+
struct alignas(1) SmallAlign {};
3132
struct alignas(32) BigAlign {};
32-
static_assert(std::same_as<decltype(detail::full_align<MyDomain, detail::EmptyBase>()), std::align_val_t>);
33-
static_assert(detail::full_align<Domain<char>, detail::EmptyBase>() == std::align_val_t(1));
34-
static_assert(detail::full_align<Domain<char, int, double>, detail::EmptyBase>() == std::align_val_t(8));
35-
static_assert(detail::full_align<Domain<char, BigAlign, double>, detail::EmptyBase>() == std::align_val_t(32));
33+
static_assert(std::same_as<decltype(detail::full_align<MyDomain, SmallAlign>()), std::align_val_t>);
34+
static_assert(detail::full_align<Domain<char>, SmallAlign>() == std::align_val_t(1));
35+
static_assert(detail::full_align<Domain<char, int, double>, SmallAlign>() == std::align_val_t(8));
36+
static_assert(detail::full_align<Domain<char, BigAlign, double>, SmallAlign>() == std::align_val_t(32));
3637
static_assert(detail::full_align<Domain<char, int, double>, BigAlign>() == std::align_val_t(32));
3738

3839
} // namespace

‎vespalib/src/vespa/vespalib/util/typed_data_layout.h‎

Lines changed: 33 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -24,11 +24,10 @@
2424
// Layout:
2525
// Used to plan which objects are needed.
2626
// Reserves handles that can later be resolved against Data.
27-
// Defines the Data and DataUP types used to access the data.
2827
//
2928
// Data:
3029
// Owns the packed storage for all objects planned by Layout.
31-
// Inherits from the Base class specified in Layout.
30+
// This class can be extended by the user to add custom data and API.
3231
//
3332
// Handle:
3433
// Reserved by Layout for a single object.
@@ -81,8 +80,6 @@ template <size_t I, typename H, typename... Ts> constexpr auto get_type_at() {
8180
}
8281
}
8382

84-
struct EmptyBase {};
85-
8683
} // namespace detail
8784

8885
template <typename H, typename... Ts> struct Domain {
@@ -107,23 +104,23 @@ template <typename T, domain D> constexpr size_t type_id() {
107104

108105
namespace detail {
109106

110-
template <domain D, typename Base> constexpr std::align_val_t full_align() {
111-
return std::align_val_t(std::max(D::max_align, alignof(Base)));
107+
template <domain D, typename Target> constexpr std::align_val_t full_align() {
108+
return std::align_val_t(std::max(D::max_align, alignof(Target)));
112109
}
113110

114-
template <domain D, typename Base> struct DataDeleter;
111+
template <domain D, typename Target> struct DataDeleter;
115112

116113
} // namespace detail
117114

118-
template <domain D, typename Base> class Data;
119-
template <domain D, typename Base = detail::EmptyBase> class Layout;
115+
template <domain D> class Data;
116+
template <domain D, typename Target = Data<D>> class Layout;
120117
class ArrayHandle;
121118

122119
class Handle {
123120
private:
124121
friend class ArrayHandle;
125122
friend class HandleIterator;
126-
template <domain D, typename Base> friend class Layout;
123+
template <domain D, typename Target> friend class Layout;
127124
uint32_t _value;
128125
static constexpr uint32_t invalid_handle = 0xffffffff;
129126
static constexpr uint32_t offset_bits = 24;
@@ -165,8 +162,8 @@ class HandleIterator {
165162

166163
class ArrayHandle {
167164
private:
168-
template <domain D, typename Base> friend class Layout;
169-
template <domain D, typename Base> friend class Data;
165+
template <domain D, typename Target> friend class Layout;
166+
template <domain D> friend class Data;
170167
Handle _base;
171168
uint32_t _size;
172169

@@ -189,25 +186,32 @@ class ArrayHandle {
189186
constexpr auto end() const noexcept { return HandleIterator(_base._value + _size); }
190187
};
191188

192-
template <typename... Ts, typename Base> class Data<Domain<Ts...>, Base> : public Base {
189+
class DataKey {
190+
private:
191+
template <domain D, typename Target> friend class Layout;
192+
constexpr DataKey() noexcept = default;
193+
};
194+
195+
template <typename... Ts> class Data<Domain<Ts...>> {
193196
private:
194197
using MyDomain = Domain<Ts...>;
195-
friend class Layout<MyDomain, Base>;
196-
friend struct detail::DataDeleter<MyDomain, Base>;
198+
template <domain D, typename Target> friend class Layout;
199+
template <domain D, typename Target> friend struct detail::DataDeleter;
197200
struct VectorRef {
198201
uint32_t pos;
199202
uint32_t len;
200203
constexpr VectorRef() noexcept : pos(0), len(0) {}
201204
};
202205
size_t _allocated;
203206
std::array<VectorRef, MyDomain::num_types> _header;
204-
constexpr Data(size_t need_size) noexcept : _allocated(need_size), _header{} {}
205207
Data(const Data&) = delete;
206208
Data(Data&&) = delete;
207209
Data& operator=(const Data&) = delete;
208210
Data& operator=(Data&&) = delete;
209211

210212
public:
213+
constexpr Data(DataKey) noexcept : _allocated{}, _header{} {}
214+
211215
template <typename T> std::span<T> all_of() noexcept {
212216
static constexpr size_t I = type_id<T, MyDomain>();
213217
if (_header[I].len == 0) {
@@ -252,31 +256,31 @@ template <typename... Ts, typename Base> class Data<Domain<Ts...>, Base> : publi
252256

253257
namespace detail {
254258

255-
template <typename... Ts, typename Base> struct DataDeleter<Domain<Ts...>, Base> {
259+
template <typename... Ts, typename Target> struct DataDeleter<Domain<Ts...>, Target> {
256260
using MyDomain = Domain<Ts...>;
257-
using MyData = Data<MyDomain, Base>;
258-
void operator()(MyData* ptr) noexcept {
261+
static_assert(std::derived_from<Target, Data<MyDomain>>);
262+
void operator()(Target* ptr) noexcept {
259263
auto destruct_array = [&]<typename T>() {
260264
for (auto& obj : ptr->template all_of<T>()) {
261265
obj.~T();
262266
}
263267
};
264268
(destruct_array.template operator()<Ts>(), ...);
265-
ptr->~MyData();
266-
::operator delete(ptr, full_align<MyDomain, Base>());
269+
ptr->~Target();
270+
::operator delete(ptr, full_align<MyDomain, Target>());
267271
}
268272
};
269273

270274
} // namespace detail
271275

272-
template <typename... Ts, typename Base> class Layout<Domain<Ts...>, Base> {
276+
template <typename... Ts, typename Target> class Layout<Domain<Ts...>, Target> {
273277
private:
274278
using MyDomain = Domain<Ts...>;
275279
std::array<uint32_t, MyDomain::num_types> counts{};
280+
static_assert(std::derived_from<Target, Data<MyDomain>>);
276281

277282
public:
278-
using MyData = Data<MyDomain, Base>;
279-
using DataUP = std::unique_ptr<MyData, detail::DataDeleter<MyDomain, Base>>;
283+
using DataUP = std::unique_ptr<Target, detail::DataDeleter<MyDomain, Target>>;
280284
template <typename T> Handle reserve() {
281285
static constexpr size_t I = type_id<T, MyDomain>();
282286
return Handle::make<I>(counts[I]++);
@@ -292,7 +296,7 @@ template <typename... Ts, typename Base> class Layout<Domain<Ts...>, Base> {
292296
return ArrayHandle::make<I>(0, counts[I]);
293297
}
294298
DataUP create_data() const {
295-
size_t need_size = sizeof(MyData);
299+
size_t need_size = sizeof(Target);
296300
[&]<size_t... Is>(std::index_sequence<Is...>) {
297301
auto handle_array = [&]<typename T, size_t I>() {
298302
if (counts[I] > 0) {
@@ -303,11 +307,12 @@ template <typename... Ts, typename Base> class Layout<Domain<Ts...>, Base> {
303307
(handle_array.template operator()<Ts, Is>(), ...);
304308
}(typename MyDomain::index_sequence{});
305309
assert(need_size <= UINT32_MAX);
306-
constexpr auto align = detail::full_align<MyDomain, Base>();
310+
constexpr auto align = detail::full_align<MyDomain, Target>();
307311
char* mem = static_cast<char*>(::operator new(need_size, align));
308-
DataUP result(new (mem) MyData(need_size));
312+
DataUP result(new (mem) Target(DataKey{}));
309313
auto& obj = *result;
310-
size_t offset = sizeof(MyData);
314+
size_t offset = sizeof(Target);
315+
obj._allocated = need_size;
311316
[&]<size_t... Is>(std::index_sequence<Is...>) {
312317
auto construct_array = [&]<typename T, size_t I>() {
313318
if (counts[I] > 0) {

0 commit comments

Comments
 (0)
⚡