[ntuple] add RPageSource::LoadPageListImpl()

A new callback that loads a particular page list from the backend
storage. As a result, the logic to load the page lists and to
populate the cluster group descriptor has been moved from the
concrete page source implementations to the RPageSource base class.

Moving on, this will allow to decouple loading the page lists from
attaching the storage.

Author: jblomer

tree/ntuple/inc/ROOT/RPageStorage.hxx

@@ -625,6 +625,7 @@ public:
          fDescriptor.IncGeneration();
          fLock.unlock();
       }
+      ROOT::RNTupleDescriptor &operator*() const { return fDescriptor; }
       ROOT::RNTupleDescriptor *operator->() const { return &fDescriptor; }
       void MoveIn(ROOT::RNTupleDescriptor desc) { fDescriptor = std::move(desc); }
    };
@@ -747,11 +748,14 @@ protected:
    /// Fills fStructureBuffer with the compressed header and footer
    virtual void LoadStructureImpl() = 0;
    /// `LoadStructureImpl()` has been called before `AttachImpl()` is called
-   virtual ROOT::RNTupleDescriptor AttachImpl(ROOT::Internal::RNTupleSerializer::EDescriptorDeserializeMode mode) = 0;
+   virtual ROOT::RNTupleDescriptor AttachImpl() = 0;
    /// Returns a new, unattached page source for the same data set
    virtual std::unique_ptr<RPageSource> CloneImpl() const = 0;
    // Only called if a task scheduler is set. No-op be default.
    virtual void UnzipClusterImpl(ROOT::Internal::RCluster *cluster);
+   // Loads a page list into the provided buffer. The buffer parameter needs to point to a memory region large enough
+   // to hold the compressed page list.
+   virtual void LoadPageListImpl(const RNTupleLocator &locator, void *buffer) = 0;
    // Returns a sealed page from storage without adding it to the page pool. The sealed pages buffer and buffer size
    // is already initialized.
    virtual void LoadSealedPageImpl(const RNTupleLocator &locator, RSealedPage &sealedPage) = 0;

tree/ntuple/inc/ROOT/RPageStorageDaos.hxx

@@ -158,11 +158,12 @@ private:
    RDaosNTupleAnchor fAnchor;
    ROOT::Internal::RNTupleDescriptorBuilder fDescriptorBuilder;
 
+   void LoadPageListImpl(const RNTupleLocator &locator, void *buffer) final;
    void LoadSealedPageImpl(const RNTupleLocator &locator, RSealedPage &sealedPage) final;
 
 protected:
    void LoadStructureImpl() final;
-   ROOT::RNTupleDescriptor AttachImpl(ROOT::Internal::RNTupleSerializer::EDescriptorDeserializeMode mode) final;
+   ROOT::RNTupleDescriptor AttachImpl() final;
    /// The cloned page source creates a new connection to the pool/container.
    std::unique_ptr<RPageSource> CloneImpl() const final;
 

tree/ntuple/inc/ROOT/RPageStorageFile.hxx

@@ -160,10 +160,11 @@ private:
 
 protected:
    void LoadStructureImpl() final;
-   ROOT::RNTupleDescriptor AttachImpl(RNTupleSerializer::EDescriptorDeserializeMode mode) final;
+   ROOT::RNTupleDescriptor AttachImpl() final;
    /// The cloned page source creates a new raw file and reader and opens its own file descriptor to the data.
    std::unique_ptr<RPageSource> CloneImpl() const final;
 
+   void LoadPageListImpl(const RNTupleLocator &locator, void *buffer) final;
    void LoadSealedPageImpl(const RNTupleLocator &locator, RSealedPage &sealedPage) final;
 
 public:

tree/ntuple/src/RPageStorage.cxx

