Skip to content

Commit 6ab801f

Browse files
jblomerjblomer
committed
[ntuple] fix memory allocation for over-aligned fields
1 parent 90cfea1 commit 6ab801f

14 files changed

Lines changed: 123 additions & 47 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
@@ -54,8 +54,9 @@ private:
5454
std::unique_ptr<RDeleter> fItemDeleter;
5555

5656
public:
57-
RArrayDeleter(std::size_t itemSize, std::size_t arrayLength, std::unique_ptr<RDeleter> itemDeleter)
58-
: fItemSize(itemSize), fArrayLength(arrayLength), fItemDeleter(std::move(itemDeleter))
57+
RArrayDeleter(std::size_t itemSize, std::size_t arrayLength, std::size_t alignment,
58+
std::unique_ptr<RDeleter> itemDeleter)
59+
: RDeleter(alignment), fItemSize(itemSize), fArrayLength(arrayLength), fItemDeleter(std::move(itemDeleter))
5960
{
6061
}
6162
void operator()(void *objPtr, bool dtorOnly) final;
@@ -130,9 +131,15 @@ class RRVecField : public RFieldBase {
130131
std::unique_ptr<RDeleter> fItemDeleter;
131132

132133
public:
133-
explicit RRVecDeleter(std::size_t itemAlignment) : fItemAlignment(itemAlignment) {}
134+
explicit RRVecDeleter(std::size_t itemAlignment)
135+
: RDeleter(ROOT::Internal::EvalRVecAlignment(itemAlignment)), fItemAlignment(itemAlignment)
136+
{
137+
}
134138
RRVecDeleter(std::size_t itemAlignment, std::size_t itemSize, std::unique_ptr<RDeleter> itemDeleter)
135-
: fItemAlignment(itemAlignment), fItemSize(itemSize), fItemDeleter(std::move(itemDeleter))
139+
: RDeleter(ROOT::Internal::EvalRVecAlignment(itemAlignment)),
140+
fItemAlignment(itemAlignment),
141+
fItemSize(itemSize),
142+
fItemDeleter(std::move(itemDeleter))
136143
{
137144
}
138145
void operator()(void *objPtr, bool dtorOnly) final;
@@ -215,11 +222,8 @@ class RVectorField : public RFieldBase {
215222
std::unique_ptr<RDeleter> fItemDeleter;
216223

217224
public:
218-
RVectorDeleter() = default;
219-
RVectorDeleter(std::size_t itemSize, std::unique_ptr<RDeleter> itemDeleter)
220-
: fItemSize(itemSize), fItemDeleter(std::move(itemDeleter))
221-
{
222-
}
225+
RVectorDeleter();
226+
RVectorDeleter(std::size_t itemSize, std::unique_ptr<RDeleter> itemDeleter);
223227
void operator()(void *objPtr, bool dtorOnly) final;
224228
};
225229

‎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: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
#ifndef ROOT_RFieldBase
1414
#define ROOT_RFieldBase
1515

16+
#include <ROOT/BitUtils.hxx>
1617
#include <ROOT/RColumn.hxx>
1718
#include <ROOT/RCreateFieldOptions.hxx>
1819
#include <ROOT/RError.hxx>
@@ -21,6 +22,7 @@
2122
#include <ROOT/RNTupleTypes.hxx>
2223

2324
#include <atomic>
25+
#include <cassert>
2426
#include <cstddef>
2527
#include <functional>
2628
#include <iterator>
@@ -107,19 +109,23 @@ protected:
107109
/// The deleter is operational without the field object and thus can be used to destruct/release a value after
108110
/// the field has been destructed.
109111
class RDeleter {
112+
std::size_t fAlignment;
113+
110114
public:
115+
explicit RDeleter(std::size_t align) : fAlignment(align) { assert(Internal::IsValidAlignment(align)); }
111116
virtual ~RDeleter() = default;
112117
virtual void operator()(void *objPtr, bool dtorOnly)
113118
{
114119
if (!dtorOnly)
115-
operator delete(objPtr);
120+
operator delete(objPtr, fAlignment);
116121
}
117122
};
118123

119124
/// A deleter for templated RFieldBase descendents where the value type is known.
120125
template <typename T>
121126
class RTypedDeleter : public RDeleter {
122127
public:
128+
RTypedDeleter() : RDeleter(alignof(T)) {}
123129
void operator()(void *objPtr, bool dtorOnly) final
124130
{
125131
std::destroy_at(static_cast<T *>(objPtr));
@@ -421,7 +427,7 @@ protected:
421427

422428
/// Constructs value in a given location of size at least GetValueSize(). Called by the base class' CreateValue().
423429
virtual void ConstructValue(void *where) const = 0;
424-
virtual std::unique_ptr<RDeleter> GetDeleter() const { return std::make_unique<RDeleter>(); }
430+
virtual std::unique_ptr<RDeleter> GetDeleter() const { return std::make_unique<RDeleter>(GetAlignment()); }
425431
/// Allow derived classes to call ConstructValue(void *) and GetDeleter() on other (sub)fields.
426432
static void CallConstructValueOn(const RFieldBase &other, void *where) { other.ConstructValue(where); }
427433
static std::unique_ptr<RDeleter> GetDeleterOf(const RFieldBase &other) { return other.GetDeleter(); }
@@ -555,9 +561,6 @@ protected:
555561
const ROOT::RNTupleDescriptor *desc, ROOT::DescriptorId_t fieldId);
556562

557563
public:
558-
/// Maximum supported alignment for field types, i.e. maximum returned by GetAlignment()
559-
static constexpr std::size_t kMaxAlignment = 4096;
560-
561564
template <bool IsConstT>
562565
class RSchemaIteratorTemplate;
563566
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: 4 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;

‎tree/ntuple/src/RFieldMeta.cxx‎

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

120-
void EnsureValidAlignment(std::size_t align)
120+
void EnsureValidAlignment(std::size_t alignment)
121121
{
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)));
122+
if (!ROOT::Internal::IsValidAlignment(alignment))
123+
throw ROOT::RException(R__FAIL(std::string("invalid alignment: ") + std::to_string(alignment)));
124124
}
125125

