Skip to content

Commit b7da805

Browse files
committed
[ntuple] use TClass alignment info where possible
1 parent 59c882e commit b7da805

12 files changed

Lines changed: 143 additions & 35 deletions

tree/ntuple/inc/ROOT/RField.hxx

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,6 @@ private:
164164
TClass *fClass;
165165
/// Additional information kept for each entry in `fSubfields`
166166
std::vector<RSubfieldInfo> fSubfieldsInfo;
167-
std::size_t fMaxAlignment = 1;
168167

169168
/// The staging area stores inputs to I/O rules according to the offsets given by the streamer info of
170169
/// "TypeName@@Version". The area is allocated depending on I/O rules resp. the source members of the I/O rules.
@@ -219,8 +218,8 @@ public:
219218
~RClassField() override;
220219

221220
std::vector<RValue> SplitValue(const RValue &value) const final;
222-
size_t GetValueSize() const final;
223-
size_t GetAlignment() const final { return fMaxAlignment; }
221+
std::size_t GetValueSize() const final;
222+
std::size_t GetAlignment() const final;
224223
std::uint32_t GetTypeVersion() const final;
225224
std::uint32_t GetTypeChecksum() const final;
226225
/// For polymorphic classes (that declare or inherit at least one virtual method), return the expected dynamic type

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -175,8 +175,8 @@ public:
175175
~RProxiedCollectionField() override = default;
176176

177177
std::vector<RValue> SplitValue(const RValue &value) const final;
178-
size_t GetValueSize() const final { return fProxy->Sizeof(); }
179-
size_t GetAlignment() const final { return alignof(std::max_align_t); }
178+
std::size_t GetValueSize() const final;
179+
std::size_t GetAlignment() const final;
180180
void AcceptVisitor(ROOT::Detail::RFieldVisitor &visitor) const final;
181181
};
182182

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

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -263,8 +263,8 @@ public:
263263
~ROptionalField() override = default;
264264

265265
std::vector<RValue> SplitValue(const RValue &value) const final;
266-
size_t GetValueSize() const final;
267-
size_t GetAlignment() const final;
266+
std::size_t GetValueSize() const final;
267+
std::size_t GetAlignment() const final;
268268
};
269269

270270
template <typename ItemT>
@@ -312,8 +312,8 @@ public:
312312
~RUniquePtrField() override = default;
313313

314314
std::vector<RValue> SplitValue(const RValue &value) const final;
315-
size_t GetValueSize() const final { return sizeof(std::unique_ptr<char>); }
316-
size_t GetAlignment() const final { return alignof(std::unique_ptr<char>); }
315+
std::size_t GetValueSize() const final { return sizeof(std::unique_ptr<char>); }
316+
std::size_t GetAlignment() const final { return alignof(std::unique_ptr<char>); }
317317
};
318318

319319
template <typename ItemT>
@@ -362,8 +362,8 @@ public:
362362
RField &operator=(RField &&other) = default;
363363
~RField() final = default;
364364

365-
size_t GetValueSize() const final { return sizeof(std::string); }
366-
size_t GetAlignment() const final { return std::alignment_of<std::string>(); }
365+
std::size_t GetValueSize() const final { return sizeof(std::string); }
366+
std::size_t GetAlignment() const final { return alignof(std::string); }
367367
void AcceptVisitor(ROOT::Detail::RFieldVisitor &visitor) const final;
368368
};
369369

@@ -435,8 +435,8 @@ public:
435435
RVariantField &operator=(RVariantField &&other) = default;
436436
~RVariantField() override = default;
437437

438-
size_t GetValueSize() const final;
439-
size_t GetAlignment() const final;
438+
std::size_t GetValueSize() const final;
439+
std::size_t GetAlignment() const final;
440440
};
441441

442442
template <typename... ItemTs>

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -260,8 +260,8 @@ public:
260260
CreateUntyped(std::string_view fieldName, std::unique_ptr<RFieldBase> itemField);
261261