@@ -226,11 +226,27 @@ void ROOT::Internal::RPageSource::LoadStructure()
 
 void ROOT::Internal::RPageSource::Attach(RNTupleSerializer::EDescriptorDeserializeMode mode)
 {
+   if (fIsAttached)
+      return;
+
    LoadStructure();
-   if (!fIsAttached) {
-      GetExclDescriptorGuard().MoveIn(AttachImpl(mode));
-      fStructureBuffer.Reset();
+
+   auto descGuard = GetExclDescriptorGuard();
+   descGuard.MoveIn(AttachImpl());
+   fStructureBuffer.Reset();
+
+   std::vector<unsigned char> buffer;
+   for (const auto &cgDesc : descGuard->GetClusterGroupIterable()) {
+      buffer.resize(cgDesc.GetPageListLength() + cgDesc.GetPageListLocator().GetNBytesOnStorage());
+      auto zipBuffer = buffer.data() + cgDesc.GetPageListLength();
+
+      LoadPageListImpl(cgDesc.GetPageListLocator(), zipBuffer);
+      RNTupleDecompressor::Unzip(zipBuffer, cgDesc.GetPageListLocator().GetNBytesOnStorage(),
+                                 cgDesc.GetPageListLength(), buffer.data());
+      RNTupleSerializer::DeserializePageList(buffer.data(), cgDesc.GetPageListLength(), cgDesc.GetId(), *descGuard,
+                                             mode);
    }
+
    fIsAttached = true;
 }
 

tree/ntuple/src/RPageStorageDaos.cxx

@@ -473,8 +473,7 @@ void ROOT::Experimental::Internal::RPageSourceDaos::LoadStructureImpl()
    }
 }
 
-ROOT::RNTupleDescriptor
-ROOT::Experimental::Internal::RPageSourceDaos::AttachImpl(RNTupleSerializer::EDescriptorDeserializeMode mode)
+ROOT::RNTupleDescriptor ROOT::Experimental::Internal::RPageSourceDaos::AttachImpl()
 {
    auto unzipBuf = reinterpret_cast<unsigned char *>(fStructureBuffer.fPtrFooter) + fAnchor.fNBytesFooter;
 
@@ -489,23 +488,14 @@ ROOT::Experimental::Internal::RPageSourceDaos::AttachImpl(RNTupleSerializer::EDe
       throw ROOT::RException(R__FAIL("LocateNTuple: ntuple name '" + fNTupleName + "' unavailable in this container."));
    }
 
-   auto desc = fDescriptorBuilder.MoveDescriptor();
+   return fDescriptorBuilder.MoveDescriptor();
+}
 
-   std::unique_ptr<unsigned char[]> buffer, zipBuffer;
+void ROOT::Experimental::Internal::RPageSourceDaos::LoadPageListImpl(const RNTupleLocator &locator, void *buffer)
+{
    daos_obj_id_t oidPageList{kOidLowPageList, static_cast<decltype(daos_obj_id_t::hi)>(fNTupleIndex)};
-   for (const auto &cgDesc : desc.GetClusterGroupIterable()) {
-      buffer = MakeUninitArray<unsigned char>(cgDesc.GetPageListLength());
-      zipBuffer = MakeUninitArray<unsigned char>(cgDesc.GetPageListLocator().GetNBytesOnStorage());
-      fDaosContainer->ReadSingleAkey(
-         zipBuffer.get(), cgDesc.GetPageListLocator().GetNBytesOnStorage(), oidPageList, kDistributionKeyDefault,
-         cgDesc.GetPageListLocator().GetPosition<RNTupleLocatorObject64>().GetLocation(), kCidMetadata);
-      RNTupleDecompressor::Unzip(zipBuffer.get(), cgDesc.GetPageListLocator().GetNBytesOnStorage(),
-                                 cgDesc.GetPageListLength(), buffer.get());
-
-      RNTupleSerializer::DeserializePageList(buffer.get(), cgDesc.GetPageListLength(), cgDesc.GetId(), desc, mode);
-   }
-
-   return desc;
+   fDaosContainer->ReadSingleAkey(buffer, locator.GetNBytesOnStorage(), oidPageList, kDistributionKeyDefault,
+                                  locator.GetPosition<RNTupleLocatorObject64>().GetLocation(), kCidMetadata);
 }
 
 std::string ROOT::Experimental::Internal::RPageSourceDaos::GetObjectClass() const

tree/ntuple/src/RPageStorageFile.cxx

@@ -477,7 +477,7 @@ void ROOT::Internal::RPageSourceFile::LoadStructureImpl()
    }
 }
 
