Skip to content

Commit ba94832

Browse files
jblomerjblomer
committed
[ntuple] fix memory allocation for over-aligned fields
1 parent 524aab6 commit ba94832

13 files changed

Lines changed: 112 additions & 46 deletions

‎tree/ntuple/inc/ROOT/RField.hxx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,7 @@ private:
158158
TClass *fClass;
159159

160160
public:
161-
explicit RClassDeleter(TClass *cl) : fClass(cl) {}
161+
explicit RClassDeleter(TClass *cl);
162162
void operator()(void *objPtr, bool dtorOnly) final;
163163
};
164164

@@ -239,7 +239,7 @@ private:
239239
TClass *fClass;
240240

241241
public:
242-
explicit RStreamerFieldDeleter(TClass *cl) : fClass(cl) {}
242+
explicit RStreamerFieldDeleter(TClass *cl);
243243
void operator()(void *objPtr, bool dtorOnly) final;
244244
};
245245

‎tree/ntuple/inc/ROOT/RField/RFieldProxiedCollection.hxx‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -128,13 +128,9 @@ protected:
128128
RCollectionIterableOnce::RIteratorFuncs fIFuncsWrite;
129129

130130
public:
131-
explicit RProxiedCollectionDeleter(std::shared_ptr<TVirtualCollectionProxy> proxy) : fProxy(proxy) {}
131+
explicit RProxiedCollectionDeleter(std::shared_ptr<TVirtualCollectionProxy> proxy);
132132
RProxiedCollectionDeleter(std::shared_ptr<TVirtualCollectionProxy> proxy, std::unique_ptr<RDeleter> itemDeleter,
133-
size_t itemSize)
134-
: fProxy(proxy), fItemDeleter(std::move(itemDeleter)), fItemSize(itemSize)
135-
{
136-
fIFuncsWrite = RCollectionIterableOnce::GetIteratorFuncs(fProxy.get(), false /* readFromDisk */);
137-
}
133+
size_t itemSize);
138134
void operator()(void *objPtr, bool dtorOnly) final;
139135
};
140136