126126
/// Create a comma-separated list of type names from the given fields. Uses either the real type names or the
@@ -608,6 +608,8 @@ void ROOT::RClassField::ConstructValue(void *where) const
608608
fClass->New(where);
609609
}
610610

611+
ROOT::RClassField::RClassDeleter::RClassDeleter(TClass *cl) : RDeleter(cl->GetClassAlignment()), fClass(cl) {}
612+
611613
void ROOT::RClassField::RClassDeleter::operator()(void *objPtr, bool dtorOnly)
612614
{
613615
fClass->Destructor(objPtr, true /* dtorOnly */);
@@ -886,6 +888,10 @@ void ROOT::Experimental::RSoAField::ConstructValue(void *where) const
886888
fSoAClass->New(where);
887889
}
888890

891+
ROOT::Experimental::RSoAField::RSoADeleter::RSoADeleter(TClass *cl) : RDeleter(cl->GetClassAlignment()), fSoAClass(cl)
892+
{
893+
}
894+
889895
void ROOT::Experimental::RSoAField::RSoADeleter::operator()(void *objPtr, bool dtorOnly)
890896
{
891897
fSoAClass->Destructor(objPtr, true /* dtorOnly */);
@@ -1242,6 +1248,22 @@ std::unique_ptr<ROOT::RFieldBase::RDeleter> ROOT::RProxiedCollectionField::GetDe
12421248
return std::make_unique<RProxiedCollectionDeleter>(fProxy);
12431249
}
12441250

1251+
ROOT::RProxiedCollectionField::RProxiedCollectionDeleter::RProxiedCollectionDeleter(
1252+
std::shared_ptr<TVirtualCollectionProxy> proxy)
1253+
: RDeleter(proxy->GetCollectionClass()->GetClassAlignment()), fProxy(proxy)
1254+
{
1255+
}
1256+
1257+
ROOT::RProxiedCollectionField::RProxiedCollectionDeleter::RProxiedCollectionDeleter(
1258+
std::shared_ptr<TVirtualCollectionProxy> proxy, std::unique_ptr<RDeleter> itemDeleter, size_t itemSize)
1259+
: RDeleter(proxy->GetCollectionClass()->GetClassAlignment()),
1260+
fProxy(proxy),
1261+
fItemDeleter(std::move(itemDeleter)),
1262+
fItemSize(itemSize)
1263+
{
1264+
fIFuncsWrite = RCollectionIterableOnce::GetIteratorFuncs(fProxy.get(), false /* readFromDisk */);
1265+
}
1266+
12451267
void ROOT::RProxiedCollectionField::RProxiedCollectionDeleter::operator()(void *objPtr, bool dtorOnly)
12461268
{
12471269
if (fItemDeleter) {
@@ -1467,6 +1489,11 @@ void ROOT::RStreamerField::ConstructValue(void *where) const
14671489
fClass->New(where);
14681490
}
14691491

1492+
ROOT::RStreamerField::RStreamerFieldDeleter::RStreamerFieldDeleter(TClass *cl)
1493+
: RDeleter(cl->GetClassAlignment()), fClass(cl)
1494+
{
1495+
}
1496+
14701497
void ROOT::RStreamerField::RStreamerFieldDeleter::operator()(void *objPtr, bool dtorOnly)
14711498
{
14721499
fClass->Destructor(objPtr, true /* dtorOnly */);
@@ -1868,7 +1895,7 @@ std::unique_ptr<ROOT::RFieldBase::RDeleter> ROOT::RVariantField::GetDeleter() co
18681895
for (const auto &f : fSubfields) {
18691896
itemDeleters.emplace_back(GetDeleterOf(*f));
18701897
}
1871-
return std::make_unique<RVariantDeleter>(fTagOffset, fVariantOffset, std::move(itemDeleters));
1898+
return std::make_unique<RVariantDeleter>(fTagOffset, fVariantOffset, GetAlignment(), std::move(itemDeleters));
18721899
}
18731900

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

0 commit comments

Comments
 (0)