262262
std::vector<RValue> SplitValue(const RValue &value) const final;
263-
size_t GetValueSize() const final { return sizeof(std::vector<char>); }
264-
size_t GetAlignment() const final { return std::alignment_of<std::vector<char>>(); }
263+
std::size_t GetValueSize() const final;
264+
std::size_t GetAlignment() const final;
265265
void AcceptVisitor(ROOT::Detail::RFieldVisitor &visitor) const final;
266266
};
267267

@@ -314,8 +314,8 @@ public:
314314

315315
std::vector<RValue> SplitValue(const RValue &value) const final;
316316

317-
size_t GetValueSize() const final { return sizeof(std::vector<bool>); }
318-
size_t GetAlignment() const final { return std::alignment_of<std::vector<bool>>(); }
317+
std::size_t GetValueSize() const final { return sizeof(std::vector<bool>); }
318+
std::size_t GetAlignment() const final { return alignof(std::vector<bool>); }
319319
void AcceptVisitor(ROOT::Detail::RFieldVisitor &visitor) const final;
320320
};
321321

@@ -409,8 +409,8 @@ public:
409409
RArrayAsVectorField &operator=(RArrayAsVectorField &&other) = default;
410410
~RArrayAsVectorField() final = default;
411411

412-
size_t GetValueSize() const final { return sizeof(std::vector<char>); }
413-
size_t GetAlignment() const final { return std::alignment_of<std::vector<char>>(); }
412+
std::size_t GetValueSize() const final;
413+
std::size_t GetAlignment() const final;
414414

415415
std::vector<RFieldBase::RValue> SplitValue(const RFieldBase::RValue &value) const final;
416416
void AcceptVisitor(ROOT::Detail::RFieldVisitor &visitor) const final;

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,6 @@ class RSoAField : public RFieldBase {
6666
/// The offset of the RVec members in the SoA type in the order of subfields of the underlying record type.
6767
/// In particular, the order is not necessarily the same then the order of RVec members in the SoA class.
6868
std::vector<std::size_t> fSoAMemberOffsets;
69-
std::size_t fMaxAlignment = 1;
7069
ROOT::Internal::RColumnIndex fNWritten;
7170

7271
RSoAField(std::string_view fieldName, const RSoAField &source); ///< Used by CloneImpl
@@ -97,7 +96,7 @@ public:
9796

9897
std::vector<RValue> SplitValue(const RValue &value) const final;
9998
size_t GetValueSize() const final;
100-
size_t GetAlignment() const final { return fMaxAlignment; }
99+
size_t GetAlignment() const final;
101100
std::uint32_t GetTypeVersion() const final;
102101
std::uint32_t GetTypeChecksum() const final;
103102
/// For polymorphic classes (that declare or inherit at least one virtual method), return the expected dynamic type

tree/ntuple/inc/ROOT/RFieldBase.hxx

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -555,6 +555,9 @@ protected:
555555
const ROOT::RNTupleDescriptor *desc, ROOT::DescriptorId_t fieldId);
556556

557557
public:
558+
/// Maximum supported alignment for field types, i.e. maximum returned by GetAlignment()
559+
static constexpr std::size_t kMaxAlignment = 4096;
560+
558561
template <bool IsConstT>
559562
class RSchemaIteratorTemplate;
560563
using RSchemaIterator = RSchemaIteratorTemplate<false>;
@@ -623,10 +626,9 @@ public:
623626
/// correct `std::variant` or all the elements of a collection. The default implementation assumes no subvalues
624627
/// and returns an empty vector.
625628
virtual std::vector<RValue> SplitValue(const RValue &value) const;
626-
/// The number of bytes taken by a value of the appropriate type
629+
/// What sizeof(T) for this type returns
627630
virtual size_t GetValueSize() const = 0;
628-
/// As a rule of thumb, the alignment is equal to the size of the type. There are, however, various exceptions
629-
/// to this rule depending on OS and CPU architecture. So enforce the alignment to be explicitly spelled out.
631+
/// What alignof(T) for this type returns
630632
virtual size_t GetAlignment() const = 0;
631633
std::uint32_t GetTraits() const { return fTraits; }
632634
bool HasReadCallbacks() const { return !fReadCallbacks.empty(); }

