[ntuple] use TClass alignment info where possible

(cherry picked from commit eec1443)

Author: jblomer

tree/ntuple/inc/ROOT/RField.hxx

@@ -167,7 +167,6 @@ private:
    TClass *fClass;
    /// Additional information kept for each entry in `fSubfields`
    std::vector<RSubfieldInfo> 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<RValue> 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

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

@@ -176,8 +176,8 @@ public:
    ~RProxiedCollectionField() override = default;
 
    std::vector<RValue> 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;
 };
 

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

@@ -264,8 +264,8 @@ public:
    ~ROptionalField() override = default;
 
    std::vector<RValue> 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 <typename ItemT>
@@ -313,8 +313,8 @@ public:
    ~RUniquePtrField() override = default;
 
    std::vector<RValue> SplitValue(const RValue &value) const final;
-   size_t GetValueSize() const final { return sizeof(std::unique_ptr<char>); }
-   size_t GetAlignment() const final { return alignof(std::unique_ptr<char>); }
+   std::size_t GetValueSize() const final { return sizeof(std::unique_ptr<char>); }
+   std::size_t GetAlignment() const final { return alignof(std::unique_ptr<char>); }
 };
 
 template <typename ItemT>
@@ -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::string>(); }
+   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 <typename... ItemTs>

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

@@ -261,8 +261,8 @@ public:
    CreateUntyped(std::string_view fieldName, std::unique_ptr<RFieldBase> itemField);
 
    std::vector<RValue> SplitValue(const RValue &value) const final;
-   size_t GetValueSize() const final { return sizeof(std::vector<char>); }
-   size_t GetAlignment() const final { return std::alignment_of<std::vector<char>>(); }
+   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<RValue> SplitValue(const RValue &value) const final;
 
-   size_t GetValueSize() const final { return sizeof(std::vector<bool>); }
-   size_t GetAlignment() const final { return std::alignment_of<std::vector<bool>>(); }
+   std::size_t GetValueSize() const final { return sizeof(std::vector<bool>); }
+   std::size_t GetAlignment() const final { return alignof(std::vector<bool>); }
    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<char>); }
-   size_t GetAlignment() const final { return std::alignment_of<std::vector<char>>(); }
+   std::size_t GetValueSize() const final;
+   std::size_t GetAlignment() const final;
 
    std::vector<RFieldBase::RValue> SplitValue(const RFieldBase::RValue &value) const final;
    void AcceptVisitor(ROOT::Detail::RFieldVisitor &visitor) const final;

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<std::size_t> 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<RValue> 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

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<RValue> 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(); }

tree/ntuple/src/RFieldMeta.cxx

@@ -15,6 +15,7 @@
 //  - RField<TObject>
 //  - RVariantField
 
+#include <ROOT/RAlignmentUtils.hxx>
 #include <ROOT/RField.hxx>
 #include <ROOT/RFieldBase.hxx>
 #include <ROOT/RFieldUtils.hxx>
@@ -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<RFieldBase> 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::RFieldBase::RValue> 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::RFieldBase::RValue> ROOT::Experimental::RSoAField::SplitValue(
    return std::vector<RValue>();
 }
 
-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::RFieldBase::RValue> 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

tree/ntuple/src/RFieldSequenceContainer.cxx

@@ -30,6 +30,16 @@ std::vector<ROOT::RFieldBase::RValue> SplitVector(std::shared_ptr<void> valuePtr
    return result;
 }
 
+std::size_t GetSizeOfVector()
+{
+   return sizeof(std::vector<char>);
+}
+
+std::size_t GetAlignOfVector()
+{
+   return alignof(std::vector<char>);
+}
+
 } // anonymous namespace
 
 ROOT::RArrayField::RArrayField(std::string_view fieldName, std::unique_ptr<RFieldBase> itemField,
@@ -648,6 +658,16 @@ std::vector<ROOT::RFieldBase::RValue> ROOT::RVectorField::SplitValue(const RValu
    return SplitVector(value.GetPtr<void>(), *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::RFieldBase::RValue> ROOT::RArrayAsVectorField::SplitValue(cons
    return SplitVector(value.GetPtr<void>(), *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);

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

tree/ntuple/test/CustomStruct.hxx

@@ -478,4 +478,11 @@ struct MemberWithCustomStreamer {
    ClassDefNV(MemberWithCustomStreamer, 2);
 };
 
+struct AlignmentDeterminedByTransientMember {
+   short int fA;
+   float fTransient; ///<!
+};
+
+struct alignas(8) AlignedAs {};
+
 #endif