[ntuple] create streamer infos from TClass names

Get the streamer info records of class fields from their meta (TClass)
normalized name. Previously, the names came from the descriptor, which
has them already RNTuple normalized.

Author: jblomer

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

@@ -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/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) {
+      auto clField = dynamic_cast<const RClassField *>(field);
+      if (!clField)
+         return;
+
+      const auto clName = clField->GetClass()->GetName();
+      auto cl = TClass::GetClass(clName);
+      if (!cl) {
+         throw RException(R__FAIL(std::string("cannot get TClass for ") + clName));
+      }
+      auto streamerInfo = cl->GetStreamerInfo(clField->GetTypeVersion());
+      if (!streamerInfo) {
+         throw RException(R__FAIL(std::string("cannot get streamerInfo for ") + clName + " [" +
+                                  std::to_string(clField->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)
@@ -76,6 +71,11 @@ ROOT_ADD_GTEST(ntuple_zip ntuple_zip.cxx LIBRARIES ROOTNTuple)
 
 ROOT_ADD_GTEST(rfield_basics rfield_basics.cxx LIBRARIES ROOTNTuple CustomStruct)
 ROOT_ADD_GTEST(rfield_class rfield_class.cxx LIBRARIES ROOTNTuple CustomStruct Physics)
+ROOT_GENERATE_DICTIONARY(RNTupleClassFieldDict ${CMAKE_CURRENT_SOURCE_DIR}/RNTupleClassFieldDict.hxx
+                         MODULE rfield_class
+                         LINKDEF RNTupleClassFieldLinkDef.h
+                         OPTIONS -inlineInputHeader
+                         DEPENDENCIES RIO)
 ROOT_ADD_GTEST(rfield_string rfield_string.cxx LIBRARIES ROOTNTuple)
 ROOT_ADD_GTEST(rfield_variant rfield_variant.cxx LIBRARIES ROOTNTuple CustomStruct)
 if(MSVC)

tree/ntuple/test/RNTupleClassFieldDict.hxx

@@ -0,0 +1,8 @@
+#ifndef ROOT_RNTuple_Test_RNTupleClassFieldDict
+#define ROOT_RNTuple_Test_RNTupleClassFieldDict
+
+#include "CustomStruct.hxx"
+
+#include <map>
+
+#endif // ROOT_RNTuple_Test_RNTupleClassFieldDict

tree/ntuple/test/RNTupleDescriptorLinkDef.h renamed to tree/ntuple/test/RNTupleClassFieldLinkDef.h

@@ -2,6 +2,5 @@
 
 #pragma link C++ class std::map < int, CustomStruct> + ;
 #pragma link C++ class std::map < int, float> + ;
-#pragma link C++ class std::unordered_map < int, float> + ;
 
 #endif

tree/ntuple/test/RNTupleDescriptorDict.hxx

@@ -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");