From 02ecd3ec829d29afa3fa94f7f6f7f721cd7d4a45 Mon Sep 17 00:00:00 2001 From: Ishaan Maheshwari Date: Mon, 12 Oct 2026 02:21:54 +0530 Subject: [PATCH 1/5] [ntuple] Grow RVec capacity geometrically in ResizeRVec When reading consecutive entries with variable-sized RVec collections, ResizeRVec previously reallocated only enough capacity for nItems. This incurred frequent reallocations across entries with growing element counts. Grow capacity geometrically (doubling, capped at int32_t max) when reallocating an already-allocated RVec, amortizing allocation costs and avoiding repeated malloc/free cycles across entry iterations. Assisted-by: antigravity:gemini --- tree/ntuple/src/RFieldSequenceContainer.cxx | 14 +++++-- tree/ntuple/test/rfield_vector.cxx | 41 ++++++++++++++++++++- 2 files changed, 51 insertions(+), 4 deletions(-) diff --git a/tree/ntuple/src/RFieldSequenceContainer.cxx b/tree/ntuple/src/RFieldSequenceContainer.cxx index 7505249e14b86..a541b5ba33c5e 100644 --- a/tree/ntuple/src/RFieldSequenceContainer.cxx +++ b/tree/ntuple/src/RFieldSequenceContainer.cxx @@ -291,13 +291,21 @@ unsigned char *ROOT::RRVecField::ResizeRVec(void *rvec, std::size_t nItems, std: } } - // TODO Increment capacity by a factor rather than just enough to fit the elements. + // Increment capacity geometrically rather than just enough to fit the elements. + std::size_t newCapacity = nItems; + if (*capacityPtr > 0) { + constexpr std::size_t maxSize = static_cast(std::numeric_limits::max()); + const std::size_t oldCap = static_cast(*capacityPtr); + const std::size_t grown = oldCap <= maxSize / 2 ? oldCap * 2 : maxSize; + newCapacity = std::min(maxSize, std::max(nItems, grown)); + } + Internal::DestroyRVecWithChecks(itemField->GetAlignment(), beginPtr, capacityPtr); // We trust that malloc returns a buffer with large enough alignment. // This might not be the case if T in RVec is over-aligned. - *beginPtr = static_cast(malloc(nItems * itemSize)); + *beginPtr = static_cast(malloc(newCapacity * itemSize)); R__ASSERT(*beginPtr != nullptr); - *capacityPtr = nItems; + *capacityPtr = static_cast(newCapacity); // Placement new for elements that were already there before the resize if (needsConstruct) { diff --git a/tree/ntuple/test/rfield_vector.cxx b/tree/ntuple/test/rfield_vector.cxx index af28671b25961..daa90fbbd01d9 100644 --- a/tree/ntuple/test/rfield_vector.cxx +++ b/tree/ntuple/test/rfield_vector.cxx @@ -154,7 +154,6 @@ TEST(RNTuple, InsideCollection) // TODO: test reading of "klassVec.v1" } - TEST(RNTuple, RVec) { FileRaii fileGuard("test_ntuple_rvec.root"); @@ -304,6 +303,46 @@ TEST(RNTuple, ReadVectorAsRVec) EXPECT_TRUE(rvec->empty()); } +TEST(RNTuple, RVecGeometricGrowth) +{ + FileRaii fileGuard("test_ntuple_rvec_geometric_growth.root"); + + { + auto m = RNTupleModel::Create(); + auto rvec = m->MakeField>("v"); + auto w = RNTupleWriter::Recreate(std::move(m), "ntpl", fileGuard.GetPath()); + + *rvec = {1, 2}; + w->Fill(); + *rvec = {1, 2, 3}; + w->Fill(); + *rvec = {1, 2, 3, 4}; + w->Fill(); + *rvec = {10}; + w->Fill(); + } + + auto m = RNTupleModel::Create(); + auto rvec = m->MakeField>("v"); + auto reader = RNTupleReader::Open(std::move(m), "ntpl", fileGuard.GetPath()); + + reader->LoadEntry(0); + EXPECT_EQ(rvec->size(), 2u); + EXPECT_EQ(rvec->capacity(), 2u); + + reader->LoadEntry(1); + EXPECT_EQ(rvec->size(), 3u); + EXPECT_GE(rvec->capacity(), 4u); + + reader->LoadEntry(2); + EXPECT_EQ(rvec->size(), 4u); + EXPECT_GE(rvec->capacity(), 4u); + + reader->LoadEntry(3); + EXPECT_EQ(rvec->size(), 1u); + EXPECT_GE(rvec->capacity(), 4u); +} + TEST(RNTuple, BoolVector) { FileRaii fileGuard("test_ntuple_boolvec.root"); From b39d0097543241f210c7bf63801e7ee8624cb680 Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Wed, 14 Oct 2026 14:18:41 +0200 Subject: [PATCH 2/5] [ntuple] more memory-saving RVec growth Grow by doubling until 16 items, then grow only by 50%. --- tree/ntuple/src/RFieldSequenceContainer.cxx | 4 +- tree/ntuple/test/rfield_vector.cxx | 48 ++++++++++++++++----- 2 files changed, 40 insertions(+), 12 deletions(-) diff --git a/tree/ntuple/src/RFieldSequenceContainer.cxx b/tree/ntuple/src/RFieldSequenceContainer.cxx index a541b5ba33c5e..be0874eb087d9 100644 --- a/tree/ntuple/src/RFieldSequenceContainer.cxx +++ b/tree/ntuple/src/RFieldSequenceContainer.cxx @@ -282,6 +282,8 @@ unsigned char *ROOT::RRVecField::ResizeRVec(void *rvec, std::size_t nItems, std: } // Resize RVec (capacity and size) + // In the regime between 0 and 16 items, grow by doubling the size. Grow larger vectors by a factor 1.5 + static constexpr std::int32_t kSmallThreshold = 16; if (std::int32_t(nItems) > *capacityPtr) { // must reallocate // Destroy old elements: useless work for trivial types, but in case the element type's constructor // allocates memory we need to release it here to avoid memleaks (e.g. if this is an RVec>) @@ -296,7 +298,7 @@ unsigned char *ROOT::RRVecField::ResizeRVec(void *rvec, std::size_t nItems, std: if (*capacityPtr > 0) { constexpr std::size_t maxSize = static_cast(std::numeric_limits::max()); const std::size_t oldCap = static_cast(*capacityPtr); - const std::size_t grown = oldCap <= maxSize / 2 ? oldCap * 2 : maxSize; + const std::size_t grown = oldCap + ((oldCap < kSmallThreshold) ? oldCap : (oldCap / 2)); newCapacity = std::min(maxSize, std::max(nItems, grown)); } diff --git a/tree/ntuple/test/rfield_vector.cxx b/tree/ntuple/test/rfield_vector.cxx index daa90fbbd01d9..19bca01a3a80b 100644 --- a/tree/ntuple/test/rfield_vector.cxx +++ b/tree/ntuple/test/rfield_vector.cxx @@ -312,35 +312,61 @@ TEST(RNTuple, RVecGeometricGrowth) auto rvec = m->MakeField>("v"); auto w = RNTupleWriter::Recreate(std::move(m), "ntpl", fileGuard.GetPath()); - *rvec = {1, 2}; - w->Fill(); - *rvec = {1, 2, 3}; + for (int i = 0; i < 6; ++i) { + w->Fill(); + rvec->push_back(i); + } + + *rvec = {10}; w->Fill(); - *rvec = {1, 2, 3, 4}; + + *rvec = ROOT::RVec(11, 0); w->Fill(); - *rvec = {10}; + + *rvec = ROOT::RVec(23, 0); w->Fill(); } auto m = RNTupleModel::Create(); auto rvec = m->MakeField>("v"); auto reader = RNTupleReader::Open(std::move(m), "ntpl", fileGuard.GetPath()); + EXPECT_EQ(9u, reader->GetNEntries()); reader->LoadEntry(0); + EXPECT_EQ(rvec->size(), 0u); + EXPECT_EQ(rvec->capacity(), 0u); + + reader->LoadEntry(1); + EXPECT_EQ(rvec->size(), 1u); + EXPECT_EQ(rvec->capacity(), 1u); + + reader->LoadEntry(2); EXPECT_EQ(rvec->size(), 2u); EXPECT_EQ(rvec->capacity(), 2u); - reader->LoadEntry(1); + reader->LoadEntry(3); EXPECT_EQ(rvec->size(), 3u); - EXPECT_GE(rvec->capacity(), 4u); + EXPECT_EQ(rvec->capacity(), 4u); - reader->LoadEntry(2); + reader->LoadEntry(4); EXPECT_EQ(rvec->size(), 4u); - EXPECT_GE(rvec->capacity(), 4u); + EXPECT_EQ(rvec->capacity(), 4u); - reader->LoadEntry(3); + reader->LoadEntry(5); + EXPECT_EQ(rvec->size(), 5u); + EXPECT_EQ(rvec->capacity(), 8u); + + reader->LoadEntry(6); EXPECT_EQ(rvec->size(), 1u); - EXPECT_GE(rvec->capacity(), 4u); + EXPECT_GE(rvec->capacity(), 8u); + + reader->LoadEntry(7); + EXPECT_EQ(rvec->size(), 11u); + EXPECT_GE(rvec->capacity(), 16u); + + reader->LoadEntry(8); + EXPECT_EQ(rvec->size(), 23u); + EXPECT_GE(rvec->capacity(), 24u); } TEST(RNTuple, BoolVector) From dfa2a7c8fb9edb7faf1f38eb5eb4691012395275 Mon Sep 17 00:00:00 2001 From: Jakob Blomer Date: Wed, 14 Oct 2026 15:20:12 +0200 Subject: [PATCH 3/5] [ntuple] release memory on shrinking RVecs As we now grow geometrically, also shrink geometrically. Half the capacity when the size falls below 1/4. --- tree/ntuple/src/RFieldSequenceContainer.cxx | 29 +++++++--- tree/ntuple/test/rfield_vector.cxx | 62 +++++++++++++++++---- 2 files changed, 71 insertions(+), 20 deletions(-) diff --git a/tree/ntuple/src/RFieldSequenceContainer.cxx b/tree/ntuple/src/RFieldSequenceContainer.cxx index be0874eb087d9..f49561e2533e3 100644 --- a/tree/ntuple/src/RFieldSequenceContainer.cxx +++ b/tree/ntuple/src/RFieldSequenceContainer.cxx @@ -282,24 +282,34 @@ unsigned char *ROOT::RRVecField::ResizeRVec(void *rvec, std::size_t nItems, std: } // Resize RVec (capacity and size) - // In the regime between 0 and 16 items, grow by doubling the size. Grow larger vectors by a factor 1.5 + // In the regime between 0 and 16 items, grow by doubling the size. Grow larger vectors by a factor 1.5 at most. + // If the new size is larger than the grown capacity, set the new capacity to the new size (i.e., don't allocate + // a larger capacity for vectors the are suddenly vastly larger than their previous state). + // TODO: investigate a longer look back window to avoid shrinking for an unusual small entry + // (see TBasket::WriteReset()). static constexpr std::int32_t kSmallThreshold = 16; - if (std::int32_t(nItems) > *capacityPtr) { // must reallocate + if ((std::int32_t(nItems) > *capacityPtr) || /* must grow */ + ((std::int32_t(nItems) > kSmallThreshold) && (std::int32_t(nItems) < *capacityPtr / 4))) /* must shrink */ { // Destroy old elements: useless work for trivial types, but in case the element type's constructor // allocates memory we need to release it here to avoid memleaks (e.g. if this is an RVec>) if (needsDestruct) { - for (std::size_t i = 0u; i < oldSize; ++i) { + for (std::size_t i = 0u; i < std::min(oldSize, nItems); ++i) { itemDeleter->operator()(*beginPtr + (i * itemSize), true /* dtorOnly */); } } - // Increment capacity geometrically rather than just enough to fit the elements. + // Increment / decrement capacity geometrically std::size_t newCapacity = nItems; if (*capacityPtr > 0) { constexpr std::size_t maxSize = static_cast(std::numeric_limits::max()); const std::size_t oldCap = static_cast(*capacityPtr); - const std::size_t grown = oldCap + ((oldCap < kSmallThreshold) ? oldCap : (oldCap / 2)); - newCapacity = std::min(maxSize, std::max(nItems, grown)); + if (nItems > oldCap) { + const std::size_t grown = oldCap + ((oldCap < kSmallThreshold) ? oldCap : (oldCap / 2)); + newCapacity = std::min(maxSize, std::max(nItems, grown)); + } else { + newCapacity = oldCap / 2; + assert(nItems <= newCapacity); + } } Internal::DestroyRVecWithChecks(itemField->GetAlignment(), beginPtr, capacityPtr); @@ -309,9 +319,12 @@ unsigned char *ROOT::RRVecField::ResizeRVec(void *rvec, std::size_t nItems, std: R__ASSERT(*beginPtr != nullptr); *capacityPtr = static_cast(newCapacity); - // Placement new for elements that were already there before the resize + // Placement new for elements that were already there before the resize. When shrank, we only need to construct + // nItems items, because that's what we going to read. When grown, the remaining elements from oldSize until + // nItems is constructed a few lines down (we need to do this always when the size grows, even when not changing + // the capacity). if (needsConstruct) { - for (std::size_t i = 0u; i < oldSize; ++i) + for (std::size_t i = 0u; i < std::min(oldSize, nItems); ++i) CallConstructValueOn(*itemField, *beginPtr + (i * itemSize)); } } diff --git a/tree/ntuple/test/rfield_vector.cxx b/tree/ntuple/test/rfield_vector.cxx index 19bca01a3a80b..53da854fef285 100644 --- a/tree/ntuple/test/rfield_vector.cxx +++ b/tree/ntuple/test/rfield_vector.cxx @@ -303,9 +303,9 @@ TEST(RNTuple, ReadVectorAsRVec) EXPECT_TRUE(rvec->empty()); } -TEST(RNTuple, RVecGeometricGrowth) +TEST(RNTuple, RVecGeometricResize) { - FileRaii fileGuard("test_ntuple_rvec_geometric_growth.root"); + FileRaii fileGuard("test_ntuple_rvec_geometric_resize.root"); { auto m = RNTupleModel::Create(); @@ -325,12 +325,27 @@ TEST(RNTuple, RVecGeometricGrowth) *rvec = ROOT::RVec(23, 0); w->Fill(); + + rvec->clear(); + w->Fill(); + + *rvec = ROOT::RVec(100, 0); + w->Fill(); + + *rvec = ROOT::RVec(101, 0); + w->Fill(); + + *rvec = {1, 2}; + w->Fill(); + + *rvec = {1, 2, 3}; + w->Fill(); } auto m = RNTupleModel::Create(); auto rvec = m->MakeField>("v"); auto reader = RNTupleReader::Open(std::move(m), "ntpl", fileGuard.GetPath()); - EXPECT_EQ(9u, reader->GetNEntries()); + EXPECT_EQ(14u, reader->GetNEntries()); reader->LoadEntry(0); EXPECT_EQ(rvec->size(), 0u); @@ -367,6 +382,26 @@ TEST(RNTuple, RVecGeometricGrowth) reader->LoadEntry(8); EXPECT_EQ(rvec->size(), 23u); EXPECT_GE(rvec->capacity(), 24u); + + reader->LoadEntry(9); + EXPECT_EQ(rvec->size(), 0u); + EXPECT_GE(rvec->capacity(), 24u); + + reader->LoadEntry(10); + EXPECT_EQ(rvec->size(), 100u); + EXPECT_GE(rvec->capacity(), 100u); + + reader->LoadEntry(11); + EXPECT_EQ(rvec->size(), 101u); + EXPECT_GE(rvec->capacity(), 150u); + + reader->LoadEntry(12); + EXPECT_EQ(rvec->size(), 2u); + EXPECT_GE(rvec->capacity(), 75u); + + reader->LoadEntry(13); + EXPECT_EQ(rvec->size(), 3u); + EXPECT_GE(rvec->capacity(), 37u); } TEST(RNTuple, BoolVector) @@ -417,6 +452,8 @@ void FillComplexVector(const std::string &fname) auto model = RNTupleModel::Create(); auto wrV = model->MakeField>("v"); + // Sequence of number of elements: 0, 10, 10000, 100, 200, 0, 1 + auto ntuple = RNTupleWriter::Recreate(std::move(model), "T", fname); ntuple->Fill(); for (unsigned i = 0; i < 10; ++i) @@ -442,6 +479,7 @@ void FillComplexVector(const std::string &fname) template