Skip to content

Commit bb61c3d

Browse files
jblomerjblomer
committed
[ntuple] fix memory allocation for over-aligned fields
1 parent e84b44c commit bb61c3d

14 files changed

Lines changed: 106 additions & 41 deletions

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,7 @@ private:
160160
TClass *fClass;
161161

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

@@ -241,7 +241,7 @@ private:
241241
TClass *fClass;
242242

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

‎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: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -102,25 +102,35 @@ 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)
118+
{
119+
R__ASSERT(fAlignment > 0 && fAlignment <= kMaxAlignment && ROOT::Internal::IsPowerOfTwo(fAlignment));
120+
}
112121
virtual ~RDeleter() = default;
113122
virtual void operator()(void *objPtr, bool dtorOnly)
114123
{
115124
if (!dtorOnly)
116-
operator delete(objPtr);
125+
operator delete(objPtr, fAlignment);
117126
}
118127
};
119128

120129
/// A deleter for templated RFieldBase descendents where the value type is known.
121130
template <typename T>
122131
class RTypedDeleter : public RDeleter {
123132
public:
133+
RTypedDeleter() : RDeleter(alignof(T)) {}
124134
void operator()(void *objPtr, bool dtorOnly) final
125135
{
126136
std::destroy_at(static_cast<T *>(objPtr));
@@ -422,7 +432,7 @@ protected:
422432

423433
/// Constructs value in a given location of size at least GetValueSize(). Called by the base class' CreateValue().
424434
virtual void ConstructValue(void *where) const = 0;
425-
virtual std::unique_ptr<RDeleter> GetDeleter() const { return std::make_unique<RDeleter>(); }
435+
virtual std::unique_ptr<RDeleter> GetDeleter() const { return std::make_unique<RDeleter>(GetAlignment()); }
426436
/// Allow derived classes to call ConstructValue(void *) and GetDeleter() on other (sub)fields.
427437
static void CallConstructValueOn(const RFieldBase &other, void *where) { other.ConstructValue(where); }
428438
static std::unique_ptr<RDeleter> GetDeleterOf(const RFieldBase &other) { return other.GetDeleter(); }
@@ -552,9 +562,6 @@ protected:
552562
const ROOT::RNTupleDescriptor *desc, ROOT::DescriptorId_t fieldId);
553563

554564
public:
555-
/// Maximum supported alignment for field types, i.e. maximum returned by GetAlignment()
556-
static constexpr std::size_t kMaxAlignment = 4096;
557-
558565
template <bool IsConstT>
559566
class RSchemaIteratorTemplate;
560567
using RSchemaIterator = RSchemaIteratorTemplate<false>;

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ std::size_t EvalRVecAlignment(std::size_t alignOfSubfield);
111111
void DestroyRVecWithChecks(std::size_t alignOfT, unsigned char **beginPtr, std::int32_t *capacityPtr);
112112

113113
// TODO: move to upcoming BitUtils.hxx
114-
inline bool IsPowerOfTwo(std::uint64_t v) { return (v & (v - 1)) == 0; }
114+
constexpr bool IsPowerOfTwo(std::uint64_t v) { return (v & (v - 1)) == 0; }
115115

116116
} // namespace Internal
117117
} // namespace ROOT

‎tree/ntuple/src/RField.cxx‎

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

732732
std::vector<ROOT::RFieldBase::RValue> ROOT::RRecordField::SplitValue(const RValue &value) const
@@ -1164,7 +1164,7 @@ std::unique_ptr<ROOT::RFieldBase::RDeleter> ROOT::ROptionalField::GetDeleter() c
11641164
{
11651165
return std::make_unique<ROptionalDeleter>(
11661166
(fSubfields[0]->GetTraits() & kTraitTriviallyDestructible) ? nullptr : GetDeleterOf(*fSubfields[0]),
1167-
fSubfields[0]->GetValueSize());
1167+
fSubfields[0]->GetValueSize(), GetAlignment());
11681168
}
11691169

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

‎tree/ntuple/src/RFieldBase.cxx‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,7 @@ void ROOT::RFieldBase::RBulkValues::ReleaseValues()
168168
}
169169
}
170170

171-
operator delete(fValues);
171+
operator delete(fValues, fField->GetAlignment());
172172
}
173173

174174
void ROOT::RFieldBase::RBulkValues::Reset(RNTupleLocalIndex firstIndex, std::size_t size)
@@ -178,7 +178,7 @@ void ROOT::RFieldBase::RBulkValues::Reset(RNTupleLocalIndex firstIndex, std::siz
178178
throw RException(R__FAIL("invalid attempt to bulk read beyond the adopted buffer"));
179179
}
180180
ReleaseValues();
181-
fValues = operator new(size * fValueSize);
181+
fValues = operator new(size * fValueSize, std::align_val_t(fField->GetAlignment()));
182182

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

634634
void *ROOT::RFieldBase::CreateObjectRawPtr() const
635635
{
636-
void *where = operator new(GetValueSize());
636+
void *where = operator new(GetValueSize(), std::align_val_t(GetAlignment()));
637637
R__ASSERT(where != nullptr);
638638
ConstructValue(where);
639639
return where;

0 commit comments

Comments
 (0)