tree/ntuple/src/RFieldMeta.cxx

Lines changed: 40 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
// - RField<TObject>
1515
// - RVariantField
1616

17+
#include <ROOT/BitUtils.hxx>
1718
#include <ROOT/RField.hxx>
1819
#include <ROOT/RFieldBase.hxx>
1920
#include <ROOT/RFieldUtils.hxx>
@@ -116,6 +117,12 @@ TEnum *EnsureValidEnum(std::string_view enumName)
116117
return e;
117118
}
118119

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+
119126
/// Create a comma-separated list of type names from the given fields. Uses either the real type names or the
120127
/// type aliases (if there are any, otherwise the actual type name). Used to construct template argument lists
121128
/// for templated types such as std::pair<...>, std::tuple<...>, std::variant<...>.
@@ -186,8 +193,7 @@ std::string BuildMapTypeName(ROOT::RMapField::EMapType mapType, const ROOT::RFie
186193
ROOT::RClassField::RClassField(std::string_view fieldName, const RClassField &source)
187194
: ROOT::RFieldBase(fieldName, source.GetTypeName(), ROOT::ENTupleStructure::kRecord, false /* isSimple */),
188195
fClass(source.fClass),
189-
fSubfieldsInfo(source.fSubfieldsInfo),
190-
fMaxAlignment(source.fMaxAlignment)
196+
fSubfieldsInfo(source.fSubfieldsInfo)
191197
{
192198
for (const auto &f : source.GetConstSubfields()) {
193199
RFieldBase::Attach(f->Clone(f->GetFieldName()));
@@ -280,7 +286,6 @@ ROOT::RClassField::~RClassField()
280286

281287
void ROOT::RClassField::Attach(std::unique_ptr<RFieldBase> child, RSubfieldInfo info)
282288
{
283-
fMaxAlignment = std::max(fMaxAlignment, child->GetAlignment());
284289
fSubfieldsInfo.push_back(info);
285290
RFieldBase::Attach(std::move(child));
286291
}
@@ -622,11 +627,18 @@ std::vector<ROOT::RFieldBase::RValue> ROOT::RClassField::SplitValue(const RValue
622627
return result;
623628
}
624629

625-
size_t ROOT::RClassField::GetValueSize() const
630+
std::size_t ROOT::RClassField::GetValueSize() const
626631
{
627632
return fClass->GetClassSize();
628633
}
629634

635+
std::size_t ROOT::RClassField::GetAlignment() const
636+
{
637+
const auto align = fClass->GetClassAlignment();
638+
EnsureValidAlignment(align);
639+
return align;
640+
}
641+
630642
std::uint32_t ROOT::RClassField::GetTypeVersion() const
631643
{
632644
return fClass->GetClassVersion();
@@ -656,8 +668,7 @@ void ROOT::RClassField::AcceptVisitor(ROOT::Detail::RFieldVisitor &visitor) cons
656668
ROOT::Experimental::RSoAField::RSoAField(std::string_view fieldName, const RSoAField &source)
657669
: ROOT::RFieldBase(fieldName, source.GetTypeName(), ROOT::ENTupleStructure::kCollection, false /* isSimple */),
658670
fSoAClass(source.fSoAClass),
659-
fSoAMemberOffsets(source.fSoAMemberOffsets),
660-
fMaxAlignment(source.fMaxAlignment)
671+
fSoAMemberOffsets(source.fSoAMemberOffsets)
661672
{
662673
fTraits = source.GetTraits();
663674
Attach(source.fSubfields[0]->Clone(source.fSubfields[0]->GetFieldName()));
@@ -756,8 +767,6 @@ ROOT::Experimental::RSoAField::RSoAField(std::string_view fieldName, TClass *clS
756767
leftType + " vs. " + rightType + ")"));
757768
}
758769

759-
fMaxAlignment = std::max(fMaxAlignment, vecField->GetAlignment());
760-
761770
assert(itr->second < fSoAMemberOffsets.size());
762771
fSoAMemberOffsets[itr->second] = dataMember->GetOffset();
763772
nMembers++;
@@ -862,7 +871,7 @@ std::vector<ROOT::RFieldBase::RValue> ROOT::Experimental::RSoAField::SplitValue(
862871
return std::vector<RValue>();
863872
}
864873

865-
size_t ROOT::Experimental::RSoAField::GetValueSize() const
874+
std::size_t ROOT::Experimental::RSoAField::GetValueSize() const
866875
{
867876
return fSoAClass->GetClassSize();
868877
}
@@ -877,6 +886,13 @@ std::uint32_t ROOT::Experimental::RSoAField::GetTypeChecksum() const
877886
return fSoAClass->GetCheckSum();
878887
}
879888

889+
std::size_t ROOT::Experimental::RSoAField::GetAlignment() const
890+
{
891+
const auto align = fSoAClass->GetClassAlignment();
892+
EnsureValidAlignment(align);
893+
return align;
894+
}
895+
880896
const std::type_info *ROOT::Experimental::RSoAField::GetPolymorphicTypeInfo() const
881897
{
882898
// TODO(jblomer): factor out
@@ -1197,6 +1213,18 @@ std::vector<ROOT::RFieldBase::RValue> ROOT::RProxiedCollectionField::SplitValue(
11971213
return result;
11981214
}
11991215

1216+
std::size_t ROOT::RProxiedCollectionField::GetValueSize() const
1217+
{
1218+
return fProxy->Sizeof();
1219+
}
1220+
1221+
std::size_t ROOT::RProxiedCollectionField::GetAlignment() const
1222+
{
1223+
const auto align = fProxy->GetCollectionClass()->GetClassAlignment();
1224+
EnsureValidAlignment(align);
1225+
return align;
1226+
}
1227+
12001228
void ROOT::RProxiedCollectionField::AcceptVisitor(ROOT::Detail::RFieldVisitor &visitor) const
12011229
{
12021230
visitor.VisitProxiedCollectionField(*this);
@@ -1404,7 +1432,9 @@ ROOT::RExtraTypeInfoDescriptor ROOT::RStreamerField::GetExtraTypeInfo() const
14041432

14051433
std::size_t ROOT::RStreamerField::GetAlignment() const
14061434
{
1407-
return std::min(alignof(std::max_align_t), GetValueSize()); // TODO(jblomer): fix me
1435+
const auto align = fClass->GetClassAlignment();
1436+
EnsureValidAlignment(align);
1437+
return align;
14081438
}
14091439

14101440
std::size_t ROOT::RStreamerField::GetValueSize() const

tree/ntuple/src/RFieldSequenceContainer.cxx

Lines changed: 35 additions & 0 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/RField.hxx>
67
#include <ROOT/RFieldBase.hxx>
78
#include <ROOT/RFieldVisitor.hxx>
@@ -29,6 +30,20 @@ std::vector<ROOT::RFieldBase::RValue> SplitVector(std::shared_ptr<void> valuePtr
2930
return result;
3031
}
3132

33+
std::size_t GetSizeOfVector()
34+
{
35+
static_assert(sizeof(std::vector<char>) ==
36+
sizeof(std::vector<ROOT::Internal::RAlignedStorage<ROOT::RFieldBase::kMaxAlignment>>));
37+
return sizeof(std::vector<char>);
38+
}
39+
40+
std::size_t GetAlignOfVector()
41+
{
42+
static_assert(alignof(std::vector<char>) ==
43+
alignof(std::vector<ROOT::Internal::RAlignedStorage<ROOT::RFieldBase::kMaxAlignment>>));
44+
return alignof(std::vector<char>);
45+
}
46+
3247
} // anonymous namespace
3348

3449
ROOT::RArrayField::RArrayField(std::string_view fieldName, std::unique_ptr<RFieldBase> itemField,
@@ -647,6 +662,16 @@ std::vector<ROOT::RFieldBase::RValue> ROOT::RVectorField::SplitValue(const RValu
647662
return SplitVector(value.GetPtr<void>(), *fSubfields[0]);
648663
}
649664

665+
std::size_t ROOT::RVectorField::GetValueSize() const
666+
{
667+
return GetSizeOfVector();
668+
}
669+
670+
std::size_t ROOT::RVectorField::GetAlignment() const
671+
{
672+
return GetAlignOfVector();
673+
}
674+
650675
void ROOT::RVectorField::AcceptVisitor(ROOT::Detail::RFieldVisitor &visitor) const
651676
{
652677
visitor.VisitVectorField(*this);
@@ -973,6 +998,16 @@ std::vector<ROOT::RFieldBase::RValue> ROOT::RArrayAsVectorField::SplitValue(cons
973998
return SplitVector(value.GetPtr<void>(), *fSubfields[0]);
974999
}
9751000

1001+
std::size_t ROOT::RArrayAsVectorField::GetValueSize() const
1002+
{
1003+
return GetSizeOfVector();
1004+
}
1005+
1006+
std::size_t ROOT::RArrayAsVectorField::GetAlignment() const
1007+
{
1008+
return GetAlignOfVector();
1009+
}
1010+
9761011
void ROOT::RArrayAsVectorField::AcceptVisitor(ROOT::Detail::RFieldVisitor &visitor) const
9771012
{
9781013
visitor.VisitArrayAsVectorField(*this);

tree/ntuple/test/CMakeLists.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ ROOT_STANDARD_LIBRARY_PACKAGE(CustomStruct
1111
HEADERS ${CMAKE_CURRENT_SOURCE_DIR}/CustomStruct.hxx
1212
SOURCES CustomStruct.cxx
1313
LINKDEF CustomStructLinkDef.h
14-
DEPENDENCIES RIO)
14+
DEPENDENCIES RIO ROOTVecOps)
1515
configure_file(CustomStruct.hxx . COPYONLY)
1616
if(MSVC AND NOT CMAKE_GENERATOR MATCHES Ninja)
1717
add_custom_command(TARGET CustomStruct POST_BUILD

tree/ntuple/test/CustomStruct.hxx

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,14 @@
11
#ifndef ROOT_RNTuple_Test_CustomStruct
22
#define ROOT_RNTuple_Test_CustomStruct
33

4+
#include <ROOT/RVec.hxx>
5+
46
#include <RtypesCore.h> // for Double32_t
57
#include <TObject.h>
68
#include <TRootIOCtor.h>
79
#include <TVirtualCollectionProxy.h>
810

11+
#include <array>
912
#include <chrono>
1013
#include <stdexcept>
1114
#include <cstddef>
@@ -478,4 +481,20 @@ struct MemberWithCustomStreamer {
478481
ClassDefNV(MemberWithCustomStreamer, 2);
479482
};
480483

484+
struct AlignmentDeterminedByTransientMember {
485+
short int fA;
486+
float fTransient; ///<!
487+
};
488+
489+
struct alignas(8) AlignedAs {};
490+
491+
struct alignas(64) OverAligned {
492+
std::uint32_t fA;
493+
};
494+
495+
struct AlignmentEnvelope {
496+
ROOT::RVec<OverAligned> fVec;
497+
std::array<OverAligned, 2> fArr;
498+
};
499+
481500
#endif

0 commit comments

Comments
 (0)