From c215356012e96b84dd8a4e36f11ec06f77c1dd49 Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Sun, 24 May 2026 22:46:15 +0200 Subject: [PATCH] [ntuple] use TClass alignment info where possible (cherry picked from commit eec14432eb82133b63bc84da3689455fba2ba39e) --- tree/ntuple/inc/ROOT/RField.hxx | 5 +- .../ROOT/RField/RFieldProxiedCollection.hxx | 4 +- tree/ntuple/inc/ROOT/RField/RFieldSTLMisc.hxx | 16 +++--- .../ROOT/RField/RFieldSequenceContainer.hxx | 12 ++--- tree/ntuple/inc/ROOT/RField/RFieldSoA.hxx | 3 +- tree/ntuple/inc/ROOT/RFieldBase.hxx | 5 +- tree/ntuple/src/RFieldMeta.cxx | 50 +++++++++++++++---- tree/ntuple/src/RFieldSequenceContainer.cxx | 30 +++++++++++ tree/ntuple/test/CMakeLists.txt | 2 +- tree/ntuple/test/CustomStruct.hxx | 7 +++ tree/ntuple/test/CustomStructLinkDef.h | 3 ++ tree/ntuple/test/rfield_class.cxx | 12 +++++ 12 files changed, 114 insertions(+), 35 deletions(-) diff --git a/tree/ntuple/inc/ROOT/RField.hxx b/tree/ntuple/inc/ROOT/RField.hxx index bc0c4f952877d..201d4ea66a775 100644 --- a/tree/ntuple/inc/ROOT/RField.hxx +++ b/tree/ntuple/inc/ROOT/RField.hxx @@ -167,7 +167,6 @@ private: TClass *fClass; /// Additional information kept for each entry in `fSubfields` std::vector fSubfieldsInfo; - std::size_t fMaxAlignment = 1; /// The staging area stores inputs to I/O rules according to the offsets given by the streamer info of /// "TypeName@@Version". The area is allocated depending on I/O rules resp. the source members of the I/O rules. @@ -222,8 +221,8 @@ public: ~RClassField() override; std::vector SplitValue(const RValue &value) const final; - size_t GetValueSize() const final; - size_t GetAlignment() const final { return fMaxAlignment; } + std::size_t GetValueSize() const final; + std::size_t GetAlignment() const final; std::uint32_t GetTypeVersion() const final; std::uint32_t GetTypeChecksum() const final; /// For polymorphic classes (that declare or inherit at least one virtual method), return the expected dynamic type diff --git a/tree/ntuple/inc/ROOT/RField/RFieldProxiedCollection.hxx b/tree/ntuple/inc/ROOT/RField/RFieldProxiedCollection.hxx index 9ee3724b58b23..4b2637471a474 100644 --- a/tree/ntuple/inc/ROOT/RField/RFieldProxiedCollection.hxx +++ b/tree/ntuple/inc/ROOT/RField/RFieldProxiedCollection.hxx @@ -176,8 +176,8 @@ public: ~RProxiedCollectionField() override = default; std::vector SplitValue(const RValue &value) const final; - size_t GetValueSize() const final { return fProxy->Sizeof(); } - size_t GetAlignment() const final { return alignof(std::max_align_t); } + std::size_t GetValueSize() const final; + std::size_t GetAlignment() const final; void AcceptVisitor(ROOT::Detail::RFieldVisitor &visitor) const final; }; diff --git a/tree/ntuple/inc/ROOT/RField/RFieldSTLMisc.hxx b/tree/ntuple/inc/ROOT/RField/RFieldSTLMisc.hxx index eeeca91d20501..6a59b3a4d5e7a 100644 --- a/tree/ntuple/inc/ROOT/RField/RFieldSTLMisc.hxx +++ b/tree/ntuple/inc/ROOT/RField/RFieldSTLMisc.hxx @@ -264,8 +264,8 @@ public: ~ROptionalField() override = default; std::vector SplitValue(const RValue &value) const final; - size_t GetValueSize() const final; - size_t GetAlignment() const final; + std::size_t GetValueSize() const final; + std::size_t GetAlignment() const final; }; template @@ -313,8 +313,8 @@ public: ~RUniquePtrField() override = default; std::vector SplitValue(const RValue &value) const final; - size_t GetValueSize() const final { return sizeof(std::unique_ptr); } - size_t GetAlignment() const final { return alignof(std::unique_ptr); } + std::size_t GetValueSize() const final { return sizeof(std::unique_ptr); } + std::size_t GetAlignment() const final { return alignof(std::unique_ptr); } }; template @@ -363,8 +363,8 @@ public: RField &operator=(RField &&other) = default; ~RField() final = default; - size_t GetValueSize() const final { return sizeof(std::string); } - size_t GetAlignment() const final { return std::alignment_of(); } + std::size_t GetValueSize() const final { return sizeof(std::string); } + std::size_t GetAlignment() const final { return alignof(std::string); } void AcceptVisitor(ROOT::Detail::RFieldVisitor &visitor) const final; }; @@ -436,8 +436,8 @@ public: RVariantField &operator=(RVariantField &&other) = default; ~RVariantField() override = default; - size_t GetValueSize() const final; - size_t GetAlignment() const final; + std::size_t GetValueSize() const final; + std::size_t GetAlignment() const final; }; template diff --git a/tree/ntuple/inc/ROOT/RField/RFieldSequenceContainer.hxx b/tree/ntuple/inc/ROOT/RField/RFieldSequenceContainer.hxx index e4b152f0ccb49..4bf35f3c6f657 100644 --- a/tree/ntuple/inc/ROOT/RField/RFieldSequenceContainer.hxx +++ b/tree/ntuple/inc/ROOT/RField/RFieldSequenceContainer.hxx @@ -261,8 +261,8 @@ public: CreateUntyped(std::string_view fieldName, std::unique_ptr itemField); std::vector SplitValue(const RValue &value) const final; - size_t GetValueSize() const final { return sizeof(std::vector); } - size_t GetAlignment() const final { return std::alignment_of>(); } + std::size_t GetValueSize() const final; + std::size_t GetAlignment() const final; void AcceptVisitor(ROOT::Detail::RFieldVisitor &visitor) const final; }; @@ -315,8 +315,8 @@ public: std::vector SplitValue(const RValue &value) const final; - size_t GetValueSize() const final { return sizeof(std::vector); } - size_t GetAlignment() const final { return std::alignment_of>(); } + std::size_t GetValueSize() const final { return sizeof(std::vector); } + std::size_t GetAlignment() const final { return alignof(std::vector); } void AcceptVisitor(ROOT::Detail::RFieldVisitor &visitor) const final; }; @@ -410,8 +410,8 @@ public: RArrayAsVectorField &operator=(RArrayAsVectorField &&other) = default; ~RArrayAsVectorField() final = default; - size_t GetValueSize() const final { return sizeof(std::vector); } - size_t GetAlignment() const final { return std::alignment_of>(); } + std::size_t GetValueSize() const final; + std::size_t GetAlignment() const final; std::vector SplitValue(const RFieldBase::RValue &value) const final; void AcceptVisitor(ROOT::Detail::RFieldVisitor &visitor) const final; diff --git a/tree/ntuple/inc/ROOT/RField/RFieldSoA.hxx b/tree/ntuple/inc/ROOT/RField/RFieldSoA.hxx index 5fdc3fc9a4583..97a625eeb3c83 100644 --- a/tree/ntuple/inc/ROOT/RField/RFieldSoA.hxx +++ b/tree/ntuple/inc/ROOT/RField/RFieldSoA.hxx @@ -67,7 +67,6 @@ class RSoAField : public RFieldBase { /// The offset of the RVec members in the SoA type in the order of subfields of the underlying record type. /// In particular, the order is not necessarily the same then the order of RVec members in the SoA class. std::vector fSoAMemberOffsets; - std::size_t fMaxAlignment = 1; ROOT::Internal::RColumnIndex fNWritten; RSoAField(std::string_view fieldName, const RSoAField &source); ///< Used by CloneImpl @@ -98,7 +97,7 @@ public: std::vector SplitValue(const RValue &value) const final; size_t GetValueSize() const final; - size_t GetAlignment() const final { return fMaxAlignment; } + size_t GetAlignment() const final; std::uint32_t GetTypeVersion() const final; std::uint32_t GetTypeChecksum() const final; /// For polymorphic classes (that declare or inherit at least one virtual method), return the expected dynamic type diff --git a/tree/ntuple/inc/ROOT/RFieldBase.hxx b/tree/ntuple/inc/ROOT/RFieldBase.hxx index 2761e391116ae..3bd6175c2f567 100644 --- a/tree/ntuple/inc/ROOT/RFieldBase.hxx +++ b/tree/ntuple/inc/ROOT/RFieldBase.hxx @@ -624,10 +624,9 @@ public: /// correct `std::variant` or all the elements of a collection. The default implementation assumes no subvalues /// and returns an empty vector. virtual std::vector SplitValue(const RValue &value) const; - /// The number of bytes taken by a value of the appropriate type + /// What sizeof(T) for this type returns virtual size_t GetValueSize() const = 0; - /// As a rule of thumb, the alignment is equal to the size of the type. There are, however, various exceptions - /// to this rule depending on OS and CPU architecture. So enforce the alignment to be explicitly spelled out. + /// What alignof(T) for this type returns virtual size_t GetAlignment() const = 0; std::uint32_t GetTraits() const { return fTraits; } bool HasReadCallbacks() const { return !fReadCallbacks.empty(); } diff --git a/tree/ntuple/src/RFieldMeta.cxx b/tree/ntuple/src/RFieldMeta.cxx index 78d3d1aba7da7..5f64332ed0e27 100644 --- a/tree/ntuple/src/RFieldMeta.cxx +++ b/tree/ntuple/src/RFieldMeta.cxx @@ -15,6 +15,7 @@ // - RField // - RVariantField +#include #include #include #include @@ -117,6 +118,12 @@ TEnum *EnsureValidEnum(std::string_view enumName) return e; } +void EnsureValidAlignment(std::size_t align) +{ + if (!ROOT::Internal::IsValidAlignment(align)) + throw ROOT::RException(R__FAIL(std::string("invalid alignment: ") + std::to_string(align))); +} + /// Create a comma-separated list of type names from the given fields. Uses either the real type names or the /// type aliases (if there are any, otherwise the actual type name). Used to construct template argument lists /// for templated types such as std::pair<...>, std::tuple<...>, std::variant<...>. @@ -187,8 +194,7 @@ std::string BuildMapTypeName(ROOT::RMapField::EMapType mapType, const ROOT::RFie ROOT::RClassField::RClassField(std::string_view fieldName, const RClassField &source) : ROOT::RFieldBase(fieldName, source.GetTypeName(), ROOT::ENTupleStructure::kRecord, false /* isSimple */), fClass(source.fClass), - fSubfieldsInfo(source.fSubfieldsInfo), - fMaxAlignment(source.fMaxAlignment) + fSubfieldsInfo(source.fSubfieldsInfo) { for (const auto &f : source.GetConstSubfields()) { RFieldBase::Attach(f->Clone(f->GetFieldName())); @@ -281,7 +287,6 @@ ROOT::RClassField::~RClassField() void ROOT::RClassField::Attach(std::unique_ptr child, RSubfieldInfo info) { - fMaxAlignment = std::max(fMaxAlignment, child->GetAlignment()); fSubfieldsInfo.push_back(info); RFieldBase::Attach(std::move(child)); } @@ -623,11 +628,18 @@ std::vector ROOT::RClassField::SplitValue(const RValue return result; } -size_t ROOT::RClassField::GetValueSize() const +std::size_t ROOT::RClassField::GetValueSize() const { return fClass->GetClassSize(); } +std::size_t ROOT::RClassField::GetAlignment() const +{ + const auto align = fClass->GetClassAlignment(); + EnsureValidAlignment(align); + return align; +} + std::uint32_t ROOT::RClassField::GetTypeVersion() const { return fClass->GetClassVersion(); @@ -657,8 +669,7 @@ void ROOT::RClassField::AcceptVisitor(ROOT::Detail::RFieldVisitor &visitor) cons ROOT::Experimental::RSoAField::RSoAField(std::string_view fieldName, const RSoAField &source) : ROOT::RFieldBase(fieldName, source.GetTypeName(), ROOT::ENTupleStructure::kCollection, false /* isSimple */), fSoAClass(source.fSoAClass), - fSoAMemberOffsets(source.fSoAMemberOffsets), - fMaxAlignment(source.fMaxAlignment) + fSoAMemberOffsets(source.fSoAMemberOffsets) { fTraits = source.GetTraits(); Attach(source.fSubfields[0]->Clone(source.fSubfields[0]->GetFieldName())); @@ -757,8 +768,6 @@ ROOT::Experimental::RSoAField::RSoAField(std::string_view fieldName, TClass *clS leftType + " vs. " + rightType + ")")); } - fMaxAlignment = std::max(fMaxAlignment, vecField->GetAlignment()); - assert(itr->second < fSoAMemberOffsets.size()); fSoAMemberOffsets[itr->second] = dataMember->GetOffset(); nMembers++; @@ -863,7 +872,7 @@ std::vector ROOT::Experimental::RSoAField::SplitValue( return std::vector(); } -size_t ROOT::Experimental::RSoAField::GetValueSize() const +std::size_t ROOT::Experimental::RSoAField::GetValueSize() const { return fSoAClass->GetClassSize(); } @@ -878,6 +887,13 @@ std::uint32_t ROOT::Experimental::RSoAField::GetTypeChecksum() const return fSoAClass->GetCheckSum(); } +std::size_t ROOT::Experimental::RSoAField::GetAlignment() const +{ + const auto align = fSoAClass->GetClassAlignment(); + EnsureValidAlignment(align); + return align; +} + const std::type_info *ROOT::Experimental::RSoAField::GetPolymorphicTypeInfo() const { // TODO(jblomer): factor out @@ -1198,6 +1214,18 @@ std::vector ROOT::RProxiedCollectionField::SplitValue( return result; } +std::size_t ROOT::RProxiedCollectionField::GetValueSize() const +{ + return fProxy->Sizeof(); +} + +std::size_t ROOT::RProxiedCollectionField::GetAlignment() const +{ + const auto align = fProxy->GetCollectionClass()->GetClassAlignment(); + EnsureValidAlignment(align); + return align; +} + void ROOT::RProxiedCollectionField::AcceptVisitor(ROOT::Detail::RFieldVisitor &visitor) const { visitor.VisitProxiedCollectionField(*this); @@ -1405,7 +1433,9 @@ ROOT::RExtraTypeInfoDescriptor ROOT::RStreamerField::GetExtraTypeInfo() const std::size_t ROOT::RStreamerField::GetAlignment() const { - return std::min(alignof(std::max_align_t), GetValueSize()); // TODO(jblomer): fix me + const auto align = fClass->GetClassAlignment(); + EnsureValidAlignment(align); + return align; } std::size_t ROOT::RStreamerField::GetValueSize() const diff --git a/tree/ntuple/src/RFieldSequenceContainer.cxx b/tree/ntuple/src/RFieldSequenceContainer.cxx index 9c96ee2990d03..0917432397507 100644 --- a/tree/ntuple/src/RFieldSequenceContainer.cxx +++ b/tree/ntuple/src/RFieldSequenceContainer.cxx @@ -30,6 +30,16 @@ std::vector SplitVector(std::shared_ptr valuePtr return result; } +std::size_t GetSizeOfVector() +{ + return sizeof(std::vector); +} + +std::size_t GetAlignOfVector() +{ + return alignof(std::vector); +} + } // anonymous namespace ROOT::RArrayField::RArrayField(std::string_view fieldName, std::unique_ptr itemField, @@ -648,6 +658,16 @@ std::vector ROOT::RVectorField::SplitValue(const RValu return SplitVector(value.GetPtr(), *fSubfields[0]); } +std::size_t ROOT::RVectorField::GetValueSize() const +{ + return GetSizeOfVector(); +} + +std::size_t ROOT::RVectorField::GetAlignment() const +{ + return GetAlignOfVector(); +} + void ROOT::RVectorField::AcceptVisitor(ROOT::Detail::RFieldVisitor &visitor) const { visitor.VisitVectorField(*this); @@ -974,6 +994,16 @@ std::vector ROOT::RArrayAsVectorField::SplitValue(cons return SplitVector(value.GetPtr(), *fSubfields[0]); } +std::size_t ROOT::RArrayAsVectorField::GetValueSize() const +{ + return GetSizeOfVector(); +} + +std::size_t ROOT::RArrayAsVectorField::GetAlignment() const +{ + return GetAlignOfVector(); +} + void ROOT::RArrayAsVectorField::AcceptVisitor(ROOT::Detail::RFieldVisitor &visitor) const { visitor.VisitArrayAsVectorField(*this); diff --git a/tree/ntuple/test/CMakeLists.txt b/tree/ntuple/test/CMakeLists.txt index 34e8b373149be..7bd5824ddfd7d 100644 --- a/tree/ntuple/test/CMakeLists.txt +++ b/tree/ntuple/test/CMakeLists.txt @@ -11,7 +11,7 @@ ROOT_STANDARD_LIBRARY_PACKAGE(CustomStruct HEADERS ${CMAKE_CURRENT_SOURCE_DIR}/CustomStruct.hxx SOURCES CustomStruct.cxx LINKDEF CustomStructLinkDef.h - DEPENDENCIES RIO) + DEPENDENCIES RIO ROOTVecOps) configure_file(CustomStruct.hxx . COPYONLY) if(MSVC AND NOT CMAKE_GENERATOR MATCHES Ninja) add_custom_command(TARGET CustomStruct POST_BUILD diff --git a/tree/ntuple/test/CustomStruct.hxx b/tree/ntuple/test/CustomStruct.hxx index c3d6e633f356b..4897be279f4eb 100644 --- a/tree/ntuple/test/CustomStruct.hxx +++ b/tree/ntuple/test/CustomStruct.hxx @@ -478,4 +478,11 @@ struct MemberWithCustomStreamer { ClassDefNV(MemberWithCustomStreamer, 2); }; +struct AlignmentDeterminedByTransientMember { + short int fA; + float fTransient; ///GetAlignment()); + + // Respect custom alignment + EXPECT_EQ(alignof(AlignedAs), RFieldBase::Create("", "AlignedAs").Unwrap()->GetAlignment()); + EXPECT_EQ(sizeof(AlignedAs), RFieldBase::Create("", "AlignedAs").Unwrap()->GetValueSize()); + EXPECT_EQ(8u, RFieldBase::Create("", "AlignedAs").Unwrap()->GetValueSize()); +}