-ROOT::RNTupleDescriptor ROOT::Internal::RPageSourceFile::AttachImpl(RNTupleSerializer::EDescriptorDeserializeMode mode)
+ROOT::RNTupleDescriptor ROOT::Internal::RPageSourceFile::AttachImpl()
 {
    auto unzipBuf = reinterpret_cast<unsigned char *>(fStructureBuffer.fPtrFooter) + fAnchor->GetNBytesFooter();
 
@@ -489,33 +489,23 @@ ROOT::RNTupleDescriptor ROOT::Internal::RPageSourceFile::AttachImpl(RNTupleSeria
                               unzipBuf);
    RNTupleSerializer::DeserializeFooter(unzipBuf, fAnchor->GetLenFooter(), fDescriptorBuilder);
 
-   auto desc = fDescriptorBuilder.MoveDescriptor();
-
    // fNTupleName is empty if and only if we created this source via CreateFromAnchor. If that's the case, this is the
    // earliest we can set the name.
    if (fNTupleName.empty())
-      fNTupleName = desc.GetName();
-
-   std::vector<unsigned char> buffer;
-   for (const auto &cgDesc : desc.GetClusterGroupIterable()) {
-      buffer.resize(std::max<size_t>(buffer.size(),
-                                     cgDesc.GetPageListLength() + cgDesc.GetPageListLocator().GetNBytesOnStorage()));
-      auto *zipBuffer = buffer.data() + cgDesc.GetPageListLength();
-      fReader.ReadBuffer(zipBuffer, cgDesc.GetPageListLocator().GetNBytesOnStorage(),
-                         cgDesc.GetPageListLocator().GetPosition<std::uint64_t>());
-      RNTupleDecompressor::Unzip(zipBuffer, cgDesc.GetPageListLocator().GetNBytesOnStorage(),
-                                 cgDesc.GetPageListLength(), buffer.data());
-
-      RNTupleSerializer::DeserializePageList(buffer.data(), cgDesc.GetPageListLength(), cgDesc.GetId(), desc, mode);
-   }
+      fNTupleName = fDescriptorBuilder.GetDescriptor().GetName();
 
    // For the page reads, we rely on the I/O scheduler to define the read requests
    fFile->SetBuffering(false);
 
    // Set file size once after buffering is turned off
    fFileSize = fFile->GetSize();
 
-   return desc;
+   return fDescriptorBuilder.MoveDescriptor();
+}
+
+void ROOT::Internal::RPageSourceFile::LoadPageListImpl(const RNTupleLocator &locator, void *buffer)
+{
+   fReader.ReadBuffer(buffer, locator.GetNBytesOnStorage(), locator.GetPosition<std::uint64_t>());
 }
 
 void ROOT::Internal::RPageSourceFile::LoadSealedPageImpl(const RNTupleLocator &locator, RSealedPage &sealedPage)

tree/ntuple/test/ntuple_cluster.cxx

@@ -39,8 +39,9 @@ namespace {
 class RPageSourceMock : public RPageSource {
 protected:
    void LoadStructureImpl() final {}
-   RNTupleDescriptor AttachImpl(RNTupleSerializer::EDescriptorDeserializeMode) final { return RNTupleDescriptor(); }
+   RNTupleDescriptor AttachImpl() final { return RNTupleDescriptor(); }
    std::unique_ptr<RPageSource> CloneImpl() const final { return nullptr; }
+   void LoadPageListImpl(const ROOT::RNTupleLocator &, void *) final {}
    void LoadSealedPageImpl(const ROOT::RNTupleLocator &, RSealedPage &) final {}
    void LoadStreamerInfo() final {}
    std::unique_ptr<ROOT::Internal::RPageSource>

tree/ntuple/test/ntuple_endian.cxx

@@ -89,11 +89,9 @@ class RPageSourceMock : public RPageSource {
 
 protected:
    void LoadStructureImpl() final {}
-   RNTupleDescriptor AttachImpl(ROOT::Internal::RNTupleSerializer::EDescriptorDeserializeMode) final
-   {
-      return RNTupleDescriptor();
-   }
+   RNTupleDescriptor AttachImpl() final { return RNTupleDescriptor(); }
    std::unique_ptr<RPageSource> CloneImpl() const final { return nullptr; }
+   void LoadPageListImpl(const ROOT::RNTupleLocator &, void *) final {}
    void LoadSealedPageImpl(const ROOT::RNTupleLocator &, RSealedPage &) final {}
    void LoadStreamerInfo() final {}
 

tree/ntuple/test/ntuple_pages.cxx

@@ -9,8 +9,9 @@ using ROOT::Internal::RPageRef;
 class RPageSourceMock : public RPageSource {
 protected:
    void LoadStructureImpl() final {}
-   RNTupleDescriptor AttachImpl(RNTupleSerializer::EDescriptorDeserializeMode) final { return RNTupleDescriptor(); }
+   RNTupleDescriptor AttachImpl() final { return RNTupleDescriptor(); }
    std::unique_ptr<RPageSource> CloneImpl() const final { return nullptr; }
+   void LoadPageListImpl(const ROOT::RNTupleLocator &, void *) final {}
    void LoadSealedPageImpl(const ROOT::RNTupleLocator &, RSealedPage &) final {}
    void LoadStreamerInfo() final {}
    std::unique_ptr<ROOT::Internal::RPageSource>