[ntuple] Create streamer info records from TClass normalized name

tree/ntuple/inc/ROOT/RField.hxx

@@ -225,6 +225,8 @@ public:
    /// For polymorphic classes (that declare or inherit at least one virtual method), return the expected dynamic type
    /// of any user object. If the class is not polymorphic, return nullptr.
    const std::type_info *GetPolymorphicTypeInfo() const;
+   /// Return the TClass instance backing this field.
+   const TClass *GetClass() const { return fClass; }
    void AcceptVisitor(ROOT::Detail::RFieldVisitor &visitor) const final;
 };
 
@@ -279,6 +281,7 @@ public:
    size_t GetAlignment() const final;
    std::uint32_t GetTypeVersion() const final;
    std::uint32_t GetTypeChecksum() const final;
+   TClass *GetClass() const { return fClass; }
    void AcceptVisitor(ROOT::Detail::RFieldVisitor &visitor) const final;
 };
 
@@ -316,7 +319,7 @@ template <typename T, typename = void>
 class RField final : public RClassField {
 public:
    static std::string TypeName() { return ROOT::Internal::GetRenormalizedTypeName(typeid(T)); }
-   RField(std::string_view name) : RClassField(name, TypeName())
+   RField(std::string_view name) : RClassField(name, Internal::GetDemangledTypeName(typeid(T)))
    {
       static_assert(std::is_class_v<T>, "no I/O support for this basic C++ type");
    }

tree/ntuple/inc/ROOT/RNTupleDescriptor.hxx

@@ -1762,9 +1762,6 @@ public:
    ///   - Logical columns of piece two
    ///   - ...
    void ShiftAliasColumns(std::uint32_t offset);
-
-   /// Get the streamer info records for custom classes. Currently requires the corresponding dictionaries to be loaded.
-   ROOT::Internal::RNTupleSerializer::StreamerInfoMap_t BuildStreamerInfos() const;
 };
 
 inline RNTupleDescriptor CloneDescriptorSchema(const RNTupleDescriptor &desc)

tree/ntuple/inc/ROOT/RPageStorage.hxx

@@ -447,7 +447,7 @@ private:
    std::vector<ROOT::RClusterDescriptor::RPageRange> fOpenPageRanges;
 
    /// Union of the streamer info records that are sent from streamer fields to the sink before committing the dataset.
-   ROOT::Internal::RNTupleSerializer::StreamerInfoMap_t fStreamerInfos;
+   ROOT::Internal::RNTupleSerializer::StreamerInfoMap_t fInfosOfStreamerFields;
 
 protected:
    /// Set of optional features supported by the persistent sink
@@ -521,7 +521,7 @@ public:
 
    /// Updates the descriptor and calls InitImpl() that handles the backend-specific details (file, DAOS, etc.)
    void InitImpl(RNTupleModel &model) final;
-   void UpdateSchema(const ROOT::Internal::RNTupleModelChangeset &changeset, ROOT::NTupleSize_t firstEntry) final;
+   void UpdateSchema(const ROOT::Internal::RNTupleModelChangeset &changeset, ROOT::NTupleSize_t firstEntry) override;
    void UpdateExtraTypeInfo(const ROOT::RExtraTypeInfoDescriptor &extraTypeInfo) final;
 
    /// Initialize sink based on an existing descriptor and fill into the descriptor builder, optionally copying over

tree/ntuple/inc/ROOT/RPageStorageFile.hxx

@@ -65,6 +65,10 @@ private:
    std::unique_ptr<ROOT::Internal::RNTupleFileWriter> fWriter;
    /// Number of bytes committed to storage in the current cluster
    std::uint64_t fNBytesCurrentCluster = 0;
+   /// On UpdateSchema(), the new class fields register the corresponding streamer info here so that the
+   /// streamer info records in the file can be properly updated on dataset commit
+   ROOT::Internal::RNTupleSerializer::StreamerInfoMap_t fInfosOfClassFields;
+
    RPageSinkFile(std::string_view ntupleName, const ROOT::RNTupleWriteOptions &options);
 
    /// We pass bytesPacked so that TFile::ls() reports a reasonable value for the compression ratio of the corresponding
@@ -98,6 +102,8 @@ public:
    RPageSinkFile(RPageSinkFile &&) = default;
    RPageSinkFile &operator=(RPageSinkFile &&) = default;
    ~RPageSinkFile() override;
