From bdb137846aed321d53e9653b9d44754d636681c4 Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Tue, 26 May 2026 23:12:47 +0200 Subject: [PATCH 01/25] [ntuple] fix argument comment --- tree/ntuple/src/RMiniFile.cxx | 12 ++++++------ tree/ntuple/src/RPageStorageFile.cxx | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/tree/ntuple/src/RMiniFile.cxx b/tree/ntuple/src/RMiniFile.cxx index 35f28cb666b71..fa4ae6d3c014b 100644 --- a/tree/ntuple/src/RMiniFile.cxx +++ b/tree/ntuple/src/RMiniFile.cxx @@ -1221,8 +1221,8 @@ std::uint64_t ROOT::Internal::RNTupleFileWriter::RImplRFile::ReserveBlobKey(size //////////////////////////////////////////////////////////////////////////////// -ROOT::Internal::RNTupleFileWriter::RNTupleFileWriter(std::string_view name, std::uint64_t maxKeySize, bool hidden) - : fIsHidden(hidden), fNTupleName(name) +ROOT::Internal::RNTupleFileWriter::RNTupleFileWriter(std::string_view name, std::uint64_t maxKeySize, bool isHidden) + : fIsHidden(isHidden), fNTupleName(name) { auto &fileSimple = fFile.emplace(); fileSimple.fControlBlock = std::make_unique(); @@ -1269,8 +1269,8 @@ ROOT::Internal::RNTupleFileWriter::Recreate(std::string_view ntupleName, std::st // RNTupleFileWriter::RImplSimple does its own buffering, turn off additional buffering from C stdio. std::setvbuf(fileStream, nullptr, _IONBF, 0); - auto writer = - std::unique_ptr(new RNTupleFileWriter(ntupleName, options.GetMaxKeySize(), /*hidden=*/false)); + auto writer = std::unique_ptr( + new RNTupleFileWriter(ntupleName, options.GetMaxKeySize(), /*isHidden=*/false)); RImplSimple &fileSimple = std::get(writer->fFile); fileSimple.fFile = fileStream; fileSimple.fDirectIO = options.GetUseDirectIO(); @@ -1309,7 +1309,7 @@ std::unique_ptr ROOT::Internal::RNTupleFileWriter::Append(std::string_view ntupleName, ROOT::Experimental::RFile &file, std::string_view ntupleDir, std::uint64_t maxKeySize) { - auto writer = std::unique_ptr(new RNTupleFileWriter(ntupleName, maxKeySize, /*hidden=*/false)); + auto writer = std::unique_ptr(new RNTupleFileWriter(ntupleName, maxKeySize, /*isHidden=*/false)); auto &rfile = writer->fFile.emplace(); rfile.fFile = &file; R__ASSERT(ntupleDir.empty() || ntupleDir[ntupleDir.size() - 1] == '/'); @@ -1321,7 +1321,7 @@ std::unique_ptr ROOT::Internal::RNTupleFileWriter::CloneAsHidden(std::string_view ntupleName) const { if (auto *file = std::get_if(&fFile)) { - return Append(ntupleName, *file->fDirectory, fNTupleAnchor.fMaxKeySize, /* hidden= */ true); + return Append(ntupleName, *file->fDirectory, fNTupleAnchor.fMaxKeySize, /* isHidden= */ true); } // TODO: support also non-TFile-based writers throw ROOT::RException(R__FAIL("cannot clone a non-TFile-based RNTupleFileWriter.")); diff --git a/tree/ntuple/src/RPageStorageFile.cxx b/tree/ntuple/src/RPageStorageFile.cxx index aad35a36cea88..c10079e2a253b 100644 --- a/tree/ntuple/src/RPageStorageFile.cxx +++ b/tree/ntuple/src/RPageStorageFile.cxx @@ -74,7 +74,7 @@ ROOT::Internal::RPageSinkFile::RPageSinkFile(std::string_view ntupleName, TDirec const ROOT::RNTupleWriteOptions &options) : RPageSinkFile(ntupleName, options) { - fWriter = RNTupleFileWriter::Append(ntupleName, fileOrDirectory, options.GetMaxKeySize(), /*hidden=*/false); + fWriter = RNTupleFileWriter::Append(ntupleName, fileOrDirectory, options.GetMaxKeySize(), /*isHidden=*/false); } ROOT::Internal::RPageSinkFile::RPageSinkFile(std::string_view ntupleName, ROOT::Experimental::RFile &file, From 6840b0aa783d2c9bbbb606ed146c07274c377749 Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Wed, 27 May 2026 11:22:54 +0200 Subject: [PATCH 02/25] [ntuple] avoid throwing impossible exception --- tree/ntuple/src/RNTupleReader.cxx | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tree/ntuple/src/RNTupleReader.cxx b/tree/ntuple/src/RNTupleReader.cxx index e26aaba93323b..dfeb557bb2d86 100644 --- a/tree/ntuple/src/RNTupleReader.cxx +++ b/tree/ntuple/src/RNTupleReader.cxx @@ -40,9 +40,8 @@ void ROOT::RNTupleReader::RActiveEntryToken::DeactivateEntry(NTupleSize_t entryN { const auto descGuard = fPtrControlBlock->fPageSource->GetSharedDescriptorGuard(); const auto clusterId = Internal::CallFindClusterIdOn(descGuard.GetRef(), entryNumber); - - if (clusterId == kInvalidDescriptorId) - throw RException(R__FAIL(std::string("entry number ") + std::to_string(entryNumber) + " out of range")); + // We acquired the given entry number so we must be able to find it back + R__ASSERT(clusterId != kInvalidDescriptorId); auto itr = fPtrControlBlock->fActiveClusters.find(clusterId); assert(itr != fPtrControlBlock->fActiveClusters.end()); From adcbd1f2714f05dcbe46a7782cba5fafa09bfaca Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Wed, 27 May 2026 11:38:06 +0200 Subject: [PATCH 03/25] [ntuple] don't throw in RNTupleModel::~RUpdater() --- tree/ntuple/src/RNTupleModel.cxx | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tree/ntuple/src/RNTupleModel.cxx b/tree/ntuple/src/RNTupleModel.cxx index d326194e31af9..5e3dcfb1ad374 100644 --- a/tree/ntuple/src/RNTupleModel.cxx +++ b/tree/ntuple/src/RNTupleModel.cxx @@ -197,8 +197,14 @@ ROOT::RNTupleModel::RUpdater::~RUpdater() // If we made any changes, we should commit them because the model was already altered. // Otherwise, we _do not_ commit -- it may be that the referenced model is already expired if the // corresponding writer is already destructed. - if (!fOpenChangeset.IsEmpty()) + if (fOpenChangeset.IsEmpty()) + return; + + try { ROOT::RNTupleModel::RUpdater::CommitUpdate(); + } catch (std::runtime_error &e) { + Fatal("RNTupleModel::RUpdater::~RUpdater", "cannot commit model changes during updater destructor: %s", e.what()); + } } void ROOT::RNTupleModel::RUpdater::BeginUpdate() From ff9976a3475384f2f8d39012a880a52a7c786244 Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Wed, 27 May 2026 12:51:00 +0200 Subject: [PATCH 04/25] [ntuple] safer use of RException --- tree/ntuple/src/RNTupleDescriptor.cxx | 2 +- tree/ntuple/src/RNTupleFillContext.cxx | 2 +- tree/ntuple/src/RNTupleMerger.cxx | 2 +- tree/ntuple/src/RNTupleParallelWriter.cxx | 2 +- tree/ntuple/src/RNTupleWriter.cxx | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/tree/ntuple/src/RNTupleDescriptor.cxx b/tree/ntuple/src/RNTupleDescriptor.cxx index b1e8407b86b68..69ff62cb1fc57 100644 --- a/tree/ntuple/src/RNTupleDescriptor.cxx +++ b/tree/ntuple/src/RNTupleDescriptor.cxx @@ -143,7 +143,7 @@ ROOT::RFieldDescriptor::CreateField(const RNTupleDescriptor &ntplDesc, const ROO return field; } catch (const RException &ex) { if (options.GetReturnInvalidOnError()) - return std::make_unique(GetFieldName(), GetTypeName(), ex.GetError().GetReport(), + return std::make_unique(GetFieldName(), GetTypeName(), ex.what(), ROOT::RInvalidField::ECategory::kGeneric); else throw ex; diff --git a/tree/ntuple/src/RNTupleFillContext.cxx b/tree/ntuple/src/RNTupleFillContext.cxx index 03da350314b2b..7f72ef2d08833 100644 --- a/tree/ntuple/src/RNTupleFillContext.cxx +++ b/tree/ntuple/src/RNTupleFillContext.cxx @@ -44,7 +44,7 @@ ROOT::RNTupleFillContext::~RNTupleFillContext() try { FlushCluster(); } catch (const RException &err) { - R__LOG_ERROR(ROOT::Internal::NTupleLog()) << "failure flushing cluster: " << err.GetError().GetReport(); + R__LOG_ERROR(ROOT::Internal::NTupleLog()) << "failure flushing cluster: " << err.what(); } if (!fStagedClusters.empty()) { diff --git a/tree/ntuple/src/RNTupleMerger.cxx b/tree/ntuple/src/RNTupleMerger.cxx index 6ff1ad0ac8679..aea1e85c706ff 100644 --- a/tree/ntuple/src/RNTupleMerger.cxx +++ b/tree/ntuple/src/RNTupleMerger.cxx @@ -725,7 +725,7 @@ ExtendDestinationModel(std::span newFields, ROOT try { mergeData.fDestination.UpdateSchema(changeset, mergeData.fNumDstEntries); } catch (const ROOT::RException &ex) { - return R__FAIL(ex.GetError().GetReport()); + return R__FAIL(ex.what()); } commonFields.reserve(commonFields.size() + newFields.size()); diff --git a/tree/ntuple/src/RNTupleParallelWriter.cxx b/tree/ntuple/src/RNTupleParallelWriter.cxx index ce3ee96e3c557..d909f38265d0c 100644 --- a/tree/ntuple/src/RNTupleParallelWriter.cxx +++ b/tree/ntuple/src/RNTupleParallelWriter.cxx @@ -144,7 +144,7 @@ ROOT::RNTupleParallelWriter::~RNTupleParallelWriter() try { CommitDataset(); } catch (const RException &err) { - R__LOG_ERROR(ROOT::Internal::NTupleLog()) << "failure committing ntuple: " << err.GetError().GetReport(); + R__LOG_ERROR(ROOT::Internal::NTupleLog()) << "failure committing ntuple: " << err.what(); } } diff --git a/tree/ntuple/src/RNTupleWriter.cxx b/tree/ntuple/src/RNTupleWriter.cxx index 4f0cd2d57f0dc..eec45995faf14 100644 --- a/tree/ntuple/src/RNTupleWriter.cxx +++ b/tree/ntuple/src/RNTupleWriter.cxx @@ -55,7 +55,7 @@ ROOT::RNTupleWriter::~RNTupleWriter() try { CommitDataset(); } catch (const RException &err) { - R__LOG_ERROR(ROOT::Internal::NTupleLog()) << "failure committing ntuple: " << err.GetError().GetReport(); + R__LOG_ERROR(ROOT::Internal::NTupleLog()) << "failure committing ntuple: " << err.what(); } } From 437c89feb9122a0af188cc6fc04a4a75222ef427 Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Wed, 27 May 2026 13:16:13 +0200 Subject: [PATCH 05/25] [ntuple] fix throwing exception in RNTupleDescriptor --- tree/ntuple/src/RNTupleDescriptor.cxx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tree/ntuple/src/RNTupleDescriptor.cxx b/tree/ntuple/src/RNTupleDescriptor.cxx index 69ff62cb1fc57..054e2d46ce94e 100644 --- a/tree/ntuple/src/RNTupleDescriptor.cxx +++ b/tree/ntuple/src/RNTupleDescriptor.cxx @@ -720,7 +720,7 @@ std::unique_ptr ROOT::RNTupleDescriptor::CreateModel(const R const auto cat = invalid.GetCategory(); bool mustThrow = cat != RInvalidField::ECategory::kUnknownStructure; if (mustThrow) - throw invalid.GetError(); + throw RException(R__FAIL(invalid.GetError())); // Not a hard error: skip the field and go on. continue; From 9c2e2df9b9b88063cb55b4e07bea47fbd63a56b4 Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Wed, 27 May 2026 13:19:26 +0200 Subject: [PATCH 06/25] [ntuple] remove wrong forward declaration of RFieldVisitor --- tree/ntuple/inc/ROOT/RField/RFieldFundamental.hxx | 7 ------- tree/ntuple/inc/ROOT/RField/RFieldSTLMisc.hxx | 7 ------- 2 files changed, 14 deletions(-) diff --git a/tree/ntuple/inc/ROOT/RField/RFieldFundamental.hxx b/tree/ntuple/inc/ROOT/RField/RFieldFundamental.hxx index 9caf202bd0171..987f6799a81a8 100644 --- a/tree/ntuple/inc/ROOT/RField/RFieldFundamental.hxx +++ b/tree/ntuple/inc/ROOT/RField/RFieldFundamental.hxx @@ -28,13 +28,6 @@ #include namespace ROOT { -namespace Experimental { - -namespace Detail { -class RFieldVisitor; -} // namespace Detail - -} // namespace Experimental //////////////////////////////////////////////////////////////////////////////// /// Template specializations for concrete C++ fundamental types diff --git a/tree/ntuple/inc/ROOT/RField/RFieldSTLMisc.hxx b/tree/ntuple/inc/ROOT/RField/RFieldSTLMisc.hxx index 0a21ecb0c28fe..908f24706a8ad 100644 --- a/tree/ntuple/inc/ROOT/RField/RFieldSTLMisc.hxx +++ b/tree/ntuple/inc/ROOT/RField/RFieldSTLMisc.hxx @@ -31,13 +31,6 @@ #include namespace ROOT { -namespace Experimental { - -namespace Detail { -class RFieldVisitor; -} // namespace Detail - -} // namespace Experimental //////////////////////////////////////////////////////////////////////////////// /// Template specializations for C++ std::atomic From 470bad9c07dba548e8c62a10e4e8581b155ca653 Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Wed, 27 May 2026 13:53:58 +0200 Subject: [PATCH 07/25] [ntuple] minor improvement in optional assignments --- tree/ntuple/src/RNTupleMerger.cxx | 4 ++-- tree/ntupleutil/src/RNTupleInspector.cxx | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tree/ntuple/src/RNTupleMerger.cxx b/tree/ntuple/src/RNTupleMerger.cxx index aea1e85c706ff..32131fabeda7f 100644 --- a/tree/ntuple/src/RNTupleMerger.cxx +++ b/tree/ntuple/src/RNTupleMerger.cxx @@ -217,7 +217,7 @@ try { "determined."; return -1; } - compression = (*firstColRange).GetCompressionSettings().value(); + compression = (*firstColRange).GetCompressionSettings(); R__LOG_INFO(NTupleMergeLog()) << "Using the first RNTuple's compression: " << *compression; } sources.push_back(std::move(source)); @@ -245,7 +245,7 @@ try { // Now merge RNTupleMerger merger{std::move(destination), std::move(model)}; RNTupleMergeOptions mergerOpts; - mergerOpts.fCompressionSettings = *compression; + mergerOpts.fCompressionSettings = compression; mergerOpts.fExtraVerbose = extraVerbose; if (auto mergingMode = ParseOptionMergingMode(mergeInfo->fOptions)) { mergerOpts.fMergingMode = *mergingMode; diff --git a/tree/ntupleutil/src/RNTupleInspector.cxx b/tree/ntupleutil/src/RNTupleInspector.cxx index 1a5b192f6ad53..20b3dc49bfa1c 100644 --- a/tree/ntupleutil/src/RNTupleInspector.cxx +++ b/tree/ntupleutil/src/RNTupleInspector.cxx @@ -73,7 +73,7 @@ void ROOT::Experimental::RNTupleInspector::CollectColumnInfo() nElems += columnRange.GetNElements(); if (!fCompressionSettings && columnRange.GetCompressionSettings()) { - fCompressionSettings = *columnRange.GetCompressionSettings(); + fCompressionSettings = columnRange.GetCompressionSettings(); } else if (fCompressionSettings && columnRange.GetCompressionSettings() && (*fCompressionSettings != *columnRange.GetCompressionSettings())) { // Note that currently all clusters and columns are compressed with the same settings and it is not yet From cc27fff36bdafb9314da8e397be7eef47a21466e Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Wed, 27 May 2026 14:18:56 +0200 Subject: [PATCH 08/25] [ntuple] minor readability improvement --- tree/ntuple/src/RPageStorage.cxx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tree/ntuple/src/RPageStorage.cxx b/tree/ntuple/src/RPageStorage.cxx index ee3269b2be9a6..e0454df8479e7 100644 --- a/tree/ntuple/src/RPageStorage.cxx +++ b/tree/ntuple/src/RPageStorage.cxx @@ -1227,8 +1227,8 @@ void ROOT::Internal::RPagePersistentSink::CommitSealedPageV(std::spansecond.fSealedPage; - if (sealedPageIt->GetDataSize() != p->GetDataSize() || - memcmp(sealedPageIt->GetBuffer(), p->GetBuffer(), p->GetDataSize())) { + if ((sealedPageIt->GetDataSize() != p->GetDataSize()) || + (memcmp(sealedPageIt->GetBuffer(), p->GetBuffer(), p->GetDataSize()) != 0)) { mask.emplace_back(true); locatorIndexes.emplace_back(iLocator++); continue; From 4ff86abd28608395ba94debaec07c0587ec65a24 Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Wed, 27 May 2026 15:14:06 +0200 Subject: [PATCH 09/25] [ntuple] minor improvement in RPageSourceFile::CreateFromAnchor() --- tree/ntuple/src/RPageStorageFile.cxx | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tree/ntuple/src/RPageStorageFile.cxx b/tree/ntuple/src/RPageStorageFile.cxx index c10079e2a253b..c8ec26ea7bc9d 100644 --- a/tree/ntuple/src/RPageStorageFile.cxx +++ b/tree/ntuple/src/RPageStorageFile.cxx @@ -395,9 +395,8 @@ ROOT::Internal::RPageSourceFile::CreateFromAnchor(const RNTuple &anchor, const R // For local TFiles, TDavixFile, TCurlFile, and TNetXNGFile, we want to open a new RRawFile to take advantage of the // faster reading. We check the exact class name to avoid classes inheriting in ROOT (for example TMemFile) or in // experiment frameworks. - std::string className = anchor.fFile->IsA()->GetName(); - auto url = anchor.fFile->GetEndpointUrl(); - auto protocol = std::string(url->GetProtocol()); + const std::string className = anchor.fFile->IsA()->GetName(); + const auto url = anchor.fFile->GetEndpointUrl(); if (className == "TFile") { rawFile = ROOT::Internal::RRawFile::Create(url->GetFile()); } else if (className == "TDavixFile" || className == "TCurlFile" || className == "TNetXNGFile") { From 777f3c82bf972892f41dc24262f45131fd948336 Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Wed, 27 May 2026 15:15:05 +0200 Subject: [PATCH 10/25] [ntuple] remove unused variable in RFieldBase::Create() --- tree/ntuple/src/RFieldBase.cxx | 1 - 1 file changed, 1 deletion(-) diff --git a/tree/ntuple/src/RFieldBase.cxx b/tree/ntuple/src/RFieldBase.cxx index e185389f1190a..40f7129b836eb 100644 --- a/tree/ntuple/src/RFieldBase.cxx +++ b/tree/ntuple/src/RFieldBase.cxx @@ -438,7 +438,6 @@ ROOT::RFieldBase::Create(const std::string &fieldName, const std::string &typeNa } else if (resolvedType.substr(0, 24) == "std::unordered_multiset<") { std::string itemTypeName = resolvedType.substr(24, resolvedType.length() - 25); auto itemField = Create("_0", itemTypeName, options, desc, maybeGetChildId(0)).Unwrap(); - auto normalizedInnerTypeName = itemField->GetTypeName(); result = std::make_unique(fieldName, RSetField::ESetType::kUnorderedMultiSet, std::move(itemField)); } else if (resolvedType.substr(0, 9) == "std::map<") { auto innerTypes = TokenizeTypeList(resolvedType.substr(9, resolvedType.length() - 10)); From 12ef34a785e83b533212b6394125406b096947ea Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Wed, 27 May 2026 15:21:21 +0200 Subject: [PATCH 11/25] [ntuple] fix use-after-move in RNTupleDescriptorBuilder::AddColumn() --- tree/ntuple/src/RNTupleDescriptor.cxx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tree/ntuple/src/RNTupleDescriptor.cxx b/tree/ntuple/src/RNTupleDescriptor.cxx index 054e2d46ce94e..e3ddb5652b0b5 100644 --- a/tree/ntuple/src/RNTupleDescriptor.cxx +++ b/tree/ntuple/src/RNTupleDescriptor.cxx @@ -1300,9 +1300,9 @@ ROOT::RResult ROOT::Internal::RNTupleDescriptorBuilder::AddColumn(RColumnD if (!columnDesc.IsAliasColumn()) fDescriptor.fNPhysicalColumns++; - fDescriptor.fColumnDescriptors.emplace(logicalId, std::move(columnDesc)); if (fDescriptor.fHeaderExtension) fDescriptor.fHeaderExtension->MarkExtendedColumn(columnDesc); + fDescriptor.fColumnDescriptors.emplace(logicalId, std::move(columnDesc)); return RResult::Success(); } From d669fe305a5c2c541ba458e90cfbab203ff77525 Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Wed, 27 May 2026 15:39:02 +0200 Subject: [PATCH 12/25] [NFC][ntuple] suppress clang-tidy false positive --- tree/ntuple/src/RColumnElement.hxx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tree/ntuple/src/RColumnElement.hxx b/tree/ntuple/src/RColumnElement.hxx index a1cba707103c6..e71763f3060a7 100644 --- a/tree/ntuple/src/RColumnElement.hxx +++ b/tree/ntuple/src/RColumnElement.hxx @@ -58,7 +58,7 @@ void UnpackBits(void *dst, const void *src, std::size_t count, std::size_t sizeo } // namespace ROOT::Internal::BitPacking -namespace { +namespace { // NOLINT(misc-anonymous-namespace-in-header): this header is only used in tests and in RColumnElement.cxx // In this namespace, common routines are defined for element packing and unpacking of ints and floats. // The following conversions and encodings exist: @@ -331,7 +331,7 @@ inline void CastZigzagSplitUnpack(void *destination, const void *source, std::si } // namespace // anonymous namespace because these definitions are not meant to be exported. -namespace { +namespace { // NOLINT(misc-anonymous-namespace-in-header): this header is only used in tests and in RColumnElement.cxx using ROOT::ENTupleColumnType; using ROOT::Internal::kTestFutureColumnType; From 49cde5460b328381831ce2a3ec1f7e92184ef84b Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Wed, 27 May 2026 17:01:35 +0200 Subject: [PATCH 13/25] [ntuple] fix widening method visibility change --- tree/ntuple/inc/ROOT/RPageNullSink.hxx | 14 ++++++++------ tree/ntuple/inc/ROOT/RPageSinkBuf.hxx | 7 +++---- tree/ntuple/inc/ROOT/RPageStorage.hxx | 10 ++++++---- tree/ntuple/src/RNTupleParallelWriter.cxx | 12 ++++++------ 4 files changed, 23 insertions(+), 20 deletions(-) diff --git a/tree/ntuple/inc/ROOT/RPageNullSink.hxx b/tree/ntuple/inc/ROOT/RPageNullSink.hxx index e46951cb3fb80..b72c104171537 100644 --- a/tree/ntuple/inc/ROOT/RPageNullSink.hxx +++ b/tree/ntuple/inc/ROOT/RPageNullSink.hxx @@ -36,6 +36,14 @@ class RPageNullSink : public ROOT::Internal::RPageSink { ROOT::DescriptorId_t fNColumns = 0; std::uint64_t fNBytesCurrentCluster = 0; +protected: + void InitImpl(ROOT::RNTupleModel &model) final + { + auto &fieldZero = ROOT::Internal::GetFieldZeroOfModel(model); + ConnectFields(fieldZero.GetMutableSubfields(), 0); + } + ROOT::Internal::RNTupleLink CommitDatasetImpl() final { return {}; } + public: RPageNullSink(std::string_view ntupleName, const ROOT::RNTupleWriteOptions &options) : RPageSink(ntupleName, options) { @@ -66,11 +74,6 @@ public: } } } - void InitImpl(ROOT::RNTupleModel &model) final - { - auto &fieldZero = ROOT::Internal::GetFieldZeroOfModel(model); - ConnectFields(fieldZero.GetMutableSubfields(), 0); - } void UpdateSchema(const ROOT::Internal::RNTupleModelChangeset &changeset, ROOT::NTupleSize_t firstEntry) final { ConnectFields(changeset.fAddedFields, firstEntry); @@ -104,7 +107,6 @@ public: } void CommitStagedClusters(std::span) final {} void CommitClusterGroup() final {} - ROOT::Internal::RNTupleLink CommitDatasetImpl() final { return {}; } void CommitAttributeSet(std::string_view, const ROOT::Internal::RNTupleLink &) final {} std::unique_ptr CloneAsHidden(std::string_view, const RNTupleWriteOptions &) const final diff --git a/tree/ntuple/inc/ROOT/RPageSinkBuf.hxx b/tree/ntuple/inc/ROOT/RPageSinkBuf.hxx index 8cda1d843c71e..53f081486c629 100644 --- a/tree/ntuple/inc/ROOT/RPageSinkBuf.hxx +++ b/tree/ntuple/inc/ROOT/RPageSinkBuf.hxx @@ -37,7 +37,6 @@ namespace Internal { */ // clang-format on class RPageSinkBuf : public RPageSink { -private: /// A buffered column. The column is not responsible for RPage memory management (i.e. ReservePage), /// which is handled by the enclosing RPageSinkBuf. class RColumnBuf { @@ -94,7 +93,6 @@ private: RPageStorage::SealedPageSequence_t fSealedPages; }; -private: /// I/O performance counters that get registered in fMetrics struct RCounters { ROOT::Experimental::Detail::RNTuplePlainCounter &fParallelZip; @@ -122,6 +120,9 @@ private: void ConnectFields(const std::vector &fields, ROOT::NTupleSize_t firstEntry); void FlushClusterImpl(std::function FlushClusterFn); + void InitImpl(ROOT::RNTupleModel &model) final; + RNTupleLink CommitDatasetImpl() final; + public: explicit RPageSinkBuf(std::unique_ptr inner); RPageSinkBuf(const RPageSinkBuf&) = delete; @@ -136,7 +137,6 @@ public: ROOT::NTupleSize_t GetNEntries() const final { return fInnerSink->GetNEntries(); } - void InitImpl(ROOT::RNTupleModel &model) final; void UpdateSchema(const RNTupleModelChangeset &changeset, ROOT::NTupleSize_t firstEntry) final; void UpdateExtraTypeInfo(const ROOT::RExtraTypeInfoDescriptor &extraTypeInfo) final; @@ -148,7 +148,6 @@ public: RStagedCluster StageCluster(ROOT::NTupleSize_t nNewEntries) final; void CommitStagedClusters(std::span clusters) final; void CommitClusterGroup() final; - RNTupleLink CommitDatasetImpl() final; void CommitAttributeSet(std::string_view attrSetName, const RNTupleLink &attrAnchorInfo) final; RPage ReservePage(ColumnHandle_t columnHandle, std::size_t nElements) final; diff --git a/tree/ntuple/inc/ROOT/RPageStorage.hxx b/tree/ntuple/inc/ROOT/RPageStorage.hxx index 5a0723fd0248d..400b8b47ec588 100644 --- a/tree/ntuple/inc/ROOT/RPageStorage.hxx +++ b/tree/ntuple/inc/ROOT/RPageStorage.hxx @@ -484,6 +484,9 @@ protected: virtual void InitImpl(unsigned char *serializedHeader, std::uint32_t length) = 0; + /// Updates the descriptor and calls InitImpl() that handles the backend-specific details (file, DAOS, etc.) + void InitImpl(RNTupleModel &model) final; + virtual RNTupleLocator CommitPageImpl(ColumnHandle_t columnHandle, const ROOT::Internal::RPage &page) = 0; virtual RNTupleLocator CommitSealedPageImpl(ROOT::DescriptorId_t physicalColumnId, const RPageStorage::RSealedPage &sealedPage) = 0; @@ -504,6 +507,9 @@ protected: virtual RNTupleLocator CommitClusterGroupImpl(unsigned char *serializedPageList, std::uint32_t length) = 0; virtual RNTupleLink CommitDatasetImpl(unsigned char *serializedFooter, std::uint32_t length) = 0; + /// \return The locator and length of the written anchor. + RNTupleLink CommitDatasetImpl() final; + /// Enables the default set of metrics provided by RPageSink. `prefix` will be used as the prefix for /// the counters registered in the internal RNTupleMetrics object. /// This set of counters can be extended by a subclass by calling `fMetrics.MakeCounter<...>()`. @@ -531,8 +537,6 @@ public: ROOT::NTupleSize_t GetNEntries() const final { return fPrevClusterNEntries; } - /// Updates the descriptor and calls InitImpl() that handles the backend-specific details (file, DAOS, etc.) - void InitImpl(RNTupleModel &model) final; void UpdateSchema(const ROOT::Internal::RNTupleModelChangeset &changeset, ROOT::NTupleSize_t firstEntry) override; void UpdateExtraTypeInfo(const ROOT::RExtraTypeInfoDescriptor &extraTypeInfo) final; @@ -551,8 +555,6 @@ public: void CommitStagedClusters(std::span clusters) final; void CommitClusterGroup() final; void CommitAttributeSet(std::string_view attrSetName, const RNTupleLink &attrAnchorInfo) final; - /// \return The locator and length of the written anchor. - RNTupleLink CommitDatasetImpl() final; }; // class RPagePersistentSink // clang-format off diff --git a/tree/ntuple/src/RNTupleParallelWriter.cxx b/tree/ntuple/src/RNTupleParallelWriter.cxx index d909f38265d0c..453d6805b9287 100644 --- a/tree/ntuple/src/RNTupleParallelWriter.cxx +++ b/tree/ntuple/src/RNTupleParallelWriter.cxx @@ -60,6 +60,12 @@ class RPageSynchronizingSink : public RPageSink { RPageSink *fInnerSink; std::mutex *fMutex; + void InitImpl(ROOT::RNTupleModel &) final {} + ROOT::Internal::RNTupleLink CommitDatasetImpl() final + { + throw ROOT::RException(R__FAIL("should never commit dataset via RPageSynchronizingSink")); + } + public: explicit RPageSynchronizingSink(RPageSink &inner, std::mutex &mutex) : RPageSink(inner.GetNTupleName(), inner.GetWriteOptions()), fInnerSink(&inner), fMutex(&mutex) @@ -76,7 +82,6 @@ class RPageSynchronizingSink : public RPageSink { NTupleSize_t GetNEntries() const final { return fInnerSink->GetNEntries(); } ColumnHandle_t AddColumn(DescriptorId_t, RColumn &) final { return {}; } - void InitImpl(ROOT::RNTupleModel &) final {} void UpdateSchema(const RNTupleModelChangeset &, NTupleSize_t) final { throw ROOT::RException(R__FAIL("UpdateSchema not supported via RPageSynchronizingSink")); @@ -107,11 +112,6 @@ class RPageSynchronizingSink : public RPageSink { throw ROOT::RException(R__FAIL("should never commit cluster group via RPageSynchronizingSink")); } - ROOT::Internal::RNTupleLink CommitDatasetImpl() final - { - throw ROOT::RException(R__FAIL("should never commit dataset via RPageSynchronizingSink")); - } - RSinkGuard GetSinkGuard() final { return RSinkGuard(fMutex); } std::unique_ptr CloneAsHidden(std::string_view, const ROOT::RNTupleWriteOptions &) const final From acdd5fa6d018fbba2aed1b2ac232694fa038444a Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Wed, 27 May 2026 17:08:14 +0200 Subject: [PATCH 14/25] [ntuple] remove unusued usings --- tree/ntuple/src/RMiniFile.cxx | 1 - tree/ntuple/src/RNTupleDescriptor.cxx | 2 -- tree/ntuple/src/RPageSinkBuf.cxx | 1 - tree/ntuple/src/RPageStorage.cxx | 1 - tree/ntuple/src/RPageStorageFile.cxx | 3 --- 5 files changed, 8 deletions(-) diff --git a/tree/ntuple/src/RMiniFile.cxx b/tree/ntuple/src/RMiniFile.cxx index fa4ae6d3c014b..93355206818fe 100644 --- a/tree/ntuple/src/RMiniFile.cxx +++ b/tree/ntuple/src/RMiniFile.cxx @@ -55,7 +55,6 @@ #endif #endif /* R__LITTLE_ENDIAN */ -using ROOT::Internal::MakeUninitArray; using ROOT::Internal::RNTupleCompressor; using ROOT::Internal::RNTupleDecompressor; using ROOT::Internal::RNTupleSerializer; diff --git a/tree/ntuple/src/RNTupleDescriptor.cxx b/tree/ntuple/src/RNTupleDescriptor.cxx index e3ddb5652b0b5..ebfed9d89b747 100644 --- a/tree/ntuple/src/RNTupleDescriptor.cxx +++ b/tree/ntuple/src/RNTupleDescriptor.cxx @@ -32,8 +32,6 @@ #include #include -using ROOT::Internal::RNTupleSerializer; - bool ROOT::RFieldDescriptor::operator==(const RFieldDescriptor &other) const { return fFieldId == other.fFieldId && fFieldVersion == other.fFieldVersion && fTypeVersion == other.fTypeVersion && diff --git a/tree/ntuple/src/RPageSinkBuf.cxx b/tree/ntuple/src/RPageSinkBuf.cxx index 1fed3a0138ced..e516e410cabb9 100644 --- a/tree/ntuple/src/RPageSinkBuf.cxx +++ b/tree/ntuple/src/RPageSinkBuf.cxx @@ -27,7 +27,6 @@ using ROOT::Experimental::Detail::RNTupleMetrics; using ROOT::Experimental::Detail::RNTuplePlainCounter; using ROOT::Experimental::Detail::RNTuplePlainTimer; using ROOT::Experimental::Detail::RNTupleTickCounter; -using ROOT::Internal::MakeUninitArray; void ROOT::Internal::RPageSinkBuf::RColumnBuf::DropBufferedPages() { diff --git a/tree/ntuple/src/RPageStorage.cxx b/tree/ntuple/src/RPageStorage.cxx index e0454df8479e7..eea646494b23c 100644 --- a/tree/ntuple/src/RPageStorage.cxx +++ b/tree/ntuple/src/RPageStorage.cxx @@ -41,7 +41,6 @@ #include #include -using ROOT::Internal::MakeUninitArray; using ROOT::Internal::RClusterDescriptorBuilder; using ROOT::Internal::RClusterGroupDescriptorBuilder; using ROOT::Internal::RColumn; diff --git a/tree/ntuple/src/RPageStorageFile.cxx b/tree/ntuple/src/RPageStorageFile.cxx index c8ec26ea7bc9d..1341b77a189bd 100644 --- a/tree/ntuple/src/RPageStorageFile.cxx +++ b/tree/ntuple/src/RPageStorageFile.cxx @@ -45,16 +45,13 @@ using ROOT::Experimental::Detail::RNTupleAtomicCounter; using ROOT::Experimental::Detail::RNTupleAtomicTimer; using ROOT::Experimental::Detail::RNTupleCalcPerf; using ROOT::Experimental::Detail::RNTupleMetrics; -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) : RPagePersistentSink(ntupleName, options) From 8537f6354eb654d96aedb54a7def5e94e254836e Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Wed, 27 May 2026 17:14:36 +0200 Subject: [PATCH 15/25] [ntuple] move RAuxiliaryProcessorField to anonymous namespace --- tree/ntuple/src/RNTupleProcessor.cxx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tree/ntuple/src/RNTupleProcessor.cxx b/tree/ntuple/src/RNTupleProcessor.cxx index 3a00539e0a82c..a6e3fb7c21910 100644 --- a/tree/ntuple/src/RNTupleProcessor.cxx +++ b/tree/ntuple/src/RNTupleProcessor.cxx @@ -398,13 +398,13 @@ void ROOT::Experimental::RNTupleChainProcessor::PrintStructureImpl(std::ostream //------------------------------------------------------------------------------ -namespace ROOT::Experimental::Internal { +namespace { class RAuxiliaryProcessorField final : public ROOT::RRecordField { private: using RFieldBase::GenerateColumns; void GenerateColumns() final { - throw RException(R__FAIL("RAuxiliaryProcessorField fields must only be used for reading")); + throw ROOT::RException(R__FAIL("RAuxiliaryProcessorField fields must only be used for reading")); } public: @@ -414,7 +414,7 @@ class RAuxiliaryProcessorField final : public ROOT::RRecordField { AttachItemFields(std::move(itemFields)); } }; -} // namespace ROOT::Experimental::Internal +} // anonymous namespace ROOT::Experimental::RNTupleJoinProcessor::RNTupleJoinProcessor(std::unique_ptr primaryProcessor, std::unique_ptr auxProcessor, From 78022c4ec836b963c87d09f05b009574547dd41d Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Wed, 27 May 2026 17:19:53 +0200 Subject: [PATCH 16/25] [ntuple] private linkage of stream operator in the merger --- tree/ntuple/src/RNTupleMerger.cxx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tree/ntuple/src/RNTupleMerger.cxx b/tree/ntuple/src/RNTupleMerger.cxx index 32131fabeda7f..3c7cadc71b76b 100644 --- a/tree/ntuple/src/RNTupleMerger.cxx +++ b/tree/ntuple/src/RNTupleMerger.cxx @@ -429,7 +429,7 @@ struct RSealedPageMergeData { std::vector> fBuffers; }; -std::ostream &operator<<(std::ostream &os, const std::optional &x) +static std::ostream &operator<<(std::ostream &os, const std::optional &x) { if (x) { os << '(' << x->fMin << ", " << x->fMax << ')'; From 25eb3e4ce41e4dc9104da828968778c4766707d4 Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Wed, 27 May 2026 17:27:52 +0200 Subject: [PATCH 17/25] [ntuple] replace std::endl by \n --- tree/ntuple/src/RFieldVisitor.cxx | 4 ++-- tree/ntuple/src/RNTupleDescriptorFmt.cxx | 6 +++--- tree/ntuple/src/RNTupleMetrics.cxx | 4 ++-- tree/ntuple/src/RNTupleReader.cxx | 6 +++--- tree/ntupleutil/src/RNTupleInspector.cxx | 9 ++++----- 5 files changed, 14 insertions(+), 15 deletions(-) diff --git a/tree/ntuple/src/RFieldVisitor.cxx b/tree/ntuple/src/RFieldVisitor.cxx index 5ca6d26a7aadc..05e34fab35743 100644 --- a/tree/ntuple/src/RFieldVisitor.cxx +++ b/tree/ntuple/src/RFieldVisitor.cxx @@ -143,7 +143,7 @@ void ROOT::Internal::RPrintValueVisitor::PrintRecord(const ROOT::RFieldBase &fie auto elems = field.SplitValue(fValue); for (auto iValue = elems.begin(); iValue != elems.end();) { if (!fPrintOptions.fPrintSingleLine) - fOutput << std::endl; + fOutput << '\n'; RPrintOptions options; options.fPrintSingleLine = fPrintOptions.fPrintSingleLine; @@ -152,7 +152,7 @@ void ROOT::Internal::RPrintValueVisitor::PrintRecord(const ROOT::RFieldBase &fie if (++iValue == elems.end()) { if (!fPrintOptions.fPrintSingleLine) - fOutput << std::endl; + fOutput << '\n'; break; } else { fOutput << ","; diff --git a/tree/ntuple/src/RNTupleDescriptorFmt.cxx b/tree/ntuple/src/RNTupleDescriptorFmt.cxx index db18b0265d3e3..8063a7e39622b 100644 --- a/tree/ntuple/src/RNTupleDescriptorFmt.cxx +++ b/tree/ntuple/src/RNTupleDescriptorFmt.cxx @@ -159,7 +159,7 @@ void ROOT::RNTupleDescriptor::PrintInfo(std::ostream &output) const << float(headerSize + footerSize) / float(nBytesOnStorage) << "\n"; output << "------------------------------------------------------------\n"; output << "CLUSTER DETAILS\n"; - output << "------------------------------------------------------------" << std::endl; + output << "------------------------------------------------------------\n"; std::sort(clusters.begin(), clusters.end()); for (unsigned int i = 0; i < clusters.size(); ++i) { @@ -168,7 +168,7 @@ void ROOT::RNTupleDescriptor::PrintInfo(std::ostream &output) const output << " " << " # Pages: " << clusters[i].fNPages << "\n"; output << " " << " Size on storage: " << clusters[i].fNBytesOnStorage << " B\n"; output << " " << " Compression: " << std::fixed << std::setprecision(2) - << float(clusters[i].fNBytesInMemory) / float(float(clusters[i].fNBytesOnStorage)) << std::endl; + << float(clusters[i].fNBytesInMemory) / float(float(clusters[i].fNBytesOnStorage)) << '\n'; } output << "------------------------------------------------------------\n"; @@ -199,6 +199,6 @@ void ROOT::RNTupleDescriptor::PrintInfo(std::ostream &output) const output << " Size on storage: " << col.fNBytesOnStorage << " B\n"; output << " Compression: " << std::fixed << std::setprecision(2) << float(col.fElementSize * col.fNElements) / float(col.fNBytesOnStorage) << "\n"; - output << "............................................................" << std::endl; + output << "............................................................\n"; } } diff --git a/tree/ntuple/src/RNTupleMetrics.cxx b/tree/ntuple/src/RNTupleMetrics.cxx index c97146010f3a6..c02fbd3d88675 100644 --- a/tree/ntuple/src/RNTupleMetrics.cxx +++ b/tree/ntuple/src/RNTupleMetrics.cxx @@ -65,12 +65,12 @@ ROOT::Experimental::Detail::RNTupleMetrics::GetCounter(std::string_view name) co void ROOT::Experimental::Detail::RNTupleMetrics::Print(std::ostream &output, const std::string &prefix) const { if (!fIsEnabled) { - output << fName << " metrics disabled!" << std::endl; + output << fName << " metrics disabled!\n"; return; } for (const auto &c : fCounters) { - output << prefix << fName << kNamespaceSeperator << c->ToString() << std::endl; + output << prefix << fName << kNamespaceSeperator << c->ToString() << '\n'; } for (const auto c : fObservedMetrics) { c->Print(output, prefix + fName + "."); diff --git a/tree/ntuple/src/RNTupleReader.cxx b/tree/ntuple/src/RNTupleReader.cxx index dfeb557bb2d86..b2d0c4f8e5e62 100644 --- a/tree/ntuple/src/RNTupleReader.cxx +++ b/tree/ntuple/src/RNTupleReader.cxx @@ -309,18 +309,18 @@ void ROOT::RNTupleReader::Show(ROOT::NTupleSize_t index, std::ostream &output) reader->LoadEntry(index); output << "{"; for (auto iValue = entry.begin(); iValue != entry.end();) { - output << std::endl; + output << '\n'; ROOT::Internal::RPrintValueVisitor visitor(*iValue, output, 1 /* level */); iValue->GetField().AcceptVisitor(visitor); if (++iValue == entry.end()) { - output << std::endl; + output << '\n'; break; } else { output << ","; } } - output << "}" << std::endl; + output << "}\n"; } const ROOT::RNTupleDescriptor &ROOT::RNTupleReader::GetDescriptor() diff --git a/tree/ntupleutil/src/RNTupleInspector.cxx b/tree/ntupleutil/src/RNTupleInspector.cxx index 20b3dc49bfa1c..2eb11d339b22e 100644 --- a/tree/ntupleutil/src/RNTupleInspector.cxx +++ b/tree/ntupleutil/src/RNTupleInspector.cxx @@ -265,21 +265,20 @@ void ROOT::Experimental::RNTupleInspector::PrintColumnTypeInfo(ENTupleInspectorP output << " column type | count | # elements | compressed bytes | uncompressed bytes | compression ratio | " "# pages \n" << "----------------|---------|-------------|------------------|--------------------|-------------------|-" - "------" - << std::endl; + "------\n"; for (const auto &[colType, typeInfo] : colTypeInfo) output << std::setw(15) << RColumnElementBase::GetColumnTypeName(colType) << " |" << std::setw(8) << typeInfo.count << " |" << std::setw(12) << typeInfo.nElems << " |" << std::setw(17) << typeInfo.compressedSize << " |" << std::setw(19) << typeInfo.uncompressedSize << " |" << std::fixed << std::setprecision(3) << std::setw(18) << typeInfo.GetCompressionFactor() << " |" << std::setw(6) - << typeInfo.nPages << " " << std::endl; + << typeInfo.nPages << " \n"; break; case ENTupleInspectorPrintFormat::kCSV: - output << "columnType,count,nElements,compressedSize,uncompressedSize,compressionFactor,nPages" << std::endl; + output << "columnType,count,nElements,compressedSize,uncompressedSize,compressionFactor,nPages\n"; for (const auto &[colType, typeInfo] : colTypeInfo) { output << RColumnElementBase::GetColumnTypeName(colType) << "," << typeInfo.count << "," << typeInfo.nElems << "," << typeInfo.compressedSize << "," << typeInfo.uncompressedSize << "," << std::fixed - << std::setprecision(3) << typeInfo.GetCompressionFactor() << "," << typeInfo.nPages << std::endl; + << std::setprecision(3) << typeInfo.GetCompressionFactor() << "," << typeInfo.nPages << '\n'; } break; default: R__ASSERT(false && "Invalid print format"); From ec3b1734dadde829d5325c6678875f7fe5476ff4 Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Wed, 27 May 2026 17:30:28 +0200 Subject: [PATCH 18/25] [ntuple] minor performance improvement in RPageSourceFile::LoadClusters() --- tree/ntuple/src/RPageStorageFile.cxx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tree/ntuple/src/RPageStorageFile.cxx b/tree/ntuple/src/RPageStorageFile.cxx index 1341b77a189bd..0d8319538b34e 100644 --- a/tree/ntuple/src/RPageStorageFile.cxx +++ b/tree/ntuple/src/RPageStorageFile.cxx @@ -672,7 +672,7 @@ ROOT::Internal::RPageSourceFile::LoadClusters(std::span clusterK std::vector readRequests; clusters.reserve(clusterKeys.size()); - for (auto key : clusterKeys) { + for (const auto &key : clusterKeys) { clusters.emplace_back(PrepareSingleCluster(key, readRequests)); } From 688fdd23e0e77d9a6bbb67e4abb4ef28be71a987 Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Wed, 27 May 2026 17:41:36 +0200 Subject: [PATCH 19/25] [ntuple] use member move in RValue move ctor --- tree/ntuple/inc/ROOT/RFieldBase.hxx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tree/ntuple/inc/ROOT/RFieldBase.hxx b/tree/ntuple/inc/ROOT/RFieldBase.hxx index 6e691a9f98d95..27a6ea59a45d6 100644 --- a/tree/ntuple/inc/ROOT/RFieldBase.hxx +++ b/tree/ntuple/inc/ROOT/RFieldBase.hxx @@ -790,7 +790,7 @@ public: fTypeInfo = nullptr; return *this; } - RValue(RValue &&other) : fField(other.fField), fObjPtr(other.fObjPtr) {} + RValue(RValue &&other) : fField(other.fField), fObjPtr(std::move(other.fObjPtr)) {} RValue &operator=(RValue &&other) { fField = other.fField; From 1d45e7b573d76d43ff7805a97d2d375572992f4b Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Wed, 27 May 2026 17:43:56 +0200 Subject: [PATCH 20/25] [ntuple] minor perf improvement in type name normalization --- tree/ntuple/src/RFieldUtils.cxx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tree/ntuple/src/RFieldUtils.cxx b/tree/ntuple/src/RFieldUtils.cxx index 6420d43cc8c47..d8b37d46baa60 100644 --- a/tree/ntuple/src/RFieldUtils.cxx +++ b/tree/ntuple/src/RFieldUtils.cxx @@ -393,7 +393,7 @@ void NormalizeTemplateArguments(std::string &templatedTypeName, int maxTemplateA // Given a type name normalized by ROOT Meta, return the type name normalized according to the RNTuple rules. std::string GetRenormalizedMetaTypeName(const std::string &metaNormalizedName) { - const auto canonicalTypePrefix = ROOT::Internal::GetCanonicalTypePrefix(metaNormalizedName); + auto canonicalTypePrefix = ROOT::Internal::GetCanonicalTypePrefix(metaNormalizedName); // RNTuple resolves Double32_t for the normalized type name but keeps Double32_t for the type alias // (also in template parameters) if (canonicalTypePrefix == "Double32_t") @@ -573,7 +573,7 @@ std::string ROOT::Internal::GetNormalizedUnresolvedTypeName(const std::string &o TClassEdit::TSplitType splitname(origName.c_str(), modType); std::string shortType; splitname.ShortType(shortType, modType); - const auto canonicalTypePrefix = GetCanonicalTypePrefix(shortType); + auto canonicalTypePrefix = GetCanonicalTypePrefix(shortType); if (canonicalTypePrefix.find('<') == std::string::npos) { // If there are no templates, the function is done. From ad41fbef29e0cf811f3b0b52af5a83878a5f3e80 Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Wed, 27 May 2026 17:53:38 +0200 Subject: [PATCH 21/25] [ntuple] make moving RPage(Ref) noexcept --- tree/ntuple/inc/ROOT/RPage.hxx | 4 ++-- tree/ntuple/inc/ROOT/RPagePool.hxx | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tree/ntuple/inc/ROOT/RPage.hxx b/tree/ntuple/inc/ROOT/RPage.hxx index 1c45836312ac0..2344a1981247e 100644 --- a/tree/ntuple/inc/ROOT/RPage.hxx +++ b/tree/ntuple/inc/ROOT/RPage.hxx @@ -81,7 +81,7 @@ public: {} RPage(const RPage &) = delete; RPage &operator=(const RPage &) = delete; - RPage(RPage &&other) + RPage(RPage &&other) noexcept { fBuffer = other.fBuffer; fPageAllocator = other.fPageAllocator; @@ -92,7 +92,7 @@ public: fClusterInfo = other.fClusterInfo; other.fPageAllocator = nullptr; } - RPage &operator=(RPage &&other) + RPage &operator=(RPage &&other) noexcept { if (this != &other) { std::swap(fBuffer, other.fBuffer); diff --git a/tree/ntuple/inc/ROOT/RPagePool.hxx b/tree/ntuple/inc/ROOT/RPagePool.hxx index 64b66aa2df4a4..7ee2aba14f31c 100644 --- a/tree/ntuple/inc/ROOT/RPagePool.hxx +++ b/tree/ntuple/inc/ROOT/RPagePool.hxx @@ -213,9 +213,9 @@ public: RPageRef(const RPageRef &other) = delete; RPageRef &operator=(const RPageRef &other) = delete; - RPageRef(RPageRef &&other) : RPageRef(other.fPage, other.fPagePool) { other.fPagePool = nullptr; } + RPageRef(RPageRef &&other) noexcept : RPageRef(other.fPage, other.fPagePool) { other.fPagePool = nullptr; } - RPageRef &operator=(RPageRef &&other) + RPageRef &operator=(RPageRef &&other) noexcept { if (this != &other) { std::swap(fPage, other.fPage); From fd69e2f5a8eab698cda95c0471804f9ae3aa92cb Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Thu, 28 May 2026 09:09:48 +0200 Subject: [PATCH 22/25] [ntuple] avoid some unnecessary value copies --- tree/ntuple/inc/ROOT/RFieldBase.hxx | 6 +++--- tree/ntuple/inc/ROOT/RFieldVisitor.hxx | 2 +- tree/ntuple/inc/ROOT/RNTupleModel.hxx | 6 +++--- tree/ntuple/inc/ROOT/RNTupleProcessorEntry.hxx | 2 +- tree/ntuple/inc/ROOT/RNTupleReader.hxx | 2 +- tree/ntuple/inc/ROOT/RNTupleView.hxx | 4 ++-- tree/ntuple/inc/ROOT/RPageSinkBuf.hxx | 2 +- tree/ntuple/inc/ROOT/RPageStorage.hxx | 14 +++++++------- tree/ntuple/src/RFieldBase.cxx | 4 ++-- tree/ntuple/src/RFieldMeta.cxx | 8 ++++---- tree/ntuple/src/RFieldSequenceContainer.cxx | 2 +- tree/ntuple/src/RNTupleModel.cxx | 13 +++++++------ tree/ntuple/src/RNTupleProcessor.cxx | 4 ++-- tree/ntuple/src/RPageSinkBuf.cxx | 2 +- tree/ntuple/src/RPageStorage.cxx | 4 ++-- tree/ntupleutil/inc/ROOT/RNTupleImporter.hxx | 4 ++-- 16 files changed, 40 insertions(+), 39 deletions(-) diff --git a/tree/ntuple/inc/ROOT/RFieldBase.hxx b/tree/ntuple/inc/ROOT/RFieldBase.hxx index 27a6ea59a45d6..2623dc5300fc7 100644 --- a/tree/ntuple/inc/ROOT/RFieldBase.hxx +++ b/tree/ntuple/inc/ROOT/RFieldBase.hxx @@ -511,7 +511,7 @@ protected: /// Set a user-defined function to be called after reading a value, giving a chance to inspect and/or modify the /// value object. /// Returns an index that can be used to remove the callback. - size_t AddReadCallback(ReadCallback_t func); + size_t AddReadCallback(const ReadCallback_t &func); void RemoveReadCallback(size_t idx); // Perform housekeeping tasks for global to cluster-local index translation @@ -778,7 +778,7 @@ private: std::shared_ptr fObjPtr; mutable std::atomic fTypeInfo = nullptr; - RValue(RFieldBase *field, std::shared_ptr objPtr) : fField(field), fObjPtr(objPtr) {} + RValue(RFieldBase *field, std::shared_ptr objPtr) : fField(field), fObjPtr(std::move(objPtr)) {} public: RValue(const RValue &other) : fField(other.fField), fObjPtr(other.fObjPtr) {} @@ -829,7 +829,7 @@ public: void Read(ROOT::NTupleSize_t globalIndex) { fField->Read(globalIndex, fObjPtr.get()); } void Read(RNTupleLocalIndex localIndex) { fField->Read(localIndex, fObjPtr.get()); } - void Bind(std::shared_ptr objPtr) { fObjPtr = objPtr; } + void Bind(std::shared_ptr objPtr) { fObjPtr = std::move(objPtr); } void BindRawPtr(void *rawPtr); /// Replace the current object pointer by a pointer to a new object constructed by the field void EmplaceNew() { fObjPtr = fField->CreateValue().GetPtr(); } diff --git a/tree/ntuple/inc/ROOT/RFieldVisitor.hxx b/tree/ntuple/inc/ROOT/RFieldVisitor.hxx index 41af5a1b76e18..8e2fb0c53a71b 100644 --- a/tree/ntuple/inc/ROOT/RFieldVisitor.hxx +++ b/tree/ntuple/inc/ROOT/RFieldVisitor.hxx @@ -199,7 +199,7 @@ private: public: RPrintValueVisitor(ROOT::RFieldBase::RValue value, std::ostream &output, unsigned int level = 0, RPrintOptions options = RPrintOptions()) - : fValue(value), fOutput{output}, fLevel(level), fPrintOptions(options) + : fValue(std::move(value)), fOutput{output}, fLevel(level), fPrintOptions(options) { } diff --git a/tree/ntuple/inc/ROOT/RNTupleModel.hxx b/tree/ntuple/inc/ROOT/RNTupleModel.hxx index eb393b63bab54..8a5df8d244096 100644 --- a/tree/ntuple/inc/ROOT/RNTupleModel.hxx +++ b/tree/ntuple/inc/ROOT/RNTupleModel.hxx @@ -314,7 +314,7 @@ public: /// ~~~ /// /// Creating projections for fields containing `std::variant` or fixed-size arrays is unsupported. - RResult AddProjectedField(std::unique_ptr field, FieldMappingFunc_t mapping); + RResult AddProjectedField(std::unique_ptr field, const FieldMappingFunc_t &mapping); /// Transitions an RNTupleModel from the *building* state to the *frozen* state, disabling adding additional fields /// and enabling creating entries from it. Freezing an already-frozen model is a no-op. Throws an RException if the @@ -418,7 +418,7 @@ struct RNTupleModelChangeset { /// \see RNTupleModel::AddProjectedField() ROOT::RResult - AddProjectedField(std::unique_ptr field, RNTupleModel::FieldMappingFunc_t mapping); + AddProjectedField(std::unique_ptr field, const RNTupleModel::FieldMappingFunc_t &mapping); }; } // namespace Internal @@ -459,7 +459,7 @@ public: void AddField(std::unique_ptr field, std::string_view parentName = ""); /// \see RNTupleModel::AddProjectedField() - RResult AddProjectedField(std::unique_ptr field, FieldMappingFunc_t mapping); + RResult AddProjectedField(std::unique_ptr field, const FieldMappingFunc_t &mapping); }; } // namespace ROOT diff --git a/tree/ntuple/inc/ROOT/RNTupleProcessorEntry.hxx b/tree/ntuple/inc/ROOT/RNTupleProcessorEntry.hxx index d72ac8f282247..42094516b8129 100644 --- a/tree/ntuple/inc/ROOT/RNTupleProcessorEntry.hxx +++ b/tree/ntuple/inc/ROOT/RNTupleProcessorEntry.hxx @@ -107,7 +107,7 @@ private: fQualifiedFieldName(qualifiedFieldName), fValue(std::move(value)), fIsValid(isValid), - fProcessorProvenance(provenance) + fProcessorProvenance(std::move(provenance)) { } }; diff --git a/tree/ntuple/inc/ROOT/RNTupleReader.hxx b/tree/ntuple/inc/ROOT/RNTupleReader.hxx index 8f3e8e191d3d2..2f9c118f18c38 100644 --- a/tree/ntuple/inc/ROOT/RNTupleReader.hxx +++ b/tree/ntuple/inc/ROOT/RNTupleReader.hxx @@ -166,7 +166,7 @@ public: void DeactivateEntry(NTupleSize_t entryNumber); explicit RActiveEntryToken(std::shared_ptr ptrControlBlock) - : fPtrControlBlock(ptrControlBlock) + : fPtrControlBlock(std::move(ptrControlBlock)) { } diff --git a/tree/ntuple/inc/ROOT/RNTupleView.hxx b/tree/ntuple/inc/ROOT/RNTupleView.hxx index 7dcf187e6f3c7..f963b1c3793c9 100644 --- a/tree/ntuple/inc/ROOT/RNTupleView.hxx +++ b/tree/ntuple/inc/ROOT/RNTupleView.hxx @@ -115,7 +115,7 @@ protected: } RNTupleViewBase(std::unique_ptr field, ROOT::RNTupleGlobalRange range, std::shared_ptr objPtr) - : fField(std::move(field)), fFieldRange(range), fValue(fField->BindValue(objPtr)) + : fField(std::move(field)), fFieldRange(range), fValue(fField->BindValue(std::move(objPtr))) { } @@ -232,7 +232,7 @@ protected: } RNTupleView(std::unique_ptr field, ROOT::RNTupleGlobalRange range, std::shared_ptr objPtr) - : RNTupleViewBase(std::move(field), range, objPtr) + : RNTupleViewBase(std::move(field), range, std::move(objPtr)) { } diff --git a/tree/ntuple/inc/ROOT/RPageSinkBuf.hxx b/tree/ntuple/inc/ROOT/RPageSinkBuf.hxx index 53f081486c629..2153e79fe3003 100644 --- a/tree/ntuple/inc/ROOT/RPageSinkBuf.hxx +++ b/tree/ntuple/inc/ROOT/RPageSinkBuf.hxx @@ -118,7 +118,7 @@ class RPageSinkBuf : public RPageSink { ROOT::DescriptorId_t fNColumns = 0; void ConnectFields(const std::vector &fields, ROOT::NTupleSize_t firstEntry); - void FlushClusterImpl(std::function FlushClusterFn); + void FlushClusterImpl(const std::function &FlushClusterFn); void InitImpl(ROOT::RNTupleModel &model) final; RNTupleLink CommitDatasetImpl() final; diff --git a/tree/ntuple/inc/ROOT/RPageStorage.hxx b/tree/ntuple/inc/ROOT/RPageStorage.hxx index 400b8b47ec588..df661029f5d8f 100644 --- a/tree/ntuple/inc/ROOT/RPageStorage.hxx +++ b/tree/ntuple/inc/ROOT/RPageStorage.hxx @@ -135,8 +135,8 @@ public: SealedPageSequence_t::const_iterator fLast; RSealedPageGroup() = default; - RSealedPageGroup(ROOT::DescriptorId_t d, SealedPageSequence_t::const_iterator b, - SealedPageSequence_t::const_iterator e) + RSealedPageGroup(ROOT::DescriptorId_t d, const SealedPageSequence_t::const_iterator &b, + const SealedPageSequence_t::const_iterator &e) : fPhysicalColumnId(d), fFirst(b), fLast(e) { } @@ -399,7 +399,7 @@ public: virtual void CommitAttributeSet(std::string_view attrSetName, const RNTupleLink &attrAnchorInfo) = 0; /// The registered callback is executed at the beginning of CommitDataset(); - void RegisterOnCommitDatasetCallback(Callback_t callback) { fOnDatasetCommitCallbacks.emplace_back(callback); } + void RegisterOnCommitDatasetCallback(const Callback_t &cb) { fOnDatasetCommitCallbacks.emplace_back(cb); } /// Run the registered callbacks and finalize the current cluster and the entrire data set. RNTupleLink CommitDataset(); @@ -746,10 +746,10 @@ protected: /// Prepare a page range read for the column set in `clusterKey`. Specifically, pages referencing the /// `kTypePageZero` locator are filled in `pageZeroMap`; otherwise, `perPageFunc` is called for each page. This is /// commonly used as part of `LoadClusters()` in derived classes. - void PrepareLoadCluster( - const ROOT::Internal::RCluster::RKey &clusterKey, ROOT::Internal::ROnDiskPageMap &pageZeroMap, - std::function - perPageFunc); + void PrepareLoadCluster(const ROOT::Internal::RCluster::RKey &clusterKey, + ROOT::Internal::ROnDiskPageMap &pageZeroMap, + const std::function &perPageFunc); /// Enables the default set of metrics provided by RPageSource. `prefix` will be used as the prefix for /// the counters registered in the internal RNTupleMetrics object. diff --git a/tree/ntuple/src/RFieldBase.cxx b/tree/ntuple/src/RFieldBase.cxx index 40f7129b836eb..cfd884dbbd70c 100644 --- a/tree/ntuple/src/RFieldBase.cxx +++ b/tree/ntuple/src/RFieldBase.cxx @@ -773,7 +773,7 @@ ROOT::RFieldBase::RBulkValues ROOT::RFieldBase::CreateBulk() ROOT::RFieldBase::RValue ROOT::RFieldBase::BindValue(std::shared_ptr objPtr) { - return RValue(this, objPtr); + return RValue(this, std::move(objPtr)); } std::size_t ROOT::RFieldBase::ReadBulk(const RBulkSpec &bulkSpec) @@ -893,7 +893,7 @@ ROOT::RFieldBase::EnsureCompatibleColumnTypes(const ROOT::RNTupleDescriptor &des "(representation index: " + std::to_string(representationIndex) + ")")); } -size_t ROOT::RFieldBase::AddReadCallback(ReadCallback_t func) +size_t ROOT::RFieldBase::AddReadCallback(const ReadCallback_t &func) { fReadCallbacks.push_back(func); fIsSimple = false; diff --git a/tree/ntuple/src/RFieldMeta.cxx b/tree/ntuple/src/RFieldMeta.cxx index cb62fe1e33291..5b97f86383703 100644 --- a/tree/ntuple/src/RFieldMeta.cxx +++ b/tree/ntuple/src/RFieldMeta.cxx @@ -402,7 +402,7 @@ ROOT::DescriptorId_t ROOT::RClassField::LookupMember(const ROOT::RNTupleDescript return idSourceMember; for (const auto &subFieldDesc : desc.GetFieldIterable(classFieldId)) { - const auto subFieldName = subFieldDesc.GetFieldName(); + const auto &subFieldName = subFieldDesc.GetFieldName(); if (subFieldName.length() > 2 && subFieldName[0] == ':' && subFieldName[1] == '_') { idSourceMember = LookupMember(desc, memberName, subFieldDesc.GetId()); if (idSourceMember != ROOT::kInvalidDescriptorId) @@ -1250,14 +1250,14 @@ std::unique_ptr ROOT::RProxiedCollectionField::GetDe ROOT::RProxiedCollectionField::RProxiedCollectionDeleter::RProxiedCollectionDeleter( std::shared_ptr proxy) - : RDeleter(proxy->GetCollectionClass()->GetClassAlignment()), fProxy(proxy) + : RDeleter(proxy->GetCollectionClass()->GetClassAlignment()), fProxy(std::move(proxy)) { } ROOT::RProxiedCollectionField::RProxiedCollectionDeleter::RProxiedCollectionDeleter( std::shared_ptr proxy, std::unique_ptr itemDeleter, size_t itemSize) : RDeleter(proxy->GetCollectionClass()->GetClassAlignment()), - fProxy(proxy), + fProxy(std::move(proxy)), fItemDeleter(std::move(itemDeleter)), fItemSize(itemSize) { @@ -1391,7 +1391,7 @@ class TBufferRecStreamer : public TBufferFile { public: TBufferRecStreamer(TBuffer::EMode mode, Int_t bufsize, RCallbackStreamerInfo callbackStreamerInfo) - : TBufferFile(mode, bufsize), fCallbackStreamerInfo(callbackStreamerInfo) + : TBufferFile(mode, bufsize), fCallbackStreamerInfo(std::move(callbackStreamerInfo)) { } void TagStreamerInfo(TVirtualStreamerInfo *info) final { fCallbackStreamerInfo(info); } diff --git a/tree/ntuple/src/RFieldSequenceContainer.cxx b/tree/ntuple/src/RFieldSequenceContainer.cxx index 4e42fc1b6b32d..1d5a713532fe7 100644 --- a/tree/ntuple/src/RFieldSequenceContainer.cxx +++ b/tree/ntuple/src/RFieldSequenceContainer.cxx @@ -15,7 +15,7 @@ namespace { -std::vector SplitVector(std::shared_ptr valuePtr, ROOT::RFieldBase &itemField) +std::vector SplitVector(const std::shared_ptr &valuePtr, ROOT::RFieldBase &itemField) { auto *vec = static_cast *>(valuePtr.get()); const auto itemSize = itemField.GetValueSize(); diff --git a/tree/ntuple/src/RNTupleModel.cxx b/tree/ntuple/src/RNTupleModel.cxx index 5e3dcfb1ad374..77e2bc6792d11 100644 --- a/tree/ntuple/src/RNTupleModel.cxx +++ b/tree/ntuple/src/RNTupleModel.cxx @@ -276,8 +276,9 @@ void ROOT::RNTupleModel::RUpdater::AddField(std::unique_ptr fi fOpenChangeset.AddField(std::move(field), parentName); } -ROOT::RResult ROOT::Internal::RNTupleModelChangeset::AddProjectedField(std::unique_ptr field, - RNTupleModel::FieldMappingFunc_t mapping) +ROOT::RResult +ROOT::Internal::RNTupleModelChangeset::AddProjectedField(std::unique_ptr field, + const RNTupleModel::FieldMappingFunc_t &mapping) { auto fieldp = field.get(); auto result = fModel.AddProjectedField(std::move(field), mapping); @@ -286,10 +287,10 @@ ROOT::RResult ROOT::Internal::RNTupleModelChangeset::AddProjectedField(std return R__FORWARD_RESULT(result); } -ROOT::RResult -ROOT::RNTupleModel::RUpdater::AddProjectedField(std::unique_ptr field, FieldMappingFunc_t mapping) +ROOT::RResult ROOT::RNTupleModel::RUpdater::AddProjectedField(std::unique_ptr field, + const FieldMappingFunc_t &mapping) { - return R__FORWARD_RESULT(fOpenChangeset.AddProjectedField(std::move(field), std::move(mapping))); + return R__FORWARD_RESULT(fOpenChangeset.AddProjectedField(std::move(field), mapping)); } void ROOT::RNTupleModel::EnsureValidFieldName(std::string_view fieldName) @@ -457,7 +458,7 @@ void ROOT::RNTupleModel::RegisterSubfield(std::string_view qualifiedFieldName) } ROOT::RResult -ROOT::RNTupleModel::AddProjectedField(std::unique_ptr field, FieldMappingFunc_t mapping) +ROOT::RNTupleModel::AddProjectedField(std::unique_ptr field, const FieldMappingFunc_t &mapping) { EnsureNotFrozen(); if (!field) diff --git a/tree/ntuple/src/RNTupleProcessor.cxx b/tree/ntuple/src/RNTupleProcessor.cxx index a6e3fb7c21910..b8b251da50fdf 100644 --- a/tree/ntuple/src/RNTupleProcessor.cxx +++ b/tree/ntuple/src/RNTupleProcessor.cxx @@ -78,9 +78,9 @@ ROOT::Experimental::RNTupleProcessor::CreateJoin(RNTupleOpenSpec primaryNTuple, throw RException(R__FAIL("join fields must be unique")); } - std::unique_ptr primaryProcessor = Create(primaryNTuple, processorName); + std::unique_ptr primaryProcessor = Create(std::move(primaryNTuple), processorName); - std::unique_ptr auxProcessor = Create(auxNTuple); + std::unique_ptr auxProcessor = Create(std::move(auxNTuple)); return CreateJoin(std::move(primaryProcessor), std::move(auxProcessor), joinFields, processorName); } diff --git a/tree/ntuple/src/RPageSinkBuf.cxx b/tree/ntuple/src/RPageSinkBuf.cxx index e516e410cabb9..7720239f1cb83 100644 --- a/tree/ntuple/src/RPageSinkBuf.cxx +++ b/tree/ntuple/src/RPageSinkBuf.cxx @@ -260,7 +260,7 @@ void ROOT::Internal::RPageSinkBuf::CommitSealedPageV( // We implement both StageCluster() and CommitCluster() because we can call CommitCluster() on the inner sink more // efficiently in a single critical section. For parallel writing, it also guarantees that we produce a fully sequential // file. -void ROOT::Internal::RPageSinkBuf::FlushClusterImpl(std::function FlushClusterFn) +void ROOT::Internal::RPageSinkBuf::FlushClusterImpl(const std::function &FlushClusterFn) { WaitForAllTasks(); assert(fBufferedUncompressed == 0 && "all buffered pages should have been processed"); diff --git a/tree/ntuple/src/RPageStorage.cxx b/tree/ntuple/src/RPageStorage.cxx index eea646494b23c..581a72f1af6c9 100644 --- a/tree/ntuple/src/RPageStorage.cxx +++ b/tree/ntuple/src/RPageStorage.cxx @@ -332,8 +332,8 @@ void ROOT::Internal::RPageSource::UnzipClusterImpl(RCluster *cluster) void ROOT::Internal::RPageSource::PrepareLoadCluster( const RCluster::RKey &clusterKey, ROnDiskPageMap &pageZeroMap, - std::function - perPageFunc) + const std::function + &perPageFunc) { auto descriptorGuard = GetSharedDescriptorGuard(); const auto &clusterDesc = descriptorGuard->GetClusterDescriptor(clusterKey.fClusterId); diff --git a/tree/ntupleutil/inc/ROOT/RNTupleImporter.hxx b/tree/ntupleutil/inc/ROOT/RNTupleImporter.hxx index 912ea6e0cc5bd..ab68dd70faef4 100644 --- a/tree/ntupleutil/inc/ROOT/RNTupleImporter.hxx +++ b/tree/ntupleutil/inc/ROOT/RNTupleImporter.hxx @@ -254,7 +254,7 @@ public: static std::unique_ptr Create(TTree *sourceTree, std::string_view destFileName); ROOT::RNTupleWriteOptions GetWriteOptions() const { return fWriteOptions; } - void SetWriteOptions(ROOT::RNTupleWriteOptions options) { fWriteOptions = options; } + void SetWriteOptions(const ROOT::RNTupleWriteOptions &options) { fWriteOptions = options; } void SetNTupleName(const std::string &name) { fNTupleName = name; } void SetMaxEntries(std::uint64_t maxEntries) { fMaxEntries = maxEntries; }; @@ -267,7 +267,7 @@ public: /// Add custom method to adjust column representations. Will be called for every field of the frozen model /// before it is attached to the page sink - void SetFieldModifier(FieldModifier_t modifier) { fFieldModifier = modifier; } + void SetFieldModifier(const FieldModifier_t &modifier) { fFieldModifier = modifier; } /// Import works in two steps: /// 1. PrepareSchema() calls SetBranchAddress() on all the TTree branches and creates the corresponding RNTuple From a7c3cf2960b57a64ad54eedf74f2a606e914a1ad Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Thu, 28 May 2026 11:31:55 +0200 Subject: [PATCH 23/25] [ntuple] mark some move constructors noexcept --- tree/ntuple/inc/ROOT/RFieldBase.hxx | 8 ++++---- tree/ntuple/inc/ROOT/RNTupleAttrWriting.hxx | 4 ++-- tree/ntuple/inc/ROOT/RNTupleReader.hxx | 4 ++-- tree/ntuple/inc/ROOT/RNTupleView.hxx | 4 ++-- tree/ntuple/inc/ROOT/RPageSinkBuf.hxx | 4 ++-- tree/ntuple/src/RFieldBase.cxx | 4 ++-- tree/ntuple/src/RNTupleReader.cxx | 5 +++-- 7 files changed, 17 insertions(+), 16 deletions(-) diff --git a/tree/ntuple/inc/ROOT/RFieldBase.hxx b/tree/ntuple/inc/ROOT/RFieldBase.hxx index 2623dc5300fc7..a60ee5c2c83bf 100644 --- a/tree/ntuple/inc/ROOT/RFieldBase.hxx +++ b/tree/ntuple/inc/ROOT/RFieldBase.hxx @@ -790,8 +790,8 @@ public: fTypeInfo = nullptr; return *this; } - RValue(RValue &&other) : fField(other.fField), fObjPtr(std::move(other.fObjPtr)) {} - RValue &operator=(RValue &&other) + RValue(RValue &&other) noexcept : fField(other.fField), fObjPtr(std::move(other.fObjPtr)) {} + RValue &operator=(RValue &&other) noexcept { fField = other.fField; fObjPtr = other.fObjPtr; @@ -927,8 +927,8 @@ public: ~RBulkValues(); RBulkValues(const RBulkValues &) = delete; RBulkValues &operator=(const RBulkValues &) = delete; - RBulkValues(RBulkValues &&other); - RBulkValues &operator=(RBulkValues &&other); + RBulkValues(RBulkValues &&other) noexcept; + RBulkValues &operator=(RBulkValues &&other) noexcept; // Sets `fValues` and `fSize`/`fCapacity` to the given values. The capacity is specified in number of values. // Once a buffer is adopted, an attempt to read more values then available throws an exception. diff --git a/tree/ntuple/inc/ROOT/RNTupleAttrWriting.hxx b/tree/ntuple/inc/ROOT/RNTupleAttrWriting.hxx index bc5861fd990e1..76fed96f23ead 100644 --- a/tree/ntuple/inc/ROOT/RNTupleAttrWriting.hxx +++ b/tree/ntuple/inc/ROOT/RNTupleAttrWriting.hxx @@ -75,10 +75,10 @@ public: RNTupleAttrPendingRange(const RNTupleAttrPendingRange &) = delete; RNTupleAttrPendingRange &operator=(const RNTupleAttrPendingRange &) = delete; - RNTupleAttrPendingRange(RNTupleAttrPendingRange &&other) { *this = std::move(other); } + RNTupleAttrPendingRange(RNTupleAttrPendingRange &&other) noexcept { *this = std::move(other); } // NOTE: explicitly implemented to make sure that 'other' gets invalidated upon move. - RNTupleAttrPendingRange &operator=(RNTupleAttrPendingRange &&other) + RNTupleAttrPendingRange &operator=(RNTupleAttrPendingRange &&other) noexcept { if (&other != this) { std::swap(fStart, other.fStart); diff --git a/tree/ntuple/inc/ROOT/RNTupleReader.hxx b/tree/ntuple/inc/ROOT/RNTupleReader.hxx index 2f9c118f18c38..c93d558c89b81 100644 --- a/tree/ntuple/inc/ROOT/RNTupleReader.hxx +++ b/tree/ntuple/inc/ROOT/RNTupleReader.hxx @@ -173,9 +173,9 @@ public: public: ~RActiveEntryToken() { Reset(); } RActiveEntryToken(const RActiveEntryToken &other); - RActiveEntryToken(RActiveEntryToken &&other); + RActiveEntryToken(RActiveEntryToken &&other) noexcept; RActiveEntryToken &operator=(const RActiveEntryToken &other); - RActiveEntryToken &operator=(RActiveEntryToken &&other); + RActiveEntryToken &operator=(RActiveEntryToken &&other) noexcept; NTupleSize_t GetEntryNumber() const { return fEntryNumber; } /// Set or replace the entry number. If the entry number is replaced, the cluster corresponding to the new diff --git a/tree/ntuple/inc/ROOT/RNTupleView.hxx b/tree/ntuple/inc/ROOT/RNTupleView.hxx index f963b1c3793c9..8acad94ebbacd 100644 --- a/tree/ntuple/inc/ROOT/RNTupleView.hxx +++ b/tree/ntuple/inc/ROOT/RNTupleView.hxx @@ -365,11 +365,11 @@ private: public: RNTupleCollectionView(const RNTupleCollectionView &other) = delete; RNTupleCollectionView &operator=(const RNTupleCollectionView &other) = delete; - RNTupleCollectionView(RNTupleCollectionView &&other) + RNTupleCollectionView(RNTupleCollectionView &&other) noexcept(false) : fSource(other.fSource), fField(std::move(other.fField)), fValue(fField.CreateValue()) { } - RNTupleCollectionView &operator=(RNTupleCollectionView &&other) + RNTupleCollectionView &operator=(RNTupleCollectionView &&other) noexcept(false) { if (this == &other) return *this; diff --git a/tree/ntuple/inc/ROOT/RPageSinkBuf.hxx b/tree/ntuple/inc/ROOT/RPageSinkBuf.hxx index 2153e79fe3003..da6d8db1a6a9f 100644 --- a/tree/ntuple/inc/ROOT/RPageSinkBuf.hxx +++ b/tree/ntuple/inc/ROOT/RPageSinkBuf.hxx @@ -52,8 +52,8 @@ class RPageSinkBuf : public RPageSink { RColumnBuf() = default; RColumnBuf(const RColumnBuf&) = delete; RColumnBuf& operator=(const RColumnBuf&) = delete; - RColumnBuf(RColumnBuf&&) = default; - RColumnBuf& operator=(RColumnBuf&&) = default; + RColumnBuf(RColumnBuf &&) noexcept = default; + RColumnBuf &operator=(RColumnBuf &&) noexcept = default; ~RColumnBuf() { DropBufferedPages(); } /// Returns a reference to the newly buffered page. The reference remains diff --git a/tree/ntuple/src/RFieldBase.cxx b/tree/ntuple/src/RFieldBase.cxx index cfd884dbbd70c..1a896167609de 100644 --- a/tree/ntuple/src/RFieldBase.cxx +++ b/tree/ntuple/src/RFieldBase.cxx @@ -129,7 +129,7 @@ void ROOT::RFieldBase::RValue::BindRawPtr(void *rawPtr) //------------------------------------------------------------------------------ -ROOT::RFieldBase::RBulkValues::RBulkValues(RBulkValues &&other) +ROOT::RFieldBase::RBulkValues::RBulkValues(RBulkValues &&other) noexcept : fField(other.fField), fValueSize(other.fValueSize), fCapacity(other.fCapacity), @@ -143,7 +143,7 @@ ROOT::RFieldBase::RBulkValues::RBulkValues(RBulkValues &&other) std::swap(fMaskAvail, other.fMaskAvail); } -ROOT::RFieldBase::RBulkValues &ROOT::RFieldBase::RBulkValues::operator=(RBulkValues &&other) +ROOT::RFieldBase::RBulkValues &ROOT::RFieldBase::RBulkValues::operator=(RBulkValues &&other) noexcept { std::swap(fField, other.fField); std::swap(fDeleter, other.fDeleter); diff --git a/tree/ntuple/src/RNTupleReader.cxx b/tree/ntuple/src/RNTupleReader.cxx index b2d0c4f8e5e62..3672603186efc 100644 --- a/tree/ntuple/src/RNTupleReader.cxx +++ b/tree/ntuple/src/RNTupleReader.cxx @@ -84,7 +84,7 @@ ROOT::RNTupleReader::RActiveEntryToken::RActiveEntryToken(const RActiveEntryToke SetEntryNumber(other.fEntryNumber); } -ROOT::RNTupleReader::RActiveEntryToken::RActiveEntryToken(RActiveEntryToken &&other) +ROOT::RNTupleReader::RActiveEntryToken::RActiveEntryToken(RActiveEntryToken &&other) noexcept { std::swap(fEntryNumber, other.fEntryNumber); std::swap(fPtrControlBlock, other.fPtrControlBlock); @@ -105,7 +105,8 @@ ROOT::RNTupleReader::RActiveEntryToken::operator=(const RActiveEntryToken &other return *this; } -ROOT::RNTupleReader::RActiveEntryToken &ROOT::RNTupleReader::RActiveEntryToken::operator=(RActiveEntryToken &&other) +ROOT::RNTupleReader::RActiveEntryToken & +ROOT::RNTupleReader::RActiveEntryToken::operator=(RActiveEntryToken &&other) noexcept { std::swap(fEntryNumber, other.fEntryNumber); std::swap(fPtrControlBlock, other.fPtrControlBlock); From bc7fc7e74d28d09adcb0073213a860b44434e3ab Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Thu, 28 May 2026 13:45:54 +0200 Subject: [PATCH 24/25] [ntuple] fix virtual destructor visibility --- tree/ntuple/inc/ROOT/RField/RFieldFundamental.hxx | 3 +-- tree/ntuple/inc/ROOT/RFieldVisitor.hxx | 2 ++ 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/tree/ntuple/inc/ROOT/RField/RFieldFundamental.hxx b/tree/ntuple/inc/ROOT/RField/RFieldFundamental.hxx index 987f6799a81a8..044bce5e3dfb1 100644 --- a/tree/ntuple/inc/ROOT/RField/RFieldFundamental.hxx +++ b/tree/ntuple/inc/ROOT/RField/RFieldFundamental.hxx @@ -412,14 +412,13 @@ protected: fPrincipalColumn = fAvailableColumns[0].get(); } - ~RRealField() override = default; - public: using Base::SetColumnRepresentatives; RRealField(std::string_view name, std::string_view typeName) : RSimpleField(name, typeName) {} RRealField(RRealField &&other) = default; RRealField &operator=(RRealField &&other) = default; + ~RRealField() override = default; /// Sets this field to use a half precision representation, occupying half as much storage space (16 bits: /// 1 sign bit, 5 exponent bits, 10 mantissa bits) on disk. diff --git a/tree/ntuple/inc/ROOT/RFieldVisitor.hxx b/tree/ntuple/inc/ROOT/RFieldVisitor.hxx index 8e2fb0c53a71b..01bca5bf7a49b 100644 --- a/tree/ntuple/inc/ROOT/RFieldVisitor.hxx +++ b/tree/ntuple/inc/ROOT/RFieldVisitor.hxx @@ -56,6 +56,8 @@ As an example of a concrete use case, see Internal::RPrintSchemaVisitor. // clang-format on class RFieldVisitor { public: + virtual ~RFieldVisitor() = default; + virtual void VisitField(const ROOT::RFieldBase &field) = 0; virtual void VisitFieldZero(const ROOT::RFieldZero &field) { VisitField(field); } virtual void VisitArrayField(const ROOT::RArrayField &field) { VisitField(field); } From f7ab0192f36cea3f4bae949c2a3513416fa03dfb Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Thu, 28 May 2026 13:52:06 +0200 Subject: [PATCH 25/25] [ntuple] fix up RNTupleDescriptorBuilder::SetNTuple() signature --- tree/ntuple/inc/ROOT/RNTupleDescriptor.hxx | 2 +- tree/ntuple/src/RNTupleDescriptor.cxx | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/tree/ntuple/inc/ROOT/RNTupleDescriptor.hxx b/tree/ntuple/inc/ROOT/RNTupleDescriptor.hxx index 6a1c35ea264ac..f8c42e30f6e4f 100644 --- a/tree/ntuple/inc/ROOT/RNTupleDescriptor.hxx +++ b/tree/ntuple/inc/ROOT/RNTupleDescriptor.hxx @@ -1763,7 +1763,7 @@ public: std::uint16_t versionPatch); void SetVersionForWriting(); - void SetNTuple(const std::string_view name, const std::string_view description); + void SetNTuple(std::string_view name, std::string_view description); /// Sets the `flag`-th bit of the feature flag to 1. /// Note that `flag` itself is not a bitmask, just the bit index of the flag to enable. void SetFeature(unsigned int flag); diff --git a/tree/ntuple/src/RNTupleDescriptor.cxx b/tree/ntuple/src/RNTupleDescriptor.cxx index ebfed9d89b747..0e86459c8b663 100644 --- a/tree/ntuple/src/RNTupleDescriptor.cxx +++ b/tree/ntuple/src/RNTupleDescriptor.cxx @@ -1093,8 +1093,7 @@ void ROOT::Internal::RNTupleDescriptorBuilder::SetVersionForWriting() fDescriptor.fVersionPatch = RNTuple::kVersionPatch; } -void ROOT::Internal::RNTupleDescriptorBuilder::SetNTuple(const std::string_view name, - const std::string_view description) +void ROOT::Internal::RNTupleDescriptorBuilder::SetNTuple(std::string_view name, std::string_view description) { fDescriptor.fName = std::string(name); fDescriptor.fDescription = std::string(description);