From baf0498c73b02c962a02e63da278e7af3762a1ae Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Fri, 26 Sep 2025 15:43:34 +0200 Subject: [PATCH 1/3] [ntuple] add multiply operator to RNTupleLocalIndex Used to scale and index by the repetition count of a field (i.e., for fixed-size array index arithmetic). --- tree/ntuple/inc/ROOT/RNTupleTypes.hxx | 5 +++++ tree/ntuple/src/RFieldSequenceContainer.cxx | 9 ++------- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/tree/ntuple/inc/ROOT/RNTupleTypes.hxx b/tree/ntuple/inc/ROOT/RNTupleTypes.hxx index aaad0036532a6..34c7180ae1362 100644 --- a/tree/ntuple/inc/ROOT/RNTupleTypes.hxx +++ b/tree/ntuple/inc/ROOT/RNTupleTypes.hxx @@ -163,6 +163,11 @@ public: return RNTupleLocalIndex(fClusterId, fIndexInCluster - off); } + RNTupleLocalIndex operator*(ROOT::NTupleSize_t repetitionFactor) const + { + return RNTupleLocalIndex(fClusterId, fIndexInCluster * repetitionFactor); + } + RNTupleLocalIndex operator++(int) /* postfix */ { auto r = *this; diff --git a/tree/ntuple/src/RFieldSequenceContainer.cxx b/tree/ntuple/src/RFieldSequenceContainer.cxx index b6db8dd1dcd34..b1d4688656a18 100644 --- a/tree/ntuple/src/RFieldSequenceContainer.cxx +++ b/tree/ntuple/src/RFieldSequenceContainer.cxx @@ -809,19 +809,14 @@ void ROOT::RArrayAsRVecField::ReadInClusterImpl(RNTupleLocalIndex localIndex, vo { auto begin = RRVecField::ResizeRVec(to, fArrayLength, fItemSize, fSubfields[0].get(), fItemDeleter.get()); - const auto &clusterId = localIndex.GetClusterId(); - const auto &indexInCluster = localIndex.GetIndexInCluster(); - if (fSubfields[0]->IsSimple()) { - GetPrincipalColumnOf(*fSubfields[0]) - ->ReadV(RNTupleLocalIndex(clusterId, indexInCluster * fArrayLength), fArrayLength, begin); + GetPrincipalColumnOf(*fSubfields[0])->ReadV(localIndex * fArrayLength, fArrayLength, begin); return; } // Read the new values into the collection elements for (std::size_t i = 0; i < fArrayLength; ++i) { - CallReadOn(*fSubfields[0], RNTupleLocalIndex(clusterId, indexInCluster * fArrayLength + i), - begin + (i * fItemSize)); + CallReadOn(*fSubfields[0], localIndex * fArrayLength + i, begin + (i * fItemSize)); } } From d3e97b706c814608dcc30ad75f2ae9b79cb36755 Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Fri, 26 Sep 2025 17:13:16 +0200 Subject: [PATCH 2/3] [ntuple] optimize bulk read for array of PoD --- .../ROOT/RField/RFieldSequenceContainer.hxx | 1 + tree/ntuple/src/RFieldSequenceContainer.cxx | 10 ++++ tree/ntuple/test/ntuple_bulk.cxx | 49 +++++++++++++++++++ 3 files changed, 60 insertions(+) diff --git a/tree/ntuple/inc/ROOT/RField/RFieldSequenceContainer.hxx b/tree/ntuple/inc/ROOT/RField/RFieldSequenceContainer.hxx index 950f442f1d85f..b8efcdeee8c0e 100644 --- a/tree/ntuple/inc/ROOT/RField/RFieldSequenceContainer.hxx +++ b/tree/ntuple/inc/ROOT/RField/RFieldSequenceContainer.hxx @@ -70,6 +70,7 @@ protected: std::size_t AppendImpl(const void *from) final; void ReadGlobalImpl(ROOT::NTupleSize_t globalIndex, void *to) final; void ReadInClusterImpl(RNTupleLocalIndex localIndex, void *to) final; + std::size_t ReadBulkImpl(const RBulkSpec &bulkSpec) final; void ReconcileOnDiskField(const RNTupleDescriptor &desc) final; diff --git a/tree/ntuple/src/RFieldSequenceContainer.cxx b/tree/ntuple/src/RFieldSequenceContainer.cxx index b1d4688656a18..0142743d2332d 100644 --- a/tree/ntuple/src/RFieldSequenceContainer.cxx +++ b/tree/ntuple/src/RFieldSequenceContainer.cxx @@ -75,6 +75,16 @@ void ROOT::RArrayField::ReadInClusterImpl(RNTupleLocalIndex localIndex, void *to } } +std::size_t ROOT::RArrayField::ReadBulkImpl(const RBulkSpec &bulkSpec) +{ + if (!fSubfields[0]->IsSimple()) + return RFieldBase::ReadBulkImpl(bulkSpec); + + GetPrincipalColumnOf(*fSubfields[0]) + ->ReadV(bulkSpec.fFirstIndex * fArrayLength, bulkSpec.fCount * fArrayLength, bulkSpec.fValues); + return RBulkSpec::kAllSet; +} + void ROOT::RArrayField::ReconcileOnDiskField(const RNTupleDescriptor &desc) { static const std::vector prefixes = {"std::array<"}; diff --git a/tree/ntuple/test/ntuple_bulk.cxx b/tree/ntuple/test/ntuple_bulk.cxx index fac813a9bfd03..0ddc13970014f 100644 --- a/tree/ntuple/test/ntuple_bulk.cxx +++ b/tree/ntuple/test/ntuple_bulk.cxx @@ -208,6 +208,55 @@ TEST(RNTupleBulk, RVec) } } +TEST(RNTupleBulk, Array) +{ + FileRaii fileGuard("test_ntuple_bulk_array.root"); + { + auto model = RNTupleModel::Create(); + auto fldArrI = model->MakeField>("aint"); + auto fld2DArrI = model->MakeField, 2>>("2daint"); + auto writer = RNTupleWriter::Recreate(std::move(model), "ntpl", fileGuard.GetPath()); + for (int i = 0; i < 3; ++i) { + fldArrI->at(0) = 2 * i; + fldArrI->at(1) = 2 * i + 1; + + fld2DArrI->at(0).at(0) = 4 * i; + fld2DArrI->at(0).at(1) = 4 * i + 1; + fld2DArrI->at(1).at(0) = 4 * i + 2; + fld2DArrI->at(1).at(1) = 4 * i + 3; + + writer->Fill(); + } + } + + auto reader = RNTupleReader::Open("ntpl", fileGuard.GetPath()); + const auto &model = reader->GetModel(); + + RFieldBase::RBulkValues bulkI = model.CreateBulk("aint"); + RFieldBase::RBulkValues bulk2DI = model.CreateBulk("2daint"); + + auto mask = std::make_unique(3); + mask[0] = true; + mask[1] = false; // the std::array field optimization should ignore the mask + mask[2] = true; + + auto iArr = static_cast *>(bulkI.ReadBulk(RNTupleLocalIndex(0, 0), mask.get(), 3)); + auto i2DArr = + static_cast, 2> *>(bulk2DI.ReadBulk(RNTupleLocalIndex(0, 0), mask.get(), 3)); + + for (int i = 0; i < 3; ++i) { + EXPECT_EQ(2 * i, iArr[i][0]); + EXPECT_EQ(2 * i + 1, iArr[i][1]); + + if (mask[i]) { + EXPECT_EQ(4 * i, i2DArr[i][0][0]); + EXPECT_EQ(4 * i + 1, i2DArr[i][0][1]); + EXPECT_EQ(4 * i + 2, i2DArr[i][1][0]); + EXPECT_EQ(4 * i + 3, i2DArr[i][1][1]); + } + } +} + TEST(RNTupleBulk, Adopted) { FileRaii fileGuard("test_ntuple_bulk_adopted.root"); From d7617f5d2f0a60c2bbfa890cbc6fe203fc5b97a4 Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Fri, 26 Sep 2025 20:34:31 +0200 Subject: [PATCH 3/3] [ntuple] optimize bulk read for RVec of array of PoD --- .../ROOT/RField/RFieldSequenceContainer.hxx | 5 +++ tree/ntuple/src/RFieldSequenceContainer.cxx | 20 ++++++++++-- tree/ntuple/test/ntuple_bulk.cxx | 32 ++++++++++++------- 3 files changed, 43 insertions(+), 14 deletions(-) diff --git a/tree/ntuple/inc/ROOT/RField/RFieldSequenceContainer.hxx b/tree/ntuple/inc/ROOT/RField/RFieldSequenceContainer.hxx index b8efcdeee8c0e..fa5705adf3423 100644 --- a/tree/ntuple/inc/ROOT/RField/RFieldSequenceContainer.hxx +++ b/tree/ntuple/inc/ROOT/RField/RFieldSequenceContainer.hxx @@ -143,6 +143,11 @@ protected: ROOT::Internal::RColumnIndex fNWritten; std::size_t fValueSize; + // For bulk read optimzation + std::size_t fBulkNRepetition = 1; + /// May be a direct PoD subfield or a sub-subfield of a fixed-size array of PoD + RFieldBase *fBulkSubfield = nullptr; + std::unique_ptr CloneImpl(std::string_view newName) const final; const RColumnRepresentations &GetColumnRepresentations() const final; void GenerateColumns() final; diff --git a/tree/ntuple/src/RFieldSequenceContainer.cxx b/tree/ntuple/src/RFieldSequenceContainer.cxx index 0142743d2332d..0f75ed69c4f39 100644 --- a/tree/ntuple/src/RFieldSequenceContainer.cxx +++ b/tree/ntuple/src/RFieldSequenceContainer.cxx @@ -237,6 +237,19 @@ ROOT::RRVecField::RRVecField(std::string_view fieldName, std::unique_ptrGetAlignment(), fSubfields[0]->GetValueSize(), GetAlignment()); + + // Determine if we can optimimize bulk reading + if (fSubfields[0]->IsSimple()) { + fBulkSubfield = fSubfields[0].get(); + } else { + if (auto f = dynamic_cast(fSubfields[0].get())) { + auto grandChildFields = fSubfields[0]->GetMutableSubfields(); + if (grandChildFields[0]->IsSimple()) { + fBulkSubfield = grandChildFields[0]; + fBulkNRepetition = f->GetLength(); + } + } + } } std::unique_ptr ROOT::RRVecField::CloneImpl(std::string_view newName) const @@ -351,14 +364,14 @@ void ROOT::RRVecField::ReadGlobalImpl(ROOT::NTupleSize_t globalIndex, void *to) std::size_t ROOT::RRVecField::ReadBulkImpl(const RBulkSpec &bulkSpec) { - if (!fSubfields[0]->IsSimple()) + if (!fBulkSubfield) return RFieldBase::ReadBulkImpl(bulkSpec); if (bulkSpec.fAuxData->empty()) { /// Initialize auxiliary memory: the first sizeof(size_t) bytes store the value size of the item field. /// The following bytes store the item values, consecutively. bulkSpec.fAuxData->resize(sizeof(std::size_t)); - *reinterpret_cast(bulkSpec.fAuxData->data()) = fSubfields[0]->GetValueSize(); + *reinterpret_cast(bulkSpec.fAuxData->data()) = fBulkNRepetition * fBulkSubfield->GetValueSize(); } const auto itemValueSize = *reinterpret_cast(bulkSpec.fAuxData->data()); unsigned char *itemValueArray = bulkSpec.fAuxData->data() + sizeof(std::size_t); @@ -410,7 +423,8 @@ std::size_t ROOT::RRVecField::ReadBulkImpl(const RBulkSpec &bulkSpec) } } - GetPrincipalColumnOf(*fSubfields[0])->ReadV(firstItemIndex, nItems, itemValueArray - delta); + GetPrincipalColumnOf(*fBulkSubfield) + ->ReadV(firstItemIndex * fBulkNRepetition, nItems * fBulkNRepetition, itemValueArray - delta); return RBulkSpec::kAllSet; } diff --git a/tree/ntuple/test/ntuple_bulk.cxx b/tree/ntuple/test/ntuple_bulk.cxx index 0ddc13970014f..5974029b6bd0b 100644 --- a/tree/ntuple/test/ntuple_bulk.cxx +++ b/tree/ntuple/test/ntuple_bulk.cxx @@ -151,21 +151,25 @@ TEST(RNTupleBulk, RVec) FileRaii fileGuard("test_ntuple_bulk_rvec.root"); { auto model = RNTupleModel::Create(); - auto fldVecI = model->MakeField("vint"); - auto fldVecS = model->MakeField>("vs"); - auto fldVecVI = model->MakeField>("vvint"); + auto ptrVecI = model->MakeField("vint"); + auto ptrVecS = model->MakeField>("vs"); + auto ptrVecVI = model->MakeField>("vvint"); + auto ptrVecArrI = model->MakeField>>("varrint"); auto writer = RNTupleWriter::Recreate(std::move(model), "ntpl", fileGuard.GetPath()); for (int i = 0; i < 10; ++i) { - fldVecI->resize(i); - fldVecS->resize(i); - fldVecVI->resize(i); + ptrVecI->resize(i); + ptrVecS->resize(i); + ptrVecVI->resize(i); + ptrVecArrI->resize(i); for (int j = 0; j < i; ++j) { - fldVecI->at(j) = j; - fldVecS->at(j).a = j; - fldVecVI->at(j).resize(j); + ptrVecI->at(j) = j; + ptrVecS->at(j).a = j; + ptrVecVI->at(j).resize(j); for (int k = 0; k < j; ++k) { - fldVecVI->at(j).at(k) = k; + ptrVecVI->at(j).at(k) = k; } + ptrVecArrI->at(j).at(0) = 1000 * i + 2 * j; + ptrVecArrI->at(j).at(1) = 1000 * i + 2 * j + 1; } writer->Fill(); } @@ -177,6 +181,7 @@ TEST(RNTupleBulk, RVec) RFieldBase::RBulkValues bulkI = model.CreateBulk("vint"); RFieldBase::RBulkValues bulkS = model.CreateBulk("vs"); RFieldBase::RBulkValues bulkVI = model.CreateBulk("vvint"); + RFieldBase::RBulkValues bulkVArrI = model.CreateBulk("varrint"); auto mask = std::make_unique(10); std::fill(mask.get(), mask.get() + 10, true); @@ -185,12 +190,17 @@ TEST(RNTupleBulk, RVec) auto iArr = static_cast(bulkI.ReadBulk(RNTupleLocalIndex(0, 0), mask.get(), 10)); auto sArr = static_cast *>(bulkS.ReadBulk(RNTupleLocalIndex(0, 0), mask.get(), 10)); auto viArr = static_cast *>(bulkVI.ReadBulk(RNTupleLocalIndex(0, 0), mask.get(), 10)); + auto arriArr = + static_cast> *>(bulkVArrI.ReadBulk(RNTupleLocalIndex(0, 0), mask.get(), 10)); for (int i = 0; i < 10; ++i) { EXPECT_EQ(i, iArr[i].size()); + EXPECT_EQ(i, arriArr[i].size()); EXPECT_EQ(i == 1 ? 0 : i, sArr[i].size()); EXPECT_EQ(i == 1 ? 0 : i, viArr[i].size()); - for (std::size_t j = 0; j < iArr[i].size(); ++j) { + for (int j = 0; j < i; ++j) { EXPECT_EQ(j, iArr[i].at(j)); + EXPECT_EQ(1000 * i + 2 * j, arriArr[i].at(j).at(0)); + EXPECT_EQ(1000 * i + 2 * j + 1, arriArr[i].at(j).at(1)); } // RVec should have all the vector elements of the bulk stored consecutively in memory if (i > 1) {