From 7ea36730d31f443faa17c3579d6913cf80aaf9af Mon Sep 17 00:00:00 2001 From: silverweed Date: Tue, 15 Apr 2025 15:57:04 +0200 Subject: [PATCH 1/2] [ntuple] Move RCluster/RClusterPool out of experimental --- .../io/hadd/merge_changeComp_check_output.C | 4 +-- tree/ntuple/inc/ROOT/RCluster.hxx | 17 ++++------ tree/ntuple/inc/ROOT/RClusterPool.hxx | 4 +-- tree/ntuple/inc/ROOT/RNTupleMerger.hxx | 11 +++---- tree/ntuple/inc/ROOT/RPageStorage.hxx | 13 ++++---- tree/ntuple/inc/ROOT/RPageStorageDaos.hxx | 14 +++++--- tree/ntuple/inc/ROOT/RPageStorageFile.hxx | 18 ++++------- tree/ntuple/src/RCluster.cxx | 13 ++++---- tree/ntuple/src/RClusterPool.cxx | 28 +++++++--------- tree/ntuple/src/RNTupleMerger.cxx | 8 +++-- tree/ntuple/src/RPageStorage.cxx | 9 +++--- tree/ntuple/src/RPageStorageDaos.cxx | 20 ++++++------ tree/ntuple/src/RPageStorageFile.cxx | 17 +++++----- tree/ntuple/test/ntuple_cluster.cxx | 32 ++++++++----------- tree/ntuple/test/ntuple_endian.cxx | 2 +- tree/ntupleutil/v7/src/RNTupleExporter.cxx | 10 +++--- 16 files changed, 102 insertions(+), 118 deletions(-) diff --git a/roottest/root/io/hadd/merge_changeComp_check_output.C b/roottest/root/io/hadd/merge_changeComp_check_output.C index 02cdf4f7690cb..9cf41f980619e 100644 --- a/roottest/root/io/hadd/merge_changeComp_check_output.C +++ b/roottest/root/io/hadd/merge_changeComp_check_output.C @@ -24,7 +24,7 @@ int merge_changeComp_check_output(int expectedCompressionRNT, int expectedCompre ROOT::Internal::RPageSourceFile source("ntpl", fnameOut, ROOT::RNTupleReadOptions()); source.Attach(); - Internal::RClusterPool pool{source}; + ROOT::Internal::RClusterPool pool{source}; const auto expCompAlgo = ROOT::RCompressionSetting::AlgorithmFromCompressionSettings(expectedCompressionRNT); const auto &desc = source.GetSharedDescriptorGuard(); @@ -44,7 +44,7 @@ int merge_changeComp_check_output(int expectedCompressionRNT, int expectedCompre std::uint64_t pageIdx = 0; for (const auto &pageInfo : pages.GetPageInfos()) { auto cluster = pool.GetCluster(clusterDesc.GetId(), {column.GetPhysicalId()}); - Internal::ROnDiskPage::Key key{column.GetPhysicalId(), pageIdx}; + ROOT::Internal::ROnDiskPage::Key key{column.GetPhysicalId(), pageIdx}; auto onDiskPage = cluster->GetOnDiskPage(key); R__ASSERT(onDiskPage); const auto actualCompAlgo = diff --git a/tree/ntuple/inc/ROOT/RCluster.hxx b/tree/ntuple/inc/ROOT/RCluster.hxx index 478c165aae95a..b206554763a8d 100644 --- a/tree/ntuple/inc/ROOT/RCluster.hxx +++ b/tree/ntuple/inc/ROOT/RCluster.hxx @@ -26,12 +26,11 @@ #include namespace ROOT { -namespace Experimental { namespace Internal { // clang-format off /** -\class ROnDiskPage +\class ROOT::Internal::ROnDiskPage \ingroup NTuple \brief A page as being stored on disk, that is packed and compressed @@ -68,16 +67,15 @@ public: }; // class ROnDiskPage } // namespace Internal -} // namespace Experimental } // namespace ROOT // For hash maps ROnDiskPage::Key --> ROnDiskPage namespace std { template <> -struct hash { +struct hash { // TODO(jblomer): quick and dirty hash, likely very sub-optimal, to be revised later. - size_t operator()(const ROOT::Experimental::Internal::ROnDiskPage::Key &key) const + size_t operator()(const ROOT::Internal::ROnDiskPage::Key &key) const { return ( (std::hash()(key.fPhysicalColumnId) ^ (hash()(key.fPageNo) << 1)) >> @@ -86,14 +84,12 @@ struct hash { }; } - namespace ROOT { -namespace Experimental { namespace Internal { // clang-format off /** -\class ROOT::Experimental::Internal::ROnDiskPageMap +\class ROOT::Internal::ROnDiskPageMap \ingroup NTuple \brief A memory region that contains packed and compressed pages @@ -122,7 +118,7 @@ public: // clang-format off /** -\class ROOT::Experimental::Internal::ROnDiskPageMapHeap +\class ROOT::Internal::ROnDiskPageMapHeap \ingroup NTuple \brief An ROnDiskPageMap that is used for an fMemory allocated as an array of unsigned char. */ @@ -142,7 +138,7 @@ public: // clang-format off /** -\class ROOT::Experimental::Internal::RCluster +\class ROOT::Internal::RCluster \ingroup NTuple \brief An in-memory subset of the packed and compressed pages of a cluster @@ -199,7 +195,6 @@ public: }; // class RCluster } // namespace Internal -} // namespace Experimental } // namespace ROOT #endif diff --git a/tree/ntuple/inc/ROOT/RClusterPool.hxx b/tree/ntuple/inc/ROOT/RClusterPool.hxx index 620c977ff4ae2..2de7beb2860b5 100644 --- a/tree/ntuple/inc/ROOT/RClusterPool.hxx +++ b/tree/ntuple/inc/ROOT/RClusterPool.hxx @@ -33,12 +33,11 @@ namespace Internal { class RPageSource; } -namespace Experimental { namespace Internal { // clang-format off /** -\class ROOT::Experimental::Internal::RClusterPool +\class ROOT::Internal::RClusterPool \ingroup NTuple \brief Managed a set of clusters containing compressed and packed pages @@ -142,7 +141,6 @@ public: }; // class RClusterPool } // namespace Internal -} // namespace Experimental } // namespace ROOT #endif diff --git a/tree/ntuple/inc/ROOT/RNTupleMerger.hxx b/tree/ntuple/inc/ROOT/RNTupleMerger.hxx index 6e0713a1d0e5c..15ea7fc63f7c0 100644 --- a/tree/ntuple/inc/ROOT/RNTupleMerger.hxx +++ b/tree/ntuple/inc/ROOT/RNTupleMerger.hxx @@ -32,6 +32,7 @@ class RNTuple; namespace Internal { class RPageAllocator; +class RClusterPool; } namespace Experimental::Internal { @@ -59,8 +60,6 @@ struct RColumnMergeInfo; struct RNTupleMergeData; struct RSealedPageMergeData; -class RClusterPool; - /// Set of merging options to pass to RNTupleMerger. /// If you're using the merger through TFileMerger you need to give it string-based options instead. /// Here is the mapping for the TFileMerger options: @@ -99,11 +98,11 @@ class RNTupleMerger final { std::optional fTaskGroup; std::unique_ptr fModel; - void MergeCommonColumns(RClusterPool &clusterPool, const ROOT::RClusterDescriptor &clusterDesc, + void MergeCommonColumns(ROOT::Internal::RClusterPool &clusterPool, const ROOT::RClusterDescriptor &clusterDesc, std::span commonColumns, - const RCluster::ColumnSet_t &commonColumnSet, std::size_t nCommonColumnsInCluster, - RSealedPageMergeData &sealedPageData, const RNTupleMergeData &mergeData, - ROOT::Internal::RPageAllocator &pageAlloc); + const ROOT::Internal::RCluster::ColumnSet_t &commonColumnSet, + std::size_t nCommonColumnsInCluster, RSealedPageMergeData &sealedPageData, + const RNTupleMergeData &mergeData, ROOT::Internal::RPageAllocator &pageAlloc); void MergeSourceClusters(ROOT::Internal::RPageSource &source, std::span commonColumns, std::span extraDstColumns, RNTupleMergeData &mergeData); diff --git a/tree/ntuple/inc/ROOT/RPageStorage.hxx b/tree/ntuple/inc/ROOT/RPageStorage.hxx index 2b37e74b20555..039188f9c61bc 100644 --- a/tree/ntuple/inc/ROOT/RPageStorage.hxx +++ b/tree/ntuple/inc/ROOT/RPageStorage.hxx @@ -667,7 +667,7 @@ protected: public: void Insert(ROOT::DescriptorId_t physicalColumnId, ROOT::Internal::RColumnElementBase::RIdentifier elementId); void Erase(ROOT::DescriptorId_t physicalColumnId, ROOT::Internal::RColumnElementBase::RIdentifier elementId); - ROOT::Experimental::Internal::RCluster::ColumnSet_t ToColumnSet() const; + ROOT::Internal::RCluster::ColumnSet_t ToColumnSet() const; bool HasColumnInfos(ROOT::DescriptorId_t physicalColumnId) const { return fColumnInfos.count(physicalColumnId) > 0; @@ -703,7 +703,7 @@ protected: /// Returns a new, unattached page source for the same data set virtual std::unique_ptr CloneImpl() const = 0; // Only called if a task scheduler is set. No-op be default. - virtual void UnzipClusterImpl(ROOT::Experimental::Internal::RCluster *cluster); + 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; @@ -712,8 +712,7 @@ protected: /// `kTypePageZero` locator are filled in `pageZeroMap`; otherwise, `perPageFunc` is called for each page. This is /// commonly used as part of `LoadClusters()` in derived classes. void PrepareLoadCluster( - const ROOT::Experimental::Internal::RCluster::RKey &clusterKey, - ROOT::Experimental::Internal::ROnDiskPageMap &pageZeroMap, + const ROOT::Internal::RCluster::RKey &clusterKey, ROOT::Internal::ROnDiskPageMap &pageZeroMap, std::function perPageFunc); @@ -805,15 +804,15 @@ public: /// for the cluster would assume an incomplete cluster and trigger loading again. /// `LoadClusters()` is typically called from the I/O thread of a cluster pool, i.e. the method runs /// concurrently to other methods of the page source. - virtual std::vector> - LoadClusters(std::span clusterKeys) = 0; + virtual std::vector> + LoadClusters(std::span clusterKeys) = 0; /// Parallel decompression and unpacking of the pages in the given cluster. The unzipped pages are supposed /// to be preloaded in a page pool attached to the source. The method is triggered by the cluster pool's /// unzip thread. It is an optional optimization, the method can safely do nothing. In particular, the /// actual implementation will only run if a task scheduler is set. In practice, a task scheduler is set /// if implicit multi-threading is turned on. - void UnzipCluster(ROOT::Experimental::Internal::RCluster *cluster); + void UnzipCluster(ROOT::Internal::RCluster *cluster); // TODO(gparolini): for symmetry with SealPage(), we should either make this private or SealPage() public. RResult diff --git a/tree/ntuple/inc/ROOT/RPageStorageDaos.hxx b/tree/ntuple/inc/ROOT/RPageStorageDaos.hxx index 68c86a1d7e0c2..4b7962bb1f1ff 100644 --- a/tree/ntuple/inc/ROOT/RPageStorageDaos.hxx +++ b/tree/ntuple/inc/ROOT/RPageStorageDaos.hxx @@ -31,12 +31,15 @@ #include namespace ROOT { -namespace Experimental { namespace Internal { -using ntuple_index_t = std::uint32_t; class RCluster; class RClusterPool; +} // namespace Internal + +namespace Experimental { +namespace Internal { +using ntuple_index_t = std::uint32_t; class RDaosPool; class RDaosContainer; class RPageAllocatorHeap; @@ -151,13 +154,13 @@ private: ntuple_index_t fNTupleIndex{0}; /// The last cluster from which a page got loaded. Points into fClusterPool->fPool - RCluster *fCurrentCluster = nullptr; + ROOT::Internal::RCluster *fCurrentCluster = nullptr; /// A container that stores object data (header/footer, pages, etc.) std::unique_ptr fDaosContainer; /// A URI to a DAOS pool of the form 'daos://pool-label/container-label' std::string fURI; /// The cluster pool asynchronously preloads the next few clusters - std::unique_ptr fClusterPool; + std::unique_ptr fClusterPool; ROOT::Internal::RNTupleDescriptorBuilder fDescriptorBuilder; @@ -177,7 +180,8 @@ public: void LoadSealedPage(ROOT::DescriptorId_t physicalColumnId, RNTupleLocalIndex localIndex, RSealedPage &sealedPage) final; - std::vector> LoadClusters(std::span clusterKeys) final; + std::vector> + LoadClusters(std::span clusterKeys) final; /// Return the object class used for user data OIDs in this ntuple. std::string GetObjectClass() const; diff --git a/tree/ntuple/inc/ROOT/RPageStorageFile.hxx b/tree/ntuple/inc/ROOT/RPageStorageFile.hxx index 18fb8975500d8..b223d008f0c07 100644 --- a/tree/ntuple/inc/ROOT/RPageStorageFile.hxx +++ b/tree/ntuple/inc/ROOT/RPageStorageFile.hxx @@ -35,11 +35,8 @@ namespace ROOT { class RNTuple; // for making RPageSourceFile a friend of RNTuple class RNTupleLocator; -namespace Experimental::Internal { -class RClusterPool; -} - namespace Internal { +class RClusterPool; class RRawFile; class RPageAllocatorHeap; @@ -131,7 +128,7 @@ private: /// Either provided by CreateFromAnchor, or read from the ROOT file given the ntuple name std::optional fAnchor; /// The last cluster from which a page got loaded. Points into fClusterPool->fPool - ROOT::Experimental::Internal::RCluster *fCurrentCluster = nullptr; + ROOT::Internal::RCluster *fCurrentCluster = nullptr; /// An RRawFile is used to request the necessary byte ranges from a local or a remote file std::unique_ptr fFile; /// Takes the fFile to read ntuple blobs from it @@ -139,7 +136,7 @@ private: /// The descriptor is created from the header and footer either in AttachImpl or in CreateFromAnchor RNTupleDescriptorBuilder fDescriptorBuilder; /// The cluster pool asynchronously preloads the next few clusters - std::unique_ptr fClusterPool; + std::unique_ptr fClusterPool; /// Populated by LoadStructureImpl(), reset at the end of Attach() RStructureBuffer fStructureBuffer; @@ -149,9 +146,8 @@ private: /// read requests for a given cluster and columns. The reead requests are appended to /// the provided vector. This way, requests can be collected for multiple clusters before /// sending them to RRawFile::ReadV(). - std::unique_ptr - PrepareSingleCluster(const ROOT::Experimental::Internal::RCluster::RKey &clusterKey, - std::vector &readRequests); + std::unique_ptr + PrepareSingleCluster(const ROOT::Internal::RCluster::RKey &clusterKey, std::vector &readRequests); protected: void LoadStructureImpl() final; @@ -180,8 +176,8 @@ public: void LoadSealedPage(ROOT::DescriptorId_t physicalColumnId, RNTupleLocalIndex localIndex, RSealedPage &sealedPage) final; - std::vector> - LoadClusters(std::span clusterKeys) final; + std::vector> + LoadClusters(std::span clusterKeys) final; }; // class RPageSourceFile } // namespace Internal diff --git a/tree/ntuple/src/RCluster.cxx b/tree/ntuple/src/RCluster.cxx index c074ad3f58544..b9e0b479653b9 100644 --- a/tree/ntuple/src/RCluster.cxx +++ b/tree/ntuple/src/RCluster.cxx @@ -20,16 +20,15 @@ #include #include -ROOT::Experimental::Internal::ROnDiskPageMap::~ROnDiskPageMap() = default; +ROOT::Internal::ROnDiskPageMap::~ROnDiskPageMap() = default; //////////////////////////////////////////////////////////////////////////////// -ROOT::Experimental::Internal::ROnDiskPageMapHeap::~ROnDiskPageMapHeap() = default; +ROOT::Internal::ROnDiskPageMapHeap::~ROnDiskPageMapHeap() = default; //////////////////////////////////////////////////////////////////////////////// -const ROOT::Experimental::Internal::ROnDiskPage * -ROOT::Experimental::Internal::RCluster::GetOnDiskPage(const ROnDiskPage::Key &key) const +const ROOT::Internal::ROnDiskPage *ROOT::Internal::RCluster::GetOnDiskPage(const ROnDiskPage::Key &key) const { const auto itr = fOnDiskPages.find(key); if (itr != fOnDiskPages.end()) @@ -37,7 +36,7 @@ ROOT::Experimental::Internal::RCluster::GetOnDiskPage(const ROnDiskPage::Key &ke return nullptr; } -void ROOT::Experimental::Internal::RCluster::Adopt(std::unique_ptr pageMap) +void ROOT::Internal::RCluster::Adopt(std::unique_ptr pageMap) { auto &pages = pageMap->fOnDiskPages; fOnDiskPages.insert(std::make_move_iterator(pages.begin()), std::make_move_iterator(pages.end())); @@ -45,7 +44,7 @@ void ROOT::Experimental::Internal::RCluster::Adopt(std::unique_ptr #include -bool ROOT::Experimental::Internal::RClusterPool::RInFlightCluster::operator<(const RInFlightCluster &other) const +bool ROOT::Internal::RClusterPool::RInFlightCluster::operator<(const RInFlightCluster &other) const { if (fClusterKey.fClusterId == other.fClusterKey.fClusterId) { if (fClusterKey.fPhysicalColumnSet.size() == other.fClusterKey.fPhysicalColumnSet.size()) { @@ -48,8 +48,7 @@ bool ROOT::Experimental::Internal::RClusterPool::RInFlightCluster::operator<(con return fClusterKey.fClusterId < other.fClusterKey.fClusterId; } -ROOT::Experimental::Internal::RClusterPool::RClusterPool(ROOT::Internal::RPageSource &pageSource, - unsigned int clusterBunchSize) +ROOT::Internal::RClusterPool::RClusterPool(ROOT::Internal::RPageSource &pageSource, unsigned int clusterBunchSize) : fPageSource(pageSource), fClusterBunchSize(clusterBunchSize), fPool(2 * clusterBunchSize), @@ -58,7 +57,7 @@ ROOT::Experimental::Internal::RClusterPool::RClusterPool(ROOT::Internal::RPageSo R__ASSERT(clusterBunchSize > 0); } -ROOT::Experimental::Internal::RClusterPool::~RClusterPool() +ROOT::Internal::RClusterPool::~RClusterPool() { { // Controlled shutdown of the I/O thread @@ -69,7 +68,7 @@ ROOT::Experimental::Internal::RClusterPool::~RClusterPool() fThreadIo.join(); } -void ROOT::Experimental::Internal::RClusterPool::ExecReadClusters() +void ROOT::Internal::RClusterPool::ExecReadClusters() { std::deque readItems; while (true) { @@ -105,8 +104,7 @@ void ROOT::Experimental::Internal::RClusterPool::ExecReadClusters() } // while (true) } -ROOT::Experimental::Internal::RCluster * -ROOT::Experimental::Internal::RClusterPool::FindInPool(ROOT::DescriptorId_t clusterId) const +ROOT::Internal::RCluster *ROOT::Internal::RClusterPool::FindInPool(ROOT::DescriptorId_t clusterId) const { for (const auto &cptr : fPool) { if (cptr && (cptr->GetId() == clusterId)) @@ -115,7 +113,7 @@ ROOT::Experimental::Internal::RClusterPool::FindInPool(ROOT::DescriptorId_t clus return nullptr; } -size_t ROOT::Experimental::Internal::RClusterPool::FindFreeSlot() const +size_t ROOT::Internal::RClusterPool::FindFreeSlot() const { auto N = fPool.size(); for (unsigned i = 0; i < N; ++i) { @@ -133,7 +131,7 @@ namespace { /// Helper class for the (cluster, column list) pairs that should be loaded in the background class RProvides { using DescriptorId_t = ROOT::DescriptorId_t; - using ColumnSet_t = ROOT::Experimental::Internal::RCluster::ColumnSet_t; + using ColumnSet_t = ROOT::Internal::RCluster::ColumnSet_t; public: struct RInfo { @@ -182,9 +180,8 @@ class RProvides { } // anonymous namespace -ROOT::Experimental::Internal::RCluster * -ROOT::Experimental::Internal::RClusterPool::GetCluster(ROOT::DescriptorId_t clusterId, - const RCluster::ColumnSet_t &physicalColumns) +ROOT::Internal::RCluster * +ROOT::Internal::RClusterPool::GetCluster(ROOT::DescriptorId_t clusterId, const RCluster::ColumnSet_t &physicalColumns) { std::set keep; RProvides provide; @@ -328,9 +325,8 @@ ROOT::Experimental::Internal::RClusterPool::GetCluster(ROOT::DescriptorId_t clus return WaitFor(clusterId, physicalColumns); } -ROOT::Experimental::Internal::RCluster * -ROOT::Experimental::Internal::RClusterPool::WaitFor(ROOT::DescriptorId_t clusterId, - const RCluster::ColumnSet_t &physicalColumns) +ROOT::Internal::RCluster * +ROOT::Internal::RClusterPool::WaitFor(ROOT::DescriptorId_t clusterId, const RCluster::ColumnSet_t &physicalColumns) { while (true) { // Fast exit: the cluster happens to be already present in the cache pool @@ -383,7 +379,7 @@ ROOT::Experimental::Internal::RClusterPool::WaitFor(ROOT::DescriptorId_t cluster } } -void ROOT::Experimental::Internal::RClusterPool::WaitForInFlightClusters() +void ROOT::Internal::RClusterPool::WaitForInFlightClusters() { while (true) { decltype(fInFlightClusters)::iterator itr; diff --git a/tree/ntuple/src/RNTupleMerger.cxx b/tree/ntuple/src/RNTupleMerger.cxx index 19797c6f06cb8..97f1f335b5467 100644 --- a/tree/ntuple/src/RNTupleMerger.cxx +++ b/tree/ntuple/src/RNTupleMerger.cxx @@ -43,6 +43,7 @@ using ROOT::ENTupleColumnType; using ROOT::RNTupleModel; using ROOT::Internal::MakeUninitArray; +using ROOT::Internal::RCluster; using ROOT::Internal::RColumnElementBase; using ROOT::Internal::RNTupleSerializer; using ROOT::Internal::RPageSink; @@ -693,7 +694,8 @@ static void GenerateZeroPagesForColumns(size_t nEntriesToGenerate, std::span commonColumns, const RCluster::ColumnSet_t &commonColumnSet, std::size_t nCommonColumnsInCluster, RSealedPageMergeData &sealedPageData, @@ -768,7 +770,7 @@ void RNTupleMerger::MergeCommonColumns(RClusterPool &clusterPool, const ROOT::RC assert(sealedPageData.fBuffers.size() == 0 || pageIdx < sealedPageData.fBuffers.size()); assert(pageInfo.GetLocator().GetType() != RNTupleLocator::kTypePageZero); - ROnDiskPage::Key key{columnId, pageIdx}; + ROOT::Internal::ROnDiskPage::Key key{columnId, pageIdx}; auto onDiskPage = cluster->GetOnDiskPage(key); const auto checksumSize = pageInfo.HasChecksum() * RPageStorage::kNBytesPageChecksum; @@ -816,7 +818,7 @@ void RNTupleMerger::MergeCommonColumns(RClusterPool &clusterPool, const ROOT::RC void RNTupleMerger::MergeSourceClusters(RPageSource &source, std::span commonColumns, std::span extraDstColumns, RNTupleMergeData &mergeData) { - RClusterPool clusterPool{source}; + ROOT::Internal::RClusterPool clusterPool{source}; std::vector missingColumns{extraDstColumns.begin(), extraDstColumns.end()}; diff --git a/tree/ntuple/src/RPageStorage.cxx b/tree/ntuple/src/RPageStorage.cxx index 1cc3938acc0b0..1652f7ca3f4f4 100644 --- a/tree/ntuple/src/RPageStorage.cxx +++ b/tree/ntuple/src/RPageStorage.cxx @@ -50,9 +50,9 @@ using ROOT::Internal::RExtraTypeInfoDescriptorBuilder; using ROOT::Internal::RFieldDescriptorBuilder; using ROOT::Internal::RNTupleSerializer; -using ROOT::Experimental::Internal::RCluster; -using ROOT::Experimental::Internal::ROnDiskPage; -using ROOT::Experimental::Internal::ROnDiskPageMap; +using ROOT::Internal::RCluster; +using ROOT::Internal::ROnDiskPage; +using ROOT::Internal::ROnDiskPageMap; using ROOT::Experimental::Detail::RNTupleAtomicCounter; using ROOT::Experimental::Detail::RNTupleAtomicTimer; @@ -136,8 +136,7 @@ void ROOT::Internal::RPageSource::RActivePhysicalColumns::Erase(ROOT::Descriptor } } -ROOT::Experimental::Internal::RCluster::ColumnSet_t -ROOT::Internal::RPageSource::RActivePhysicalColumns::ToColumnSet() const +ROOT::Internal::RCluster::ColumnSet_t ROOT::Internal::RPageSource::RActivePhysicalColumns::ToColumnSet() const { RCluster::ColumnSet_t result; for (const auto &[physicalColumnId, _] : fColumnInfos) diff --git a/tree/ntuple/src/RPageStorageDaos.cxx b/tree/ntuple/src/RPageStorageDaos.cxx index 3f5e0a0279ea8..9fb04336d7786 100644 --- a/tree/ntuple/src/RPageStorageDaos.cxx +++ b/tree/ntuple/src/RPageStorageDaos.cxx @@ -45,6 +45,7 @@ using AttributeKey_t = ROOT::Experimental::Internal::RDaosContainer::AttributeKe using DistributionKey_t = ROOT::Experimental::Internal::RDaosContainer::DistributionKey_t; using ntuple_index_t = ROOT::Experimental::Internal::ntuple_index_t; using ROOT::Internal::MakeUninitArray; +using ROOT::Internal::RCluster; using ROOT::Internal::RNTupleCompressor; using ROOT::Internal::RNTupleDecompressor; using ROOT::Internal::RNTupleSerializer; @@ -489,8 +490,8 @@ ROOT::Experimental::Internal::RPageSourceDaos::RPageSourceDaos(std::string_view const ROOT::RNTupleReadOptions &options) : RPageSource(ntupleName, options), fURI(uri), - fClusterPool( - std::make_unique(*this, ROOT::Internal::RNTupleReadOptionsManip::GetClusterBunchSize(options))) + fClusterPool(std::make_unique( + *this, ROOT::Internal::RNTupleReadOptionsManip::GetClusterBunchSize(options))) { EnableDefaultMetrics("RPageSourceDaos"); @@ -642,7 +643,7 @@ ROOT::Internal::RPageRef ROOT::Experimental::Internal::RPageSourceDaos::LoadPage if (!cachedPageRef.Get().IsNull()) return cachedPageRef; - ROnDiskPage::Key key(columnId, pageInfo.GetPageNumber()); + ROOT::Internal::ROnDiskPage::Key key(columnId, pageInfo.GetPageNumber()); auto onDiskPage = fCurrentCluster->GetOnDiskPage(key); R__ASSERT(onDiskPage && (sealedPage.GetBufferSize() == onDiskPage->GetSize())); sealedPage.SetBuffer(onDiskPage->GetAddress()); @@ -667,7 +668,7 @@ std::unique_ptr ROOT::Experimental::Internal::RPage return std::unique_ptr(clone); } -std::vector> +std::vector> ROOT::Experimental::Internal::RPageSourceDaos::LoadClusters(std::span clusterKeys) { struct RDaosSealedPageLocator { @@ -692,7 +693,7 @@ ROOT::Experimental::Internal::RPageSourceDaos::LoadClusters(std::span> onDiskPages; unsigned clusterBufSz = 0, nPages = 0; - auto pageZeroMap = std::make_unique(); + auto pageZeroMap = std::make_unique(); PrepareLoadCluster( clusterKey, *pageZeroMap, [&](ROOT::DescriptorId_t physicalColumnId, ROOT::NTupleSize_t pageNo, @@ -710,7 +711,8 @@ ROOT::Experimental::Internal::RPageSourceDaos::LoadClusters(std::span(std::unique_ptr(clusterBuffer)); + auto pageMap = + std::make_unique(std::unique_ptr(clusterBuffer)); auto cageBuffer = clusterBuffer; // Fill the cluster page map and the read requests for the RDaosContainer::ReadV() call @@ -722,8 +724,8 @@ ROOT::Experimental::Internal::RPageSourceDaos::LoadClusters(std::spanRegister(key, ROnDiskPage(cageBuffer + s.fCageOffset, s.fBufferSize)); + ROOT::Internal::ROnDiskPage::Key key(s.fColumnId, s.fPageNo); + pageMap->Register(key, ROOT::Internal::ROnDiskPage(cageBuffer + s.fCageOffset, s.fBufferSize)); cageSz += s.fBufferSize; } @@ -751,7 +753,7 @@ ROOT::Experimental::Internal::RPageSourceDaos::LoadClusters(std::spanfNClusterLoaded.Add(clusterKeys.size()); - std::vector> clusters; + std::vector> clusters; RDaosContainer::MultiObjectRWOperation_t readRequests; for (auto key : clusterKeys) { clusters.emplace_back(fnPrepareSingleCluster(key, readRequests)); diff --git a/tree/ntuple/src/RPageStorageFile.cxx b/tree/ntuple/src/RPageStorageFile.cxx index c70cb7a4b9b7d..545c4b9972a61 100644 --- a/tree/ntuple/src/RPageStorageFile.cxx +++ b/tree/ntuple/src/RPageStorageFile.cxx @@ -42,15 +42,15 @@ #include using ROOT::Experimental::Detail::RNTupleAtomicTimer; -using ROOT::Experimental::Internal::RCluster; -using ROOT::Experimental::Internal::RClusterPool; -using ROOT::Experimental::Internal::ROnDiskPage; -using ROOT::Experimental::Internal::ROnDiskPageMap; using ROOT::Internal::MakeUninitArray; +using ROOT::Internal::RCluster; +using ROOT::Internal::RClusterPool; using ROOT::Internal::RNTupleCompressor; using ROOT::Internal::RNTupleDecompressor; using ROOT::Internal::RNTupleFileWriter; using ROOT::Internal::RNTupleSerializer; +using ROOT::Internal::ROnDiskPage; +using ROOT::Internal::ROnDiskPageMap; using ROOT::Internal::RPagePool; ROOT::Internal::RPageSinkFile::RPageSinkFile(std::string_view ntupleName, const ROOT::RNTupleWriteOptions &options) @@ -491,7 +491,7 @@ std::unique_ptr ROOT::Internal::RPageSourceFile::Cl return std::unique_ptr(clone); } -std::unique_ptr +std::unique_ptr ROOT::Internal::RPageSourceFile::PrepareSingleCluster(const RCluster::RKey &clusterKey, std::vector &readRequests) { @@ -599,8 +599,7 @@ ROOT::Internal::RPageSourceFile::PrepareSingleCluster(const RCluster::RKey &clus // Register the on disk pages in a page map auto buffer = new unsigned char[reinterpret_cast(req.fBuffer) + req.fSize]; - auto pageMap = - std::make_unique(std::unique_ptr(buffer)); + auto pageMap = std::make_unique(std::unique_ptr(buffer)); for (const auto &s : onDiskPages) { ROnDiskPage::Key key(s.fColumnId, s.fPageNo); pageMap->Register(key, ROnDiskPage(buffer + s.fBufPos, s.fSize)); @@ -618,12 +617,12 @@ ROOT::Internal::RPageSourceFile::PrepareSingleCluster(const RCluster::RKey &clus return cluster; } -std::vector> +std::vector> ROOT::Internal::RPageSourceFile::LoadClusters(std::span clusterKeys) { fCounters->fNClusterLoaded.Add(clusterKeys.size()); - std::vector> clusters; + std::vector> clusters; std::vector readRequests; clusters.reserve(clusterKeys.size()); diff --git a/tree/ntuple/test/ntuple_cluster.cxx b/tree/ntuple/test/ntuple_cluster.cxx index e807052324901..896d9f27c4c89 100644 --- a/tree/ntuple/test/ntuple_cluster.cxx +++ b/tree/ntuple/test/ntuple_cluster.cxx @@ -24,9 +24,10 @@ #include #include -using ROOT::Experimental::Internal::RCluster; -using ROOT::Experimental::Internal::RClusterPool; -using ROOT::Experimental::Internal::ROnDiskPage; +using ROOT::Internal::RCluster; +using ROOT::Internal::RClusterPool; +using ROOT::Internal::ROnDiskPage; +using ROOT::Internal::ROnDiskPageMapHeap; using ROOT::Internal::RPageRef; using ROOT::Internal::RPageSource; @@ -45,7 +46,7 @@ class RPageSourceMock : public RPageSource { public: /// Records the cluster IDs requests by LoadClusters() calls std::vector fReqsClusterIds; - std::vector fReqsColumns; + std::vector fReqsColumns; RPageSourceMock() : RPageSource("test", RNTupleReadOptions()) { @@ -74,7 +75,7 @@ class RPageSourceMock : public RPageSource { fReqsClusterIds.emplace_back(key.fClusterId); fReqsColumns.emplace_back(key.fPhysicalColumnSet); auto cluster = std::make_unique(key.fClusterId); - auto pageMap = std::make_unique(); + auto pageMap = std::make_unique(); for (auto colId : key.fPhysicalColumnSet) { pageMap->Register(ROnDiskPage::Key(colId, 0), ROnDiskPage(nullptr, 0)); cluster->SetColumnAvailable(colId); @@ -90,18 +91,17 @@ class RPageSourceMock : public RPageSource { TEST(Cluster, Allocate) { - auto cluster = new ROOT::Experimental::Internal::ROnDiskPageMapHeap(nullptr); + auto cluster = new ROnDiskPageMapHeap(nullptr); delete cluster; - cluster = new ROOT::Experimental::Internal::ROnDiskPageMapHeap(std::make_unique(1)); + cluster = new ROnDiskPageMapHeap(std::make_unique(1)); delete cluster; } TEST(Cluster, Basics) { auto memory = new unsigned char[3]; - auto pageMap = - std::make_unique(std::unique_ptr(memory)); + auto pageMap = std::make_unique(std::unique_ptr(memory)); pageMap->Register(ROnDiskPage::Key(5, 0), ROnDiskPage(&memory[0], 1)); pageMap->Register(ROnDiskPage::Key(5, 1), ROnDiskPage(&memory[1], 2)); auto cluster = std::make_unique(0); @@ -121,14 +121,12 @@ TEST(Cluster, Basics) TEST(Cluster, AdoptPageMaps) { auto mem1 = new unsigned char[3]; - auto pageMap1 = - std::make_unique(std::unique_ptr(mem1)); + auto pageMap1 = std::make_unique(std::unique_ptr(mem1)); pageMap1->Register(ROnDiskPage::Key(5, 0), ROnDiskPage(&mem1[0], 1)); pageMap1->Register(ROnDiskPage::Key(5, 1), ROnDiskPage(&mem1[1], 2)); // Column 5 is in both mem1 and mem2 but that should not hurt auto mem2 = new unsigned char[4]; - auto pageMap2 = - std::make_unique(std::unique_ptr(mem2)); + auto pageMap2 = std::make_unique(std::unique_ptr(mem2)); pageMap2->Register(ROnDiskPage::Key(5, 0), ROnDiskPage(&mem2[0], 1)); pageMap2->Register(ROnDiskPage::Key(5, 1), ROnDiskPage(&mem2[1], 2)); pageMap2->Register(ROnDiskPage::Key(6, 0), ROnDiskPage(&mem2[3], 1)); @@ -158,8 +156,7 @@ TEST(Cluster, AdoptPageMaps) TEST(Cluster, AdoptClusters) { auto mem1 = new unsigned char[3]; - auto pageMap1 = - std::make_unique(std::unique_ptr(mem1)); + auto pageMap1 = std::make_unique(std::unique_ptr(mem1)); pageMap1->Register(ROnDiskPage::Key(5, 0), ROnDiskPage(&mem1[0], 1)); pageMap1->Register(ROnDiskPage::Key(5, 1), ROnDiskPage(&mem1[1], 2)); auto cluster1 = std::make_unique(0); @@ -168,8 +165,7 @@ TEST(Cluster, AdoptClusters) // Column 5 is in both clusters but that should not hurt auto mem2 = new unsigned char[4]; - auto pageMap2 = - std::make_unique(std::unique_ptr(mem2)); + auto pageMap2 = std::make_unique(std::unique_ptr(mem2)); pageMap2->Register(ROnDiskPage::Key(5, 0), ROnDiskPage(&mem2[0], 1)); pageMap2->Register(ROnDiskPage::Key(5, 1), ROnDiskPage(&mem2[1], 2)); pageMap2->Register(ROnDiskPage::Key(6, 0), ROnDiskPage(&mem2[3], 1)); @@ -336,7 +332,7 @@ TEST(PageStorageFile, LoadClusters) EXPECT_NE(ROOT::kInvalidDescriptorId, colId); } - std::vector clusterKeys; + std::vector clusterKeys; clusterKeys.push_back({0, {}}); auto cluster = std::move(source.LoadClusters(clusterKeys)[0]); EXPECT_EQ(0U, cluster->GetId()); diff --git a/tree/ntuple/test/ntuple_endian.cxx b/tree/ntuple/test/ntuple_endian.cxx index 7610d63875b42..2443d6c22904e 100644 --- a/tree/ntuple/test/ntuple_endian.cxx +++ b/tree/ntuple/test/ntuple_endian.cxx @@ -24,7 +24,7 @@ using ROOT::ENTupleColumnType; using ROOT::NTupleSize_t; using ROOT::RNTupleDescriptor; using ROOT::RNTupleModel; -using ROOT::Experimental::Internal::RCluster; +using ROOT::Internal::RCluster; using ROOT::Internal::RColumn; using ROOT::Internal::RColumnElementBase; using ROOT::Internal::RPage; diff --git a/tree/ntupleutil/v7/src/RNTupleExporter.cxx b/tree/ntupleutil/v7/src/RNTupleExporter.cxx index 434cbe447fc67..c2db3ce071f6c 100644 --- a/tree/ntupleutil/v7/src/RNTupleExporter.cxx +++ b/tree/ntupleutil/v7/src/RNTupleExporter.cxx @@ -117,14 +117,14 @@ RNTupleExporter::ExportPages(ROOT::Internal::RPageSource &source, const RPagesOp source.Attach(); auto desc = source.GetSharedDescriptorGuard(); - RClusterPool clusterPool{source}; + ROOT::Internal::RClusterPool clusterPool{source}; // Collect column info std::vector columnInfos; const RAddColumnsResult addColRes = AddColumnsFromField(columnInfos, desc.GetRef(), desc->GetFieldZero(), options); // Collect ColumnSet for the cluster pool query - RCluster::ColumnSet_t columnSet; + ROOT::Internal::RCluster::ColumnSet_t columnSet; columnSet.reserve(columnInfos.size()); for (const auto &colInfo : columnInfos) { columnSet.emplace(colInfo.fColDesc->GetPhysicalId()); @@ -141,7 +141,7 @@ RNTupleExporter::ExportPages(ROOT::Internal::RPageSource &source, const RPagesOp int prevIntPercent = 0; while (clusterId != ROOT::kInvalidDescriptorId) { const auto &clusterDesc = desc->GetClusterDescriptor(clusterId); - const RCluster *cluster = clusterPool.GetCluster(clusterId, columnSet); + const ROOT::Internal::RCluster *cluster = clusterPool.GetCluster(clusterId, columnSet); for (const auto &colInfo : columnInfos) { auto columnId = colInfo.fColDesc->GetPhysicalId(); const auto &pages = clusterDesc.GetPageRange(columnId); @@ -155,8 +155,8 @@ RNTupleExporter::ExportPages(ROOT::Internal::RPageSource &source, const RPagesOp assert(!colRange.IsSuppressed() || pages.GetPageInfos().empty()); for (const auto &pageInfo : pages.GetPageInfos()) { - ROnDiskPage::Key key{columnId, pageIdx}; - const ROnDiskPage *onDiskPage = cluster->GetOnDiskPage(key); + ROOT::Internal::ROnDiskPage::Key key{columnId, pageIdx}; + const ROOT::Internal::ROnDiskPage *onDiskPage = cluster->GetOnDiskPage(key); // dump the page const void *pageBuf = onDiskPage->GetAddress(); From a2abd369496134bd10a1a2b7b0e656cf90e614a3 Mon Sep 17 00:00:00 2001 From: silverweed Date: Tue, 15 Apr 2025 16:38:21 +0200 Subject: [PATCH 2/2] [ntuple][skip-ci] fix some wrong Experimental leftovers in comments --- tree/ntuple/inc/ROOT/RField/RFieldSequenceContainer.hxx | 2 +- tree/ntuple/inc/ROOT/RNTupleDescriptor.hxx | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tree/ntuple/inc/ROOT/RField/RFieldSequenceContainer.hxx b/tree/ntuple/inc/ROOT/RField/RFieldSequenceContainer.hxx index 86356756f369b..c817742ae9fd6 100644 --- a/tree/ntuple/inc/ROOT/RField/RFieldSequenceContainer.hxx +++ b/tree/ntuple/inc/ROOT/RField/RFieldSequenceContainer.hxx @@ -313,7 +313,7 @@ public: //////////////////////////////////////////////////////////////////////////////// /** -\class ROOT::Experimental::RArrayAsRVecField +\class ROOT::RArrayAsRVecField \brief A field for fixed-size arrays that are represented as RVecs in memory. \ingroup NTuple This class is used only for reading. In particular, it helps exposing diff --git a/tree/ntuple/inc/ROOT/RNTupleDescriptor.hxx b/tree/ntuple/inc/ROOT/RNTupleDescriptor.hxx index 3587b6071723b..b9a0a8a50e618 100644 --- a/tree/ntuple/inc/ROOT/RNTupleDescriptor.hxx +++ b/tree/ntuple/inc/ROOT/RNTupleDescriptor.hxx @@ -235,7 +235,7 @@ class RClusterDescriptor final { public: // clang-format off /** - \class ROOT::Experimental::RClusterDescriptor::RColumnRange + \class ROOT::RClusterDescriptor::RColumnRange \ingroup NTuple \brief The window of element indexes of a particular column in a particular cluster */