diff --git a/roottest/root/io/hadd/merge_changeComp_check_output.C b/roottest/root/io/hadd/merge_changeComp_check_output.C index cad0053cf1792..8d03dfbe8897c 100644 --- a/roottest/root/io/hadd/merge_changeComp_check_output.C +++ b/roottest/root/io/hadd/merge_changeComp_check_output.C @@ -23,6 +23,7 @@ int merge_changeComp_check_output(int expectedCompressionRNT, int expectedCompre ROOT::Internal::RPageSourceFile source("ntpl", fnameOut, ROOT::RNTupleReadOptions()); source.Attach(); + source.LoadAllPageLists(); ROOT::Internal::RClusterPool pool{source}; diff --git a/tree/dataframe/src/RNTupleDS.cxx b/tree/dataframe/src/RNTupleDS.cxx index 59ec6f0191ec5..135dd04f18ac8 100644 --- a/tree/dataframe/src/RNTupleDS.cxx +++ b/tree/dataframe/src/RNTupleDS.cxx @@ -552,6 +552,7 @@ void ROOT::RDF::RNTupleDS::PrepareNextRanges() source = CreatePageSource(fNTupleName, fFileNames[fNextFileIndex]); } source->Attach(); + source->LoadAllPageLists(); fNextFileIndex++; auto nEntries = source->GetNEntries(); @@ -565,7 +566,7 @@ void ROOT::RDF::RNTupleDS::PrepareNextRanges() const auto rangesByCluster = [&source]() { // Take the shared lock of the descriptor just for the time necessary auto descGuard = source->GetSharedDescriptorGuard(); - return ROOT::Internal::GetClusterBoundaries(descGuard.GetRef()); + return ROOT::Internal::GetClusterBoundaries(descGuard.GetConstRef()); }(); const unsigned int nRangesByCluster = rangesByCluster.size(); @@ -891,6 +892,8 @@ ROOT::Internal::RDF::GetClustersAndEntries(std::string_view ntupleName, std::str { auto source = ROOT::Internal::RPageSource::Create(ntupleName, location); source->Attach(); + // TODO(jblomer): improve MT scheduling such that not all page lists are required right away + source->LoadAllPageLists(); auto descGuard = source->GetSharedDescriptorGuard(); - return std::make_pair(ROOT::Internal::GetClusterBoundaries(descGuard.GetRef()), descGuard->GetNEntries()); + return std::make_pair(ROOT::Internal::GetClusterBoundaries(descGuard.GetConstRef()), descGuard->GetNEntries()); } diff --git a/tree/dataframe/test/dataframe_snapshot_ntuple.cxx b/tree/dataframe/test/dataframe_snapshot_ntuple.cxx index d4e92e393a823..b6428d4713895 100644 --- a/tree/dataframe/test/dataframe_snapshot_ntuple.cxx +++ b/tree/dataframe/test/dataframe_snapshot_ntuple.cxx @@ -125,7 +125,9 @@ TEST(RDFSnapshotRNTuple, WriteOpts) EXPECT_EQ(columns, sdf->GetColumnNames()); - auto reader = RNTupleReader::Open("ntuple", fileGuard.GetPath()); + ROOT::RNTupleReadOptions ntplOpts; + ntplOpts.SetMetadataMode(ROOT::RNTupleReadOptions::EMetadataMode::kEager); + auto reader = RNTupleReader::Open("ntuple", fileGuard.GetPath(), ntplOpts); EXPECT_EQ(2, reader->GetDescriptor().GetNClusters()); EXPECT_EQ(2, reader->GetDescriptor().GetClusterDescriptor(0).GetPageRange(0).GetPageInfos().size()); EXPECT_EQ(2, reader->GetDescriptor().GetClusterDescriptor(1).GetPageRange(0).GetPageInfos().size()); @@ -184,7 +186,9 @@ TEST(RDFSnapshotRNTuple, DefaultCompressionSettings) EXPECT_EQ(columns, sdf->GetColumnNames()); - auto reader = RNTupleReader::Open("ntuple", fileGuard.GetPath()); + ROOT::RNTupleReadOptions ntplOpts; + ntplOpts.SetMetadataMode(ROOT::RNTupleReadOptions::EMetadataMode::kEager); + auto reader = RNTupleReader::Open("ntuple", fileGuard.GetPath(), ntplOpts); auto compSettings = *reader->GetDescriptor().GetClusterDescriptor(0).GetColumnRange(0).GetCompressionSettings(); // The RNTuple default should be 505 EXPECT_EQ(505, compSettings); @@ -206,7 +210,9 @@ TEST(RDFSnapshotRNTuple, Compression) EXPECT_EQ(columns, sdf->GetColumnNames()); - auto reader = RNTupleReader::Open("ntuple", fileGuard.GetPath()); + ROOT::RNTupleReadOptions ntplOpts; + ntplOpts.SetMetadataMode(ROOT::RNTupleReadOptions::EMetadataMode::kEager); + auto reader = RNTupleReader::Open("ntuple", fileGuard.GetPath(), ntplOpts); auto compSettings = *reader->GetDescriptor().GetClusterDescriptor(0).GetColumnRange(0).GetCompressionSettings(); EXPECT_EQ(404, compSettings); } diff --git a/tree/ntuple/inc/ROOT/RNTupleReadOptions.hxx b/tree/ntuple/inc/ROOT/RNTupleReadOptions.hxx index 442756ce48bab..807afa7046642 100644 --- a/tree/ntuple/inc/ROOT/RNTupleReadOptions.hxx +++ b/tree/ntuple/inc/ROOT/RNTupleReadOptions.hxx @@ -66,6 +66,16 @@ This is useful, e.g., in the context of RDataFrame where the threads are fully m + +`EMetadataMode` +EMetadataMode +EMetadataMode::kDefault + +Set whether cluster metadata (page locations) are loaded entirely on open or on demand during entry iteration. +By default, the RNTuple reader loads metadata on demand. + + + `EnableMetrics` `bool` @@ -93,12 +103,19 @@ public: kDefault, }; + enum class EMetadataMode { + kEager, + kOnDemand, + kDefault = kOnDemand, + }; + private: EClusterCache fClusterCache = EClusterCache::kDefault; /// The number of cluster to be prefetched in a single batch; this option is transitional and will be replaced /// by an option that allows to control the amount of memory that the prefetcher uses. unsigned int fClusterBunchSize = 1; EImplicitMT fUseImplicitMT = EImplicitMT::kDefault; + EMetadataMode fMetadataMode = EMetadataMode::kDefault; bool fEnableMetrics = false; /// Largest header or footer envelope a page source will accept from an anchor it does not fully trust, /// such as the S3 anchor, which is a separately stored object. Guards the allocation made from those sizes. @@ -111,6 +128,9 @@ public: EImplicitMT GetUseImplicitMT() const { return fUseImplicitMT; } void SetUseImplicitMT(EImplicitMT val) { fUseImplicitMT = val; } + EMetadataMode GetMetadataMode() const { return fMetadataMode; } + void SetMetadataMode(EMetadataMode val) { fMetadataMode = val; } + bool GetEnableMetrics() const { return fEnableMetrics; } void SetEnableMetrics(bool val) { fEnableMetrics = val; } }; // class RNTupleReadOptions diff --git a/tree/ntuple/inc/ROOT/RNTupleView.hxx b/tree/ntuple/inc/ROOT/RNTupleView.hxx index 592357f9fc187..f0bc91aa2f923 100644 --- a/tree/ntuple/inc/ROOT/RNTupleView.hxx +++ b/tree/ntuple/inc/ROOT/RNTupleView.hxx @@ -93,7 +93,7 @@ protected: std::unique_ptr field; { auto descGuard = pageSource.GetSharedDescriptorGuard(); - const auto &desc = descGuard.GetRef(); + const auto &desc = descGuard.GetConstRef(); const auto &fieldDesc = desc.GetFieldDescriptor(fieldId); if constexpr (std::is_void_v) { if (typeName.empty()) @@ -275,7 +275,7 @@ protected: static ROOT::RField CreateField(ROOT::DescriptorId_t fieldId, ROOT::Internal::RPageSource &pageSource) { auto descGuard = pageSource.GetSharedDescriptorGuard(); - const auto &desc = descGuard.GetRef(); + const auto &desc = descGuard.GetConstRef(); const auto &fieldDesc = desc.GetFieldDescriptor(fieldId); if (!Internal::IsMatchingFieldType(fieldDesc.GetTypeName())) { throw RException(R__FAIL("type mismatch for field " + fieldDesc.GetFieldName() + ": " + @@ -340,7 +340,7 @@ private: std::string fieldName; { auto descGuard = source->GetSharedDescriptorGuard(); - const auto &desc = descGuard.GetRef(); + const auto &desc = descGuard.GetConstRef(); const auto &fieldDesc = desc.GetFieldDescriptor(fieldId); if (fieldDesc.GetStructure() != ROOT::ENTupleStructure::kCollection) { throw RException( diff --git a/tree/ntuple/inc/ROOT/RPageStorage.hxx b/tree/ntuple/inc/ROOT/RPageStorage.hxx index a205bba72155b..94c3dec969aff 100644 --- a/tree/ntuple/inc/ROOT/RPageStorage.hxx +++ b/tree/ntuple/inc/ROOT/RPageStorage.hxx @@ -40,6 +40,7 @@ #include #include #include +#include #include namespace ROOT { @@ -619,6 +620,7 @@ public: std::shared_mutex *fLock = nullptr; public: + RSharedDescriptorGuard() = default; RSharedDescriptorGuard(const ROOT::RNTupleDescriptor &desc, std::shared_mutex &lock) : fDescriptor(&desc), fLock(&lock) { @@ -626,35 +628,112 @@ public: } RSharedDescriptorGuard(const RSharedDescriptorGuard &) = delete; RSharedDescriptorGuard &operator=(const RSharedDescriptorGuard &) = delete; - RSharedDescriptorGuard(RSharedDescriptorGuard &&) = default; - RSharedDescriptorGuard &operator=(RSharedDescriptorGuard &&) = default; - ~RSharedDescriptorGuard() { fLock->unlock_shared(); } + RSharedDescriptorGuard(RSharedDescriptorGuard &&other) + { + std::swap(fDescriptor, other.fDescriptor); + std::swap(fLock, other.fLock); + } + RSharedDescriptorGuard &operator=(RSharedDescriptorGuard &&other) + { + if (this == &other) + return *this; + std::swap(fDescriptor, other.fDescriptor); + std::swap(fLock, other.fLock); + return *this; + } + ~RSharedDescriptorGuard() + { + if (IsValid()) + Release(); + } const ROOT::RNTupleDescriptor *operator->() const { return fDescriptor; } - const ROOT::RNTupleDescriptor &GetRef() const { return *fDescriptor; } + const ROOT::RNTupleDescriptor &GetConstRef() const { return *fDescriptor; } + bool IsValid() const { return fLock; } + void Release() + { + fLock->unlock_shared(); + fLock = nullptr; + fDescriptor = nullptr; + } }; /// An RAII wrapper used for the writable access to `RPageSource::fDescriptor`. See `GetSharedDescriptorGuard()`. class RExclDescriptorGuard { - ROOT::RNTupleDescriptor &fDescriptor; - std::shared_mutex &fLock; + ROOT::RNTupleDescriptor *fDescriptor = nullptr; + std::shared_mutex *fLock = nullptr; public: - RExclDescriptorGuard(ROOT::RNTupleDescriptor &desc, std::shared_mutex &lock) : fDescriptor(desc), fLock(lock) + RExclDescriptorGuard() = default; + RExclDescriptorGuard(ROOT::RNTupleDescriptor &desc, std::shared_mutex &lock) : fDescriptor(&desc), fLock(&lock) { - fLock.lock(); + fLock->lock(); } RExclDescriptorGuard(const RExclDescriptorGuard &) = delete; RExclDescriptorGuard &operator=(const RExclDescriptorGuard &) = delete; - RExclDescriptorGuard(RExclDescriptorGuard &&) = delete; - RExclDescriptorGuard &operator=(RExclDescriptorGuard &&) = delete; + RExclDescriptorGuard(RExclDescriptorGuard &&other) + { + std::swap(fDescriptor, other.fDescriptor); + std::swap(fLock, other.fLock); + } + RExclDescriptorGuard &operator=(RExclDescriptorGuard &&other) + { + if (this == &other) + return *this; + std::swap(fDescriptor, other.fDescriptor); + std::swap(fLock, other.fLock); + return *this; + } ~RExclDescriptorGuard() { - fDescriptor.IncGeneration(); - fLock.unlock(); + if (IsValid()) + Release(); + } + ROOT::RNTupleDescriptor &operator*() const { return *fDescriptor; } + ROOT::RNTupleDescriptor *operator->() const { return fDescriptor; } + void MoveIn(ROOT::RNTupleDescriptor desc) { *fDescriptor = std::move(desc); } + bool IsValid() const { return fLock; } + void Release() + { + fDescriptor->IncGeneration(); + fLock->unlock(); + fLock = nullptr; + fDescriptor = nullptr; + } + }; + + /// Either a shared or an exclusive lock guard + class RAnyDescriptorGuard { + private: + std::variant fAnyGuard; + + public: + RAnyDescriptorGuard(RSharedDescriptorGuard sharedGuard) : fAnyGuard(std::move(sharedGuard)) {} + RAnyDescriptorGuard(RExclDescriptorGuard exclGuard) : fAnyGuard(std::move(exclGuard)) {} + RAnyDescriptorGuard(const RAnyDescriptorGuard &) = delete; + RAnyDescriptorGuard &operator=(const RAnyDescriptorGuard &) = delete; + RAnyDescriptorGuard(RAnyDescriptorGuard &&) = default; + RAnyDescriptorGuard &operator=(RAnyDescriptorGuard &&) = default; + ~RAnyDescriptorGuard() = default; + + const ROOT::RNTupleDescriptor &GetConstRef() const + { + return (fAnyGuard.index() == 0) ? std::get<0>(fAnyGuard).GetConstRef() : std::get<1>(fAnyGuard).operator*(); + } + const ROOT::RNTupleDescriptor *operator->() const + { + return (fAnyGuard.index() == 0) ? std::get<0>(fAnyGuard).operator->() : std::get<1>(fAnyGuard).operator->(); + } + bool IsValid() const + { + return (fAnyGuard.index() == 0) ? std::get<0>(fAnyGuard).IsValid() : std::get<1>(fAnyGuard).IsValid(); + } + void Release() + { + if (fAnyGuard.index() == 0) + std::get<0>(fAnyGuard).Release(); + else + std::get<1>(fAnyGuard).Release(); } - ROOT::RNTupleDescriptor &operator*() const { return fDescriptor; } - ROOT::RNTupleDescriptor *operator->() const { return &fDescriptor; } - void MoveIn(ROOT::RNTupleDescriptor desc) { fDescriptor = std::move(desc); } }; private: @@ -696,6 +775,17 @@ private: bool fIsAttached = false; ///< Set to true once `Attach()` is called bool fHasStreamerInfosRegistered = false; ///< Set to true when RegisterStreamerInfos() is called. + /// The interpretation of page lists, set in Attach() + Internal::RNTupleSerializer::EDescriptorDeserializeMode fDeserializationMode = + Internal::RNTupleSerializer::EDescriptorDeserializeMode::kRaw; + + /// This vector is aligned with the cluster groups in the descriptor. It stores the cumulative number of clusters + /// in the cluster groups. Given a cluster ID, we can thus quickly determine the cluster group that the cluster + /// comes from. While descriptor IDs in a descriptor in general are arbitrary, for the descriptor in the page source, + /// that was created from a serialized on-disk representation, we know that cluster and cluster group IDs are + /// issued consecutively. + std::vector fCumulativeClusterCounts; + /// The active columns are implicitly defined by the model fields or views RActivePhysicalColumns fActivePhysicalColumns; @@ -719,11 +809,21 @@ private: /// Pages of pinned clusters won't be evicted from the page pool. std::unordered_set fPinnedClusters; + /// Ensures that fDescriptor has cluster details loaded for the given cluster group ID. This method is expected + /// to be called while holding the lock passed by descGuard. The returned descriptor guard is either the + /// passed one or a new, exclusive guard if cluster details needed to be loaded. + RAnyDescriptorGuard EnsureClusterDetails(DescriptorId_t cgId, RAnyDescriptorGuard descGuard); + /// Uses binary search in fCumulativeClusterCounts to determine the cluster group that the cluster ID belongs in. + DescriptorId_t FindClusterGroupId(DescriptorId_t clusterId) const; + /// Does nothing if fLastUsedCluster == clusterId. Otherwise, updated fLastUsedCluster /// and evict unused paged from the page pool of all previous clusters. /// Must not be called when the descriptor guard is taken. void UpdateLastUsedCluster(ROOT::DescriptorId_t clusterId); + // Populate the cluster details of the given cluster group. Must hold an exclusive descriptor lock when calling. + // No-op if the cluster details are already present. + void LoadPageList(DescriptorId_t clusterGroupId, const RExclDescriptorGuard &exclGuard); // Common treatment of zero pages in LoadPageFromSummary() ROOT::Internal::RPageRef LoadZeroPage(ColumnHandle_t columnHandle, const RPageSummary &pageSummary); // Once the page is found to be missing in the page cache and all information about the page is collected, @@ -857,18 +957,20 @@ public: /// Open the physical storage container and deserialize header and footer void Attach(ROOT::Internal::RNTupleSerializer::EDescriptorDeserializeMode mode = ROOT::Internal::RNTupleSerializer::EDescriptorDeserializeMode::kForReading); + /// Load all remaining cluster details + void LoadAllPageLists(); ROOT::NTupleSize_t GetNEntries(); ROOT::NTupleSize_t GetNElements(ROOT::DescriptorId_t physicalColumnId); /// Returns a shared descriptor guard to ensure that the returned cluster id is useable, i.e. that the /// corresponding cluster was not meanwhile evicted from the set of active clusters. - RSharedDescriptorGuard + RAnyDescriptorGuard FindClusterId(ROOT::DescriptorId_t physicalColumnId, ROOT::NTupleSize_t index, ROOT::DescriptorId_t &cid); /// An overload of FindClusterId that searches using a certain column element index. - RSharedDescriptorGuard FindClusterId(ROOT::NTupleSize_t entryIdx, ROOT::DescriptorId_t &cid); + RAnyDescriptorGuard FindClusterId(ROOT::NTupleSize_t entryIdx, ROOT::DescriptorId_t &cid); /// Uses FindClusterId to search for the cluster with the entry index following the last entry index of the /// given cluster. - RSharedDescriptorGuard FindNextClusterId(ROOT::DescriptorId_t clusterId, ROOT::DescriptorId_t &nextId); + RAnyDescriptorGuard FindNextClusterId(ROOT::DescriptorId_t clusterId, ROOT::DescriptorId_t &nextId); /// Promise to only read from the given entry range. If set, prevents the cluster pool from reading-ahead beyond /// the given range. The range needs to be within `[0, GetNEntries())`. diff --git a/tree/ntuple/inc/ROOT/RPageStorageDaos.hxx b/tree/ntuple/inc/ROOT/RPageStorageDaos.hxx index b5ff59bddd0e6..66b33b121d43d 100644 --- a/tree/ntuple/inc/ROOT/RPageStorageDaos.hxx +++ b/tree/ntuple/inc/ROOT/RPageStorageDaos.hxx @@ -26,6 +26,7 @@ #include #include #include +#include #include #include @@ -151,6 +152,8 @@ private: /// A container that stores object data (header/footer, pages, etc.) std::unique_ptr fDaosContainer; + /// Protects the DAOS container + std::mutex fContainerLock; /// A URI to a DAOS pool of the form 'daos://pool-label/container-label' std::string fURI; diff --git a/tree/ntuple/inc/ROOT/RPageStorageFile.hxx b/tree/ntuple/inc/ROOT/RPageStorageFile.hxx index d94feff68f1e7..38b4aa7905aa9 100644 --- a/tree/ntuple/inc/ROOT/RPageStorageFile.hxx +++ b/tree/ntuple/inc/ROOT/RPageStorageFile.hxx @@ -24,6 +24,7 @@ #include #include #include +#include #include #include #include @@ -139,6 +140,8 @@ private: RNTupleDescriptorBuilder fDescriptorBuilder; /// Tracks the last read offset for seek distance calculation std::uint64_t fLastOffset = 0; + /// Protects the raw file, the reader, and fLastOffset + std::mutex fReaderLock; /// File-specific I/O performance counters struct RFileCounters { @@ -160,6 +163,9 @@ private: std::unique_ptr PrepareSingleCluster(const ROOT::Internal::RCluster::RKey &clusterKey, std::vector &readRequests); + // Helper to calculate the distance between fLastOffset and offset and to update the skip counter accordingly + void UpdateSkipCounter(std::uint64_t offset); + protected: void LoadStructureImpl() final; ROOT::RNTupleDescriptor AttachImpl() final; diff --git a/tree/ntuple/src/RClusterPool.cxx b/tree/ntuple/src/RClusterPool.cxx index 4f64cd14a0e33..3b9a38f68e779 100644 --- a/tree/ntuple/src/RClusterPool.cxx +++ b/tree/ntuple/src/RClusterPool.cxx @@ -205,7 +205,7 @@ ROOT::Internal::RClusterPool::GetCluster(ROOT::DescriptorId_t clusterId, const R auto cid = next; auto descriptorGuard = fPageSource.FindNextClusterId(cid, next); - if (next != ROOT::kInvalidNTupleIndex) { + if (next != ROOT::kInvalidDescriptorId) { if (!fPageSource.GetEntryRange().IntersectsWith(descriptorGuard->GetClusterDescriptor(next))) next = ROOT::kInvalidDescriptorId; } diff --git a/tree/ntuple/src/RFieldBase.cxx b/tree/ntuple/src/RFieldBase.cxx index 05a5254c7d8d5..4762beeeceef8 100644 --- a/tree/ntuple/src/RFieldBase.cxx +++ b/tree/ntuple/src/RFieldBase.cxx @@ -1048,7 +1048,7 @@ void ROOT::RFieldBase::ConnectPageSource(ROOT::Internal::RPageSource &pageSource if (!fIsArtificial) { auto descGuard = pageSource.GetSharedDescriptorGuard(); - ReconcileOnDiskField(descGuard.GetRef()); + ReconcileOnDiskField(descGuard.GetConstRef()); } for (auto &f : fSubfields) { @@ -1061,7 +1061,7 @@ void ROOT::RFieldBase::ConnectPageSource(ROOT::Internal::RPageSource &pageSource // Do not generate columns nor set fColumnRepresentatives for artificial fields. if (!fIsArtificial) { const auto descriptorGuard = pageSource.GetSharedDescriptorGuard(); - const ROOT::RNTupleDescriptor &desc = descriptorGuard.GetRef(); + const ROOT::RNTupleDescriptor &desc = descriptorGuard.GetConstRef(); GenerateColumns(desc); if (fColumnRepresentatives.empty()) { // If we didn't get columns from the descriptor, ensure that we actually expect a field without columns diff --git a/tree/ntuple/src/RFieldMeta.cxx b/tree/ntuple/src/RFieldMeta.cxx index dd1d70955a31d..f5976f2807f18 100644 --- a/tree/ntuple/src/RFieldMeta.cxx +++ b/tree/ntuple/src/RFieldMeta.cxx @@ -520,7 +520,7 @@ std::unique_ptr ROOT::RClassField::BeforeConnectPageSource(ROO SetStagingClass(GetTypeName(), GetTypeVersion()); } else { const auto descriptorGuard = pageSource.GetSharedDescriptorGuard(); - const ROOT::RNTupleDescriptor &desc = descriptorGuard.GetRef(); + const ROOT::RNTupleDescriptor &desc = descriptorGuard.GetConstRef(); const auto &fieldDesc = desc.GetFieldDescriptor(GetOnDiskId()); if (fieldDesc.GetStructure() == ENTupleStructure::kStreamer) { @@ -608,12 +608,13 @@ std::unique_ptr ROOT::RClassField::BeforeConnectPageSource(ROO } if (nInMemoryBaseClasses != 0 && nOnDiskBaseClasses != 0 && nInMemoryBaseClasses != nOnDiskBaseClasses) { - throw RException(R__FAIL(std::string("incompatible number of base classes for field ") + GetFieldName() + ": " + - GetTypeName() + ", " + std::to_string(nInMemoryBaseClasses) + - " base classes in memory " - " vs. " + - std::to_string(nOnDiskBaseClasses) + " base classes on-disk\n" + - Internal::GetTypeTraceReport(*this, pageSource.GetSharedDescriptorGuard().GetRef()))); + throw RException( + R__FAIL(std::string("incompatible number of base classes for field ") + GetFieldName() + ": " + GetTypeName() + + ", " + std::to_string(nInMemoryBaseClasses) + + " base classes in memory " + " vs. " + + std::to_string(nOnDiskBaseClasses) + " base classes on-disk\n" + + Internal::GetTypeTraceReport(*this, pageSource.GetSharedDescriptorGuard().GetConstRef()))); } return nullptr; diff --git a/tree/ntuple/src/RNTupleJoinTable.cxx b/tree/ntuple/src/RNTupleJoinTable.cxx index eacfa45d76578..d531266893455 100644 --- a/tree/ntuple/src/RNTupleJoinTable.cxx +++ b/tree/ntuple/src/RNTupleJoinTable.cxx @@ -24,32 +24,36 @@ ROOT::Experimental::Internal::RNTupleJoinTable::REntryMapping::REntryMapping( "std::uint32_t", "std::uint64_t"}; pageSource.Attach(); - auto desc = pageSource.GetSharedDescriptorGuard(); - std::vector> fields; std::vector fieldValues; fieldValues.reserve(fJoinFieldNames.size()); + { + auto descGuard = pageSource.GetSharedDescriptorGuard(); + + for (const auto &fieldName : fJoinFieldNames) { + auto fieldId = descGuard->FindFieldId(fieldName); + if (fieldId == ROOT::kInvalidDescriptorId) { + throw RException(R__FAIL("could not find join field \"" + std::string(fieldName) + "\" in RNTuple \"" + + descGuard->GetName() + "\"")); + } - for (const auto &fieldName : fJoinFieldNames) { - auto fieldId = desc->FindFieldId(fieldName); - if (fieldId == ROOT::kInvalidDescriptorId) - throw RException(R__FAIL("could not find join field \"" + std::string(fieldName) + "\" in RNTuple \"" + - pageSource.GetNTupleName() + "\"")); - - const auto &fieldDesc = desc->GetFieldDescriptor(fieldId); + const auto &fieldDesc = descGuard->GetFieldDescriptor(fieldId); - if (allowedTypes.find(fieldDesc.GetTypeName()) == allowedTypes.end()) { - throw RException(R__FAIL("cannot use field \"" + fieldName + "\" with type \"" + fieldDesc.GetTypeName() + - "\" in join table: only integral types are allowed")); - } + if (allowedTypes.find(fieldDesc.GetTypeName()) == allowedTypes.end()) { + throw RException(R__FAIL("cannot use field \"" + fieldName + "\" with type \"" + fieldDesc.GetTypeName() + + "\" in join table: only integral types are allowed")); + } - auto field = std::make_unique>(fieldDesc.GetFieldName()); - field->SetOnDiskId(fieldDesc.GetId()); - ROOT::Internal::CallConnectPageSourceOnField(*field, pageSource); + auto field = std::make_unique>(fieldDesc.GetFieldName()); + field->SetOnDiskId(fieldDesc.GetId()); - fieldValues.emplace_back(field->CreateValue()); - fJoinFieldValueSizes.emplace_back(field->GetValueSize()); - fields.emplace_back(std::move(field)); + fieldValues.emplace_back(field->CreateValue()); + fJoinFieldValueSizes.emplace_back(field->GetValueSize()); + fields.emplace_back(std::move(field)); + } + } // descGuard + for (auto &f : fields) { + ROOT::Internal::CallConnectPageSourceOnField(*f, pageSource); } std::vector castJoinValues; diff --git a/tree/ntuple/src/RNTupleMerger.cxx b/tree/ntuple/src/RNTupleMerger.cxx index b44247301b4af..e666513c8dbf6 100644 --- a/tree/ntuple/src/RNTupleMerger.cxx +++ b/tree/ntuple/src/RNTupleMerger.cxx @@ -194,9 +194,9 @@ try { // Get the compression of this RNTuple and use it as the output compression. // We currently assume all column ranges have the same compression, so we just peek at the first one. source->Attach(RNTupleSerializer::EDescriptorDeserializeMode::kRaw); - auto descGuard = source->GetSharedDescriptorGuard(); - auto clusterGroupIterable = descGuard->GetClusterGroupIterable(); - if (clusterGroupIterable.empty()) { + DescriptorId_t firstClusterId = kInvalidDescriptorId; + auto descGuard = source->FindClusterId(0, firstClusterId); + if (firstClusterId == kInvalidDescriptorId) { R__LOG_ERROR(NTupleMergeLog()) << "Asked to use the first source's compression as the output compression, but the " "first source (file '" @@ -205,9 +205,7 @@ try { "determined."; return -1; } - const auto firstClusterGroup = clusterGroupIterable.begin(); - R__ASSERT(firstClusterGroup->HasClusterDetails()); - const auto &firstCluster = descGuard->GetClusterDescriptor(firstClusterGroup->GetClusterIds()[0]); + const auto &firstCluster = descGuard->GetClusterDescriptor(firstClusterId); auto colRangeIter = firstCluster.GetColumnRangeIterable(); auto firstColRange = colRangeIter.begin(); if (firstColRange == colRangeIter.end()) { @@ -234,8 +232,9 @@ try { if (outNTuple) { auto outSource = RPageSourceFile::CreateFromAnchor(*outNTuple); outSource->Attach(RNTupleSerializer::EDescriptorDeserializeMode::kForWriting); + outSource->LoadAllPageLists(); auto desc = outSource->GetSharedDescriptorGuard(); - model = destination->InitFromDescriptor(desc.GetRef(), true /* copyClusters */); + model = destination->InitFromDescriptor(desc.GetConstRef(), true /* copyClusters */); } // Interface conversion @@ -997,7 +996,8 @@ class RNTupleMergeStrategyDefault final : public RNTupleMergeStrategy { void InitSource(RPageSource &source) final { source.Attach(RNTupleSerializer::EDescriptorDeserializeMode::kForWriting); - fSrcDescriptor = &source.GetSharedDescriptorGuard().GetRef(); + source.LoadAllPageLists(); + fSrcDescriptor = &source.GetSharedDescriptorGuard().GetConstRef(); if (!fDstModel) { constexpr bool copyClusters = false; diff --git a/tree/ntuple/src/RNTupleProcessor.cxx b/tree/ntuple/src/RNTupleProcessor.cxx index 08d157a253bc6..a3540cb9d024e 100644 --- a/tree/ntuple/src/RNTupleProcessor.cxx +++ b/tree/ntuple/src/RNTupleProcessor.cxx @@ -163,7 +163,7 @@ ROOT::Experimental::RNTupleSingleProcessor::CreateAndConnectField(const std::str qualifiedFieldName.find("R_rntproc_join_") == 0 ? qualifiedFieldName.substr(15) : qualifiedFieldName; auto descGuard = fPageSource->GetSharedDescriptorGuard(); - const auto &desc = descGuard.GetRef(); + const auto &desc = descGuard.GetConstRef(); const auto onDiskFieldId = desc.FindFieldId(onDiskFieldName); @@ -258,7 +258,7 @@ void ROOT::Experimental::RNTupleSingleProcessor::AddAllFieldsToEntry( { Initialize(); auto descGuard = fPageSource->GetSharedDescriptorGuard(); - const auto &desc = descGuard.GetRef(); + const auto &desc = descGuard.GetConstRef(); auto fnAddSubfields = [this, &desc, &provenance](const ROOT::RFieldDescriptor &field, auto &fn) -> void { std::string fieldName = desc.GetQualifiedFieldName(field.GetId()); if (auto prefix = provenance.Get(); !prefix.empty()) diff --git a/tree/ntuple/src/RNTupleReader.cxx b/tree/ntuple/src/RNTupleReader.cxx index 960f1e83a0478..7960749a21055 100644 --- a/tree/ntuple/src/RNTupleReader.cxx +++ b/tree/ntuple/src/RNTupleReader.cxx @@ -282,7 +282,10 @@ void ROOT::RNTupleReader::PrintInfo(const ENTupleInfo what, std::ostream &output output << std::flush; break; } - case ENTupleInfo::kStorageDetails: fSource->GetSharedDescriptorGuard()->PrintInfo(output); break; + case ENTupleInfo::kStorageDetails: + fSource->LoadAllPageLists(); + fSource->GetSharedDescriptorGuard()->PrintInfo(output); + break; case ENTupleInfo::kMetrics: fMetrics.Print(output); break; default: // Unhandled case, internal error diff --git a/tree/ntuple/src/RNTupleView.cxx b/tree/ntuple/src/RNTupleView.cxx index b9694e7bf97f7..687b3c686c10a 100644 --- a/tree/ntuple/src/RNTupleView.cxx +++ b/tree/ntuple/src/RNTupleView.cxx @@ -26,7 +26,7 @@ ROOT::Internal::GetFieldRange(const ROOT::RFieldBase &field, ROOT::Internal::RPa { auto descGuard = pageSource.GetSharedDescriptorGuard(); - const auto &desc = descGuard.GetRef(); + const auto &desc = descGuard.GetConstRef(); auto parentId = desc.GetFieldDescriptor(field.GetOnDiskId()).GetParentId(); while (parentId != desc.GetFieldZeroId()) { diff --git a/tree/ntuple/src/RPageStorage.cxx b/tree/ntuple/src/RPageStorage.cxx index ef2aba2f034ab..caf2ef81650cc 100644 --- a/tree/ntuple/src/RPageStorage.cxx +++ b/tree/ntuple/src/RPageStorage.cxx @@ -238,25 +238,52 @@ void ROOT::Internal::RPageSource::Attach(RNTupleSerializer::EDescriptorDeseriali if (fIsAttached) return; + fDeserializationMode = mode; + LoadStructure(); + { + auto descGuard = GetExclDescriptorGuard(); + descGuard.MoveIn(AttachImpl()); + fStructureBuffer.Reset(); + + NTupleSize_t sumClusterCount = 0; + fCumulativeClusterCounts.reserve(descGuard->GetNClusterGroups()); + for (const auto &cgDesc : descGuard->GetClusterGroupIterable()) { + sumClusterCount += cgDesc.GetNClusters(); + fCumulativeClusterCounts.emplace_back(sumClusterCount); + } + } + + if (fOptions.GetMetadataMode() == RNTupleReadOptions::EMetadataMode::kEager) + LoadAllPageLists(); + + fIsAttached = true; +} + +void ROOT::Internal::RPageSource::LoadAllPageLists() +{ auto descGuard = GetExclDescriptorGuard(); - descGuard.MoveIn(AttachImpl()); - fStructureBuffer.Reset(); - std::vector buffer; for (const auto &cgDesc : descGuard->GetClusterGroupIterable()) { - buffer.resize(cgDesc.GetPageListLength() + cgDesc.GetPageListLocator().GetNBytesOnStorage()); - auto zipBuffer = buffer.data() + cgDesc.GetPageListLength(); - - LoadPageListImpl(cgDesc.GetPageListLocator(), zipBuffer); - RNTupleDecompressor::Unzip(zipBuffer, cgDesc.GetPageListLocator().GetNBytesOnStorage(), - cgDesc.GetPageListLength(), buffer.data()); - RNTupleSerializer::DeserializePageList(buffer.data(), cgDesc.GetPageListLength(), cgDesc.GetId(), *descGuard, - mode); + LoadPageList(cgDesc.GetId(), descGuard); } +} - fIsAttached = true; +void ROOT::Internal::RPageSource::LoadPageList(DescriptorId_t clusterGroupId, const RExclDescriptorGuard &exclGuard) +{ + const auto &cgDesc = exclGuard->GetClusterGroupDescriptor(clusterGroupId); + if (cgDesc.HasClusterDetails()) + return; + + auto buffer = + MakeUninitArray(cgDesc.GetPageListLength() + cgDesc.GetPageListLocator().GetNBytesOnStorage()); + auto zipBuffer = buffer.get() + cgDesc.GetPageListLength(); + LoadPageListImpl(cgDesc.GetPageListLocator(), zipBuffer); + RNTupleDecompressor::Unzip(zipBuffer, cgDesc.GetPageListLocator().GetNBytesOnStorage(), cgDesc.GetPageListLength(), + buffer.get()); + RNTupleSerializer::DeserializePageList(buffer.get(), cgDesc.GetPageListLength(), cgDesc.GetId(), *exclGuard, + fDeserializationMode); } std::unique_ptr ROOT::Internal::RPageSource::Clone() const @@ -266,10 +293,44 @@ std::unique_ptr ROOT::Internal::RPageSource::Clone( clone->GetExclDescriptorGuard().MoveIn(GetSharedDescriptorGuard()->Clone()); clone->fHasStructure = true; clone->fIsAttached = true; + clone->fDeserializationMode = fDeserializationMode; + clone->fCumulativeClusterCounts = fCumulativeClusterCounts; } return clone; } +ROOT::Internal::RPageSource::RAnyDescriptorGuard +ROOT::Internal::RPageSource::EnsureClusterDetails(DescriptorId_t cgId, RAnyDescriptorGuard descGuard) +{ + assert(descGuard.IsValid()); + if ((cgId == kInvalidDescriptorId) || descGuard->GetClusterGroupDescriptor(cgId).HasClusterDetails()) + return descGuard; + + descGuard.Release(); + + auto exclGuard = GetExclDescriptorGuard(); + + if (exclGuard->GetClusterGroupDescriptor(cgId).HasClusterDetails()) { + // unlikely, but between releasing the shared guard and acquiring the exclusive guard, the cluster details + // may have appeared + return exclGuard; + } + + LoadPageList(cgId, exclGuard); + + return exclGuard; +} + +ROOT::DescriptorId_t ROOT::Internal::RPageSource::FindClusterGroupId(DescriptorId_t clusterId) const +{ + // We use `cluster + 1` because fCumulativeClusterCounts stores the cumulative _number_ of clusters and not + // the last cluster IDs. + auto iter = std::lower_bound(fCumulativeClusterCounts.begin(), fCumulativeClusterCounts.end(), clusterId + 1); + if (iter == fCumulativeClusterCounts.end()) + return kInvalidDescriptorId; + return std::distance(fCumulativeClusterCounts.begin(), iter); +} + ROOT::NTupleSize_t ROOT::Internal::RPageSource::GetNEntries() { return GetSharedDescriptorGuard()->GetNEntries(); @@ -277,46 +338,51 @@ ROOT::NTupleSize_t ROOT::Internal::RPageSource::GetNEntries() ROOT::NTupleSize_t ROOT::Internal::RPageSource::GetNElements(ROOT::DescriptorId_t physicalColumnId) { - auto descGuard = GetSharedDescriptorGuard(); - if (descGuard->GetNClusters() == 0) + auto sharedGuard = GetSharedDescriptorGuard(); + if (sharedGuard->GetNClusters() == 0) return 0; - auto itr = descGuard->GetClusterGroupIterable().begin(); - itr += descGuard->GetNClusterGroups() - 1; - R__ASSERT(itr->HasClusterDetails()); - const auto &cd = descGuard->GetClusterDescriptor(itr->GetClusterIds().back()); + const auto lastClusterGroupId = sharedGuard->GetNClusterGroups() - 1; + auto descGuard = EnsureClusterDetails(lastClusterGroupId, std::move(sharedGuard)); + + const auto &cgDesc = descGuard->GetClusterGroupDescriptor(lastClusterGroupId); + const auto &cd = descGuard->GetClusterDescriptor(cgDesc.GetClusterIds().back()); R__ASSERT(cd.ContainsColumn(physicalColumnId)); + const auto &columnRange = cd.GetColumnRange(physicalColumnId); return columnRange.GetFirstElementIndex() + columnRange.GetNElements(); } -ROOT::Internal::RPageSource::RSharedDescriptorGuard +ROOT::Internal::RPageSource::RAnyDescriptorGuard ROOT::Internal::RPageSource::FindNextClusterId(ROOT::DescriptorId_t clusterId, ROOT::DescriptorId_t &nextId) { - NTupleSize_t firstEntryInNextCluster = kInvalidNTupleIndex; - { - auto descriptorGuard = GetSharedDescriptorGuard(); - const auto &clusterDesc = descriptorGuard->GetClusterDescriptor(clusterId); - firstEntryInNextCluster = clusterDesc.GetFirstEntryIndex() + clusterDesc.GetNEntries(); + // At the cost of an additional cluster descriptor lock, we could use a more general approach that looks for the + // cluster containing the entry index after the last entry of the current cluster. + // At the moment, however, we know that cluster IDs are consecutive and clusters are not sharded. + auto sharedGuard = GetSharedDescriptorGuard(); + assert(clusterId < sharedGuard->GetNClusters()); + nextId = clusterId + 1; + if (nextId == sharedGuard->GetNClusters()) { + nextId = kInvalidDescriptorId; + return sharedGuard; } - return FindClusterId(firstEntryInNextCluster, nextId); + return EnsureClusterDetails(FindClusterGroupId(nextId), std::move(sharedGuard)); } -ROOT::Internal::RPageSource::RSharedDescriptorGuard +ROOT::Internal::RPageSource::RAnyDescriptorGuard ROOT::Internal::RPageSource::FindClusterId(ROOT::NTupleSize_t entryIdx, ROOT::DescriptorId_t &cid) { cid = ROOT::kInvalidDescriptorId; - auto descGuard = GetSharedDescriptorGuard(); - const auto &desc = descGuard.GetRef(); + RAnyDescriptorGuard descGuard = GetSharedDescriptorGuard(); - if (desc.GetNClusterGroups() == 0) + if (descGuard->GetNClusterGroups() == 0) return descGuard; // Binary search in the cluster group list, followed by a binary search in the clusters of that cluster group - auto cgIter = desc.GetClusterGroupIterable().begin(); + const auto cgIter = descGuard->GetClusterGroupIterable().begin(); std::size_t cgLeft = 0; - std::size_t cgRight = desc.GetNClusterGroups() - 1; + std::size_t cgRight = descGuard->GetNClusterGroups() - 1; while (cgLeft <= cgRight) { const std::size_t cgMidpoint = (cgLeft + cgRight) / 2; const auto &cgDesc = *(cgIter + cgMidpoint); @@ -334,13 +400,15 @@ ROOT::Internal::RPageSource::FindClusterId(ROOT::NTupleSize_t entryIdx, ROOT::De // Binary search in the current cluster group; since we already checked the element range boundaries, // the element must be in that cluster group. - const auto &clusterIds = cgDesc.GetClusterIds(); - R__ASSERT(!clusterIds.empty()); + const auto tgtClusterGroupId = cgDesc.GetId(); + descGuard = EnsureClusterDetails(tgtClusterGroupId, std::move(descGuard)); + + const auto &clusterIds = descGuard->GetClusterGroupDescriptor(tgtClusterGroupId).GetClusterIds(); std::size_t clusterLeft = 0; std::size_t clusterRight = clusterIds.size() - 1; while (clusterLeft <= clusterRight) { const std::size_t clusterMidpoint = (clusterLeft + clusterRight) / 2; - const auto &clusterDesc = desc.GetClusterDescriptor(clusterIds[clusterMidpoint]); + const auto &clusterDesc = descGuard->GetClusterDescriptor(clusterIds[clusterMidpoint]); if (clusterDesc.GetFirstEntryIndex() > entryIdx) { R__ASSERT(clusterMidpoint > 0); @@ -361,40 +429,37 @@ ROOT::Internal::RPageSource::FindClusterId(ROOT::NTupleSize_t entryIdx, ROOT::De return descGuard; } -ROOT::Internal::RPageSource::RSharedDescriptorGuard +ROOT::Internal::RPageSource::RAnyDescriptorGuard ROOT::Internal::RPageSource::FindClusterId(DescriptorId_t physicalColumnId, NTupleSize_t index, DescriptorId_t &cid) { cid = ROOT::kInvalidDescriptorId; - auto descGuard = GetSharedDescriptorGuard(); - const auto &desc = descGuard.GetRef(); + RAnyDescriptorGuard descGuard = GetSharedDescriptorGuard(); - if (desc.GetNClusterGroups() == 0) + if (descGuard->GetNClusterGroups() == 0) return descGuard; // Binary search in the cluster group list, followed by a binary search in the clusters of that cluster group - auto cgIter = desc.GetClusterGroupIterable().begin(); std::size_t cgLeft = 0; - std::size_t cgRight = desc.GetNClusterGroups() - 1; + std::size_t cgRight = descGuard->GetNClusterGroups() - 1; while (cgLeft <= cgRight) { const std::size_t cgMidpoint = (cgLeft + cgRight) / 2; + descGuard = EnsureClusterDetails(cgMidpoint, std::move(descGuard)); + + const auto cgIter = descGuard->GetClusterGroupIterable().begin(); const auto &clusterIds = (cgIter + cgMidpoint)->GetClusterIds(); R__ASSERT(!clusterIds.empty()); - const auto &clusterDesc = desc.GetClusterDescriptor(clusterIds.front()); - // this may happen if the RNTuple has an empty schema - if (!clusterDesc.ContainsColumn(physicalColumnId)) - return descGuard; - - const auto firstElementInGroup = clusterDesc.GetColumnRange(physicalColumnId).GetFirstElementIndex(); - if (firstElementInGroup > index) { + const auto &firstColumnRange = + descGuard->GetClusterDescriptor(clusterIds.front()).GetColumnRange(physicalColumnId); + if (firstColumnRange.GetFirstElementIndex() > index) { // Look into the lower half of cluster groups R__ASSERT(cgMidpoint > 0); cgRight = cgMidpoint - 1; continue; } - const auto &lastColumnRange = desc.GetClusterDescriptor(clusterIds.back()).GetColumnRange(physicalColumnId); + const auto &lastColumnRange = descGuard->GetClusterDescriptor(clusterIds.back()).GetColumnRange(physicalColumnId); if ((lastColumnRange.GetFirstElementIndex() + lastColumnRange.GetNElements()) <= index) { // Look into the upper half of cluster groups cgLeft = cgMidpoint + 1; @@ -408,7 +473,7 @@ ROOT::Internal::RPageSource::FindClusterId(DescriptorId_t physicalColumnId, NTup while (clusterLeft <= clusterRight) { const std::size_t clusterMidpoint = (clusterLeft + clusterRight) / 2; const auto clusterId = clusterIds[clusterMidpoint]; - const auto &columnRange = desc.GetClusterDescriptor(clusterId).GetColumnRange(physicalColumnId); + const auto &columnRange = descGuard->GetClusterDescriptor(clusterId).GetColumnRange(physicalColumnId); if (columnRange.Contains(index)) { cid = clusterId; @@ -439,7 +504,7 @@ void ROOT::Internal::RPageSource::UnzipCluster(RCluster *cluster) RNTupleAtomicTimer timer(fCounters->fTimeWallUnzip, fCounters->fTimeCpuUnzip); const auto clusterId = cluster->GetId(); - auto descriptorGuard = GetSharedDescriptorGuard(); + auto descriptorGuard = EnsureClusterDetails(FindClusterGroupId(clusterId), GetSharedDescriptorGuard()); const auto &clusterDescriptor = descriptorGuard->GetClusterDescriptor(clusterId); fPreloadedClusters[clusterDescriptor.GetFirstEntryIndex()] = clusterId; @@ -534,7 +599,7 @@ void ROOT::Internal::RPageSource::PrepareLoadCluster( const std::function &perPageFunc) { - auto descriptorGuard = GetSharedDescriptorGuard(); + auto descriptorGuard = EnsureClusterDetails(FindClusterGroupId(clusterKey.fClusterId), GetSharedDescriptorGuard()); const auto &clusterDesc = descriptorGuard->GetClusterDescriptor(clusterKey.fClusterId); for (auto physicalColumnId : clusterKey.fPhysicalColumnSet) { @@ -561,8 +626,11 @@ void ROOT::Internal::RPageSource::UpdateLastUsedCluster(ROOT::DescriptorId_t clu if (fLastUsedCluster == clusterId) return; - ROOT::NTupleSize_t firstEntryIndex = - GetSharedDescriptorGuard()->GetClusterDescriptor(clusterId).GetFirstEntryIndex(); + ROOT::NTupleSize_t firstEntryIndex = kInvalidNTupleIndex; + { + auto descGuard = EnsureClusterDetails(FindClusterGroupId(clusterId), GetSharedDescriptorGuard()); + firstEntryIndex = descGuard->GetClusterDescriptor(clusterId).GetFirstEntryIndex(); + } auto itr = fPreloadedClusters.begin(); while ((itr != fPreloadedClusters.end()) && (itr->first < firstEntryIndex)) { if (fPinnedClusters.count(itr->second) > 0) { @@ -597,7 +665,7 @@ void ROOT::Internal::RPageSource::LoadSealedPage(ROOT::DescriptorId_t physicalCo ROOT::RClusterDescriptor::RPageInfo pageInfo; { - auto descriptorGuard = GetSharedDescriptorGuard(); + auto descriptorGuard = EnsureClusterDetails(FindClusterGroupId(clusterId), GetSharedDescriptorGuard()); const auto &clusterDescriptor = descriptorGuard->GetClusterDescriptor(clusterId); pageInfo = clusterDescriptor.GetPageRange(physicalColumnId).Find(localIndex.GetIndexInCluster()); } @@ -747,7 +815,7 @@ ROOT::Internal::RPageSource::LoadPage(ColumnHandle_t columnHandle, RNTupleLocalI RPageSummary pageSummary; { - auto descriptorGuard = GetSharedDescriptorGuard(); + auto descriptorGuard = EnsureClusterDetails(FindClusterGroupId(clusterId), GetSharedDescriptorGuard()); const auto &clusterDescriptor = descriptorGuard->GetClusterDescriptor(clusterId); const auto &columnRange = clusterDescriptor.GetColumnRange(columnId); if (columnRange.IsSuppressed()) diff --git a/tree/ntuple/src/RPageStorageDaos.cxx b/tree/ntuple/src/RPageStorageDaos.cxx index 30329c778e1b8..c619bfa1ede52 100644 --- a/tree/ntuple/src/RPageStorageDaos.cxx +++ b/tree/ntuple/src/RPageStorageDaos.cxx @@ -35,6 +35,7 @@ #include #include #include +#include #include #include #include @@ -481,6 +482,7 @@ ROOT::RNTupleDescriptor ROOT::Experimental::Internal::RPageSourceDaos::AttachImp void ROOT::Experimental::Internal::RPageSourceDaos::LoadPageListImpl(const RNTupleLocator &locator, unsigned char *buffer) { + std::lock_guard lockGuard(fContainerLock); daos_obj_id_t oidPageList{kOidLowPageList, static_cast(fNTupleIndex)}; fDaosContainer->ReadSingleAkey(buffer, locator.GetNBytesOnStorage(), oidPageList, kDistributionKeyDefault, locator.GetPosition().GetLocation()); @@ -494,6 +496,7 @@ std::string ROOT::Experimental::Internal::RPageSourceDaos::GetObjectClass() cons void ROOT::Experimental::Internal::RPageSourceDaos::LoadSealedPageImpl(const RNTupleLocator &locator, RSealedPage &sealedPage) { + std::lock_guard lockGuard(fContainerLock); RDaosKey daosKey = GetPageDaosKey(fNTupleIndex, locator.GetPosition().GetLocation()); fDaosContainer->ReadSingleAkey(const_cast(sealedPage.GetBuffer()), sealedPage.GetBufferSize(), daosKey.fOid, daosKey.fDkey, daosKey.fAkey); @@ -587,6 +590,7 @@ ROOT::Experimental::Internal::RPageSourceDaos::LoadClusters(std::spanfTimeWallRead, fCounters->fTimeCpuRead); + std::lock_guard lockGuard(fContainerLock); if (int err = fDaosContainer->ReadV(readRequests)) throw ROOT::RException(R__FAIL("ReadV: error" + std::string(d_errstr(err)))); } diff --git a/tree/ntuple/src/RPageStorageFile.cxx b/tree/ntuple/src/RPageStorageFile.cxx index 5a6749b0d953a..7fdb8f4a400eb 100644 --- a/tree/ntuple/src/RPageStorageFile.cxx +++ b/tree/ntuple/src/RPageStorageFile.cxx @@ -34,12 +34,11 @@ #include #include #include +#include #include #include -#include - -#include #include +#include using ROOT::Experimental::Detail::RNTupleAtomicCounter; using ROOT::Experimental::Detail::RNTupleAtomicTimer; @@ -491,24 +490,35 @@ ROOT::RNTupleDescriptor ROOT::Internal::RPageSourceFile::AttachImpl() return fDescriptorBuilder.MoveDescriptor(); } +void ROOT::Internal::RPageSourceFile::UpdateSkipCounter(std::uint64_t offset) +{ + // Track seek distance (excluding file structure reads) + if (fLastOffset == 0) + return; + + assert(fFileCounters); + const auto distance = + static_cast(std::abs(static_cast(offset) - static_cast(fLastOffset))); + fFileCounters->fSzSkip.Add(distance); +} + void ROOT::Internal::RPageSourceFile::LoadPageListImpl(const RNTupleLocator &locator, unsigned char *buffer) { - fReader.ReadBuffer(buffer, locator.GetNBytesOnStorage(), locator.GetPosition()); + RNTupleAtomicTimer timer(fCounters->fTimeWallRead, fCounters->fTimeCpuRead); + std::lock_guard lockGuard(fReaderLock); + const auto offset = locator.GetPosition(); + UpdateSkipCounter(offset); + fReader.ReadBuffer(buffer, locator.GetNBytesOnStorage(), offset); + fLastOffset = offset + locator.GetNBytesOnStorage(); } void ROOT::Internal::RPageSourceFile::LoadSealedPageImpl(const RNTupleLocator &locator, RSealedPage &sealedPage) { RNTupleAtomicTimer timer(fCounters->fTimeWallRead, fCounters->fTimeCpuRead); + std::lock_guard lockGuard(fReaderLock); const auto offset = locator.GetPosition(); - // Track seek distance (excluding file structure reads) - if (fLastOffset != 0) { - R__ASSERT(fFileCounters); - const auto distance = static_cast( - std::abs(static_cast(offset) - static_cast(fLastOffset))); - fFileCounters->fSzSkip.Add(distance); - } - fReader.ReadBuffer(const_cast(sealedPage.GetBuffer()), sealedPage.GetBufferSize(), - locator.GetPosition()); + UpdateSkipCounter(offset); + fReader.ReadBuffer(const_cast(sealedPage.GetBuffer()), sealedPage.GetBufferSize(), offset); fLastOffset = offset + sealedPage.GetBufferSize(); } @@ -683,15 +693,12 @@ ROOT::Internal::RPageSourceFile::LoadClusters(std::span clusterK } } + std::lock_guard lockGuard(fReaderLock); + // Track seek distance for each read request (excluding file structure reads) - R__ASSERT(fFileCounters); for (std::size_t i = 0; i < nBatch; ++i) { const auto offset = readRequests[iReq + i].fOffset; - if (fLastOffset != 0) { - const auto distance = static_cast(std::abs( - static_cast(offset) - static_cast(fLastOffset))); - fFileCounters->fSzSkip.Add(distance); - } + UpdateSkipCounter(offset); fLastOffset = offset + readRequests[iReq + i].fSize; } diff --git a/tree/ntuple/test/ntuple_basics.cxx b/tree/ntuple/test/ntuple_basics.cxx index 1737711744d38..5b50092f87f82 100644 --- a/tree/ntuple/test/ntuple_basics.cxx +++ b/tree/ntuple/test/ntuple_basics.cxx @@ -446,7 +446,9 @@ TEST(RNTuple, PageSize) } } - auto ntuple = RNTupleReader::Open("ntuple", fileGuard.GetPath()); + RNTupleReadOptions opts; + opts.SetMetadataMode(RNTupleReadOptions::EMetadataMode::kEager); + auto ntuple = RNTupleReader::Open("ntuple", fileGuard.GetPath(), opts); const auto &col0_pages = ntuple->GetDescriptor().GetClusterDescriptor(0).GetPageRange(0); // 1000 column elements / 50 elements per page EXPECT_EQ(20, col0_pages.GetPageInfos().size()); diff --git a/tree/ntuple/test/ntuple_checksum.cxx b/tree/ntuple/test/ntuple_checksum.cxx index a650fc6e9d302..9bf065cdf2aa7 100644 --- a/tree/ntuple/test/ntuple_checksum.cxx +++ b/tree/ntuple/test/ntuple_checksum.cxx @@ -61,7 +61,9 @@ TEST(RNTupleChecksum, VerifyOnLoad) ROOT::DescriptorId_t pyColId; ROOT::DescriptorId_t pzColId; ROOT::DescriptorId_t clusterId; - auto pageSource = RPageSource::Create("ntpl", fileGuard.GetPath()); + RNTupleReadOptions opts; + opts.SetMetadataMode(RNTupleReadOptions::EMetadataMode::kEager); + auto pageSource = RPageSource::Create("ntpl", fileGuard.GetPath(), opts); pageSource->Attach(); { auto descGuard = pageSource->GetSharedDescriptorGuard(); @@ -97,7 +99,9 @@ TEST(RNTupleChecksum, OmitPageChecksum) writer->Fill(); writer.reset(); - auto pageSource = RPageSource::Create("ntpl", fileGuard.GetPath()); + RNTupleReadOptions opts; + opts.SetMetadataMode(RNTupleReadOptions::EMetadataMode::kEager); + auto pageSource = RPageSource::Create("ntpl", fileGuard.GetPath(), opts); pageSource->Attach(); auto descGuard = pageSource->GetSharedDescriptorGuard(); const auto pxColId = descGuard->FindPhysicalColumnId(descGuard->FindFieldId("px"), 0, 0); diff --git a/tree/ntuple/test/ntuple_cluster.cxx b/tree/ntuple/test/ntuple_cluster.cxx index 6121a09d61e1e..30b223f384186 100644 --- a/tree/ntuple/test/ntuple_cluster.cxx +++ b/tree/ntuple/test/ntuple_cluster.cxx @@ -37,29 +37,27 @@ namespace { * Used to track LoadClusters calls triggered by ClusterPool::GetCluster */ class RPageSourceMock : public RPageSource { + std::uint32_t fSzPageList = 0; + std::unique_ptr fBufPageList; + protected: void LoadStructureImpl() final {} - RNTupleDescriptor AttachImpl() final { return RNTupleDescriptor(); } - std::unique_ptr CloneImpl() const final { return nullptr; } - void LoadPageListImpl(const ROOT::RNTupleLocator &, unsigned char *) final {} - void LoadSealedPageImpl(const ROOT::RNTupleLocator &, RSealedPage &) final {} - std::unique_ptr - OpenWithDifferentAnchor(const ROOT::Internal::RNTupleLink &, const ROOT::RNTupleReadOptions &) final - { - throw ROOT::RException(R__FAIL("method not implemented")); - } - -public: - /// Records the cluster IDs requests by LoadClusters() calls - std::vector fReqsClusterIds; - std::vector fReqsColumns; - - RPageSourceMock() : RPageSource("test", RNTupleReadOptions()) + RNTupleDescriptor AttachImpl() final { ROOT::Internal::RNTupleDescriptorBuilder descBuilder; // Should set the version from the anchor; here it is fine to use the same version as for writing. descBuilder.SetVersionForWriting(); descBuilder.SetNTuple("ntpl", ""); + descBuilder.AddField(ROOT::Internal::RFieldDescriptorBuilder() + .FieldId(0) + .FieldName("") + .Structure(ROOT::ENTupleStructure::kRecord) + .MoveDescriptor() + .Unwrap()); + + auto ctx = RNTupleSerializer::SerializeHeader(nullptr, descBuilder.GetDescriptor()).Unwrap(); + + std::vector physClusterIDs; for (unsigned i = 0; i <= 5; ++i) { descBuilder.AddCluster(ROOT::Internal::RClusterDescriptorBuilder() .ClusterId(i) @@ -67,14 +65,45 @@ class RPageSourceMock : public RPageSource { .NEntries(1) .MoveDescriptor() .Unwrap()); + physClusterIDs.emplace_back(ctx.MapClusterId(i)); } + + fSzPageList = + RNTupleSerializer::SerializePageList(nullptr, descBuilder.GetDescriptor(), physClusterIDs, ctx).Unwrap(); + fBufPageList = MakeUninitArray(fSzPageList); + RNTupleSerializer::SerializePageList(fBufPageList.get(), descBuilder.GetDescriptor(), physClusterIDs, ctx); + RNTupleLocator locator; + locator.SetNBytesOnStorage(fSzPageList); + ROOT::Internal::RClusterGroupDescriptorBuilder cgBuilder; + cgBuilder.PageListLocator(locator).PageListLength(fSzPageList); cgBuilder.ClusterGroupId(0).MinEntry(0).EntrySpan(6).NClusters(6); cgBuilder.AddSortedClusters({0, 1, 2, 3, 4, 5}); descBuilder.AddClusterGroup(cgBuilder.MoveDescriptor().Unwrap()); - auto descriptorGuard = GetExclDescriptorGuard(); - descriptorGuard.MoveIn(descBuilder.MoveDescriptor()); + + // We need to return a descriptor without clusters + auto desc = descBuilder.MoveDescriptor(); + desc.DropClusterGroupDetails(0); + return desc; + } + std::unique_ptr CloneImpl() const final { return nullptr; } + void LoadPageListImpl(const ROOT::RNTupleLocator &, unsigned char *buf) final + { + memcpy(buf, fBufPageList.get(), fSzPageList); } + void LoadSealedPageImpl(const ROOT::RNTupleLocator &, RSealedPage &) final {} + std::unique_ptr + OpenWithDifferentAnchor(const ROOT::Internal::RNTupleLink &, const ROOT::RNTupleReadOptions &) final + { + throw ROOT::RException(R__FAIL("method not implemented")); + } + +public: + /// Records the cluster IDs requests by LoadClusters() calls + std::vector fReqsClusterIds; + std::vector fReqsColumns; + + RPageSourceMock() : RPageSource("test", RNTupleReadOptions()) {} std::vector> LoadClusters(std::span clusterKeys) final { std::vector> result; @@ -202,6 +231,7 @@ TEST(Cluster, AdoptClusters) TEST(ClusterPool, GetClusterBasics) { RPageSourceMock p1; + p1.Attach(ROOT::Internal::RNTupleSerializer::EDescriptorDeserializeMode::kForReading); RClusterPool c1(p1, 1); c1.GetCluster(3, {0}); c1.WaitForInFlightClusters(); @@ -212,6 +242,7 @@ TEST(ClusterPool, GetClusterBasics) EXPECT_EQ(RCluster::ColumnSet_t({0}), p1.fReqsColumns[1]); RPageSourceMock p2; + p2.Attach(ROOT::Internal::RNTupleSerializer::EDescriptorDeserializeMode::kForReading); { RClusterPool c2(p2, 2); c2.GetCluster(0, {0}); @@ -228,6 +259,7 @@ TEST(ClusterPool, GetClusterBasics) EXPECT_EQ(RCluster::ColumnSet_t({0}), p2.fReqsColumns[3]); RPageSourceMock p3; + p3.Attach(ROOT::Internal::RNTupleSerializer::EDescriptorDeserializeMode::kForReading); { RClusterPool c3(p3, 2); c3.GetCluster(0, {0}); @@ -241,6 +273,7 @@ TEST(ClusterPool, GetClusterBasics) EXPECT_EQ(3U, p3.fReqsClusterIds[3]); RPageSourceMock p4; + p4.Attach(ROOT::Internal::RNTupleSerializer::EDescriptorDeserializeMode::kForReading); { RClusterPool c4(p4, 3); c4.GetCluster(2, {0}); @@ -256,6 +289,7 @@ TEST(ClusterPool, GetClusterBasics) TEST(ClusterPool, SetEntryRange) { RPageSourceMock p1; + p1.Attach(ROOT::Internal::RNTupleSerializer::EDescriptorDeserializeMode::kForReading); p1.SetEntryRange({0, 6}); RClusterPool c1(p1, 1); c1.GetCluster(3, {0}); @@ -265,6 +299,7 @@ TEST(ClusterPool, SetEntryRange) EXPECT_EQ(4U, p1.fReqsClusterIds[1]); RPageSourceMock p2; + p2.Attach(ROOT::Internal::RNTupleSerializer::EDescriptorDeserializeMode::kForReading); p2.SetEntryRange({3, 1}); RClusterPool c2(p2, 1); c2.GetCluster(3, {0}); @@ -273,6 +308,7 @@ TEST(ClusterPool, SetEntryRange) EXPECT_EQ(3U, p2.fReqsClusterIds[0]); RPageSourceMock p3; + p3.Attach(ROOT::Internal::RNTupleSerializer::EDescriptorDeserializeMode::kForReading); p3.SetEntryRange({0, 1}); RClusterPool c3(p3, 1); c3.GetCluster(3, {0}); @@ -281,6 +317,7 @@ TEST(ClusterPool, SetEntryRange) EXPECT_EQ(3U, p3.fReqsClusterIds[0]); RPageSourceMock p4; + p4.Attach(ROOT::Internal::RNTupleSerializer::EDescriptorDeserializeMode::kForReading); p4.SetEntryRange({0, 3}); RClusterPool c4(p4, 2); c4.GetCluster(0, {0}); @@ -294,6 +331,7 @@ TEST(ClusterPool, SetEntryRange) TEST(ClusterPool, GetClusterIncrementally) { RPageSourceMock p1; + p1.Attach(ROOT::Internal::RNTupleSerializer::EDescriptorDeserializeMode::kForReading); RClusterPool c1(p1, 1); c1.GetCluster(3, {0}); c1.WaitForInFlightClusters(); @@ -311,6 +349,7 @@ TEST(ClusterPool, GetClusterIncrementally) TEST(ClusterPool, PinCluster) { RPageSourceMock p1; + p1.Attach(ROOT::Internal::RNTupleSerializer::EDescriptorDeserializeMode::kForReading); RClusterPool c1(p1, 1); p1.PinCluster(3); c1.GetCluster(3, {0}); diff --git a/tree/ntuple/test/ntuple_descriptor.cxx b/tree/ntuple/test/ntuple_descriptor.cxx index 8367608bae62a..bc982f67caf44 100644 --- a/tree/ntuple/test/ntuple_descriptor.cxx +++ b/tree/ntuple/test/ntuple_descriptor.cxx @@ -633,7 +633,9 @@ TEST(RClusterGroupDescriptorIterable, Ordering) RNTupleWriter::Append(RNTupleModel::Create(), "empty", *f); } - auto reader = RNTupleReader::Open("ntuple", fileGuard.GetPath()); + RNTupleReadOptions opts; + opts.SetMetadataMode(RNTupleReadOptions::EMetadataMode::kEager); + auto reader = RNTupleReader::Open("ntuple", fileGuard.GetPath(), opts); const auto &desc = reader->GetDescriptor(); EXPECT_EQ(3u, desc.GetNClusterGroups()); @@ -674,7 +676,9 @@ TEST(RClusterDescriptor, GetNBytesOnStorage) writer->Fill(); } - auto ntuple = RNTupleReader::Open("ntuple", fileGuard.GetPath()); + RNTupleReadOptions opts; + opts.SetMetadataMode(RNTupleReadOptions::EMetadataMode::kEager); + auto ntuple = RNTupleReader::Open("ntuple", fileGuard.GetPath(), opts); const auto &desc = ntuple->GetDescriptor(); EXPECT_EQ(8 + 8 + 8 + 3, desc.GetActiveClusterIterable().begin()->GetNBytesOnStorage()); } diff --git a/tree/ntuple/test/ntuple_extended.cxx b/tree/ntuple/test/ntuple_extended.cxx index d7a0fee9ac9ab..4874f14ccf6a2 100644 --- a/tree/ntuple/test/ntuple_extended.cxx +++ b/tree/ntuple/test/ntuple_extended.cxx @@ -181,7 +181,9 @@ TEST(RNTuple, LargePages) writer.reset(); } - auto reader = RNTupleReader::Open("ntpl", fileGuard.GetPath()); + RNTupleReadOptions opts; + opts.SetMetadataMode(RNTupleReadOptions::EMetadataMode::kEager); + auto reader = RNTupleReader::Open("ntpl", fileGuard.GetPath(), opts); const auto &desc = reader->GetDescriptor(); const auto rndColId = desc.FindPhysicalColumnId(desc.FindFieldId("rnd"), 0, 0); const auto &clusterDesc = *desc.GetActiveClusterIterable().begin(); diff --git a/tree/ntuple/test/ntuple_limits.cxx b/tree/ntuple/test/ntuple_limits.cxx index 9b37e29611b17..ed769ae4b93f4 100644 --- a/tree/ntuple/test/ntuple_limits.cxx +++ b/tree/ntuple/test/ntuple_limits.cxx @@ -77,7 +77,6 @@ TEST(RNTuple, Limits_ManyClusters) EXPECT_EQ(reader->GetNEntries(), NumClusters); EXPECT_EQ(descriptor.GetNClusters(), NumClusters); - EXPECT_EQ(descriptor.GetNActiveClusters(), NumClusters); auto id = model.GetDefaultEntry().GetPtr("id"); for (int i = 0; i < NumClusters; i++) { @@ -147,7 +146,9 @@ TEST(RNTuple, Limits_ManyPages) } } - auto reader = RNTupleReader::Open("myNTuple", fileGuard.GetPath()); + RNTupleReadOptions opts; + opts.SetMetadataMode(RNTupleReadOptions::EMetadataMode::kEager); + auto reader = RNTupleReader::Open("myNTuple", fileGuard.GetPath(), opts); const auto &descriptor = reader->GetDescriptor(); const auto &model = reader->GetModel(); auto fieldId = descriptor.FindFieldId("id"); @@ -189,7 +190,9 @@ TEST(RNTuple, Limits_ManyPagesOneEntry) writer->Fill(); } - auto reader = RNTupleReader::Open("myNTuple", fileGuard.GetPath()); + RNTupleReadOptions opts; + opts.SetMetadataMode(RNTupleReadOptions::EMetadataMode::kEager); + auto reader = RNTupleReader::Open("myNTuple", fileGuard.GetPath(), opts); const auto &descriptor = reader->GetDescriptor(); const auto &model = reader->GetModel(); auto fieldId = descriptor.FindFieldId("ids"); diff --git a/tree/ntuple/test/ntuple_merger.cxx b/tree/ntuple/test/ntuple_merger.cxx index 76912f77d9346..27287d804d165 100644 --- a/tree/ntuple/test/ntuple_merger.cxx +++ b/tree/ntuple/test/ntuple_merger.cxx @@ -826,7 +826,9 @@ static bool VerifyPageCompression(const std::string_view fileName, std::uint32_t // Check that the advertised compression is correct bool ok = true; { - auto reader = RNTupleReader::Open("ntuple", fileName); + RNTupleReadOptions opts; + opts.SetMetadataMode(RNTupleReadOptions::EMetadataMode::kEager); + auto reader = RNTupleReader::Open("ntuple", fileName, opts); auto compSettings = *reader->GetDescriptor().GetClusterDescriptor(0).GetColumnRange(0).GetCompressionSettings(); if (compSettings != expectedComp) { std::cerr << "Advertised compression is wrong: " << compSettings << " instead of " << expectedComp << "\n"; @@ -837,9 +839,6 @@ static bool VerifyPageCompression(const std::string_view fileName, std::uint32_t // Check that the actual compression is correct auto source = RPageSource::Create("ntuple", fileName); source->Attach(); - auto descriptor = source->GetSharedDescriptorGuard(); - const auto &columnDesc = descriptor->GetColumnDescriptor(0); - const auto colElement = ROOT::Internal::RColumnElementBase::Generate(columnDesc.GetType()); RPageStorage::RSealedPage sealedPage; source->LoadSealedPage(0, {0, 0}, sealedPage); auto buffer = MakeUninitArray(sealedPage.GetBufferSize()); diff --git a/tree/ntuple/test/ntuple_metrics.cxx b/tree/ntuple/test/ntuple_metrics.cxx index 24bf2a8b23c53..301df6fa921e6 100644 --- a/tree/ntuple/test/ntuple_metrics.cxx +++ b/tree/ntuple/test/ntuple_metrics.cxx @@ -137,24 +137,28 @@ TEST(Metrics, IOMetrics) ntupleReader->EnableMetrics(); EXPECT_TRUE(ntupleReader->GetMetrics().IsEnabled()); - auto view = ntupleReader->GetView("ints"); - for (auto i : *ntupleReader) { - (void)view(i); - } - const auto &metrics = ntupleReader->GetMetrics(); auto *randomness = metrics.GetCounter("RNTupleReader.RPageSourceFile.randomness"); auto *sparseness = metrics.GetCounter("RNTupleReader.RPageSourceFile.sparseness"); auto *szSkip = metrics.GetCounter("RNTupleReader.RPageSourceFile.szSkip"); auto *szFile = metrics.GetCounter("RNTupleReader.RPageSourceFile.szFile"); - ASSERT_NE(randomness, nullptr); ASSERT_NE(sparseness, nullptr); ASSERT_NE(szSkip, nullptr); ASSERT_NE(szFile, nullptr); - EXPECT_GE(szSkip->GetValueAsInt(), 0); + EXPECT_EQ(szSkip->GetValueAsInt(), 0); EXPECT_GT(szFile->GetValueAsInt(), 0); + + auto view = ntupleReader->GetView("ints"); + const auto pageListSkip = szSkip->GetValueAsInt(); + EXPECT_GE(pageListSkip, 0); + + for (auto i : *ntupleReader) { + (void)view(i); + } + + EXPECT_GE(szSkip->GetValueAsInt(), pageListSkip); } } diff --git a/tree/ntuple/test/ntuple_multi_column.cxx b/tree/ntuple/test/ntuple_multi_column.cxx index 963c5fabd9277..80f0cb474d49a 100644 --- a/tree/ntuple/test/ntuple_multi_column.cxx +++ b/tree/ntuple/test/ntuple_multi_column.cxx @@ -27,7 +27,9 @@ TEST(RNTuple, MultiColumnRepresentationSimple) writer->Fill(); } - auto reader = RNTupleReader::Open("ntpl", fileGuard.GetPath()); + RNTupleReadOptions opts; + opts.SetMetadataMode(RNTupleReadOptions::EMetadataMode::kEager); + auto reader = RNTupleReader::Open("ntpl", fileGuard.GetPath(), opts); EXPECT_EQ(3u, reader->GetView("px").GetFieldRange().size()); const auto &desc = reader->GetDescriptor(); diff --git a/tree/ntuple/test/ntuple_parallel_writer.cxx b/tree/ntuple/test/ntuple_parallel_writer.cxx index ebb04cd977ebd..23f94108f8ff2 100644 --- a/tree/ntuple/test/ntuple_parallel_writer.cxx +++ b/tree/ntuple/test/ntuple_parallel_writer.cxx @@ -262,7 +262,9 @@ TEST(RNTupleParallelWriter, StagedMultiColumn) c->CommitStagedClusters(); } - auto reader = RNTupleReader::Open("ntpl", fileGuard.GetPath()); + RNTupleReadOptions opts; + opts.SetMetadataMode(RNTupleReadOptions::EMetadataMode::kEager); + auto reader = RNTupleReader::Open("ntpl", fileGuard.GetPath(), opts); EXPECT_EQ(3u, reader->GetView("px").GetFieldRange().size()); const auto &desc = reader->GetDescriptor(); @@ -417,7 +419,9 @@ TEST(RNTupleFillContext, FlushColumns) } // If FlushColumns() worked, there will be two pages with one element each. - auto reader = RNTupleReader::Open("f", fileGuard.GetPath()); + RNTupleReadOptions opts; + opts.SetMetadataMode(RNTupleReadOptions::EMetadataMode::kEager); + auto reader = RNTupleReader::Open("f", fileGuard.GetPath(), opts); const auto &descriptor = reader->GetDescriptor(); auto fieldId = descriptor.FindFieldId("pt"); diff --git a/tree/ntuple/test/ntuple_storage.cxx b/tree/ntuple/test/ntuple_storage.cxx index 3021278c81ab0..da8c0156daf21 100644 --- a/tree/ntuple/test/ntuple_storage.cxx +++ b/tree/ntuple/test/ntuple_storage.cxx @@ -214,7 +214,9 @@ TEST(RNTuple, PageFilling) writer->Fill(); } - auto reader = RNTupleReader::Open("ntpl", fileGuard.GetPath()); + RNTupleReadOptions opts; + opts.SetMetadataMode(RNTupleReadOptions::EMetadataMode::kEager); + auto reader = RNTupleReader::Open("ntpl", fileGuard.GetPath(), opts); const auto &desc = reader->GetDescriptor(); EXPECT_EQ(1u, desc.GetNClusters()); const auto colIdX = desc.FindLogicalColumnId(desc.FindFieldId("x"), 0, 0); @@ -262,7 +264,9 @@ TEST(RNTuple, PageFillingString) ntuple->Fill(); // main write page is half full here; RColumn::AppendV should flush the shadow page } - auto ntuple = RNTupleReader::Open("ntpl", fileGuard.GetPath()); + RNTupleReadOptions opts; + opts.SetMetadataMode(RNTupleReadOptions::EMetadataMode::kEager); + auto ntuple = RNTupleReader::Open("ntpl", fileGuard.GetPath(), opts); auto viewX = ntuple->GetView("x"); ASSERT_EQ(5u, ntuple->GetNEntries()); EXPECT_EQ("01234567890123456", viewX(0)); @@ -317,7 +321,9 @@ TEST(RNTuple, FlushColumns) } // If FlushColumns() worked, there will be two pages with one element each. - auto reader = RNTupleReader::Open("f", fileGuard.GetPath()); + RNTupleReadOptions opts; + opts.SetMetadataMode(RNTupleReadOptions::EMetadataMode::kEager); + auto reader = RNTupleReader::Open("f", fileGuard.GetPath(), opts); const auto &descriptor = reader->GetDescriptor(); auto fieldId = descriptor.FindFieldId("pt"); @@ -924,8 +930,10 @@ TEST(RPageStorageFile, MultiKeyBlob_Pages) auto iU = modelUcmp->MakeField("i"); auto fC = modelComp->MakeField("f"); auto iC = modelComp->MakeField("i"); - auto ntupleUcmp = RNTupleReader::Open(std::move(modelUcmp), "myNTuple", fileGuardUcmp.GetPath()); - auto ntupleComp = RNTupleReader::Open(std::move(modelComp), "myNTuple", fileGuardComp.GetPath()); + RNTupleReadOptions opts; + opts.SetMetadataMode(RNTupleReadOptions::EMetadataMode::kEager); + auto ntupleUcmp = RNTupleReader::Open(std::move(modelUcmp), "myNTuple", fileGuardUcmp.GetPath(), opts); + auto ntupleComp = RNTupleReader::Open(std::move(modelComp), "myNTuple", fileGuardComp.GetPath(), opts); // Verify that the pages are larger than maxKeySize EXPECT_GT(ntupleComp->GetDescriptor() @@ -1099,7 +1107,9 @@ TEST(RPageSink, SamePageMerging) writer->Fill(); writer.reset(); - auto reader = RNTupleReader::Open("ntpl", fileGuard.GetPath()); + RNTupleReadOptions opts; + opts.SetMetadataMode(RNTupleReadOptions::EMetadataMode::kEager); + auto reader = RNTupleReader::Open("ntpl", fileGuard.GetPath(), opts); EXPECT_EQ(1u, reader->GetNEntries()); const auto &desc = reader->GetDescriptor(); @@ -1305,3 +1315,48 @@ TEST(RPageSink, AddColumnRepresentationWithData) EXPECT_EQ(col1.GetType(), ROOT::ENTupleColumnType::kInt32); EXPECT_EQ(col2.GetType(), ROOT::ENTupleColumnType::kSplitInt32); } + +TEST(RPageSource, MetadataMode) +{ + FileRaii fileGuard("test_ntuple_page_source_metadata_mode.root"); + + auto model = RNTupleModel::Create(); + auto ptrCtr = model->MakeField("ctr"); + { + auto writer = RNTupleWriter::Recreate(std::move(model), "ntpl", fileGuard.GetPath()); + for (int i = 0; i < 1000; ++i) { + *ptrCtr = i; + writer->Fill(); + writer->CommitCluster(true /* commit cluster group */); + } + } + + RNTupleReadOptions opts; + opts.SetMetadataMode(RNTupleReadOptions::EMetadataMode::kEager); + auto reader = RNTupleReader::Open("ntpl", fileGuard.GetPath(), opts); + EXPECT_EQ(1000u, reader->GetDescriptor().GetNClusterGroups()); + EXPECT_EQ(1000u, reader->GetDescriptor().GetNClusters()); + EXPECT_EQ(1000u, reader->GetDescriptor().GetNActiveClusters()); + + opts.SetMetadataMode(RNTupleReadOptions::EMetadataMode::kDefault); + reader = RNTupleReader::Open("ntpl", fileGuard.GetPath(), opts); + EXPECT_EQ(1000u, reader->GetDescriptor().GetNClusterGroups()); + EXPECT_EQ(1000u, reader->GetDescriptor().GetNClusters()); + EXPECT_EQ(0u, reader->GetDescriptor().GetNActiveClusters()); + + opts.SetMetadataMode(RNTupleReadOptions::EMetadataMode::kOnDemand); + reader = RNTupleReader::Open("ntpl", fileGuard.GetPath(), opts); + EXPECT_EQ(1000u, reader->GetDescriptor().GetNClusterGroups()); + EXPECT_EQ(1000u, reader->GetDescriptor().GetNClusters()); + EXPECT_EQ(0u, reader->GetDescriptor().GetNActiveClusters()); + + ptrCtr = reader->GetModel().GetDefaultEntry().GetPtr("ctr"); + reader->LoadEntry(500); + EXPECT_EQ(500, *ptrCtr); + reader->LoadEntry(999); + EXPECT_EQ(999, *ptrCtr); + reader->LoadEntry(0); + EXPECT_EQ(0, *ptrCtr); + EXPECT_LT(0, reader->GetDescriptor().GetNActiveClusters()); + EXPECT_GT(1000, reader->GetDescriptor().GetNActiveClusters()); +} diff --git a/tree/ntuple/test/ntuple_storage_daos.cxx b/tree/ntuple/test/ntuple_storage_daos.cxx index 0c91bacc8887f..706cbec4fd11a 100644 --- a/tree/ntuple/test/ntuple_storage_daos.cxx +++ b/tree/ntuple/test/ntuple_storage_daos.cxx @@ -275,7 +275,9 @@ TEST_F(RPageStorageDaos, DisabledSamePageMerging) writer->Fill(); writer.reset(); - auto reader = RNTupleReader::Open("ntpl", daosUri); + RNTupleReadOptions opts; + opts.SetMetadataMode(RNTupleReadOptions::EMetadataMode::kEager); + auto reader = RNTupleReader::Open("ntpl", daosUri, opts); EXPECT_EQ(1u, reader->GetNEntries()); const auto &desc = reader->GetDescriptor(); @@ -324,6 +326,7 @@ TEST_F(RPageStorageDaos, Checksum) ROOT::DescriptorId_t clusterId; auto pageSource = RPageSource::Create("ntpl", daosUri); pageSource->Attach(); + pageSource->LoadAllPageLists(); { auto descGuard = pageSource->GetSharedDescriptorGuard(); pxColId = descGuard->FindPhysicalColumnId(descGuard->FindFieldId("px"), 0, 0); diff --git a/tree/ntuple/test/ntuple_storage_s3.cxx b/tree/ntuple/test/ntuple_storage_s3.cxx index b2faf0a43ccac..c3d6251f0b68b 100644 --- a/tree/ntuple/test/ntuple_storage_s3.cxx +++ b/tree/ntuple/test/ntuple_storage_s3.cxx @@ -1052,7 +1052,9 @@ TEST_F(RPageSourceS3Wire, RoundTripManyPagesPerCluster) } } - auto reader = ROOT::RNTupleReader::Open("manypages", uri); + ROOT::RNTupleReadOptions opts; + opts.SetMetadataMode(ROOT::RNTupleReadOptions::EMetadataMode::kEager); + auto reader = ROOT::RNTupleReader::Open("manypages", uri, opts); EXPECT_EQ(1000u, reader->GetNEntries()); // Guard the premise of the test: one cluster, many pages per column. diff --git a/tree/ntuple/test/ntuple_test.cxx b/tree/ntuple/test/ntuple_test.cxx index 5e2e9bd6a4a2e..caef0abfd3163 100644 --- a/tree/ntuple/test/ntuple_test.cxx +++ b/tree/ntuple/test/ntuple_test.cxx @@ -32,7 +32,9 @@ void CreateCorruptedRNTuple(const std::string &uri) writer.reset(); // Load sealed pages to memory - auto pageSource = RPageSource::Create("ntpl", uri); + RNTupleReadOptions opts; + opts.SetMetadataMode(RNTupleReadOptions::EMetadataMode::kEager); + auto pageSource = RPageSource::Create("ntpl", uri, opts); pageSource->Attach(); auto descGuard = pageSource->GetSharedDescriptorGuard(); const auto pxColId = descGuard->FindPhysicalColumnId(descGuard->FindFieldId("px"), 0, 0); diff --git a/tree/ntuple/test/rfield_string.cxx b/tree/ntuple/test/rfield_string.cxx index 7a2b7343519cb..c45d5f043255a 100644 --- a/tree/ntuple/test/rfield_string.cxx +++ b/tree/ntuple/test/rfield_string.cxx @@ -19,7 +19,9 @@ TEST(RNTuple, ReadString) } } - auto ntuple = RNTupleReader::Open(ntupleName, fileGuard.GetPath()); + RNTupleReadOptions opts; + opts.SetMetadataMode(RNTupleReadOptions::EMetadataMode::kEager); + auto ntuple = RNTupleReader::Open(ntupleName, fileGuard.GetPath(), opts); auto viewSt = ntuple->GetView("st"); if (ntuple->GetDescriptor().GetClusterDescriptor(0).GetPageRange(1).GetPageInfos().size() < 2) { FAIL(); // This means all entries are inside the same page and numEntries should be increased. diff --git a/tree/ntupleutil/src/RNTupleExporter.cxx b/tree/ntupleutil/src/RNTupleExporter.cxx index 487863349cd9c..fdeab68633f90 100644 --- a/tree/ntupleutil/src/RNTupleExporter.cxx +++ b/tree/ntupleutil/src/RNTupleExporter.cxx @@ -118,13 +118,15 @@ RNTupleExporter::ExportPages(ROOT::Internal::RPageSource &source, const RPagesOp // make sure the source is attached source.Attach(); + source.LoadAllPageLists(); auto desc = source.GetSharedDescriptorGuard(); ROOT::Internal::RClusterPool clusterPool{source}; // Collect column info std::vector columnInfos; - const RAddColumnsResult addColRes = AddColumnsFromField(columnInfos, desc.GetRef(), desc->GetFieldZero(), options); + const RAddColumnsResult addColRes = + AddColumnsFromField(columnInfos, desc.GetConstRef(), desc->GetFieldZero(), options); // Collect ColumnSet for the cluster pool query ROOT::Internal::RCluster::ColumnSet_t columnSet; @@ -133,7 +135,7 @@ RNTupleExporter::ExportPages(ROOT::Internal::RPageSource &source, const RPagesOp columnSet.emplace(colInfo.fColDesc->GetPhysicalId()); } - const auto nPages = CountPages(desc.GetRef(), columnInfos); + const auto nPages = CountPages(desc.GetConstRef(), columnInfos); const bool showProgress = (options.fFlags & RPagesOptions::kShowProgressBar) != 0; res.fExportedFileNames.reserve(nPages); diff --git a/tree/ntupleutil/src/RNTupleInspector.cxx b/tree/ntupleutil/src/RNTupleInspector.cxx index ad50768d0ffe8..1b1ac6a66e71e 100644 --- a/tree/ntupleutil/src/RNTupleInspector.cxx +++ b/tree/ntupleutil/src/RNTupleInspector.cxx @@ -35,6 +35,7 @@ ROOT::Experimental::RNTupleInspector::RNTupleInspector(std::unique_ptrAttach(); + fPageSource->LoadAllPageLists(); auto descriptorGuard = fPageSource->GetSharedDescriptorGuard(); fDescriptor = descriptorGuard->Clone();