From c834482c63b94f75c687f3fb5e35574443c55254 Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Fri, 22 May 2026 13:46:36 +0200 Subject: [PATCH 1/7] [ntuple] RPageStorage::RClusterInfo --> RPageSummary --- tree/ntuple/inc/ROOT/RPageStorage.hxx | 11 ++++--- tree/ntuple/inc/ROOT/RPageStorageDaos.hxx | 2 +- tree/ntuple/inc/ROOT/RPageStorageFile.hxx | 2 +- tree/ntuple/src/RPageStorage.cxx | 36 +++++++++++------------ tree/ntuple/src/RPageStorageDaos.cxx | 16 +++++----- tree/ntuple/src/RPageStorageFile.cxx | 16 +++++----- tree/ntuple/test/ntuple_cluster.cxx | 2 +- tree/ntuple/test/ntuple_endian.cxx | 2 +- tree/ntuple/test/ntuple_pages.cxx | 2 +- 9 files changed, 44 insertions(+), 45 deletions(-) diff --git a/tree/ntuple/inc/ROOT/RPageStorage.hxx b/tree/ntuple/inc/ROOT/RPageStorage.hxx index 3385244ad3262..2011db86b12dc 100644 --- a/tree/ntuple/inc/ROOT/RPageStorage.hxx +++ b/tree/ntuple/inc/ROOT/RPageStorage.hxx @@ -693,14 +693,13 @@ protected: } }; - /// Summarizes cluster-level information that are necessary to load a certain page. - /// Used by LoadPageImpl(). - struct RClusterInfo { + /// Summarizes meta-data is necessary to load a certain page. Used by LoadPageImpl(). + struct RPageSummary { ROOT::DescriptorId_t fClusterId = 0; - /// Location of the page on disk - ROOT::RClusterDescriptor::RPageInfoExtended fPageInfo; /// The first element number of the page's column in the given cluster std::uint64_t fColumnOffset = 0; + /// Location of the page on disk + ROOT::RClusterDescriptor::RPageInfoExtended fPageInfo; }; std::unique_ptr fCounters; @@ -728,7 +727,7 @@ protected: virtual void UnzipClusterImpl(ROOT::Internal::RCluster *cluster); // Returns a page from storage if not found in the page pool. Should be able to handle zero page locators. virtual ROOT::Internal::RPageRef - LoadPageImpl(ColumnHandle_t columnHandle, const RClusterInfo &clusterInfo, ROOT::NTupleSize_t idxInCluster) = 0; + LoadPageImpl(ColumnHandle_t columnHandle, const RPageSummary &pageSummary, ROOT::NTupleSize_t idxInCluster) = 0; /// Prepare a page range read for the column set in `clusterKey`. Specifically, pages referencing the /// `kTypePageZero` locator are filled in `pageZeroMap`; otherwise, `perPageFunc` is called for each page. This is diff --git a/tree/ntuple/inc/ROOT/RPageStorageDaos.hxx b/tree/ntuple/inc/ROOT/RPageStorageDaos.hxx index e58dc09dd8039..bc4e0f981ec80 100644 --- a/tree/ntuple/inc/ROOT/RPageStorageDaos.hxx +++ b/tree/ntuple/inc/ROOT/RPageStorageDaos.hxx @@ -160,7 +160,7 @@ private: ROOT::Internal::RNTupleDescriptorBuilder fDescriptorBuilder; ROOT::Internal::RPageRef - LoadPageImpl(ColumnHandle_t columnHandle, const RClusterInfo &clusterInfo, ROOT::NTupleSize_t idxInCluster) final; + LoadPageImpl(ColumnHandle_t columnHandle, const RPageSummary &pageSummary, ROOT::NTupleSize_t idxInCluster) final; protected: void LoadStructureImpl() final {} diff --git a/tree/ntuple/inc/ROOT/RPageStorageFile.hxx b/tree/ntuple/inc/ROOT/RPageStorageFile.hxx index 44cb68be8fc74..859b8495f55ec 100644 --- a/tree/ntuple/inc/ROOT/RPageStorageFile.hxx +++ b/tree/ntuple/inc/ROOT/RPageStorageFile.hxx @@ -181,7 +181,7 @@ protected: std::unique_ptr CloneImpl() const final; RPageRef - LoadPageImpl(ColumnHandle_t columnHandle, const RClusterInfo &clusterInfo, ROOT::NTupleSize_t idxInCluster) final; + LoadPageImpl(ColumnHandle_t columnHandle, const RPageSummary &pageSummary, ROOT::NTupleSize_t idxInCluster) final; public: RPageSourceFile(std::string_view ntupleName, std::string_view path, const ROOT::RNTupleReadOptions &options); diff --git a/tree/ntuple/src/RPageStorage.cxx b/tree/ntuple/src/RPageStorage.cxx index b1018152999d0..7941bce7d3480 100644 --- a/tree/ntuple/src/RPageStorage.cxx +++ b/tree/ntuple/src/RPageStorage.cxx @@ -405,30 +405,30 @@ ROOT::Internal::RPageSource::LoadPage(ColumnHandle_t columnHandle, ROOT::NTupleS } std::uint64_t idxInCluster; - RClusterInfo clusterInfo; + RPageSummary pageSummary; { auto descriptorGuard = GetSharedDescriptorGuard(); - clusterInfo.fClusterId = descriptorGuard->FindClusterId(columnId, globalIndex); + pageSummary.fClusterId = descriptorGuard->FindClusterId(columnId, globalIndex); - if (clusterInfo.fClusterId == ROOT::kInvalidDescriptorId) + if (pageSummary.fClusterId == ROOT::kInvalidDescriptorId) throw RException(R__FAIL("entry with index " + std::to_string(globalIndex) + " out of bounds")); - const auto &clusterDescriptor = descriptorGuard->GetClusterDescriptor(clusterInfo.fClusterId); + const auto &clusterDescriptor = descriptorGuard->GetClusterDescriptor(pageSummary.fClusterId); const auto &columnRange = clusterDescriptor.GetColumnRange(columnId); if (columnRange.IsSuppressed()) return ROOT::Internal::RPageRef(); - clusterInfo.fColumnOffset = columnRange.GetFirstElementIndex(); - R__ASSERT(clusterInfo.fColumnOffset <= globalIndex); - idxInCluster = globalIndex - clusterInfo.fColumnOffset; - clusterInfo.fPageInfo = clusterDescriptor.GetPageRange(columnId).Find(idxInCluster); + pageSummary.fColumnOffset = columnRange.GetFirstElementIndex(); + R__ASSERT(pageSummary.fColumnOffset <= globalIndex); + idxInCluster = globalIndex - pageSummary.fColumnOffset; + pageSummary.fPageInfo = clusterDescriptor.GetPageRange(columnId).Find(idxInCluster); } - if (clusterInfo.fPageInfo.GetLocator().GetType() == RNTupleLocator::kTypeUnknown) + if (pageSummary.fPageInfo.GetLocator().GetType() == RNTupleLocator::kTypeUnknown) throw RException(R__FAIL("tried to read a page with an unknown locator")); - UpdateLastUsedCluster(clusterInfo.fClusterId); - return LoadPageImpl(columnHandle, clusterInfo, idxInCluster); + UpdateLastUsedCluster(pageSummary.fClusterId); + return LoadPageImpl(columnHandle, pageSummary, idxInCluster); } ROOT::Internal::RPageRef @@ -448,7 +448,7 @@ ROOT::Internal::RPageSource::LoadPage(ColumnHandle_t columnHandle, RNTupleLocalI if (clusterId == kInvalidDescriptorId) throw RException(R__FAIL("entry out of bounds")); - RClusterInfo clusterInfo; + RPageSummary pageSummary; { auto descriptorGuard = GetSharedDescriptorGuard(); const auto &clusterDescriptor = descriptorGuard->GetClusterDescriptor(clusterId); @@ -456,16 +456,16 @@ ROOT::Internal::RPageSource::LoadPage(ColumnHandle_t columnHandle, RNTupleLocalI if (columnRange.IsSuppressed()) return ROOT::Internal::RPageRef(); - clusterInfo.fClusterId = clusterId; - clusterInfo.fColumnOffset = columnRange.GetFirstElementIndex(); - clusterInfo.fPageInfo = clusterDescriptor.GetPageRange(columnId).Find(idxInCluster); + pageSummary.fClusterId = clusterId; + pageSummary.fColumnOffset = columnRange.GetFirstElementIndex(); + pageSummary.fPageInfo = clusterDescriptor.GetPageRange(columnId).Find(idxInCluster); } - if (clusterInfo.fPageInfo.GetLocator().GetType() == RNTupleLocator::kTypeUnknown) + if (pageSummary.fPageInfo.GetLocator().GetType() == RNTupleLocator::kTypeUnknown) throw RException(R__FAIL("tried to read a page with an unknown locator")); - UpdateLastUsedCluster(clusterInfo.fClusterId); - return LoadPageImpl(columnHandle, clusterInfo, idxInCluster); + UpdateLastUsedCluster(clusterId); + return LoadPageImpl(columnHandle, pageSummary, idxInCluster); } void ROOT::Internal::RPageSource::EnableDefaultMetrics(const std::string &prefix) diff --git a/tree/ntuple/src/RPageStorageDaos.cxx b/tree/ntuple/src/RPageStorageDaos.cxx index 655529c3ab5fe..9ef052948f959 100644 --- a/tree/ntuple/src/RPageStorageDaos.cxx +++ b/tree/ntuple/src/RPageStorageDaos.cxx @@ -551,12 +551,12 @@ void ROOT::Experimental::Internal::RPageSourceDaos::LoadSealedPage(ROOT::Descrip } ROOT::Internal::RPageRef ROOT::Experimental::Internal::RPageSourceDaos::LoadPageImpl(ColumnHandle_t columnHandle, - const RClusterInfo &clusterInfo, + const RPageSummary &pageSummary, ROOT::NTupleSize_t idxInCluster) { - const auto columnId = columnHandle.fPhysicalId; - const auto clusterId = clusterInfo.fClusterId; - const auto &pageInfo = clusterInfo.fPageInfo; + const auto &columnId = columnHandle.fPhysicalId; + const auto &clusterId = pageSummary.fClusterId; + const auto &pageInfo = pageSummary.fPageInfo; const auto element = columnHandle.fColumn->GetElement(); const auto elementSize = element->GetSize(); @@ -566,8 +566,8 @@ ROOT::Internal::RPageRef ROOT::Experimental::Internal::RPageSourceDaos::LoadPage auto pageZero = fPageAllocator->NewPage(elementSize, pageInfo.GetNElements()); pageZero.GrowUnchecked(pageInfo.GetNElements()); memset(pageZero.GetBuffer(), 0, pageZero.GetNBytes()); - pageZero.SetWindow(clusterInfo.fColumnOffset + pageInfo.GetFirstElementIndex(), - ROOT::Internal::RPage::RClusterInfo(clusterId, clusterInfo.fColumnOffset)); + pageZero.SetWindow(pageSummary.fColumnOffset + pageInfo.GetFirstElementIndex(), + ROOT::Internal::RPage::RClusterInfo(clusterId, pageSummary.fColumnOffset)); return fPagePool.RegisterPage(std::move(pageZero), ROOT::Internal::RPagePool::RKey{columnId, elementInMemoryType}); } @@ -611,8 +611,8 @@ ROOT::Internal::RPageRef ROOT::Experimental::Internal::RPageSourceDaos::LoadPage fCounters->fSzUnzip.Add(elementSize * pageInfo.GetNElements()); } - newPage.SetWindow(clusterInfo.fColumnOffset + pageInfo.GetFirstElementIndex(), - ROOT::Internal::RPage::RClusterInfo(clusterId, clusterInfo.fColumnOffset)); + newPage.SetWindow(pageSummary.fColumnOffset + pageInfo.GetFirstElementIndex(), + ROOT::Internal::RPage::RClusterInfo(clusterId, pageSummary.fColumnOffset)); fCounters->fNPageUnsealed.Inc(); return fPagePool.RegisterPage(std::move(newPage), ROOT::Internal::RPagePool::RKey{columnId, elementInMemoryType}); } diff --git a/tree/ntuple/src/RPageStorageFile.cxx b/tree/ntuple/src/RPageStorageFile.cxx index c3cb0d9320678..f63923c6b6a07 100644 --- a/tree/ntuple/src/RPageStorageFile.cxx +++ b/tree/ntuple/src/RPageStorageFile.cxx @@ -548,12 +548,12 @@ void ROOT::Internal::RPageSourceFile::LoadSealedPage(ROOT::DescriptorId_t physic } ROOT::Internal::RPageRef ROOT::Internal::RPageSourceFile::LoadPageImpl(ColumnHandle_t columnHandle, - const RClusterInfo &clusterInfo, + const RPageSummary &pageSummary, ROOT::NTupleSize_t idxInCluster) { - const auto columnId = columnHandle.fPhysicalId; - const auto clusterId = clusterInfo.fClusterId; - const auto pageInfo = clusterInfo.fPageInfo; + const auto &columnId = columnHandle.fPhysicalId; + const auto &clusterId = pageSummary.fClusterId; + const auto &pageInfo = pageSummary.fPageInfo; const auto element = columnHandle.fColumn->GetElement(); const auto elementSize = element->GetSize(); @@ -563,8 +563,8 @@ ROOT::Internal::RPageRef ROOT::Internal::RPageSourceFile::LoadPageImpl(ColumnHan auto pageZero = fPageAllocator->NewPage(elementSize, pageInfo.GetNElements()); pageZero.GrowUnchecked(pageInfo.GetNElements()); memset(pageZero.GetBuffer(), 0, pageZero.GetNBytes()); - pageZero.SetWindow(clusterInfo.fColumnOffset + pageInfo.GetFirstElementIndex(), - ROOT::Internal::RPage::RClusterInfo(clusterId, clusterInfo.fColumnOffset)); + pageZero.SetWindow(pageSummary.fColumnOffset + pageInfo.GetFirstElementIndex(), + ROOT::Internal::RPage::RClusterInfo(clusterId, pageSummary.fColumnOffset)); return fPagePool.RegisterPage(std::move(pageZero), RPagePool::RKey{columnId, elementInMemoryType}); } @@ -616,8 +616,8 @@ ROOT::Internal::RPageRef ROOT::Internal::RPageSourceFile::LoadPageImpl(ColumnHan fCounters->fSzUnzip.Add(elementSize * pageInfo.GetNElements()); } - newPage.SetWindow(clusterInfo.fColumnOffset + pageInfo.GetFirstElementIndex(), - ROOT::Internal::RPage::RClusterInfo(clusterId, clusterInfo.fColumnOffset)); + newPage.SetWindow(pageSummary.fColumnOffset + pageInfo.GetFirstElementIndex(), + ROOT::Internal::RPage::RClusterInfo(clusterId, pageSummary.fColumnOffset)); fCounters->fNPageUnsealed.Inc(); return fPagePool.RegisterPage(std::move(newPage), RPagePool::RKey{columnId, elementInMemoryType}); } diff --git a/tree/ntuple/test/ntuple_cluster.cxx b/tree/ntuple/test/ntuple_cluster.cxx index 9534ceaa2a08c..9bbf40b7c2ad3 100644 --- a/tree/ntuple/test/ntuple_cluster.cxx +++ b/tree/ntuple/test/ntuple_cluster.cxx @@ -41,7 +41,7 @@ class RPageSourceMock : public RPageSource { void LoadStructureImpl() final {} RNTupleDescriptor AttachImpl(RNTupleSerializer::EDescriptorDeserializeMode) final { return RNTupleDescriptor(); } std::unique_ptr CloneImpl() const final { return nullptr; } - RPageRef LoadPageImpl(ColumnHandle_t, const RClusterInfo &, ROOT::NTupleSize_t) final { return RPageRef(); } + RPageRef LoadPageImpl(ColumnHandle_t, const RPageSummary &, ROOT::NTupleSize_t) final { return RPageRef(); } void LoadStreamerInfo() final {} std::unique_ptr OpenWithDifferentAnchor(const ROOT::Internal::RNTupleLink &, const ROOT::RNTupleReadOptions &) final diff --git a/tree/ntuple/test/ntuple_endian.cxx b/tree/ntuple/test/ntuple_endian.cxx index 4144f2eb02ea3..c8cba573088fa 100644 --- a/tree/ntuple/test/ntuple_endian.cxx +++ b/tree/ntuple/test/ntuple_endian.cxx @@ -93,7 +93,7 @@ class RPageSourceMock : public RPageSource { return RNTupleDescriptor(); } std::unique_ptr CloneImpl() const final { return nullptr; } - RPageRef LoadPageImpl(ColumnHandle_t, const RClusterInfo &, ROOT::NTupleSize_t) final { return RPageRef(); } + RPageRef LoadPageImpl(ColumnHandle_t, const RPageSummary &, ROOT::NTupleSize_t) final { return RPageRef(); } void LoadStreamerInfo() final {} std::unique_ptr diff --git a/tree/ntuple/test/ntuple_pages.cxx b/tree/ntuple/test/ntuple_pages.cxx index 4752fc747430f..2390b3ceb6acc 100644 --- a/tree/ntuple/test/ntuple_pages.cxx +++ b/tree/ntuple/test/ntuple_pages.cxx @@ -11,7 +11,7 @@ class RPageSourceMock : public RPageSource { void LoadStructureImpl() final {} RNTupleDescriptor AttachImpl(RNTupleSerializer::EDescriptorDeserializeMode) final { return RNTupleDescriptor(); } std::unique_ptr CloneImpl() const final { return nullptr; } - RPageRef LoadPageImpl(ColumnHandle_t, const RClusterInfo &, ROOT::NTupleSize_t) final { return RPageRef(); } + RPageRef LoadPageImpl(ColumnHandle_t, const RPageSummary &, ROOT::NTupleSize_t) final { return RPageRef(); } void LoadStreamerInfo() final {} std::unique_ptr OpenWithDifferentAnchor(const ROOT::Internal::RNTupleLink &, const ROOT::RNTupleReadOptions &) final From 8e4fa79c84ae2f6e865381f1ab37bb9e154dab5f Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Fri, 22 May 2026 14:10:38 +0200 Subject: [PATCH 2/7] [ntuple] remove unneeded 2nd page cache lookup in LoadPageImpl --- tree/ntuple/src/RPageStorageDaos.cxx | 5 ----- tree/ntuple/src/RPageStorageFile.cxx | 5 ----- 2 files changed, 10 deletions(-) diff --git a/tree/ntuple/src/RPageStorageDaos.cxx b/tree/ntuple/src/RPageStorageDaos.cxx index 9ef052948f959..e7e2af7413c3a 100644 --- a/tree/ntuple/src/RPageStorageDaos.cxx +++ b/tree/ntuple/src/RPageStorageDaos.cxx @@ -593,11 +593,6 @@ ROOT::Internal::RPageRef ROOT::Experimental::Internal::RPageSourceDaos::LoadPage fCurrentCluster = fClusterPool.GetCluster(clusterId, fActivePhysicalColumns.ToColumnSet()); R__ASSERT(fCurrentCluster->ContainsColumn(columnId)); - auto cachedPageRef = fPagePool.GetPage(ROOT::Internal::RPagePool::RKey{columnId, elementInMemoryType}, - RNTupleLocalIndex(clusterId, idxInCluster)); - if (!cachedPageRef.Get().IsNull()) - return cachedPageRef; - ROOT::Internal::ROnDiskPage::Key key(columnId, pageInfo.GetPageNumber()); auto onDiskPage = fCurrentCluster->GetOnDiskPage(key); R__ASSERT(onDiskPage && (sealedPage.GetBufferSize() == onDiskPage->GetSize())); diff --git a/tree/ntuple/src/RPageStorageFile.cxx b/tree/ntuple/src/RPageStorageFile.cxx index f63923c6b6a07..33b480a109d23 100644 --- a/tree/ntuple/src/RPageStorageFile.cxx +++ b/tree/ntuple/src/RPageStorageFile.cxx @@ -598,11 +598,6 @@ ROOT::Internal::RPageRef ROOT::Internal::RPageSourceFile::LoadPageImpl(ColumnHan fCurrentCluster = fClusterPool.GetCluster(clusterId, fActivePhysicalColumns.ToColumnSet()); R__ASSERT(fCurrentCluster->ContainsColumn(columnId)); - auto cachedPageRef = - fPagePool.GetPage(RPagePool::RKey{columnId, elementInMemoryType}, RNTupleLocalIndex(clusterId, idxInCluster)); - if (!cachedPageRef.Get().IsNull()) - return cachedPageRef; - ROnDiskPage::Key key(columnId, pageInfo.GetPageNumber()); auto onDiskPage = fCurrentCluster->GetOnDiskPage(key); R__ASSERT(onDiskPage && (sealedPage.GetBufferSize() == onDiskPage->GetSize())); From 7b3023fe5b3e6db3985e03351ed9f0aa6f792d9d Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Fri, 22 May 2026 14:18:12 +0200 Subject: [PATCH 3/7] [ntuple] remove unneeded parameter from RPageSource::LoadPageImpl() --- tree/ntuple/inc/ROOT/RPageStorage.hxx | 3 +-- tree/ntuple/inc/ROOT/RPageStorageDaos.hxx | 3 +-- tree/ntuple/inc/ROOT/RPageStorageFile.hxx | 3 +-- tree/ntuple/src/RPageStorage.cxx | 11 ++++------- tree/ntuple/src/RPageStorageDaos.cxx | 3 +-- tree/ntuple/src/RPageStorageFile.cxx | 5 ++--- tree/ntuple/test/ntuple_cluster.cxx | 2 +- tree/ntuple/test/ntuple_endian.cxx | 2 +- tree/ntuple/test/ntuple_pages.cxx | 2 +- 9 files changed, 13 insertions(+), 21 deletions(-) diff --git a/tree/ntuple/inc/ROOT/RPageStorage.hxx b/tree/ntuple/inc/ROOT/RPageStorage.hxx index 2011db86b12dc..ba0c33c00ffdf 100644 --- a/tree/ntuple/inc/ROOT/RPageStorage.hxx +++ b/tree/ntuple/inc/ROOT/RPageStorage.hxx @@ -726,8 +726,7 @@ protected: // Only called if a task scheduler is set. No-op be default. virtual void UnzipClusterImpl(ROOT::Internal::RCluster *cluster); // Returns a page from storage if not found in the page pool. Should be able to handle zero page locators. - virtual ROOT::Internal::RPageRef - LoadPageImpl(ColumnHandle_t columnHandle, const RPageSummary &pageSummary, ROOT::NTupleSize_t idxInCluster) = 0; + virtual ROOT::Internal::RPageRef LoadPageImpl(ColumnHandle_t columnHandle, const RPageSummary &pageSummary) = 0; /// Prepare a page range read for the column set in `clusterKey`. Specifically, pages referencing the /// `kTypePageZero` locator are filled in `pageZeroMap`; otherwise, `perPageFunc` is called for each page. This is diff --git a/tree/ntuple/inc/ROOT/RPageStorageDaos.hxx b/tree/ntuple/inc/ROOT/RPageStorageDaos.hxx index bc4e0f981ec80..83d98f2bda5db 100644 --- a/tree/ntuple/inc/ROOT/RPageStorageDaos.hxx +++ b/tree/ntuple/inc/ROOT/RPageStorageDaos.hxx @@ -159,8 +159,7 @@ private: ROOT::Internal::RNTupleDescriptorBuilder fDescriptorBuilder; - ROOT::Internal::RPageRef - LoadPageImpl(ColumnHandle_t columnHandle, const RPageSummary &pageSummary, ROOT::NTupleSize_t idxInCluster) final; + ROOT::Internal::RPageRef LoadPageImpl(ColumnHandle_t columnHandle, const RPageSummary &pageSummary) final; protected: void LoadStructureImpl() final {} diff --git a/tree/ntuple/inc/ROOT/RPageStorageFile.hxx b/tree/ntuple/inc/ROOT/RPageStorageFile.hxx index 859b8495f55ec..5110a62354535 100644 --- a/tree/ntuple/inc/ROOT/RPageStorageFile.hxx +++ b/tree/ntuple/inc/ROOT/RPageStorageFile.hxx @@ -180,8 +180,7 @@ protected: /// The cloned page source creates a new raw file and reader and opens its own file descriptor to the data. std::unique_ptr CloneImpl() const final; - RPageRef - LoadPageImpl(ColumnHandle_t columnHandle, const RPageSummary &pageSummary, ROOT::NTupleSize_t idxInCluster) final; + RPageRef LoadPageImpl(ColumnHandle_t columnHandle, const RPageSummary &pageSummary) final; public: RPageSourceFile(std::string_view ntupleName, std::string_view path, const ROOT::RNTupleReadOptions &options); diff --git a/tree/ntuple/src/RPageStorage.cxx b/tree/ntuple/src/RPageStorage.cxx index 7941bce7d3480..3d398b4257ccb 100644 --- a/tree/ntuple/src/RPageStorage.cxx +++ b/tree/ntuple/src/RPageStorage.cxx @@ -404,7 +404,6 @@ ROOT::Internal::RPageSource::LoadPage(ColumnHandle_t columnHandle, ROOT::NTupleS return cachedPageRef; } - std::uint64_t idxInCluster; RPageSummary pageSummary; { auto descriptorGuard = GetSharedDescriptorGuard(); @@ -420,22 +419,20 @@ ROOT::Internal::RPageSource::LoadPage(ColumnHandle_t columnHandle, ROOT::NTupleS pageSummary.fColumnOffset = columnRange.GetFirstElementIndex(); R__ASSERT(pageSummary.fColumnOffset <= globalIndex); - idxInCluster = globalIndex - pageSummary.fColumnOffset; - pageSummary.fPageInfo = clusterDescriptor.GetPageRange(columnId).Find(idxInCluster); + pageSummary.fPageInfo = clusterDescriptor.GetPageRange(columnId).Find(globalIndex - pageSummary.fColumnOffset); } if (pageSummary.fPageInfo.GetLocator().GetType() == RNTupleLocator::kTypeUnknown) throw RException(R__FAIL("tried to read a page with an unknown locator")); UpdateLastUsedCluster(pageSummary.fClusterId); - return LoadPageImpl(columnHandle, pageSummary, idxInCluster); + return LoadPageImpl(columnHandle, pageSummary); } ROOT::Internal::RPageRef ROOT::Internal::RPageSource::LoadPage(ColumnHandle_t columnHandle, RNTupleLocalIndex localIndex) { const auto clusterId = localIndex.GetClusterId(); - const auto idxInCluster = localIndex.GetIndexInCluster(); const auto columnId = columnHandle.fPhysicalId; const auto columnElementId = columnHandle.fColumn->GetElement()->GetIdentifier(); auto cachedPageRef = @@ -458,14 +455,14 @@ ROOT::Internal::RPageSource::LoadPage(ColumnHandle_t columnHandle, RNTupleLocalI pageSummary.fClusterId = clusterId; pageSummary.fColumnOffset = columnRange.GetFirstElementIndex(); - pageSummary.fPageInfo = clusterDescriptor.GetPageRange(columnId).Find(idxInCluster); + pageSummary.fPageInfo = clusterDescriptor.GetPageRange(columnId).Find(localIndex.GetIndexInCluster()); } if (pageSummary.fPageInfo.GetLocator().GetType() == RNTupleLocator::kTypeUnknown) throw RException(R__FAIL("tried to read a page with an unknown locator")); UpdateLastUsedCluster(clusterId); - return LoadPageImpl(columnHandle, pageSummary, idxInCluster); + return LoadPageImpl(columnHandle, pageSummary); } void ROOT::Internal::RPageSource::EnableDefaultMetrics(const std::string &prefix) diff --git a/tree/ntuple/src/RPageStorageDaos.cxx b/tree/ntuple/src/RPageStorageDaos.cxx index e7e2af7413c3a..def58077fa4b7 100644 --- a/tree/ntuple/src/RPageStorageDaos.cxx +++ b/tree/ntuple/src/RPageStorageDaos.cxx @@ -551,8 +551,7 @@ void ROOT::Experimental::Internal::RPageSourceDaos::LoadSealedPage(ROOT::Descrip } ROOT::Internal::RPageRef ROOT::Experimental::Internal::RPageSourceDaos::LoadPageImpl(ColumnHandle_t columnHandle, - const RPageSummary &pageSummary, - ROOT::NTupleSize_t idxInCluster) + const RPageSummary &pageSummary) { const auto &columnId = columnHandle.fPhysicalId; const auto &clusterId = pageSummary.fClusterId; diff --git a/tree/ntuple/src/RPageStorageFile.cxx b/tree/ntuple/src/RPageStorageFile.cxx index 33b480a109d23..91609d8ac8860 100644 --- a/tree/ntuple/src/RPageStorageFile.cxx +++ b/tree/ntuple/src/RPageStorageFile.cxx @@ -547,9 +547,8 @@ void ROOT::Internal::RPageSourceFile::LoadSealedPage(ROOT::DescriptorId_t physic sealedPage.VerifyChecksumIfEnabled().ThrowOnError(); } -ROOT::Internal::RPageRef ROOT::Internal::RPageSourceFile::LoadPageImpl(ColumnHandle_t columnHandle, - const RPageSummary &pageSummary, - ROOT::NTupleSize_t idxInCluster) +ROOT::Internal::RPageRef +ROOT::Internal::RPageSourceFile::LoadPageImpl(ColumnHandle_t columnHandle, const RPageSummary &pageSummary) { const auto &columnId = columnHandle.fPhysicalId; const auto &clusterId = pageSummary.fClusterId; diff --git a/tree/ntuple/test/ntuple_cluster.cxx b/tree/ntuple/test/ntuple_cluster.cxx index 9bbf40b7c2ad3..5c9341b0f2cc0 100644 --- a/tree/ntuple/test/ntuple_cluster.cxx +++ b/tree/ntuple/test/ntuple_cluster.cxx @@ -41,7 +41,7 @@ class RPageSourceMock : public RPageSource { void LoadStructureImpl() final {} RNTupleDescriptor AttachImpl(RNTupleSerializer::EDescriptorDeserializeMode) final { return RNTupleDescriptor(); } std::unique_ptr CloneImpl() const final { return nullptr; } - RPageRef LoadPageImpl(ColumnHandle_t, const RPageSummary &, ROOT::NTupleSize_t) final { return RPageRef(); } + RPageRef LoadPageImpl(ColumnHandle_t, const RPageSummary &) final { return RPageRef(); } void LoadStreamerInfo() final {} std::unique_ptr OpenWithDifferentAnchor(const ROOT::Internal::RNTupleLink &, const ROOT::RNTupleReadOptions &) final diff --git a/tree/ntuple/test/ntuple_endian.cxx b/tree/ntuple/test/ntuple_endian.cxx index c8cba573088fa..353e51cc410e3 100644 --- a/tree/ntuple/test/ntuple_endian.cxx +++ b/tree/ntuple/test/ntuple_endian.cxx @@ -93,7 +93,7 @@ class RPageSourceMock : public RPageSource { return RNTupleDescriptor(); } std::unique_ptr CloneImpl() const final { return nullptr; } - RPageRef LoadPageImpl(ColumnHandle_t, const RPageSummary &, ROOT::NTupleSize_t) final { return RPageRef(); } + RPageRef LoadPageImpl(ColumnHandle_t, const RPageSummary &) final { return RPageRef(); } void LoadStreamerInfo() final {} std::unique_ptr diff --git a/tree/ntuple/test/ntuple_pages.cxx b/tree/ntuple/test/ntuple_pages.cxx index 2390b3ceb6acc..033059212e626 100644 --- a/tree/ntuple/test/ntuple_pages.cxx +++ b/tree/ntuple/test/ntuple_pages.cxx @@ -11,7 +11,7 @@ class RPageSourceMock : public RPageSource { void LoadStructureImpl() final {} RNTupleDescriptor AttachImpl(RNTupleSerializer::EDescriptorDeserializeMode) final { return RNTupleDescriptor(); } std::unique_ptr CloneImpl() const final { return nullptr; } - RPageRef LoadPageImpl(ColumnHandle_t, const RPageSummary &, ROOT::NTupleSize_t) final { return RPageRef(); } + RPageRef LoadPageImpl(ColumnHandle_t, const RPageSummary &) final { return RPageRef(); } void LoadStreamerInfo() final {} std::unique_ptr OpenWithDifferentAnchor(const ROOT::Internal::RNTupleLink &, const ROOT::RNTupleReadOptions &) final From 38094ea03c37747f842a12662ef2a01e7121bcf9 Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Fri, 22 May 2026 14:53:31 +0200 Subject: [PATCH 4/7] [ntuple] factor out loading of zero pages --- tree/ntuple/inc/ROOT/RPageStorage.hxx | 24 +++++++++++++---------- tree/ntuple/src/RPageStorage.cxx | 28 +++++++++++++++++++++++++-- tree/ntuple/src/RPageStorageDaos.cxx | 10 ---------- tree/ntuple/src/RPageStorageFile.cxx | 9 --------- 4 files changed, 40 insertions(+), 31 deletions(-) diff --git a/tree/ntuple/inc/ROOT/RPageStorage.hxx b/tree/ntuple/inc/ROOT/RPageStorage.hxx index ba0c33c00ffdf..3461a0231d896 100644 --- a/tree/ntuple/inc/ROOT/RPageStorage.hxx +++ b/tree/ntuple/inc/ROOT/RPageStorage.hxx @@ -567,6 +567,16 @@ The page source also gives access to the ntuple's metadata. */ // clang-format on class RPageSource : public RPageStorage { +protected: + /// Summarizes meta-data is necessary to load a certain page. Used by LoadPageImpl(). + struct RPageSummary { + ROOT::DescriptorId_t fClusterId = 0; + /// The first element number of the page's column in the given cluster + std::uint64_t fColumnOffset = 0; + /// Location of the page on disk + ROOT::RClusterDescriptor::RPageInfoExtended fPageInfo; + }; + public: /// Used in SetEntryRange / GetEntryRange struct REntryRange { @@ -640,6 +650,9 @@ private: /// Must not be called when the descriptor guard is taken. void UpdateLastUsedCluster(ROOT::DescriptorId_t clusterId); + // Common treatment of zero pages that would otherwise needed to be handled in LoadPageImpl() + ROOT::Internal::RPageRef LoadZeroPage(ColumnHandle_t columnHandle, const RPageSummary &pageSummary); + protected: /// Default I/O performance counters that get registered in `fMetrics` struct RCounters { @@ -693,15 +706,6 @@ protected: } }; - /// Summarizes meta-data is necessary to load a certain page. Used by LoadPageImpl(). - struct RPageSummary { - ROOT::DescriptorId_t fClusterId = 0; - /// The first element number of the page's column in the given cluster - std::uint64_t fColumnOffset = 0; - /// Location of the page on disk - ROOT::RClusterDescriptor::RPageInfoExtended fPageInfo; - }; - std::unique_ptr fCounters; ROOT::RNTupleReadOptions fOptions; @@ -725,7 +729,7 @@ protected: virtual std::unique_ptr CloneImpl() const = 0; // Only called if a task scheduler is set. No-op be default. virtual void UnzipClusterImpl(ROOT::Internal::RCluster *cluster); - // Returns a page from storage if not found in the page pool. Should be able to handle zero page locators. + // Returns a page from storage if not found in the page pool. Will never receive requests for zero pages. virtual ROOT::Internal::RPageRef LoadPageImpl(ColumnHandle_t columnHandle, const RPageSummary &pageSummary) = 0; /// Prepare a page range read for the column set in `clusterKey`. Specifically, pages referencing the diff --git a/tree/ntuple/src/RPageStorage.cxx b/tree/ntuple/src/RPageStorage.cxx index 3d398b4257ccb..551dc4105afb8 100644 --- a/tree/ntuple/src/RPageStorage.cxx +++ b/tree/ntuple/src/RPageStorage.cxx @@ -392,6 +392,24 @@ void ROOT::Internal::RPageSource::UpdateLastUsedCluster(ROOT::DescriptorId_t clu fLastUsedCluster = clusterId; } +ROOT::Internal::RPageRef +ROOT::Internal::RPageSource::LoadZeroPage(ColumnHandle_t columnHandle, const RPageSummary &pageSummary) +{ + const auto &pageInfo = pageSummary.fPageInfo; + assert(pageInfo.GetLocator().GetType() == RNTupleLocator::kTypePageZero); + + const auto element = columnHandle.fColumn->GetElement(); + const auto elementSize = element->GetSize(); + const auto elementInMemoryType = element->GetIdentifier().fInMemoryType; + + auto pageZero = fPageAllocator->NewPage(elementSize, pageInfo.GetNElements()); + pageZero.GrowUnchecked(pageInfo.GetNElements()); + std::memset(pageZero.GetBuffer(), 0, pageZero.GetNBytes()); + pageZero.SetWindow(pageSummary.fColumnOffset + pageInfo.GetFirstElementIndex(), + RPage::RClusterInfo(pageSummary.fClusterId, pageSummary.fColumnOffset)); + return fPagePool.RegisterPage(std::move(pageZero), RPagePool::RKey{columnHandle.fPhysicalId, elementInMemoryType}); +} + ROOT::Internal::RPageRef ROOT::Internal::RPageSource::LoadPage(ColumnHandle_t columnHandle, ROOT::NTupleSize_t globalIndex) { @@ -422,8 +440,11 @@ ROOT::Internal::RPageSource::LoadPage(ColumnHandle_t columnHandle, ROOT::NTupleS pageSummary.fPageInfo = clusterDescriptor.GetPageRange(columnId).Find(globalIndex - pageSummary.fColumnOffset); } - if (pageSummary.fPageInfo.GetLocator().GetType() == RNTupleLocator::kTypeUnknown) + if (pageSummary.fPageInfo.GetLocator().GetType() == RNTupleLocator::kTypeUnknown) { throw RException(R__FAIL("tried to read a page with an unknown locator")); + } else if (pageSummary.fPageInfo.GetLocator().GetType() == RNTupleLocator::kTypePageZero) { + return LoadZeroPage(columnHandle, pageSummary); + } UpdateLastUsedCluster(pageSummary.fClusterId); return LoadPageImpl(columnHandle, pageSummary); @@ -458,8 +479,11 @@ ROOT::Internal::RPageSource::LoadPage(ColumnHandle_t columnHandle, RNTupleLocalI pageSummary.fPageInfo = clusterDescriptor.GetPageRange(columnId).Find(localIndex.GetIndexInCluster()); } - if (pageSummary.fPageInfo.GetLocator().GetType() == RNTupleLocator::kTypeUnknown) + if (pageSummary.fPageInfo.GetLocator().GetType() == RNTupleLocator::kTypeUnknown) { throw RException(R__FAIL("tried to read a page with an unknown locator")); + } else if (pageSummary.fPageInfo.GetLocator().GetType() == RNTupleLocator::kTypePageZero) { + return LoadZeroPage(columnHandle, pageSummary); + } UpdateLastUsedCluster(clusterId); return LoadPageImpl(columnHandle, pageSummary); diff --git a/tree/ntuple/src/RPageStorageDaos.cxx b/tree/ntuple/src/RPageStorageDaos.cxx index def58077fa4b7..539659211d957 100644 --- a/tree/ntuple/src/RPageStorageDaos.cxx +++ b/tree/ntuple/src/RPageStorageDaos.cxx @@ -561,16 +561,6 @@ ROOT::Internal::RPageRef ROOT::Experimental::Internal::RPageSourceDaos::LoadPage const auto elementSize = element->GetSize(); const auto elementInMemoryType = element->GetIdentifier().fInMemoryType; - if (pageInfo.GetLocator().GetType() == RNTupleLocator::kTypePageZero) { - auto pageZero = fPageAllocator->NewPage(elementSize, pageInfo.GetNElements()); - pageZero.GrowUnchecked(pageInfo.GetNElements()); - memset(pageZero.GetBuffer(), 0, pageZero.GetNBytes()); - pageZero.SetWindow(pageSummary.fColumnOffset + pageInfo.GetFirstElementIndex(), - ROOT::Internal::RPage::RClusterInfo(clusterId, pageSummary.fColumnOffset)); - return fPagePool.RegisterPage(std::move(pageZero), - ROOT::Internal::RPagePool::RKey{columnId, elementInMemoryType}); - } - RSealedPage sealedPage; sealedPage.SetNElements(pageInfo.GetNElements()); sealedPage.SetHasChecksum(pageInfo.HasChecksum()); diff --git a/tree/ntuple/src/RPageStorageFile.cxx b/tree/ntuple/src/RPageStorageFile.cxx index 91609d8ac8860..85616044fde5d 100644 --- a/tree/ntuple/src/RPageStorageFile.cxx +++ b/tree/ntuple/src/RPageStorageFile.cxx @@ -558,15 +558,6 @@ ROOT::Internal::RPageSourceFile::LoadPageImpl(ColumnHandle_t columnHandle, const const auto elementSize = element->GetSize(); const auto elementInMemoryType = element->GetIdentifier().fInMemoryType; - if (pageInfo.GetLocator().GetType() == RNTupleLocator::kTypePageZero) { - auto pageZero = fPageAllocator->NewPage(elementSize, pageInfo.GetNElements()); - pageZero.GrowUnchecked(pageInfo.GetNElements()); - memset(pageZero.GetBuffer(), 0, pageZero.GetNBytes()); - pageZero.SetWindow(pageSummary.fColumnOffset + pageInfo.GetFirstElementIndex(), - ROOT::Internal::RPage::RClusterInfo(clusterId, pageSummary.fColumnOffset)); - return fPagePool.RegisterPage(std::move(pageZero), RPagePool::RKey{columnId, elementInMemoryType}); - } - RSealedPage sealedPage; sealedPage.SetNElements(pageInfo.GetNElements()); sealedPage.SetHasChecksum(pageInfo.HasChecksum()); From 8f8a27e906b1dd8b9c597859d1c92e9e06496ce8 Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Fri, 22 May 2026 15:40:40 +0200 Subject: [PATCH 5/7] [ntuple] factor out common code in LoadSealedPage() --- tree/ntuple/inc/ROOT/RPageStorage.hxx | 6 ++-- tree/ntuple/inc/ROOT/RPageStorageDaos.hxx | 4 +-- tree/ntuple/inc/ROOT/RPageStorageFile.hxx | 4 +-- tree/ntuple/src/RPageStorage.cxx | 30 +++++++++++++++++ tree/ntuple/src/RPageStorageDaos.cxx | 40 +++++------------------ tree/ntuple/src/RPageStorageFile.cxx | 29 ++-------------- tree/ntuple/test/ntuple_cluster.cxx | 2 +- tree/ntuple/test/ntuple_endian.cxx | 2 +- tree/ntuple/test/ntuple_pages.cxx | 2 +- 9 files changed, 50 insertions(+), 69 deletions(-) diff --git a/tree/ntuple/inc/ROOT/RPageStorage.hxx b/tree/ntuple/inc/ROOT/RPageStorage.hxx index 3461a0231d896..e638c69317456 100644 --- a/tree/ntuple/inc/ROOT/RPageStorage.hxx +++ b/tree/ntuple/inc/ROOT/RPageStorage.hxx @@ -731,6 +731,9 @@ protected: virtual void UnzipClusterImpl(ROOT::Internal::RCluster *cluster); // Returns a page from storage if not found in the page pool. Will never receive requests for zero pages. virtual ROOT::Internal::RPageRef LoadPageImpl(ColumnHandle_t columnHandle, const RPageSummary &pageSummary) = 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; /// Prepare a page range read for the column set in `clusterKey`. Specifically, pages referencing the /// `kTypePageZero` locator are filled in `pageZeroMap`; otherwise, `perPageFunc` is called for each page. This is @@ -818,8 +821,7 @@ public: /// The `fSize` and `fNElements` member of the sealedPage parameters are always set. If `sealedPage.fBuffer` is /// `nullptr`, no data will be copied but the returned size information can be used by the caller to allocate a large /// enough buffer and call `LoadSealedPage` again. - virtual void - LoadSealedPage(ROOT::DescriptorId_t physicalColumnId, RNTupleLocalIndex localIndex, RSealedPage &sealedPage) = 0; + void LoadSealedPage(ROOT::DescriptorId_t physicalColumnId, RNTupleLocalIndex localIndex, RSealedPage &sealedPage); /// Populates all the pages of the given cluster ids and columns; it is possible that some columns do not /// contain any pages. The page source may load more columns than the minimal necessary set from `columns`. diff --git a/tree/ntuple/inc/ROOT/RPageStorageDaos.hxx b/tree/ntuple/inc/ROOT/RPageStorageDaos.hxx index 83d98f2bda5db..f2a6580d7d793 100644 --- a/tree/ntuple/inc/ROOT/RPageStorageDaos.hxx +++ b/tree/ntuple/inc/ROOT/RPageStorageDaos.hxx @@ -160,6 +160,7 @@ private: ROOT::Internal::RNTupleDescriptorBuilder fDescriptorBuilder; ROOT::Internal::RPageRef LoadPageImpl(ColumnHandle_t columnHandle, const RPageSummary &pageSummary) final; + void LoadSealedPageImpl(const RNTupleLocator &locator, RSealedPage &sealedPage) final; protected: void LoadStructureImpl() final {} @@ -171,9 +172,6 @@ public: RPageSourceDaos(std::string_view ntupleName, std::string_view uri, const ROOT::RNTupleReadOptions &options); ~RPageSourceDaos() override; - void - LoadSealedPage(ROOT::DescriptorId_t physicalColumnId, RNTupleLocalIndex localIndex, RSealedPage &sealedPage) final; - std::vector> LoadClusters(std::span clusterKeys) final; diff --git a/tree/ntuple/inc/ROOT/RPageStorageFile.hxx b/tree/ntuple/inc/ROOT/RPageStorageFile.hxx index 5110a62354535..7cd74993f2451 100644 --- a/tree/ntuple/inc/ROOT/RPageStorageFile.hxx +++ b/tree/ntuple/inc/ROOT/RPageStorageFile.hxx @@ -181,6 +181,7 @@ protected: std::unique_ptr CloneImpl() const final; RPageRef LoadPageImpl(ColumnHandle_t columnHandle, const RPageSummary &pageSummary) final; + void LoadSealedPageImpl(const RNTupleLocator &locator, RSealedPage &sealedPage) final; public: RPageSourceFile(std::string_view ntupleName, std::string_view path, const ROOT::RNTupleReadOptions &options); @@ -200,9 +201,6 @@ public: std::unique_ptr OpenWithDifferentAnchor(const ROOT::Internal::RNTupleLink &anchorLink, const ROOT::RNTupleReadOptions &options = {}) final; - void - LoadSealedPage(ROOT::DescriptorId_t physicalColumnId, RNTupleLocalIndex localIndex, RSealedPage &sealedPage) final; - std::vector> LoadClusters(std::span clusterKeys) final; diff --git a/tree/ntuple/src/RPageStorage.cxx b/tree/ntuple/src/RPageStorage.cxx index 551dc4105afb8..825e183b379aa 100644 --- a/tree/ntuple/src/RPageStorage.cxx +++ b/tree/ntuple/src/RPageStorage.cxx @@ -392,6 +392,36 @@ void ROOT::Internal::RPageSource::UpdateLastUsedCluster(ROOT::DescriptorId_t clu fLastUsedCluster = clusterId; } +void ROOT::Internal::RPageSource::LoadSealedPage(ROOT::DescriptorId_t physicalColumnId, RNTupleLocalIndex localIndex, + RSealedPage &sealedPage) +{ + const auto clusterId = localIndex.GetClusterId(); + + ROOT::RClusterDescriptor::RPageInfo pageInfo; + { + auto descriptorGuard = GetSharedDescriptorGuard(); + const auto &clusterDescriptor = descriptorGuard->GetClusterDescriptor(clusterId); + pageInfo = clusterDescriptor.GetPageRange(physicalColumnId).Find(localIndex.GetIndexInCluster()); + } + + sealedPage.SetBufferSize(pageInfo.GetLocator().GetNBytesOnStorage() + pageInfo.HasChecksum() * kNBytesPageChecksum); + sealedPage.SetNElements(pageInfo.GetNElements()); + sealedPage.SetHasChecksum(pageInfo.HasChecksum()); + + if (!sealedPage.GetBuffer()) + return; + + if (pageInfo.GetLocator().GetType() == RNTupleLocator::kTypePageZero) { + assert(!pageInfo.HasChecksum()); + memcpy(const_cast(sealedPage.GetBuffer()), ROOT::Internal::RPage::GetPageZeroBuffer(), + sealedPage.GetBufferSize()); + return; + } + + LoadSealedPageImpl(pageInfo.GetLocator(), sealedPage); + sealedPage.VerifyChecksumIfEnabled().ThrowOnError(); +} + ROOT::Internal::RPageRef ROOT::Internal::RPageSource::LoadZeroPage(ColumnHandle_t columnHandle, const RPageSummary &pageSummary) { diff --git a/tree/ntuple/src/RPageStorageDaos.cxx b/tree/ntuple/src/RPageStorageDaos.cxx index 539659211d957..ae22ca596e5a1 100644 --- a/tree/ntuple/src/RPageStorageDaos.cxx +++ b/tree/ntuple/src/RPageStorageDaos.cxx @@ -515,39 +515,15 @@ std::string ROOT::Experimental::Internal::RPageSourceDaos::GetObjectClass() cons return fDaosContainer->GetDefaultObjectClass().ToString(); } -void ROOT::Experimental::Internal::RPageSourceDaos::LoadSealedPage(ROOT::DescriptorId_t physicalColumnId, - RNTupleLocalIndex localIndex, - RSealedPage &sealedPage) +void ROOT::Experimental::Internal::RPageSourceDaos::LoadSealedPageImpl(const RNTupleLocator &, RSealedPage &) { - const auto clusterId = localIndex.GetClusterId(); - - ROOT::RClusterDescriptor::RPageInfo pageInfo; - { - auto descriptorGuard = GetSharedDescriptorGuard(); - const auto &clusterDescriptor = descriptorGuard->GetClusterDescriptor(clusterId); - pageInfo = clusterDescriptor.GetPageRange(physicalColumnId).Find(localIndex.GetIndexInCluster()); - } - - sealedPage.SetBufferSize(pageInfo.GetLocator().GetNBytesOnStorage() + pageInfo.HasChecksum() * kNBytesPageChecksum); - sealedPage.SetNElements(pageInfo.GetNElements()); - sealedPage.SetHasChecksum(pageInfo.HasChecksum()); - if (!sealedPage.GetBuffer()) - return; - - if (pageInfo.GetLocator().GetType() == RNTupleLocator::kTypePageZero) { - assert(!pageInfo.HasChecksum()); - memcpy(const_cast(sealedPage.GetBuffer()), ROOT::Internal::RPage::GetPageZeroBuffer(), - sealedPage.GetBufferSize()); - return; - } - - RDaosKey daosKey = - GetPageDaosKey(fNTupleIndex, clusterId, physicalColumnId, - pageInfo.GetLocator().GetPosition().GetLocation()); - fDaosContainer->ReadSingleAkey(const_cast(sealedPage.GetBuffer()), sealedPage.GetBufferSize(), daosKey.fOid, - daosKey.fDkey, daosKey.fAkey); - - sealedPage.VerifyChecksumIfEnabled().ThrowOnError(); + throw ROOT::RException(R__FAIL("temporarily not implemented")); + // RDaosKey daosKey = + // GetPageDaosKey(fNTupleIndex, clusterId, physicalColumnId, + // pageInfo.GetLocator().GetPosition().GetLocation()); + // fDaosContainer->ReadSingleAkey(const_cast(sealedPage.GetBuffer()), sealedPage.GetBufferSize(), + // daosKey.fOid, + // daosKey.fDkey, daosKey.fAkey); } ROOT::Internal::RPageRef ROOT::Experimental::Internal::RPageSourceDaos::LoadPageImpl(ColumnHandle_t columnHandle, diff --git a/tree/ntuple/src/RPageStorageFile.cxx b/tree/ntuple/src/RPageStorageFile.cxx index 85616044fde5d..29a8714e6fef1 100644 --- a/tree/ntuple/src/RPageStorageFile.cxx +++ b/tree/ntuple/src/RPageStorageFile.cxx @@ -518,33 +518,10 @@ ROOT::RNTupleDescriptor ROOT::Internal::RPageSourceFile::AttachImpl(RNTupleSeria return desc; } -void ROOT::Internal::RPageSourceFile::LoadSealedPage(ROOT::DescriptorId_t physicalColumnId, - RNTupleLocalIndex localIndex, RSealedPage &sealedPage) +void ROOT::Internal::RPageSourceFile::LoadSealedPageImpl(const RNTupleLocator &locator, RSealedPage &sealedPage) { - const auto clusterId = localIndex.GetClusterId(); - - ROOT::RClusterDescriptor::RPageInfo pageInfo; - { - auto descriptorGuard = GetSharedDescriptorGuard(); - const auto &clusterDescriptor = descriptorGuard->GetClusterDescriptor(clusterId); - pageInfo = clusterDescriptor.GetPageRange(physicalColumnId).Find(localIndex.GetIndexInCluster()); - } - - sealedPage.SetBufferSize(pageInfo.GetLocator().GetNBytesOnStorage() + pageInfo.HasChecksum() * kNBytesPageChecksum); - sealedPage.SetNElements(pageInfo.GetNElements()); - sealedPage.SetHasChecksum(pageInfo.HasChecksum()); - if (!sealedPage.GetBuffer()) - return; - if (pageInfo.GetLocator().GetType() != RNTupleLocator::kTypePageZero) { - fReader.ReadBuffer(const_cast(sealedPage.GetBuffer()), sealedPage.GetBufferSize(), - pageInfo.GetLocator().GetPosition()); - } else { - assert(!pageInfo.HasChecksum()); - memcpy(const_cast(sealedPage.GetBuffer()), ROOT::Internal::RPage::GetPageZeroBuffer(), - sealedPage.GetBufferSize()); - } - - sealedPage.VerifyChecksumIfEnabled().ThrowOnError(); + fReader.ReadBuffer(const_cast(sealedPage.GetBuffer()), sealedPage.GetBufferSize(), + locator.GetPosition()); } ROOT::Internal::RPageRef diff --git a/tree/ntuple/test/ntuple_cluster.cxx b/tree/ntuple/test/ntuple_cluster.cxx index 5c9341b0f2cc0..882c9be8b2cab 100644 --- a/tree/ntuple/test/ntuple_cluster.cxx +++ b/tree/ntuple/test/ntuple_cluster.cxx @@ -42,6 +42,7 @@ class RPageSourceMock : public RPageSource { RNTupleDescriptor AttachImpl(RNTupleSerializer::EDescriptorDeserializeMode) final { return RNTupleDescriptor(); } std::unique_ptr CloneImpl() const final { return nullptr; } RPageRef LoadPageImpl(ColumnHandle_t, const RPageSummary &) final { return RPageRef(); } + void LoadSealedPageImpl(const ROOT::RNTupleLocator &, RSealedPage &) final {} void LoadStreamerInfo() final {} std::unique_ptr OpenWithDifferentAnchor(const ROOT::Internal::RNTupleLink &, const ROOT::RNTupleReadOptions &) final @@ -75,7 +76,6 @@ class RPageSourceMock : public RPageSource { auto descriptorGuard = GetExclDescriptorGuard(); descriptorGuard.MoveIn(descBuilder.MoveDescriptor()); } - void LoadSealedPage(ROOT::DescriptorId_t, ROOT::RNTupleLocalIndex, RSealedPage &) final {} std::vector> LoadClusters(std::span clusterKeys) final { std::vector> result; diff --git a/tree/ntuple/test/ntuple_endian.cxx b/tree/ntuple/test/ntuple_endian.cxx index 353e51cc410e3..35cb3efd9684b 100644 --- a/tree/ntuple/test/ntuple_endian.cxx +++ b/tree/ntuple/test/ntuple_endian.cxx @@ -94,6 +94,7 @@ class RPageSourceMock : public RPageSource { } std::unique_ptr CloneImpl() const final { return nullptr; } RPageRef LoadPageImpl(ColumnHandle_t, const RPageSummary &) final { return RPageRef(); } + void LoadSealedPageImpl(const ROOT::RNTupleLocator &, RSealedPage &) final {} void LoadStreamerInfo() final {} std::unique_ptr @@ -115,7 +116,6 @@ class RPageSourceMock : public RPageSource { return fPagePool.RegisterPage(std::move(page), key); } RPageRef LoadPage(ColumnHandle_t, ROOT::RNTupleLocalIndex) final { return RPageRef(); } - void LoadSealedPage(ROOT::DescriptorId_t, ROOT::RNTupleLocalIndex, RSealedPage &) final {} std::vector> LoadClusters(std::span) final { return {}; } }; } // anonymous namespace diff --git a/tree/ntuple/test/ntuple_pages.cxx b/tree/ntuple/test/ntuple_pages.cxx index 033059212e626..ee106e59567e6 100644 --- a/tree/ntuple/test/ntuple_pages.cxx +++ b/tree/ntuple/test/ntuple_pages.cxx @@ -12,6 +12,7 @@ class RPageSourceMock : public RPageSource { RNTupleDescriptor AttachImpl(RNTupleSerializer::EDescriptorDeserializeMode) final { return RNTupleDescriptor(); } std::unique_ptr CloneImpl() const final { return nullptr; } RPageRef LoadPageImpl(ColumnHandle_t, const RPageSummary &) final { return RPageRef(); } + void LoadSealedPageImpl(const ROOT::RNTupleLocator &, RSealedPage &) final {} void LoadStreamerInfo() final {} std::unique_ptr OpenWithDifferentAnchor(const ROOT::Internal::RNTupleLink &, const ROOT::RNTupleReadOptions &) final @@ -21,7 +22,6 @@ class RPageSourceMock : public RPageSource { public: RPageSourceMock() : RPageSource("test", RNTupleReadOptions()) {} - void LoadSealedPage(ROOT::DescriptorId_t, ROOT::RNTupleLocalIndex, RSealedPage &) final {} std::vector> LoadClusters(std::span) final { return std::vector>(); From 9df13e09b9c453e895ffb10ddd6683b37418395f Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Fri, 22 May 2026 16:52:05 +0200 Subject: [PATCH 6/7] [ntuple] remove EDaosMapping The per-cluster/per-page OID mapping option was already unused (hard-coded). Going forward, the DAOS backend should only use the one-object-per-page option. --- tree/ntuple/src/RPageStorageDaos.cxx | 53 +++++++----------------- tree/ntuple/test/ntuple_storage_daos.cxx | 32 ++++++++++++++ 2 files changed, 47 insertions(+), 38 deletions(-) diff --git a/tree/ntuple/src/RPageStorageDaos.cxx b/tree/ntuple/src/RPageStorageDaos.cxx index ae22ca596e5a1..7bd6bce88b135 100644 --- a/tree/ntuple/src/RPageStorageDaos.cxx +++ b/tree/ntuple/src/RPageStorageDaos.cxx @@ -49,12 +49,6 @@ using ROOT::Internal::RNTupleCompressor; using ROOT::Internal::RNTupleDecompressor; using ROOT::Internal::RNTupleSerializer; -/// \brief RNTuple page-DAOS mappings -enum EDaosMapping { - kOidPerCluster, - kOidPerPage -}; - struct RDaosKey { daos_obj_id_t fOid; DistributionKey_t fDkey; @@ -77,21 +71,11 @@ static constexpr decltype(daos_obj_id_t::lo) kOidLowPageList = -2; static constexpr daos_oclass_id_t kCidMetadata = OC_SX; -static constexpr EDaosMapping kDefaultDaosMapping = kOidPerCluster; - -template -RDaosKey GetPageDaosKey(ROOT::Experimental::Internal::ntuple_index_t ntplId, long unsigned clusterId, - long unsigned columnId, long unsigned pageCount) +RDaosKey GetPageDaosKey(ROOT::Experimental::Internal::ntuple_index_t ntplId, long unsigned pageCount) { - if constexpr (mapping == kOidPerCluster) { - return RDaosKey{daos_obj_id_t{static_cast(clusterId), - static_cast(ntplId)}, - static_cast(columnId), static_cast(pageCount)}; - } else if constexpr (mapping == kOidPerPage) { - return RDaosKey{daos_obj_id_t{static_cast(pageCount), - static_cast(ntplId)}, - kDistributionKeyDefault, kAttributeKeyDefault}; - } + return RDaosKey{daos_obj_id_t{static_cast(pageCount), + static_cast(ntplId)}, + kDistributionKeyDefault, kAttributeKeyDefault}; } struct RDaosURI { @@ -308,15 +292,14 @@ ROOT::RNTupleLocator ROOT::Experimental::Internal::RPageSinkDaos::CommitPageImpl } ROOT::RNTupleLocator -ROOT::Experimental::Internal::RPageSinkDaos::CommitSealedPageImpl(ROOT::DescriptorId_t physicalColumnId, +ROOT::Experimental::Internal::RPageSinkDaos::CommitSealedPageImpl(ROOT::DescriptorId_t, const RPageStorage::RSealedPage &sealedPage) { auto pageId = fPageId.fetch_add(1); - ROOT::DescriptorId_t clusterId = fDescriptorBuilder.GetDescriptor().GetNActiveClusters(); { Detail::RNTupleAtomicTimer timer(fCounters->fTimeWallWrite, fCounters->fTimeCpuWrite); - RDaosKey daosKey = GetPageDaosKey(fNTupleIndex, clusterId, physicalColumnId, pageId); + RDaosKey daosKey = GetPageDaosKey(fNTupleIndex, pageId); fDaosContainer->WriteSingleAkey(sealedPage.GetBuffer(), sealedPage.GetBufferSize(), daosKey.fOid, daosKey.fDkey, daosKey.fAkey); } @@ -340,7 +323,6 @@ ROOT::Experimental::Internal::RPageSinkDaos::CommitSealedPageVImpl(std::span(s.GetBuffer()), s.GetBufferSize()); - RDaosKey daosKey = - GetPageDaosKey(fNTupleIndex, clusterId, range.fPhysicalColumnId, pageId); + RDaosKey daosKey = GetPageDaosKey(fNTupleIndex, pageId); auto odPair = RDaosContainer::ROidDkeyPair{daosKey.fOid, daosKey.fDkey}; auto [it, ret] = writeRequests.emplace(odPair, RDaosContainer::RWOperation(odPair)); it->second.Insert(daosKey.fAkey, pageIov); @@ -515,15 +496,12 @@ std::string ROOT::Experimental::Internal::RPageSourceDaos::GetObjectClass() cons return fDaosContainer->GetDefaultObjectClass().ToString(); } -void ROOT::Experimental::Internal::RPageSourceDaos::LoadSealedPageImpl(const RNTupleLocator &, RSealedPage &) +void ROOT::Experimental::Internal::RPageSourceDaos::LoadSealedPageImpl(const RNTupleLocator &locator, + RSealedPage &sealedPage) { - throw ROOT::RException(R__FAIL("temporarily not implemented")); - // RDaosKey daosKey = - // GetPageDaosKey(fNTupleIndex, clusterId, physicalColumnId, - // pageInfo.GetLocator().GetPosition().GetLocation()); - // fDaosContainer->ReadSingleAkey(const_cast(sealedPage.GetBuffer()), sealedPage.GetBufferSize(), - // daosKey.fOid, - // daosKey.fDkey, daosKey.fAkey); + RDaosKey daosKey = GetPageDaosKey(fNTupleIndex, locator.GetPosition().GetLocation()); + fDaosContainer->ReadSingleAkey(const_cast(sealedPage.GetBuffer()), sealedPage.GetBufferSize(), daosKey.fOid, + daosKey.fDkey, daosKey.fAkey); } ROOT::Internal::RPageRef ROOT::Experimental::Internal::RPageSourceDaos::LoadPageImpl(ColumnHandle_t columnHandle, @@ -545,8 +523,8 @@ ROOT::Internal::RPageRef ROOT::Experimental::Internal::RPageSourceDaos::LoadPage if (fOptions.GetClusterCache() == ROOT::RNTupleReadOptions::EClusterCache::kOff) { directReadBuffer = MakeUninitArray(sealedPage.GetBufferSize()); - RDaosKey daosKey = GetPageDaosKey( - fNTupleIndex, clusterId, columnId, pageInfo.GetLocator().GetPosition().GetLocation()); + RDaosKey daosKey = + GetPageDaosKey(fNTupleIndex, pageInfo.GetLocator().GetPosition().GetLocation()); fDaosContainer->ReadSingleAkey(directReadBuffer.get(), sealedPage.GetBufferSize(), daosKey.fOid, daosKey.fDkey, daosKey.fAkey); fCounters->fNPageRead.Inc(); @@ -633,8 +611,7 @@ ROOT::Experimental::Internal::RPageSourceDaos::LoadClusters(std::span(fNTupleIndex, sealedLoc.fClusterId, sealedLoc.fColumnId, - sealedLoc.fPageId); + RDaosKey daosKey = GetPageDaosKey(fNTupleIndex, sealedLoc.fPageId); auto odPair = RDaosContainer::ROidDkeyPair{daosKey.fOid, daosKey.fDkey}; auto [itReq, ret] = readRequests.emplace(odPair, RDaosContainer::RWOperation(odPair)); itReq->second.Insert(daosKey.fAkey, iov); diff --git a/tree/ntuple/test/ntuple_storage_daos.cxx b/tree/ntuple/test/ntuple_storage_daos.cxx index dc2f4410fece4..1ee84e5371c17 100644 --- a/tree/ntuple/test/ntuple_storage_daos.cxx +++ b/tree/ntuple/test/ntuple_storage_daos.cxx @@ -158,6 +158,38 @@ TEST_F(RPageStorageDaos, Options) EXPECT_EQ(1U, source.GetNEntries()); } +TEST_F(RPageStorageDaos, LoadSealedPage) +{ + std::string daosUri = RegisterLabel("ntuple-test-load-sealed-page"); + const std::string_view ntupleName("ntuple"); + + { + auto model = RNTupleModel::Create(); + auto ptrPt = model->MakeField("pt"); + + RNTupleWriteOptions options; + options.SetCompression(0); + auto writer = RNTupleWriter::Recreate(std::move(model), ntupleName, daosUri, options); + *ptrPt = 1.0; + writer->Fill(); + } + + ROOT::Experimental::Internal::RPageSourceDaos source(ntupleName, daosUri, ROOT::RNTupleReadOptions()); + source.Attach(); + RPageStorage::RSealedPage sealedPage; + source.LoadSealedPage(0, RNTupleLocalIndex{0, 0}, sealedPage); + EXPECT_TRUE(sealedPage.GetHasChecksum()); + ASSERT_EQ(12u, sealedPage.GetBufferSize()); + + unsigned char buffer[12]; + sealedPage.SetBuffer(buffer); + source.LoadSealedPage(0, RNTupleLocalIndex{0, 0}, sealedPage); + + float pt = 0; + memcpy(&pt, sealedPage.GetBuffer(), sizeof(pt)); + EXPECT_FLOAT_EQ(1.0, pt); +} + TEST_F(RPageStorageDaos, MultipleNTuplesPerContainer) { std::string daosUri = RegisterLabel("ntuple-test-multiple"); From 26fff4b959d2347a9e9d98309ebe004b5e94db85 Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Fri, 22 May 2026 17:16:35 +0200 Subject: [PATCH 7/7] [ntuple] return page (not page ref) from LoadPageImpl() --- tree/ntuple/inc/ROOT/RPageStorage.hxx | 2 +- tree/ntuple/inc/ROOT/RPageStorageDaos.hxx | 2 +- tree/ntuple/inc/ROOT/RPageStorageFile.hxx | 2 +- tree/ntuple/src/RPageStorage.cxx | 16 ++++++++-------- tree/ntuple/src/RPageStorageDaos.cxx | 11 ++++------- tree/ntuple/src/RPageStorageFile.cxx | 9 +++------ tree/ntuple/test/ntuple_cluster.cxx | 2 +- tree/ntuple/test/ntuple_endian.cxx | 2 +- tree/ntuple/test/ntuple_pages.cxx | 2 +- 9 files changed, 21 insertions(+), 27 deletions(-) diff --git a/tree/ntuple/inc/ROOT/RPageStorage.hxx b/tree/ntuple/inc/ROOT/RPageStorage.hxx index e638c69317456..baf4f5c3cc0db 100644 --- a/tree/ntuple/inc/ROOT/RPageStorage.hxx +++ b/tree/ntuple/inc/ROOT/RPageStorage.hxx @@ -730,7 +730,7 @@ protected: // Only called if a task scheduler is set. No-op be default. virtual void UnzipClusterImpl(ROOT::Internal::RCluster *cluster); // Returns a page from storage if not found in the page pool. Will never receive requests for zero pages. - virtual ROOT::Internal::RPageRef LoadPageImpl(ColumnHandle_t columnHandle, const RPageSummary &pageSummary) = 0; + virtual ROOT::Internal::RPage LoadPageImpl(ColumnHandle_t columnHandle, const RPageSummary &pageSummary) = 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; diff --git a/tree/ntuple/inc/ROOT/RPageStorageDaos.hxx b/tree/ntuple/inc/ROOT/RPageStorageDaos.hxx index f2a6580d7d793..04e573353898b 100644 --- a/tree/ntuple/inc/ROOT/RPageStorageDaos.hxx +++ b/tree/ntuple/inc/ROOT/RPageStorageDaos.hxx @@ -159,7 +159,7 @@ private: ROOT::Internal::RNTupleDescriptorBuilder fDescriptorBuilder; - ROOT::Internal::RPageRef LoadPageImpl(ColumnHandle_t columnHandle, const RPageSummary &pageSummary) final; + ROOT::Internal::RPage LoadPageImpl(ColumnHandle_t columnHandle, const RPageSummary &pageSummary) final; void LoadSealedPageImpl(const RNTupleLocator &locator, RSealedPage &sealedPage) final; protected: diff --git a/tree/ntuple/inc/ROOT/RPageStorageFile.hxx b/tree/ntuple/inc/ROOT/RPageStorageFile.hxx index 7cd74993f2451..f3b39e1ef0939 100644 --- a/tree/ntuple/inc/ROOT/RPageStorageFile.hxx +++ b/tree/ntuple/inc/ROOT/RPageStorageFile.hxx @@ -180,7 +180,7 @@ protected: /// The cloned page source creates a new raw file and reader and opens its own file descriptor to the data. std::unique_ptr CloneImpl() const final; - RPageRef LoadPageImpl(ColumnHandle_t columnHandle, const RPageSummary &pageSummary) final; + RPage LoadPageImpl(ColumnHandle_t columnHandle, const RPageSummary &pageSummary) final; void LoadSealedPageImpl(const RNTupleLocator &locator, RSealedPage &sealedPage) final; public: diff --git a/tree/ntuple/src/RPageStorage.cxx b/tree/ntuple/src/RPageStorage.cxx index 825e183b379aa..740dadc25325d 100644 --- a/tree/ntuple/src/RPageStorage.cxx +++ b/tree/ntuple/src/RPageStorage.cxx @@ -444,9 +444,8 @@ ROOT::Internal::RPageRef ROOT::Internal::RPageSource::LoadPage(ColumnHandle_t columnHandle, ROOT::NTupleSize_t globalIndex) { const auto columnId = columnHandle.fPhysicalId; - const auto columnElementId = columnHandle.fColumn->GetElement()->GetIdentifier(); - auto cachedPageRef = - fPagePool.GetPage(ROOT::Internal::RPagePool::RKey{columnId, columnElementId.fInMemoryType}, globalIndex); + const auto elementInMemoryType = columnHandle.fColumn->GetElement()->GetIdentifier().fInMemoryType; + auto cachedPageRef = fPagePool.GetPage(ROOT::Internal::RPagePool::RKey{columnId, elementInMemoryType}, globalIndex); if (!cachedPageRef.Get().IsNull()) { UpdateLastUsedCluster(cachedPageRef.Get().GetClusterInfo().GetId()); return cachedPageRef; @@ -477,7 +476,8 @@ ROOT::Internal::RPageSource::LoadPage(ColumnHandle_t columnHandle, ROOT::NTupleS } UpdateLastUsedCluster(pageSummary.fClusterId); - return LoadPageImpl(columnHandle, pageSummary); + return fPagePool.RegisterPage(LoadPageImpl(columnHandle, pageSummary), + RPagePool::RKey{columnId, elementInMemoryType}); } ROOT::Internal::RPageRef @@ -485,9 +485,8 @@ ROOT::Internal::RPageSource::LoadPage(ColumnHandle_t columnHandle, RNTupleLocalI { const auto clusterId = localIndex.GetClusterId(); const auto columnId = columnHandle.fPhysicalId; - const auto columnElementId = columnHandle.fColumn->GetElement()->GetIdentifier(); - auto cachedPageRef = - fPagePool.GetPage(ROOT::Internal::RPagePool::RKey{columnId, columnElementId.fInMemoryType}, localIndex); + const auto elementInMemoryType = columnHandle.fColumn->GetElement()->GetIdentifier().fInMemoryType; + auto cachedPageRef = fPagePool.GetPage(ROOT::Internal::RPagePool::RKey{columnId, elementInMemoryType}, localIndex); if (!cachedPageRef.Get().IsNull()) { UpdateLastUsedCluster(clusterId); return cachedPageRef; @@ -516,7 +515,8 @@ ROOT::Internal::RPageSource::LoadPage(ColumnHandle_t columnHandle, RNTupleLocalI } UpdateLastUsedCluster(clusterId); - return LoadPageImpl(columnHandle, pageSummary); + return fPagePool.RegisterPage(LoadPageImpl(columnHandle, pageSummary), + RPagePool::RKey{columnId, elementInMemoryType}); } void ROOT::Internal::RPageSource::EnableDefaultMetrics(const std::string &prefix) diff --git a/tree/ntuple/src/RPageStorageDaos.cxx b/tree/ntuple/src/RPageStorageDaos.cxx index 7bd6bce88b135..8d8f2ba0df31e 100644 --- a/tree/ntuple/src/RPageStorageDaos.cxx +++ b/tree/ntuple/src/RPageStorageDaos.cxx @@ -504,16 +504,13 @@ void ROOT::Experimental::Internal::RPageSourceDaos::LoadSealedPageImpl(const RNT daosKey.fDkey, daosKey.fAkey); } -ROOT::Internal::RPageRef ROOT::Experimental::Internal::RPageSourceDaos::LoadPageImpl(ColumnHandle_t columnHandle, - const RPageSummary &pageSummary) +ROOT::Internal::RPage ROOT::Experimental::Internal::RPageSourceDaos::LoadPageImpl(ColumnHandle_t columnHandle, + const RPageSummary &pageSummary) { const auto &columnId = columnHandle.fPhysicalId; const auto &clusterId = pageSummary.fClusterId; const auto &pageInfo = pageSummary.fPageInfo; - const auto element = columnHandle.fColumn->GetElement(); - const auto elementSize = element->GetSize(); - const auto elementInMemoryType = element->GetIdentifier().fInMemoryType; RSealedPage sealedPage; sealedPage.SetNElements(pageInfo.GetNElements()); @@ -546,13 +543,13 @@ ROOT::Internal::RPageRef ROOT::Experimental::Internal::RPageSourceDaos::LoadPage { Detail::RNTupleAtomicTimer timer(fCounters->fTimeWallUnzip, fCounters->fTimeCpuUnzip); newPage = UnsealPage(sealedPage, *element).Unwrap(); - fCounters->fSzUnzip.Add(elementSize * pageInfo.GetNElements()); + fCounters->fSzUnzip.Add(element->GetSize() * pageInfo.GetNElements()); } newPage.SetWindow(pageSummary.fColumnOffset + pageInfo.GetFirstElementIndex(), ROOT::Internal::RPage::RClusterInfo(clusterId, pageSummary.fColumnOffset)); fCounters->fNPageUnsealed.Inc(); - return fPagePool.RegisterPage(std::move(newPage), ROOT::Internal::RPagePool::RKey{columnId, elementInMemoryType}); + return newPage; } std::unique_ptr ROOT::Experimental::Internal::RPageSourceDaos::CloneImpl() const diff --git a/tree/ntuple/src/RPageStorageFile.cxx b/tree/ntuple/src/RPageStorageFile.cxx index 29a8714e6fef1..92cad882a8e05 100644 --- a/tree/ntuple/src/RPageStorageFile.cxx +++ b/tree/ntuple/src/RPageStorageFile.cxx @@ -524,16 +524,13 @@ void ROOT::Internal::RPageSourceFile::LoadSealedPageImpl(const RNTupleLocator &l locator.GetPosition()); } -ROOT::Internal::RPageRef +ROOT::Internal::RPage ROOT::Internal::RPageSourceFile::LoadPageImpl(ColumnHandle_t columnHandle, const RPageSummary &pageSummary) { const auto &columnId = columnHandle.fPhysicalId; const auto &clusterId = pageSummary.fClusterId; const auto &pageInfo = pageSummary.fPageInfo; - const auto element = columnHandle.fColumn->GetElement(); - const auto elementSize = element->GetSize(); - const auto elementInMemoryType = element->GetIdentifier().fInMemoryType; RSealedPage sealedPage; sealedPage.SetNElements(pageInfo.GetNElements()); @@ -575,13 +572,13 @@ ROOT::Internal::RPageSourceFile::LoadPageImpl(ColumnHandle_t columnHandle, const { RNTupleAtomicTimer timer(fCounters->fTimeWallUnzip, fCounters->fTimeCpuUnzip); newPage = UnsealPage(sealedPage, *element).Unwrap(); - fCounters->fSzUnzip.Add(elementSize * pageInfo.GetNElements()); + fCounters->fSzUnzip.Add(element->GetSize() * pageInfo.GetNElements()); } newPage.SetWindow(pageSummary.fColumnOffset + pageInfo.GetFirstElementIndex(), ROOT::Internal::RPage::RClusterInfo(clusterId, pageSummary.fColumnOffset)); fCounters->fNPageUnsealed.Inc(); - return fPagePool.RegisterPage(std::move(newPage), RPagePool::RKey{columnId, elementInMemoryType}); + return newPage; } std::unique_ptr ROOT::Internal::RPageSourceFile::CloneImpl() const diff --git a/tree/ntuple/test/ntuple_cluster.cxx b/tree/ntuple/test/ntuple_cluster.cxx index 882c9be8b2cab..b072a48d8e770 100644 --- a/tree/ntuple/test/ntuple_cluster.cxx +++ b/tree/ntuple/test/ntuple_cluster.cxx @@ -41,7 +41,7 @@ class RPageSourceMock : public RPageSource { void LoadStructureImpl() final {} RNTupleDescriptor AttachImpl(RNTupleSerializer::EDescriptorDeserializeMode) final { return RNTupleDescriptor(); } std::unique_ptr CloneImpl() const final { return nullptr; } - RPageRef LoadPageImpl(ColumnHandle_t, const RPageSummary &) final { return RPageRef(); } + RPage LoadPageImpl(ColumnHandle_t, const RPageSummary &) final { return RPage(); } void LoadSealedPageImpl(const ROOT::RNTupleLocator &, RSealedPage &) final {} void LoadStreamerInfo() final {} std::unique_ptr diff --git a/tree/ntuple/test/ntuple_endian.cxx b/tree/ntuple/test/ntuple_endian.cxx index 35cb3efd9684b..7078e2fb56ba6 100644 --- a/tree/ntuple/test/ntuple_endian.cxx +++ b/tree/ntuple/test/ntuple_endian.cxx @@ -93,7 +93,7 @@ class RPageSourceMock : public RPageSource { return RNTupleDescriptor(); } std::unique_ptr CloneImpl() const final { return nullptr; } - RPageRef LoadPageImpl(ColumnHandle_t, const RPageSummary &) final { return RPageRef(); } + RPage LoadPageImpl(ColumnHandle_t, const RPageSummary &) final { return RPage(); } void LoadSealedPageImpl(const ROOT::RNTupleLocator &, RSealedPage &) final {} void LoadStreamerInfo() final {} diff --git a/tree/ntuple/test/ntuple_pages.cxx b/tree/ntuple/test/ntuple_pages.cxx index ee106e59567e6..a209ff377ce7a 100644 --- a/tree/ntuple/test/ntuple_pages.cxx +++ b/tree/ntuple/test/ntuple_pages.cxx @@ -11,7 +11,7 @@ class RPageSourceMock : public RPageSource { void LoadStructureImpl() final {} RNTupleDescriptor AttachImpl(RNTupleSerializer::EDescriptorDeserializeMode) final { return RNTupleDescriptor(); } std::unique_ptr CloneImpl() const final { return nullptr; } - RPageRef LoadPageImpl(ColumnHandle_t, const RPageSummary &) final { return RPageRef(); } + RPage LoadPageImpl(ColumnHandle_t, const RPageSummary &) final { return RPage(); } void LoadSealedPageImpl(const ROOT::RNTupleLocator &, RSealedPage &) final {} void LoadStreamerInfo() final {} std::unique_ptr