Skip to content

Commit 27ddc85

Browse files
jblomerjblomer
committed
[ntuple] fix memory allocation for over-aligned fields
1 parent 5434b6a commit 27ddc85

13 files changed

Lines changed: 111 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: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -233,8 +233,8 @@ 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) {}
238238
void operator()(void *objPtr, bool dtorOnly) final;
239239
};
240240

@@ -284,7 +284,8 @@ class RUniquePtrField : public RNullableField {
284284
std::unique_ptr<RDeleter> fItemDeleter;
285285

286286
public:
287-
explicit RUniquePtrDeleter(std::unique_ptr<RDeleter> itemDeleter) : fItemDeleter(std::move(itemDeleter)) {}
287+
explicit RUniquePtrDeleter(std::unique_ptr<RDeleter> itemDeleter)
288+
: RDeleter(alignof(std::unique_ptr<char>)), fItemDeleter(std::move(itemDeleter)) {}
288289
void operator()(void *objPtr, bool dtorOnly) final;
289290
};
290291

@@ -388,9 +389,10 @@ private:
388389
std::vector<std::unique_ptr<RDeleter>> fItemDeleters;
389390

390391
public:
391-
RVariantDeleter(std::size_t tagOffset, std::size_t variantOffset,
392+
RVariantDeleter(std::size_t tagOffset, std::size_t variantOffset, std::size_t alignment,
392393
std::vector<std::unique_ptr<RDeleter>> itemDeleters)
393-
: fTagOffset(tagOffset), fVariantOffset(variantOffset), fItemDeleters(std::move(itemDeleters))
394+
: RDeleter(alignment), fTagOffset(tagOffset), fVariantOffset(variantOffset),
395+
fItemDeleters(std::move(itemDeleters))
394396
{
395397
}
396398
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: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -102,25 +102,32 @@ 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;
111116
public:
117+
explicit RDeleter(std::size_t alignment) : fAlignment(alignment) { EnsureValidAlignment(alignment); }
112118
virtual ~RDeleter() = default;
113119
virtual void operator()(void *objPtr, bool dtorOnly)
114120
{
115121
if (!dtorOnly)
116-
operator delete(objPtr);
122+
operator delete(objPtr, fAlignment);
117123
}
118124
};
119125

120126
/// A deleter for templated RFieldBase descendents where the value type is known.
121127
template <typename T>
122128
class RTypedDeleter : public RDeleter {
123129
public:
130+
RTypedDeleter() : RDeleter(alignof(T)) {}
124131
void operator()(void *objPtr, bool dtorOnly) final
125132
{
126133
std::destroy_at(static_cast<T *>(objPtr));
@@ -422,10 +429,12 @@ protected:
422429

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

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

558567
public:
559-
/// Maximum supported alignment for field types, i.e. maximum returned by GetAlignment()
560-
static constexpr std::size_t kMaxAlignment = 4096;
561-
562568
template <bool IsConstT>
563569
class RSchemaIteratorTemplate;
564570
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: 32 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,10 @@ 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+
{
608+
}
609+
612610
void ROOT::RClassField::RClassDeleter::operator()(void *objPtr, bool dtorOnly)
613611
{
614612
fClass->Destructor(objPtr, true /* dtorOnly */);
@@ -860,6 +858,11 @@ void ROOT::Experimental::RSoAField::ConstructValue(void *where) const
860858
fSoAClass->New(where);
861859
}
862860

861+
ROOT::Experimental::RSoAField::RSoADeleter::RSoADeleter(TClass *cl)
862+
: RDeleter(cl->GetClassAlignment()), fSoAClass(cl)
863+
{
864+
}
865+
863866
void ROOT::Experimental::RSoAField::RSoADeleter::operator()(void *objPtr, bool dtorOnly)
864867
{
865868
fSoAClass->Destructor(objPtr, true /* dtorOnly */);
@@ -1190,6 +1193,23 @@ std::unique_ptr<ROOT::RFieldBase::RDeleter> ROOT::RProxiedCollectionField::GetDe
11901193
return std::make_unique<RProxiedCollectionDeleter>(fProxy);
11911194
}
11921195

1196+
ROOT::RProxiedCollectionField::RProxiedCollectionDeleter::RProxiedCollectionDeleter(
1197+
std::shared_ptr<TVirtualCollectionProxy> proxy)
1198+
: RDeleter(proxy->GetCollectionClass()->GetClassAlignment())
1199+
, fProxy(proxy)
1200+
{
1201+
}
1202+
1203+
ROOT::RProxiedCollectionField::RProxiedCollectionDeleter::RProxiedCollectionDeleter(
1204+
std::shared_ptr<TVirtualCollectionProxy> proxy, std::unique_ptr<RDeleter> itemDeleter, size_t itemSize)
1205+
: RDeleter(proxy->GetCollectionClass()->GetClassAlignment())
1206+
, fProxy(proxy)
1207+
, fItemDeleter(std::move(itemDeleter))
1208+
, fItemSize(itemSize)
1209+
{
1210+
fIFuncsWrite = RCollectionIterableOnce::GetIteratorFuncs(fProxy.get(), false /* readFromDisk */);
1211+
}
1212+
11931213
void ROOT::RProxiedCollectionField::RProxiedCollectionDeleter::operator()(void *objPtr, bool dtorOnly)
11941214
{
11951215
if (fItemDeleter) {
@@ -1415,6 +1435,11 @@ void ROOT::RStreamerField::ConstructValue(void *where) const
14151435
fClass->New(where);
14161436
}
14171437

1438+
ROOT::RStreamerField::RStreamerFieldDeleter::RStreamerFieldDeleter(TClass *cl)
1439+
: RDeleter(cl->GetClassAlignment()), fClass(cl)
1440+
{
1441+
}
1442+
14181443
void ROOT::RStreamerField::RStreamerFieldDeleter::operator()(void *objPtr, bool dtorOnly)
14191444
{
14201445
fClass->Destructor(objPtr, true /* dtorOnly */);
@@ -1816,7 +1841,7 @@ std::unique_ptr<ROOT::RFieldBase::RDeleter> ROOT::RVariantField::GetDeleter() co
18161841
for (const auto &f : fSubfields) {
18171842
itemDeleters.emplace_back(GetDeleterOf(*f));
18181843
}
1819-
return std::make_unique<RVariantDeleter>(fTagOffset, fVariantOffset, std::move(itemDeleters));
1844+
return std::make_unique<RVariantDeleter>(fTagOffset, fVariantOffset, GetAlignment(), std::move(itemDeleters));
18201845
}
18211846

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

0 commit comments

Comments
 (0)