‎tree/ntuple/inc/ROOT/RField/RFieldRecord.hxx‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,8 +59,9 @@ class RRecordField : public RFieldBase {
5959
std::vector<std::size_t> fOffsets;
6060

6161
public:
62-
RRecordDeleter(std::vector<std::unique_ptr<RDeleter>> itemDeleters, const std::vector<std::size_t> &offsets)
63-
: fItemDeleters(std::move(itemDeleters)), fOffsets(offsets)
62+
RRecordDeleter(std::vector<std::unique_ptr<RDeleter>> itemDeleters, const std::vector<std::size_t> &offsets,
63+
std::size_t alignment)
64+
: RDeleter(alignment), fItemDeleters(std::move(itemDeleters)), fOffsets(offsets)
6465
{
6566
}
6667
void operator()(void *objPtr, bool dtorOnly) final;

‎tree/ntuple/inc/ROOT/RField/RFieldSTLMisc.hxx‎

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -233,8 +233,10 @@ class ROptionalField : public RNullableField {
233233
std::size_t fEngagementPtrOffset = 0;
234234

235235
public:
236-
ROptionalDeleter(std::unique_ptr<RDeleter> itemDeleter, std::size_t engagementPtrOffset)
237-
: fItemDeleter(std::move(itemDeleter)), fEngagementPtrOffset(engagementPtrOffset) {}
236+
ROptionalDeleter(std::unique_ptr<RDeleter> itemDeleter, std::size_t engagementPtrOffset, std::size_t alignment)
237+
: RDeleter(alignment), fItemDeleter(std::move(itemDeleter)), fEngagementPtrOffset(engagementPtrOffset)
238+
{
239+
}
238240
void operator()(void *objPtr, bool dtorOnly) final;
239241
};
240242

@@ -284,7 +286,10 @@ class RUniquePtrField : public RNullableField {
284286
std::unique_ptr<RDeleter> fItemDeleter;
285287

286288
public:
287-
explicit RUniquePtrDeleter(std::unique_ptr<RDeleter> itemDeleter) : fItemDeleter(std::move(itemDeleter)) {}
289+
explicit RUniquePtrDeleter(std::unique_ptr<RDeleter> itemDeleter)
290+
: RDeleter(alignof(std::unique_ptr<char>)), fItemDeleter(std::move(itemDeleter))
291+
{
292+
}
288293
void operator()(void *objPtr, bool dtorOnly) final;
289294
};
290295

@@ -388,9 +393,12 @@ private:
388393
std::vector<std::unique_ptr<RDeleter>> fItemDeleters;
389394

390395
public:
391-
RVariantDeleter(std::size_t tagOffset, std::size_t variantOffset,
396+
RVariantDeleter(std::size_t tagOffset, std::size_t variantOffset, std::size_t alignment,
392397
std::vector<std::unique_ptr<RDeleter>> itemDeleters)
393-
: fTagOffset(tagOffset), fVariantOffset(variantOffset), fItemDeleters(std::move(itemDeleters))
398+
: RDeleter(alignment),
399+
fTagOffset(tagOffset),
400+
fVariantOffset(variantOffset),
401+
fItemDeleters(std::move(itemDeleters))
394402
{
395403
}
396404
void operator()(void *objPtr, bool dtorOnly) final;

‎tree/ntuple/inc/ROOT/RField/RFieldSequenceContainer.hxx‎

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,9 @@ private:
5151
std::unique_ptr<RDeleter> fItemDeleter;
5252

5353
public:
54-
RArrayDeleter(std::size_t itemSize, std::size_t arrayLength, std::unique_ptr<RDeleter> itemDeleter)
55-
: fItemSize(itemSize), fArrayLength(arrayLength), fItemDeleter(std::move(itemDeleter))
54+
RArrayDeleter(std::size_t itemSize, std::size_t arrayLength, std::size_t alignment,
55+
std::unique_ptr<RDeleter> itemDeleter)
56+
: RDeleter(alignment), fItemSize(itemSize), fArrayLength(arrayLength), fItemDeleter(std::move(itemDeleter))
5657
{
5758
}
5859
void operator()(void *objPtr, bool dtorOnly) final;
@@ -126,9 +127,15 @@ class RRVecField : public RFieldBase {
126127
std::unique_ptr<RDeleter> fItemDeleter;
127128

128129
public:
129-
explicit RRVecDeleter(std::size_t itemAlignment) : fItemAlignment(itemAlignment) {}
130+
explicit RRVecDeleter(std::size_t itemAlignment)
131+
: RDeleter(ROOT::Internal::EvalRVecAlignment(itemAlignment)), fItemAlignment(itemAlignment)
132+
{
133+
}
130134
RRVecDeleter(std::size_t itemAlignment, std::size_t itemSize, std::unique_ptr<RDeleter> itemDeleter)
131-
: fItemAlignment(itemAlignment), fItemSize(itemSize), fItemDeleter(std::move(itemDeleter))
135+
: RDeleter(ROOT::Internal::EvalRVecAlignment(itemAlignment)),
136+
fItemAlignment(itemAlignment),
137+
fItemSize(itemSize),
138+
fItemDeleter(std::move(itemDeleter))
132139
{
133140
}
134141
void operator()(void *objPtr, bool dtorOnly) final;
@@ -211,11 +218,8 @@ class RVectorField : public RFieldBase {
211218
std::unique_ptr<RDeleter> fItemDeleter;
212219

213220
public:
214-
RVectorDeleter() = default;
215-
RVectorDeleter(std::size_t itemSize, std::unique_ptr<RDeleter> itemDeleter)
216-
: fItemSize(itemSize), fItemDeleter(std::move(itemDeleter))
217-
{
218-
}
221+
RVectorDeleter();
222+
RVectorDeleter(std::size_t itemSize, std::unique_ptr<RDeleter> itemDeleter);
219223
void operator()(void *objPtr, bool dtorOnly) final;
220224
};
221225

‎tree/ntuple/inc/ROOT/RField/RFieldSoA.hxx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ class RSoAField : public RFieldBase {
5858
TClass *fSoAClass;
5959

6060
public:
61-
explicit RSoADeleter(TClass *cl) : fSoAClass(cl) {}
61+
explicit RSoADeleter(TClass *cl);
6262
void operator()(void *objPtr, bool dtorOnly) final;
6363
};
6464

‎tree/ntuple/inc/ROOT/RFieldBase.hxx‎

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -102,25 +102,33 @@ class RFieldBase {
102102

103103
using ReadCallback_t = std::function<void(void *)>;
104104

105+
public:
106+
/// Maximum supported alignment for field types, i.e. maximum returned by GetAlignment()
107+
static constexpr std::size_t kMaxAlignment = 4096;
108+
105109
protected:
106110
/// A functor to release the memory acquired by CreateValue() (memory and constructor).
107111
/// This implementation works for types with a trivial destructor. More complex fields implement a derived deleter.
108112
/// The deleter is operational without the field object and thus can be used to destruct/release a value after
109113
/// the field has been destructed.
110114
class RDeleter {
115+
std::size_t fAlignment;
116+
111117
public:
118+
explicit RDeleter(std::size_t alignment) : fAlignment(alignment) { EnsureValidAlignment(alignment); }
112119
virtual ~RDeleter() = default;
113120
virtual void operator()(void *objPtr, bool dtorOnly)
114121
{
115122
if (!dtorOnly)
116-
operator delete(objPtr);
123+
operator delete(objPtr, fAlignment);
117124
}
118125
};
119126

120127
/// A deleter for templated RFieldBase descendents where the value type is known.
121128
template <typename T>
122129
class RTypedDeleter : public RDeleter {
123130
public:
131+
RTypedDeleter() : RDeleter(alignof(T)) {}
124132
void operator()(void *objPtr, bool dtorOnly) final
125133
{
126134
std::destroy_at(static_cast<T *>(objPtr));
@@ -422,10 +430,12 @@ protected:
422430

423431
/// Constructs value in a given location of size at least GetValueSize(). Called by the base class' CreateValue().
424432
virtual void ConstructValue(void *where) const = 0;
425-
virtual std::unique_ptr<RDeleter> GetDeleter() const { return std::make_unique<RDeleter>(); }
433+
virtual std::unique_ptr<RDeleter> GetDeleter() const { return std::make_unique<RDeleter>(GetAlignment()); }
426434
/// Allow derived classes to call ConstructValue(void *) and GetDeleter() on other (sub)fields.
427435
static void CallConstructValueOn(const RFieldBase &other, void *where) { other.ConstructValue(where); }
428436
static std::unique_ptr<RDeleter> GetDeleterOf(const RFieldBase &other) { return other.GetDeleter(); }
437+
/// Throws if alignment is not a power of two between 1 and kMaxAlignment.
438+
static void EnsureValidAlignment(std::size_t alignment);
429439

430440
/// Allow parents to mark their childs as artificial fields (used in class and record fields)
431441
static void CallSetArtificialOn(RFieldBase &other) { other.SetArtificial(); }
@@ -556,9 +566,6 @@ protected:
556566
const ROOT::RNTupleDescriptor *desc, ROOT::DescriptorId_t fieldId);
557567

558568
public:
559-
/// Maximum supported alignment for field types, i.e. maximum returned by GetAlignment()
560-
static constexpr std::size_t kMaxAlignment = 4096;
561-
562569
template <bool IsConstT>
563570
class RSchemaIteratorTemplate;
564571
using RSchemaIterator = RSchemaIteratorTemplate<false>;

‎tree/ntuple/src/RField.cxx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -731,7 +731,7 @@ std::unique_ptr<ROOT::RFieldBase::RDeleter> ROOT::RRecordField::GetDeleter() con
731731
for (const auto &f : fSubfields) {
732732
itemDeleters.emplace_back(GetDeleterOf(*f));
733733
}
734-
return std::make_unique<RRecordDeleter>(std::move(itemDeleters), fOffsets);
734+
return std::make_unique<RRecordDeleter>(std::move(itemDeleters), fOffsets, GetAlignment());
735735
}
736736

737737
std::vector<ROOT::RFieldBase::RValue> ROOT::RRecordField::SplitValue(const RValue &value) const
@@ -1169,7 +1169,7 @@ std::unique_ptr<ROOT::RFieldBase::RDeleter> ROOT::ROptionalField::GetDeleter() c
11691169
{
11701170
return std::make_unique<ROptionalDeleter>(
11711171
(fSubfields[0]->GetTraits() & kTraitTriviallyDestructible) ? nullptr : GetDeleterOf(*fSubfields[0]),
1172-
fSubfields[0]->GetValueSize());
1172+
fSubfields[0]->GetValueSize(), GetAlignment());
11731173
}
11741174

11751175
std::vector<ROOT::RFieldBase::RValue> ROOT::ROptionalField::SplitValue(const RValue &value) const

‎tree/ntuple/src/RFieldBase.cxx‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
/// \author Jonas Hahnfeld <jonas.hahnfeld@cern.ch>
44
/// \date 2024-11-19
55

6+
#include <ROOT/BitUtils.hxx>
67
#include <ROOT/RError.hxx>
78
#include <ROOT/RField.hxx>
89
#include <ROOT/RFieldBase.hxx>
@@ -168,7 +169,7 @@ void ROOT::RFieldBase::RBulkValues::ReleaseValues()
168169
}
169170
}
170171

171-
operator delete(fValues);
172+
operator delete(fValues, fField->GetAlignment());
172173
}
173174

