Skip to content

Commit 139382c

Browse files
jblomerjblomer
committed
[ntuple] fix memory allocation for over-aligned fields
1 parent b7da805 commit 139382c

14 files changed

Lines changed: 130 additions & 50 deletions

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

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

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

@@ -238,7 +238,7 @@ private:
238238
TClass *fClass;
239239

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

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

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

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

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

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

6060
public:
61-
RRecordDeleter(std::vector<std::unique_ptr<RDeleter>> itemDeleters, const std::vector<std::size_t> &offsets)
62-
: fItemDeleters(std::move(itemDeleters)), fOffsets(offsets)
61+
RRecordDeleter(std::vector<std::unique_ptr<RDeleter>> itemDeleters, const std::vector<std::size_t> &offsets,
62+
std::size_t alignment)
63+
: RDeleter(alignment), fItemDeleters(std::move(itemDeleters)), fOffsets(offsets)
6364
{
6465
}
6566
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
@@ -232,8 +232,10 @@ class ROptionalField : public RNullableField {
232232
std::size_t fEngagementPtrOffset = 0;
233233

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

@@ -283,7 +285,10 @@ class RUniquePtrField : public RNullableField {
283285
std::unique_ptr<RDeleter> fItemDeleter;
284286

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

@@ -387,9 +392,12 @@ private:
387392
std::vector<std::unique_ptr<RDeleter>> fItemDeleters;
388393

389394
public:
390-
RVariantDeleter(std::size_t tagOffset, std::size_t variantOffset,
395+
RVariantDeleter(std::size_t tagOffset, std::size_t variantOffset, std::size_t alignment,
391396
std::vector<std::unique_ptr<RDeleter>> itemDeleters)
392-
: fTagOffset(tagOffset), fVariantOffset(variantOffset), fItemDeleters(std::move(itemDeleters))
397+
: RDeleter(alignment),
398+
fTagOffset(tagOffset),
399+
fVariantOffset(variantOffset),
400+
fItemDeleters(std::move(itemDeleters))
393401
{
394402
}
395403
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
@@ -50,8 +50,9 @@ private:
5050
std::unique_ptr<RDeleter> fItemDeleter;
5151

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

127128
public:
128-
explicit RRVecDeleter(std::size_t itemAlignment) : fItemAlignment(itemAlignment) {}
129+
explicit RRVecDeleter(std::size_t itemAlignment)
130+
: RDeleter(ROOT::Internal::EvalRVecAlignment(itemAlignment)), fItemAlignment(itemAlignment)
131+
{
132+
}
129133
RRVecDeleter(std::size_t itemAlignment, std::size_t itemSize, std::unique_ptr<RDeleter> itemDeleter)
130-
: fItemAlignment(itemAlignment), fItemSize(itemSize), fItemDeleter(std::move(itemDeleter))
134+
: RDeleter(ROOT::Internal::EvalRVecAlignment(itemAlignment)),
135+
fItemAlignment(itemAlignment),
136+
fItemSize(itemSize),
137+
fItemDeleter(std::move(itemDeleter))
131138
{
132139
}
133140
void operator()(void *objPtr, bool dtorOnly) final;
@@ -210,11 +217,8 @@ class RVectorField : public RFieldBase {
210217
std::unique_ptr<RDeleter> fItemDeleter;
211218

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

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

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

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

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

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

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

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

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

422430
/// Constructs value in a given location of size at least GetValueSize(). Called by the base class' CreateValue().
423431
virtual void ConstructValue(void *where) const = 0;
424-
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()); }
425433
/// Allow derived classes to call ConstructValue(void *) and GetDeleter() on other (sub)fields.
426434
static void CallConstructValueOn(const RFieldBase &other, void *where) { other.ConstructValue(where); }
427435
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);
428438

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

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

‎tree/ntuple/src/RField.cxx‎

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

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

11741174
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
@@ -2,6 +2,7 @@
22
/// \author Jonas Hahnfeld <jonas.hahnfeld@cern.ch>
33
/// \date 2024-11-19
44

5+
#include <ROOT/BitUtils.hxx>
56
#include <ROOT/RError.hxx>
67
#include <ROOT/RField.hxx>
78
#include <ROOT/RFieldBase.hxx>
@@ -167,7 +168,7 @@ void ROOT::RFieldBase::RBulkValues::ReleaseValues()
167168
}
168169
}
169170

170-
operator delete(fValues);
171+
operator delete(fValues, fField->GetAlignment());
171172
}
172173

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

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

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

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

‎tree/ntuple/src/RFieldMeta.cxx‎

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

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

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

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

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

1433+
ROOT::RStreamerField::RStreamerFieldDeleter::RStreamerFieldDeleter(TClass *cl)
1434+
: RDeleter(cl->GetClassAlignment()), fClass(cl)
1435+
{
1436+
}
1437+
14171438
void ROOT::RStreamerField::RStreamerFieldDeleter::operator()(void *objPtr, bool dtorOnly)
14181439
{
14191440
fClass->Destructor(objPtr, true /* dtorOnly */);
@@ -1815,7 +1836,7 @@ std::unique_ptr<ROOT::RFieldBase::RDeleter> ROOT::RVariantField::GetDeleter() co
18151836
for (const auto &f : fSubfields) {
18161837
itemDeleters.emplace_back(GetDeleterOf(*f));
18171838
}
1818-
return std::make_unique<RVariantDeleter>(fTagOffset, fVariantOffset, std::move(itemDeleters));
1839+
return std::make_unique<RVariantDeleter>(fTagOffset, fVariantOffset, GetAlignment(), std::move(itemDeleters));
18191840
}
18201841

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

0 commit comments

Comments
 (0)