+
+   void UpdateSchema(const ROOT::Internal::RNTupleModelChangeset &changeset, ROOT::NTupleSize_t firstEntry) final;
 }; // class RPageSinkFile
 
 // clang-format off

tree/ntuple/src/RNTupleDescriptor.cxx

@@ -24,7 +24,6 @@
 
 #include <RZip.h>
 #include <TError.h>
-#include <TVirtualStreamerInfo.h>
 
 #include <algorithm>
 #include <cstdint>
@@ -1412,48 +1411,6 @@ ROOT::Internal::RNTupleDescriptorBuilder::AddAttributeSet(Experimental::RNTupleA
    return RResult<void>::Success();
 }
 
-RNTupleSerializer::StreamerInfoMap_t ROOT::Internal::RNTupleDescriptorBuilder::BuildStreamerInfos() const
-{
-   RNTupleSerializer::StreamerInfoMap_t streamerInfoMap;
-   const auto &desc = GetDescriptor();
-
-   std::function<void(const RFieldDescriptor &)> fnWalkFieldTree;
-   fnWalkFieldTree = [&desc, &streamerInfoMap, &fnWalkFieldTree](const RFieldDescriptor &fieldDesc) {
-      if (fieldDesc.IsCustomClass()) {
-         // Add streamer info for this class to streamerInfoMap
-         auto cl = TClass::GetClass(fieldDesc.GetTypeName().c_str());
-         if (!cl) {
-            throw RException(R__FAIL(std::string("cannot get TClass for ") + fieldDesc.GetTypeName()));
-         }
-         auto streamerInfo = cl->GetStreamerInfo(fieldDesc.GetTypeVersion());
-         if (!streamerInfo) {
-            throw RException(R__FAIL(std::string("cannot get streamerInfo for ") + fieldDesc.GetTypeName()));
-         }
-         streamerInfoMap[streamerInfo->GetNumber()] = streamerInfo;
-      }
-
-      // Recursively traverse sub fields
-      for (const auto &subFieldDesc : desc.GetFieldIterable(fieldDesc)) {
-         fnWalkFieldTree(subFieldDesc);
-      }
-   };
-
-   fnWalkFieldTree(desc.GetFieldZero());
-
-   // Add the streamer info records from streamer fields: because of runtime polymorphism we may need to add additional
-   // types not covered by the type names stored in the field headers
-   for (const auto &extraTypeInfo : desc.GetExtraTypeInfoIterable()) {
-      if (extraTypeInfo.GetContentId() != EExtraTypeInfoIds::kStreamerInfo)
-         continue;
-      // Ideally, we would avoid deserializing the streamer info records of the streamer fields that we just serialized.
-      // However, this happens only once at the end of writing and only when streamer fields are used, so the
-      // preference here is for code simplicity.
-      streamerInfoMap.merge(RNTupleSerializer::DeserializeStreamerInfos(extraTypeInfo.GetContent()).Unwrap());
-   }
-
-   return streamerInfoMap;
-}
-
 ROOT::RClusterDescriptor::RColumnRangeIterable ROOT::RClusterDescriptor::GetColumnRangeIterable() const
 {
    return RColumnRangeIterable(*this);

tree/ntuple/src/RPageStorage.cxx

@@ -927,7 +927,7 @@ void ROOT::Internal::RPagePersistentSink::UpdateExtraTypeInfo(const ROOT::RExtra
    if (extraTypeInfo.GetContentId() != EExtraTypeInfoIds::kStreamerInfo)
       throw RException(R__FAIL("ROOT bug: unexpected type extra info in UpdateExtraTypeInfo()"));
 
-   fStreamerInfos.merge(RNTupleSerializer::DeserializeStreamerInfos(extraTypeInfo.GetContent()).Unwrap());
+   fInfosOfStreamerFields.merge(RNTupleSerializer::DeserializeStreamerInfos(extraTypeInfo.GetContent()).Unwrap());
 }
 
 void ROOT::Internal::RPagePersistentSink::InitImpl(ROOT::RNTupleModel &model)
@@ -1255,7 +1255,7 @@ void ROOT::Internal::RPagePersistentSink::CommitClusterGroup()
 
 void ROOT::Internal::RPagePersistentSink::CommitDatasetImpl()
 {
-   if (!fStreamerInfos.empty()) {
+   if (!fInfosOfStreamerFields.empty()) {
       // De-duplicate extra type infos before writing. Usually we won't have them already in the descriptor, but
       // this may happen when we are writing back an already-existing RNTuple, e.g. when doing incremental merging.
       for (const auto &etDesc : fDescriptorBuilder.GetDescriptor().GetExtraTypeInfoIterable()) {
@@ -1265,13 +1265,13 @@ void ROOT::Internal::RPagePersistentSink::CommitDatasetImpl()
             R__ASSERT(etDesc.GetTypeName().empty());
             R__ASSERT(etDesc.GetTypeVersion() == 0);
             auto etInfo = RNTupleSerializer::DeserializeStreamerInfos(etDesc.GetContent()).Unwrap();
-            fStreamerInfos.merge(etInfo);
+            fInfosOfStreamerFields.merge(etInfo);
          }
       }
 
       RExtraTypeInfoDescriptorBuilder extraInfoBuilder;
       extraInfoBuilder.ContentId(EExtraTypeInfoIds::kStreamerInfo)
-         .Content(RNTupleSerializer::SerializeStreamerInfos(fStreamerInfos));
+         .Content(RNTupleSerializer::SerializeStreamerInfos(fInfosOfStreamerFields));
       fDescriptorBuilder.ReplaceExtraTypeInfo(extraInfoBuilder.MoveDescriptor().Unwrap());
    }
 

tree/ntuple/src/RPageStorageFile.cxx

@@ -30,6 +30,7 @@
 #include <RVersion.h>
 #include <TDirectory.h>
 #include <TError.h>
+#include <TVirtualStreamerInfo.h>
 
 #include <algorithm>
 #include <cstdio>
@@ -85,6 +86,37 @@ void ROOT::Internal::RPageSinkFile::InitImpl(unsigned char *serializedHeader, st
    fWriter->WriteNTupleHeader(zipBuffer.get(), szZipHeader, length);
 }
 
+void ROOT::Internal::RPageSinkFile::UpdateSchema(const ROOT::Internal::RNTupleModelChangeset &changeset,
+                                                 ROOT::NTupleSize_t firstEntry)
+{
+   RPagePersistentSink::UpdateSchema(changeset, firstEntry);
+
+   auto fnAddStreamerInfo = [this](const ROOT::RFieldBase *field) {
+      const TClass *cl = nullptr;
+      if (auto classField = dynamic_cast<const RClassField *>(field)) {
+         cl = classField->GetClass();
+      } else if (auto streamerField = dynamic_cast<const RStreamerField *>(field)) {
+         cl = streamerField->GetClass();
+      }
+      if (!cl)
+         return;
+
+      auto streamerInfo = cl->GetStreamerInfo(field->GetTypeVersion());
+      if (!streamerInfo) {
+         throw RException(R__FAIL(std::string("cannot get streamerInfo for ") + cl->GetName() + " [" +
+                                  std::to_string(field->GetTypeVersion()) + "]"));
+      }
+      fInfosOfClassFields[streamerInfo->GetNumber()] = streamerInfo;
+   };
+
+   for (const auto field : changeset.fAddedFields) {
+      fnAddStreamerInfo(field);
+      for (const auto &subField : *field) {
+         fnAddStreamerInfo(&subField);
+      }
+   }
+}
+
 inline ROOT::RNTupleLocator
 ROOT::Internal::RPageSinkFile::WriteSealedPage(const RPageStorage::RSealedPage &sealedPage, std::size_t bytesPacked)
 {
@@ -244,7 +276,18 @@ ROOT::Internal::RPageSinkFile::CommitClusterGroupImpl(unsigned char *serializedP
 
 void ROOT::Internal::RPageSinkFile::CommitDatasetImpl(unsigned char *serializedFooter, std::uint32_t length)
 {
-   fWriter->UpdateStreamerInfos(fDescriptorBuilder.BuildStreamerInfos());
+   // Add the streamer info records from streamer fields: because of runtime polymorphism we may need to add additional
+   // types not covered by the type names of the class fields
+   for (const auto &extraTypeInfo : fDescriptorBuilder.GetDescriptor().GetExtraTypeInfoIterable()) {
+      if (extraTypeInfo.GetContentId() != EExtraTypeInfoIds::kStreamerInfo)
+         continue;
+      // Ideally, we would avoid deserializing the streamer info records of the streamer fields that we just serialized.
+      // However, this happens only once at the end of writing and only when streamer fields are used, so the
+      // preference here is for code simplicity.
+      fInfosOfClassFields.merge(RNTupleSerializer::DeserializeStreamerInfos(extraTypeInfo.GetContent()).Unwrap());
+   }
+   fWriter->UpdateStreamerInfos(fInfosOfClassFields);
+
    auto bufFooterZip = MakeUninitArray<unsigned char>(length);
    auto szFooterZip =
       RNTupleCompressor::Zip(serializedFooter, length, GetWriteOptions().GetCompression(), bufFooterZip.get());

tree/ntuple/test/CMakeLists.txt

@@ -30,11 +30,6 @@ ROOT_GENERATE_DICTIONARY(RXTupleDict ${CMAKE_CURRENT_SOURCE_DIR}/RXTuple.hxx
                        LINKDEF RXTupleLinkDef.h
                        DEPENDENCIES RIO)
 ROOT_ADD_GTEST(ntuple_descriptor ntuple_descriptor.cxx LIBRARIES ROOTNTuple)
-ROOT_GENERATE_DICTIONARY(RNTupleDescriptorDict ${CMAKE_CURRENT_SOURCE_DIR}/RNTupleDescriptorDict.hxx
-                         MODULE ntuple_descriptor
-                         LINKDEF RNTupleDescriptorLinkDef.h
-                         OPTIONS -inlineInputHeader
-                         DEPENDENCIES RIO)
 
 ROOT_ADD_GTEST(ntuple_endian ntuple_endian.cxx LIBRARIES ROOTNTuple)
 ROOT_ADD_GTEST(ntuple_evolution_type ntuple_evolution_type.cxx LIBRARIES ROOTNTuple)

tree/ntuple/test/CustomStruct.hxx

@@ -11,6 +11,7 @@
 #include <cstddef>
 #include <cstdint>
 #include <functional>
+#include <map>
 #include <random>
 #include <string>
 #include <variant>
@@ -103,6 +104,12 @@ public:
    T fMember;
 };
 
+class EdmContainer {
+public:
+   // Used to test that the streamer info for fWrapper will use long long
+   EdmWrapper<long long> fWrapper;
+};
+
 template <typename T>
 struct EdmHashTrait {
    using value_type = T;

tree/ntuple/test/CustomStructLinkDef.h

@@ -27,8 +27,13 @@
 #pragma link C++ class IOConstructor+;
 #pragma link C++ class LowPrecisionFloats+;
 
+#pragma link C++ class std::map<int, CustomStruct>+ ;
+#pragma link C++ class std::map<int, float>+ ;
+
 #pragma link C++ class EdmWrapper<CustomStruct> +;
 #pragma link C++ class EdmHash < 1> + ;
+#pragma link C++ class EdmWrapper<long long>+;
+#pragma link C++ class EdmContainer;
 
 #pragma link C++ class DataVector < int, double> + ;
 #pragma link C++ class DataVector < int, float> + ;

tree/ntuple/test/ntuple_descriptor.cxx

@@ -609,88 +609,6 @@ TEST(RNTupleDescriptor, Clone)
    EXPECT_EQ(desc, clone);
 }
 
-TEST(RNTupleDescriptor, BuildStreamerInfos)
-{
-   auto fnBuildStreamerInfosOf = [](const RFieldBase &field) -> RNTupleSerializer::StreamerInfoMap_t {
-      RNTupleDescriptorBuilder descBuilder;
-      descBuilder.SetNTuple("test", "");
-      descBuilder.AddField(
-         RFieldDescriptorBuilder().FieldId(0).Structure(ROOT::ENTupleStructure::kRecord).MakeDescriptor().Unwrap());
-      auto fieldBuilder = RFieldDescriptorBuilder::FromField(field);
-      descBuilder.AddField(fieldBuilder.FieldId(1).MakeDescriptor().Unwrap());
-      descBuilder.AddFieldLink(0, 1);
-      int i = 2;
-      // In this test, we only support field hierarchies up to 2 levels
-      for (const auto &child : field.GetConstSubfields()) {
-         fieldBuilder = RFieldDescriptorBuilder::FromField(*child);
-         descBuilder.AddField(fieldBuilder.FieldId(i).MakeDescriptor().Unwrap());
-         descBuilder.AddFieldLink(1, i);
-         const auto childId = i;
-         i++;
-         for (const auto &grandChild : child->GetConstSubfields()) {
-            fieldBuilder = RFieldDescriptorBuilder::FromField(*grandChild);
-            descBuilder.AddField(fieldBuilder.FieldId(i).MakeDescriptor().Unwrap());
-            descBuilder.AddFieldLink(childId, i);
-            i++;
-         }
-      }
-      return descBuilder.BuildStreamerInfos();
-   };
-
-   RNTupleSerializer::StreamerInfoMap_t streamerInfoMap;
-
-   streamerInfoMap = fnBuildStreamerInfosOf(*RFieldBase::Create("f", "float").Unwrap());
-   EXPECT_TRUE(streamerInfoMap.empty());
-
-   streamerInfoMap = fnBuildStreamerInfosOf(*RFieldBase::Create("f", "std::vector<float>").Unwrap());
-   EXPECT_TRUE(streamerInfoMap.empty());
-
-   streamerInfoMap = fnBuildStreamerInfosOf(*RFieldBase::Create("f", "std::pair<float, float>").Unwrap());
-   EXPECT_TRUE(streamerInfoMap.empty());
-
-   streamerInfoMap = fnBuildStreamerInfosOf(*RFieldBase::Create("f", "std::map<int, float>").Unwrap());
-   EXPECT_TRUE(streamerInfoMap.empty());
-
-   streamerInfoMap = fnBuildStreamerInfosOf(*RFieldBase::Create("f", "std::unordered_map<int, float>").Unwrap());
-   EXPECT_TRUE(streamerInfoMap.empty());
-
-   std::vector<std::unique_ptr<RFieldBase>> itemFields;
-   streamerInfoMap = fnBuildStreamerInfosOf(ROOT::RRecordField("f", std::move(itemFields)));
-   EXPECT_TRUE(streamerInfoMap.empty());
-
-   streamerInfoMap = fnBuildStreamerInfosOf(*RFieldBase::Create("f", "CustomStruct").Unwrap());
-   EXPECT_EQ(1u, streamerInfoMap.size());
-   EXPECT_STREQ("CustomStruct", streamerInfoMap.begin()->second->GetName());
-
-   streamerInfoMap = fnBuildStreamerInfosOf(*RFieldBase::Create("f", "std::vector<CustomStruct>").Unwrap());
-   EXPECT_EQ(1u, streamerInfoMap.size());
-   EXPECT_STREQ("CustomStruct", streamerInfoMap.begin()->second->GetName());
-
-   streamerInfoMap = fnBuildStreamerInfosOf(*RFieldBase::Create("f", "std::map<int, CustomStruct>").Unwrap());
-   EXPECT_EQ(1u, streamerInfoMap.size());
-   EXPECT_STREQ("CustomStruct", streamerInfoMap.begin()->second->GetName());
-
-   streamerInfoMap = fnBuildStreamerInfosOf(*RFieldBase::Create("f", "DerivedA").Unwrap());
-   EXPECT_EQ(2u, streamerInfoMap.size());
-   std::vector<std::string> typeNames;
-   for (const auto &[_, si] : streamerInfoMap) {
-      typeNames.emplace_back(si->GetName());
-   }
-   std::sort(typeNames.begin(), typeNames.end());
-   EXPECT_STREQ("CustomStruct", typeNames[0].c_str());
-   EXPECT_STREQ("DerivedA", typeNames[1].c_str());
-
-   streamerInfoMap = fnBuildStreamerInfosOf(*RFieldBase::Create("f", "std::pair<CustomStruct, DerivedA>").Unwrap());
-   EXPECT_EQ(2u, streamerInfoMap.size());
-   typeNames.clear();
-   for (const auto &[_, si] : streamerInfoMap) {
-      typeNames.emplace_back(si->GetName());
-   }
-   std::sort(typeNames.begin(), typeNames.end());
-   EXPECT_STREQ("CustomStruct", typeNames[0].c_str());
-   EXPECT_STREQ("DerivedA", typeNames[1].c_str());
-}
-
 TEST(RNTupleDescriptor, CloneSchema)
 {
    FileRaii fileGuard("test_ntuple_desc_cloneschema.root");