From 831abdd7d286035c9ff023b215755d0480572810 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alberto=20Francisco=20Solaz=20Garc=C3=ADa?= Date: Wed, 19 Aug 2026 13:04:46 +0200 Subject: [PATCH 1/3] [ntuple] Mark ROOT_EXPORT_RNTUPLE_METRICS as experimental --- roottest/root/ntuple/metrics/CMakeLists.txt | 2 +- tree/ntuple/inc/ROOT/RNTupleMetrics.hxx | 4 ++-- tree/ntuple/src/RNTupleMetrics.cxx | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/roottest/root/ntuple/metrics/CMakeLists.txt b/roottest/root/ntuple/metrics/CMakeLists.txt index 24fb47b881f7b..61b762854f78b 100644 --- a/roottest/root/ntuple/metrics/CMakeLists.txt +++ b/roottest/root/ntuple/metrics/CMakeLists.txt @@ -1,6 +1,6 @@ ROOTTEST_ADD_TEST(metrics_env_enabled MACRO test_rntuple_metrics_env.C - ENVIRONMENT ROOT_EXPORT_RNTUPLE_METRICS=metrics_env_enabled.root + ENVIRONMENT ROOT_EXPERIMENTAL_EXPORT_RNTUPLE_METRICS=metrics_env_enabled.root OUTREF metrics_env_enabled.ref) ROOTTEST_ADD_TEST(metrics_env_disabled diff --git a/tree/ntuple/inc/ROOT/RNTupleMetrics.hxx b/tree/ntuple/inc/ROOT/RNTupleMetrics.hxx index 965234e254851..87f522cff9685 100644 --- a/tree/ntuple/inc/ROOT/RNTupleMetrics.hxx +++ b/tree/ntuple/inc/ROOT/RNTupleMetrics.hxx @@ -283,8 +283,8 @@ using RNTupleAtomicTimer = RNTupleTimer std::string { - if (const char *env = gSystem->Getenv("ROOT_EXPORT_RNTUPLE_METRICS"); env && *env) + if (const char *env = gSystem->Getenv("ROOT_EXPERIMENTAL_EXPORT_RNTUPLE_METRICS"); env && *env) return env; return ""; }(); From f353a2ac35f6c2aae515e6ebfd0e093fafbdce03 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alberto=20Francisco=20Solaz=20Garc=C3=ADa?= Date: Wed, 19 Aug 2026 13:08:01 +0200 Subject: [PATCH 2/3] [ntuple] Fix GetMetricsExportPath caching bug The export path was memoized in a local static variable, so it was fixed by the first call and ignored later changes. I triggered this bug when needed to set ROOT_EXPORT_RNTUPLE_METRICS via gSystem->Setenv(). --- tree/ntuple/inc/ROOT/RNTupleMetrics.hxx | 2 +- tree/ntuple/src/RNTupleMetrics.cxx | 12 +++++------- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/tree/ntuple/inc/ROOT/RNTupleMetrics.hxx b/tree/ntuple/inc/ROOT/RNTupleMetrics.hxx index 87f522cff9685..8be60714bb6e2 100644 --- a/tree/ntuple/inc/ROOT/RNTupleMetrics.hxx +++ b/tree/ntuple/inc/ROOT/RNTupleMetrics.hxx @@ -332,7 +332,7 @@ public: const RNTuplePerfCounter *GetCounter(std::string_view name) const; void ObserveMetrics(RNTupleMetrics &observee); - static const std::string &GetMetricsExportPath(); + static std::string GetMetricsExportPath(); void Print(std::ostream &output, const std::string &prefix = "") const; void Enable(); diff --git a/tree/ntuple/src/RNTupleMetrics.cxx b/tree/ntuple/src/RNTupleMetrics.cxx index 37f1ceefbd5ec..ce52861c11b3d 100644 --- a/tree/ntuple/src/RNTupleMetrics.cxx +++ b/tree/ntuple/src/RNTupleMetrics.cxx @@ -93,12 +93,10 @@ void ROOT::Experimental::Detail::RNTupleMetrics::ObserveMetrics(RNTupleMetrics & fObservedMetrics.push_back(&observee); } -const std::string &ROOT::Experimental::Detail::RNTupleMetrics::GetMetricsExportPath() +std::string ROOT::Experimental::Detail::RNTupleMetrics::GetMetricsExportPath() { - static const std::string path = []() -> std::string { - if (const char *env = gSystem->Getenv("ROOT_EXPERIMENTAL_EXPORT_RNTUPLE_METRICS"); env && *env) - return env; - return ""; - }(); - return path; + if (const char *env = gSystem->Getenv("ROOT_EXPERIMENTAL_EXPORT_RNTUPLE_METRICS"); env && *env) + return env; + + return {}; } From 455e75313670265560a45ea25ab134af8ee94658 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alberto=20Francisco=20Solaz=20Garc=C3=ADa?= Date: Wed, 19 Aug 2026 13:09:02 +0200 Subject: [PATCH 3/3] [ntuple] Add new RNTupleMetrics API Implement the desired behaviour of `ROOT_EXPERIMENTAL_EXPORT_RNTUPLE_METRICS`. Previously it just enabled all metrics automatically, ignoring the value passed to the variable. As of this commit, the new environment variable to allow the user to state its intention to activate metrics and where to store them in a usable way which doesn't require source code modifications. If the environment variable `ROOT_EXPERIMENTAL_EXPORT_RNTUPLE_METRICS` is set, metrics are automatically: - Enabled on construction, and any counter added afterwards through MakeCounter() is enabled as well. - Stored in the `.root` file path (relative or absolute) set by `ROOT_EXPORT_RNTUPLE_METRICS` on destruction The generated .root file will contain one RNTuple for each component with its metrics enabled. Each RNTuple will have a flat schema consisting of fully qualified counter names, such as RPageSinkFile_timeCpuZip. There is one entry per counter, which means if a counter appears more than once in a given RNTuple, all instances will be merged. Comes with a unit test. Modifies the output of the integration test roottest-root-ntuple-metrics-metrics_env_enabled, as the implementation of the automatic metrics storage uses the RNTupleMerger under the hood, which triggers a warning message. --- .../ntuple/metrics/metrics_env_enabled.ref | 1 + tree/ntuple/inc/ROOT/RNTupleMetrics.hxx | 24 +++- tree/ntuple/src/RNTupleMetrics.cxx | 104 +++++++++++++++++- tree/ntuple/src/RNTupleParallelWriter.cxx | 2 +- tree/ntuple/src/RNTupleReader.cxx | 4 +- tree/ntuple/src/RNTupleWriter.cxx | 2 +- tree/ntuple/test/ntuple_metrics.cxx | 97 ++++++++++++++++ 7 files changed, 224 insertions(+), 10 deletions(-) diff --git a/roottest/root/ntuple/metrics/metrics_env_enabled.ref b/roottest/root/ntuple/metrics/metrics_env_enabled.ref index 289372a209b92..378512a40c9b7 100644 --- a/roottest/root/ntuple/metrics/metrics_env_enabled.ref +++ b/roottest/root/ntuple/metrics/metrics_env_enabled.ref @@ -1,3 +1,4 @@ Processing test_rntuple_metrics_env.C... Metrics enabled: true nPageCommitted: 1 +Warning in : Merging RNTuples is experimental diff --git a/tree/ntuple/inc/ROOT/RNTupleMetrics.hxx b/tree/ntuple/inc/ROOT/RNTupleMetrics.hxx index 8be60714bb6e2..931ec984da9f8 100644 --- a/tree/ntuple/inc/ROOT/RNTupleMetrics.hxx +++ b/tree/ntuple/inc/ROOT/RNTupleMetrics.hxx @@ -295,22 +295,36 @@ private: std::vector> fCounters; std::vector fObservedMetrics; std::string fName; + std::string fNTupleName; + std::string fExportPath; bool fIsEnabled = false; bool Contains(const std::string &name) const; + /// Retrieves each counter of the observed metrics down to the class instances which own the counters and pairs them + /// with their fully qualified names + void CollectCounters(std::vector> &counters, + const std::string &prefix = "") const; + void ExportToRootFile(); + public: - explicit RNTupleMetrics(const std::string &name) : fName(name) + explicit RNTupleMetrics(const std::string &name) : RNTupleMetrics(name, "") {} + RNTupleMetrics(const std::string &name, const std::string &ntupleName) : fName(name), fNTupleName(ntupleName) { - // TODO: Use the value of `GetMetricsExportPath` to save the contents of the metrics in a `.root` file - if (!GetMetricsExportPath().empty()) - Enable(); + const std::string exportPath = GetMetricsExportPath(); + if (exportPath.empty()) + return; + + if (!fNTupleName.empty()) + fExportPath = exportPath; + + Enable(); } RNTupleMetrics(const RNTupleMetrics &other) = delete; RNTupleMetrics & operator=(const RNTupleMetrics &other) = delete; RNTupleMetrics(RNTupleMetrics &&other) = default; RNTupleMetrics & operator=(RNTupleMetrics &&other) = default; - ~RNTupleMetrics() = default; + ~RNTupleMetrics(); // TODO(jblomer): return a reference template diff --git a/tree/ntuple/src/RNTupleMetrics.cxx b/tree/ntuple/src/RNTupleMetrics.cxx index ce52861c11b3d..29025159176e8 100644 --- a/tree/ntuple/src/RNTupleMetrics.cxx +++ b/tree/ntuple/src/RNTupleMetrics.cxx @@ -14,11 +14,40 @@ #include +#include +#include +#include +#include +#include + +#include +#include +#include #include +#include +#include #include +#include -#include +namespace { + +std::mutex &GetMetricsExportMutex() +{ + static std::mutex mutex; + return mutex; +} + +thread_local bool gSuppressMetricsExport = false; + +struct RSuppressMetricsRaii { + RSuppressMetricsRaii() { gSuppressMetricsExport = true; } + RSuppressMetricsRaii(const RSuppressMetricsRaii &other) = delete; + RSuppressMetricsRaii &operator=(const RSuppressMetricsRaii &other) = delete; + ~RSuppressMetricsRaii() { gSuppressMetricsExport = false; } +}; + +} // anonymous namespace ROOT::Experimental::Detail::RNTuplePerfCounter::~RNTuplePerfCounter() { @@ -95,8 +124,81 @@ void ROOT::Experimental::Detail::RNTupleMetrics::ObserveMetrics(RNTupleMetrics & std::string ROOT::Experimental::Detail::RNTupleMetrics::GetMetricsExportPath() { + if (gSuppressMetricsExport) + return {}; + if (const char *env = gSystem->Getenv("ROOT_EXPERIMENTAL_EXPORT_RNTUPLE_METRICS"); env && *env) return env; return {}; } + +ROOT::Experimental::Detail::RNTupleMetrics::~RNTupleMetrics() +{ + if (!fIsEnabled || fExportPath.empty()) + return; + + ExportToRootFile(); +} + +void ROOT::Experimental::Detail::RNTupleMetrics::CollectCounters( + std::vector> &counters, const std::string &prefix) const +{ + constexpr char kSeparator = '_'; + + R__ASSERT(!fName.empty()); + + std::string qualifiedName = prefix; + if (!qualifiedName.empty()) + qualifiedName += kSeparator; + qualifiedName += fName; + + for (const auto &counter : fCounters) + counters.emplace_back(qualifiedName + kSeparator + counter->GetName(), counter.get()); + + for (const auto *observed : fObservedMetrics) + observed->CollectCounters(counters, qualifiedName); +} + +void ROOT::Experimental::Detail::RNTupleMetrics::ExportToRootFile() +{ + R__ASSERT(!fExportPath.empty()); + R__ASSERT(!fNTupleName.empty()); + + std::vector> counters; + CollectCounters(counters); + R__ASSERT(!counters.empty()); + + // Avoid inifinite recursion (an RNTupleWriter is created to print the metrics) + RSuppressMetricsRaii suppressGuard; + + auto model = ROOT::RNTupleModel::Create(); + for (const auto &[fullyQualifiedName, counter] : counters) { + if (const auto *calc = dynamic_cast(counter)) + *model->MakeField(fullyQualifiedName) = calc->GetValue(); + else + *model->MakeField(fullyQualifiedName) = counter->GetValueAsInt(); + } + + TMemFile memoryFile(fNTupleName.c_str(), "RECREATE"); + { + auto writer = ROOT::RNTupleWriter::Append(std::move(model), fNTupleName, memoryFile); + writer->Fill(); + writer->CommitDataset(); + } + + std::lock_guard lock(GetMetricsExportMutex()); + + TFileMerger merger; + merger.SetMergeOptions(std::string_view("rntuple.MergingMode=Union")); + + if (!merger.OutputFile(fExportPath.c_str(), "UPDATE")) { + R__LOG_ERROR(ROOT::Internal::NTupleLog()) << "cannot open metrics export file '" << fExportPath << "'"; + return; + } + + if (!merger.AddFile(&memoryFile) || !merger.PartialMerge(TFileMerger::kAll | TFileMerger::kIncremental)) { + R__LOG_ERROR(ROOT::Internal::NTupleLog()) + << "cannot merge metrics for '" << fNTupleName << "' into '" << fExportPath << "'"; + } +} diff --git a/tree/ntuple/src/RNTupleParallelWriter.cxx b/tree/ntuple/src/RNTupleParallelWriter.cxx index 453d6805b9287..6729a4d317212 100644 --- a/tree/ntuple/src/RNTupleParallelWriter.cxx +++ b/tree/ntuple/src/RNTupleParallelWriter.cxx @@ -129,7 +129,7 @@ class RPageSynchronizingSink : public RPageSink { ROOT::RNTupleParallelWriter::RNTupleParallelWriter(std::unique_ptr model, std::unique_ptr sink) - : fSink(std::move(sink)), fModel(std::move(model)), fMetrics("RNTupleParallelWriter") + : fSink(std::move(sink)), fModel(std::move(model)), fMetrics("RNTupleParallelWriter", fSink->GetNTupleName()) { if (fModel->GetRegisteredSubfieldNames().size() > 0) { throw RException(R__FAIL("cannot create an RNTupleParallelWriter from a model with registered subfields")); diff --git a/tree/ntuple/src/RNTupleReader.cxx b/tree/ntuple/src/RNTupleReader.cxx index 3672603186efc..896f05e75c8cf 100644 --- a/tree/ntuple/src/RNTupleReader.cxx +++ b/tree/ntuple/src/RNTupleReader.cxx @@ -153,7 +153,7 @@ void ROOT::RNTupleReader::InitPageSource(bool enableMetrics) ROOT::RNTupleReader::RNTupleReader(std::unique_ptr model, std::unique_ptr source, const ROOT::RNTupleReadOptions &options) - : fSource(std::move(source)), fModel(std::move(model)), fMetrics("RNTupleReader") + : fSource(std::move(source)), fModel(std::move(model)), fMetrics("RNTupleReader", fSource->GetNTupleName()) { // TODO(jblomer): properly support projected fields auto &projectedFields = ROOT::Internal::GetProjectedFieldsOfModel(*fModel); @@ -167,7 +167,7 @@ ROOT::RNTupleReader::RNTupleReader(std::unique_ptr model, ROOT::RNTupleReader::RNTupleReader(std::unique_ptr source, const ROOT::RNTupleReadOptions &options) - : fSource(std::move(source)), fModel(nullptr), fMetrics("RNTupleReader") + : fSource(std::move(source)), fModel(nullptr), fMetrics("RNTupleReader", fSource->GetNTupleName()) { InitPageSource(options.GetEnableMetrics()); } diff --git a/tree/ntuple/src/RNTupleWriter.cxx b/tree/ntuple/src/RNTupleWriter.cxx index eec45995faf14..2c320794a6e69 100644 --- a/tree/ntuple/src/RNTupleWriter.cxx +++ b/tree/ntuple/src/RNTupleWriter.cxx @@ -37,7 +37,7 @@ static bool IsReservedRNTupleAttrSetName(std::string_view name) ROOT::RNTupleWriter::RNTupleWriter(std::unique_ptr model, std::unique_ptr sink) - : fFillContext(std::move(model), std::move(sink)), fMetrics("RNTupleWriter") + : fFillContext(std::move(model), std::move(sink)), fMetrics("RNTupleWriter", fFillContext.fSink->GetNTupleName()) { #ifdef R__USE_IMT if (IsImplicitMTEnabled() && diff --git a/tree/ntuple/test/ntuple_metrics.cxx b/tree/ntuple/test/ntuple_metrics.cxx index 4f7deb7d8262c..24bf2a8b23c53 100644 --- a/tree/ntuple/test/ntuple_metrics.cxx +++ b/tree/ntuple/test/ntuple_metrics.cxx @@ -1,3 +1,6 @@ +#include "TEnv.h" +#include "TKey.h" +#include "TSystem.h" #include "ntuple_test.hxx" #include @@ -154,3 +157,97 @@ TEST(Metrics, IOMetrics) EXPECT_GT(szFile->GetValueAsInt(), 0); } } + +namespace { + +void WriteDummyEntries(std::string_view fileName, std::string_view ntupleName) +{ + auto model = RNTupleModel::Create(); + auto pt = model->MakeField("float_field"); + + auto writer = RNTupleWriter::Recreate(std::move(model), ntupleName, fileName); + + *pt = 1.0; + writer->Fill(); + writer->CommitDataset(); +} + +void ReadMetrics(const std::string fileName, std::ostream &output) +{ + TFile file(fileName.c_str()); + + std::set ntupleNames; + for (auto *keyObject : *file.GetListOfKeys()) { + auto *key = static_cast(keyObject); + if (std::string(key->GetClassName()) == "ROOT::RNTuple") + ntupleNames.insert(key->GetName()); + } + + for (const auto &ntupleName : ntupleNames) { + auto reader = ROOT::RNTupleReader::Open(ntupleName, fileName); + const auto &descriptor = reader->GetDescriptor(); + + output << ntupleName << "\n\n"; + for (auto counterId : descriptor.GetFieldZero().GetLinkIds()) { + std::string fieldName = descriptor.GetFieldDescriptor(counterId).GetFieldName(); + std::string type = descriptor.GetFieldDescriptor(counterId).GetTypeName(); + + output << std::string(8, ' ') << fieldName << " (" << type << ")\n"; + } + output << "\n"; + } +} +} // namespace + +TEST(Metrics, EnvironmentVariableExport) +{ + FileRaii fileGuard("test_environment_variable_export_export.root"); + + const std::string metricsFileName = "environment_variable_export_metrics.root"; + + gSystem->Setenv("ROOT_EXPERIMENTAL_EXPORT_RNTUPLE_METRICS", metricsFileName.c_str()); + + WriteDummyEntries(fileGuard.GetPath().c_str(), "rntuple_name_1"); + WriteDummyEntries(fileGuard.GetPath().c_str(), "rntuple_name_1"); + WriteDummyEntries(fileGuard.GetPath().c_str(), "rntuple_name_2"); + + gSystem->Unsetenv("ROOT_EXPERIMENTAL_EXPORT_RNTUPLE_METRICS"); + + std::stringstream printedMetricsStream; + ReadMetrics(metricsFileName, printedMetricsStream); + + const std::string printedMetricsString = std::move(printedMetricsStream).str(); + const std::string expected = R"(rntuple_name_1 + + RNTupleWriter_RPageSinkBuf_ParallelZip (std::int64_t) + RNTupleWriter_RPageSinkBuf_timeWallZip (std::int64_t) + RNTupleWriter_RPageSinkBuf_timeWallCriticalSection (std::int64_t) + RNTupleWriter_RPageSinkBuf_timeCpuZip (std::int64_t) + RNTupleWriter_RPageSinkBuf_timeCpuCriticalSection (std::int64_t) + RNTupleWriter_RPageSinkBuf_RPageSinkFile_nPageCommitted (std::int64_t) + RNTupleWriter_RPageSinkBuf_RPageSinkFile_szWritePayload (std::int64_t) + RNTupleWriter_RPageSinkBuf_RPageSinkFile_szZip (std::int64_t) + RNTupleWriter_RPageSinkBuf_RPageSinkFile_timeWallWrite (std::int64_t) + RNTupleWriter_RPageSinkBuf_RPageSinkFile_timeWallZip (std::int64_t) + RNTupleWriter_RPageSinkBuf_RPageSinkFile_timeCpuWrite (std::int64_t) + RNTupleWriter_RPageSinkBuf_RPageSinkFile_timeCpuZip (std::int64_t) + +rntuple_name_2 + + RNTupleWriter_RPageSinkBuf_ParallelZip (std::int64_t) + RNTupleWriter_RPageSinkBuf_timeWallZip (std::int64_t) + RNTupleWriter_RPageSinkBuf_timeWallCriticalSection (std::int64_t) + RNTupleWriter_RPageSinkBuf_timeCpuZip (std::int64_t) + RNTupleWriter_RPageSinkBuf_timeCpuCriticalSection (std::int64_t) + RNTupleWriter_RPageSinkBuf_RPageSinkFile_nPageCommitted (std::int64_t) + RNTupleWriter_RPageSinkBuf_RPageSinkFile_szWritePayload (std::int64_t) + RNTupleWriter_RPageSinkBuf_RPageSinkFile_szZip (std::int64_t) + RNTupleWriter_RPageSinkBuf_RPageSinkFile_timeWallWrite (std::int64_t) + RNTupleWriter_RPageSinkBuf_RPageSinkFile_timeWallZip (std::int64_t) + RNTupleWriter_RPageSinkBuf_RPageSinkFile_timeCpuWrite (std::int64_t) + RNTupleWriter_RPageSinkBuf_RPageSinkFile_timeCpuZip (std::int64_t) + +)"; + + EXPECT_EQ(printedMetricsString, expected); +}