174175
void ROOT::RFieldBase::RBulkValues::Reset(RNTupleLocalIndex firstIndex, std::size_t size)
@@ -178,7 +179,7 @@ void ROOT::RFieldBase::RBulkValues::Reset(RNTupleLocalIndex firstIndex, std::siz
178179
throw RException(R__FAIL("invalid attempt to bulk read beyond the adopted buffer"));
179180
}
180181
ReleaseValues();
181-
fValues = operator new(size * fValueSize);
182+
fValues = operator new(size * fValueSize, std::align_val_t(fField->GetAlignment()));
182183

183184
if (!(fField->GetTraits() & RFieldBase::kTraitTriviallyConstructible)) {
184185
for (std::size_t i = 0; i < size; ++i) {
@@ -633,7 +634,7 @@ std::size_t ROOT::RFieldBase::ReadBulkImpl(const RBulkSpec &bulkSpec)
633634

634635
void *ROOT::RFieldBase::CreateObjectRawPtr() const
635636
{
636-
void *where = operator new(GetValueSize());
637+
void *where = operator new(GetValueSize(), std::align_val_t(GetAlignment()));
637638
R__ASSERT(where != nullptr);
638639
ConstructValue(where);
639640
return where;
@@ -645,6 +646,12 @@ ROOT::RFieldBase::RValue ROOT::RFieldBase::CreateValue()
645646
return RValue(this, std::shared_ptr<void>(obj, RSharedPtrDeleter(GetDeleter())));
646647
}
647648

649+
void ROOT::RFieldBase::EnsureValidAlignment(std::size_t alignment)
650+
{
651+
if (alignment == 0 || alignment > ROOT::RFieldBase::kMaxAlignment || !ROOT::Internal::IsPowerOfTwo(alignment))
652+
throw ROOT::RException(R__FAIL(std::string("invalid alignment: ") + std::to_string(alignment)));
653+
}
654+
648655
std::vector<ROOT::RFieldBase::RValue> ROOT::RFieldBase::SplitValue(const RValue & /*value*/) const
649656
{
650657
return std::vector<RValue>();

‎tree/ntuple/src/RFieldMeta.cxx‎

Lines changed: 28 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -118,12 +118,6 @@ TEnum *EnsureValidEnum(std::string_view enumName)
118118
return e;
119119
}
120120

121-
void EnsureValidAlignment(std::size_t align)
122-
{
123-
if (align == 0 || align > ROOT::RFieldBase::kMaxAlignment || !ROOT::Internal::IsPowerOfTwo(align))
124-
throw ROOT::RException(R__FAIL(std::string("invalid alignment: ") + std::to_string(align)));
125-
}
126-
127121
/// Create a comma-separated list of type names from the given fields. Uses either the real type names or the
128122
/// type aliases (if there are any, otherwise the actual type name). Used to construct template argument lists
129123
/// for templated types such as std::pair<...>, std::tuple<...>, std::variant<...>.
@@ -609,6 +603,8 @@ void ROOT::RClassField::ConstructValue(void *where) const
609603
fClass->New(where);
610604
}
611605

606+
ROOT::RClassField::RClassDeleter::RClassDeleter(TClass *cl) : RDeleter(cl->GetClassAlignment()), fClass(cl) {}
607+
612608
void ROOT::RClassField::RClassDeleter::operator()(void *objPtr, bool dtorOnly)
613609
{
614610
fClass->Destructor(objPtr, true /* dtorOnly */);
@@ -860,6 +856,10 @@ void ROOT::Experimental::RSoAField::ConstructValue(void *where) const
860856
fSoAClass->New(where);
861857
}
862858

859+
ROOT::Experimental::RSoAField::RSoADeleter::RSoADeleter(TClass *cl) : RDeleter(cl->GetClassAlignment()), fSoAClass(cl)
860+
{
861+
}
862+
863863
void ROOT::Experimental::RSoAField::RSoADeleter::operator()(void *objPtr, bool dtorOnly)
864864
{
865865
fSoAClass->Destructor(objPtr, true /* dtorOnly */);
@@ -1190,6 +1190,22 @@ std::unique_ptr<ROOT::RFieldBase::RDeleter> ROOT::RProxiedCollectionField::GetDe
11901190
return std::make_unique<RProxiedCollectionDeleter>(fProxy);
11911191
}
11921192

1193+
ROOT::RProxiedCollectionField::RProxiedCollectionDeleter::RProxiedCollectionDeleter(
1194+
std::shared_ptr<TVirtualCollectionProxy> proxy)
1195+
: RDeleter(proxy->GetCollectionClass()->GetClassAlignment()), fProxy(proxy)
1196+
{
1197+
}
1198+
1199+
ROOT::RProxiedCollectionField::RProxiedCollectionDeleter::RProxiedCollectionDeleter(
1200+
std::shared_ptr<TVirtualCollectionProxy> proxy, std::unique_ptr<RDeleter> itemDeleter, size_t itemSize)
1201+
: RDeleter(proxy->GetCollectionClass()->GetClassAlignment()),
1202+
fProxy(proxy),
1203+
fItemDeleter(std::move(itemDeleter)),
1204+
fItemSize(itemSize)
1205+
{
1206+
fIFuncsWrite = RCollectionIterableOnce::GetIteratorFuncs(fProxy.get(), false /* readFromDisk */);
1207+
}
1208+
11931209
void ROOT::RProxiedCollectionField::RProxiedCollectionDeleter::operator()(void *objPtr, bool dtorOnly)
11941210
{
11951211
if (fItemDeleter) {
@@ -1415,6 +1431,11 @@ void ROOT::RStreamerField::ConstructValue(void *where) const
14151431
fClass->New(where);
14161432
}
14171433

1434+
ROOT::RStreamerField::RStreamerFieldDeleter::RStreamerFieldDeleter(TClass *cl)
1435+
: RDeleter(cl->GetClassAlignment()), fClass(cl)
1436+
{
1437+
}
1438+
14181439
void ROOT::RStreamerField::RStreamerFieldDeleter::operator()(void *objPtr, bool dtorOnly)
14191440
{
14201441
fClass->Destructor(objPtr, true /* dtorOnly */);
@@ -1816,7 +1837,7 @@ std::unique_ptr<ROOT::RFieldBase::RDeleter> ROOT::RVariantField::GetDeleter() co
18161837
for (const auto &f : fSubfields) {
18171838
itemDeleters.emplace_back(GetDeleterOf(*f));
18181839
}
1819-
return std::make_unique<RVariantDeleter>(fTagOffset, fVariantOffset, std::move(itemDeleters));
1840+
return std::make_unique<RVariantDeleter>(fTagOffset, fVariantOffset, GetAlignment(), std::move(itemDeleters));
18201841
}
18211842

18221843
size_t ROOT::RVariantField::GetAlignment() const

0 commit comments

Comments
 (0)