From 9385972c165531ab64d3cb22592deb8bea4e1bc9 Mon Sep 17 00:00:00 2001 From: Josh Bendavid Date: Thu, 21 May 2026 11:11:19 +0000 Subject: [PATCH 1/3] [tree] fix I/O rule for vector with nested split source When reading a split TTree branch holding vector, where T has a schema- evolution rule whose source member is itself nested inside a struct, the rule fired but the new top-level member stayed at its default value because the on-file staging area was never populated. The leaf sub-branch for the nested source (e.g. ve99.fDeep.fDeepMember) could not resolve its path in the user (target) class layout, so it was marked missing and skipped at read time, leaving the staging area used by the rule uninitialized. Detect this case in TBranchElement::InitializeOffsets by also looking up the data member in the parent's on-file staging class. When found, propagate fOnfileObject to the sub-branch, flag it with the new kReadFromStagingArray bit, and re-build its read action sequence. The sequence is then wrapped with UseCacheVectorLoop so the inner action iterates over the staging area with the staging element stride and writes at the offset within the staging element, leaving the rule with a properly filled staging area to read from. Fixes #19773. Co-Authored-By: Claude Opus 4.7 (1M context) --- io/io/inc/TStreamerInfoActions.h | 7 +++++ io/io/src/TStreamerInfoActions.cxx | 15 ++++++++++ tree/tree/inc/TBranchElement.h | 3 +- tree/tree/src/TBranchElement.cxx | 47 ++++++++++++++++++++++++++++++ 4 files changed, 71 insertions(+), 1 deletion(-) diff --git a/io/io/inc/TStreamerInfoActions.h b/io/io/inc/TStreamerInfoActions.h index 6cc37943ac8f8..0c8aac6721dd0 100644 --- a/io/io/inc/TStreamerInfoActions.h +++ b/io/io/inc/TStreamerInfoActions.h @@ -213,6 +213,13 @@ namespace TStreamerInfoActions { void AddToOffset(Int_t delta); void SetMissing(); + /// Replace each action with `UseCacheVectorLoop` wrapping the original + /// action. Used by TBranchElement when a sub-branch must read its data + /// into the parent's on-file staging area (e.g. a split branch supplying + /// the source of a schema-evolution rule whose source is a nested + /// struct member). + void WrapAllActionsWithUseCacheVectorLoop(TVirtualStreamerInfo *info); + TActionSequence *CreateCopy(); static TActionSequence *CreateReadMemberWiseActions(TVirtualStreamerInfo *info, TVirtualCollectionProxy &proxy); static TActionSequence *CreateReadMemberWiseActions(TVirtualStreamerInfo &info, std::unique_ptr loopConfig); diff --git a/io/io/src/TStreamerInfoActions.cxx b/io/io/src/TStreamerInfoActions.cxx index bebd784eacd7d..84f3f9d7bbc82 100644 --- a/io/io/src/TStreamerInfoActions.cxx +++ b/io/io/src/TStreamerInfoActions.cxx @@ -5416,6 +5416,21 @@ void TStreamerInfoActions::TActionSequence::AddToOffset(Int_t delta) } } +void TStreamerInfoActions::TActionSequence::WrapAllActionsWithUseCacheVectorLoop(TVirtualStreamerInfo *info) +{ + // Replace each action in the sequence with a UseCacheVectorLoop wrapping + // the original action. After this call, the actions iterate over the + // staging area pushed onto the buffer's data cache stack rather than the + // original (user) iteration range. The element offsets stored on the + // inner actions therefore must be relative to the staging element layout. + + for (auto &configured : fActions) { + TConfiguredAction inner(configured); + configured.fLoopAction = UseCacheVectorLoop; + configured.fConfiguration = new TConfigurationUseCache(info, inner, /*repeat*/ kFALSE); + } +} + void TStreamerInfoActions::TActionSequence::SetMissing() { // Add the (potentially negative) delta to all the configuration's offset. This is used by diff --git a/tree/tree/inc/TBranchElement.h b/tree/tree/inc/TBranchElement.h index 39c762041c414..262725fd7c47d 100644 --- a/tree/tree/inc/TBranchElement.h +++ b/tree/tree/inc/TBranchElement.h @@ -51,7 +51,8 @@ class TBranchElement : public TBranch { kOwnOnfileObj = BIT(19), ///< We are the owner of fOnfileObject. kAddressSet = BIT(20), ///< The addressing set have been called for this branch kMakeClass = BIT(21), ///< This branch has been switched to using the MakeClass Mode - kDecomposedObj = BIT(21) ///< More explicit alias for kMakeClass. + kDecomposedObj = BIT(21), ///< More explicit alias for kMakeClass. + kReadFromStagingArray = BIT(23) ///< This split sub-branch must read its data into the parent's on-file staging area (e.g. for a schema-evolution rule with a nested split source). }; diff --git a/tree/tree/src/TBranchElement.cxx b/tree/tree/src/TBranchElement.cxx index a8227be2e1bac..e3ab14d9ff8e1 100644 --- a/tree/tree/src/TBranchElement.cxx +++ b/tree/tree/src/TBranchElement.cxx @@ -3695,6 +3695,18 @@ void TBranchElement::InitializeOffsets() dataName.Replace(dotpos,endpos-dotpos,subBranchElement->GetFullName()); } TRealData* rd = pClass->GetRealData(dataName); + TRealData* stagingRd = nullptr; + if (!rd && fOnfileObject && fOnfileObject->fClass) { + // The data member does not exist in the user (target) class. + // If the parent owns an on-file staging area (e.g. for an I/O + // rule whose source is a struct member that has itself been + // split on disk), try the staging class. When found, the + // sub-branch will be redirected to read its bytes into the + // staging area rather than be skipped. + stagingRd = fOnfileObject->fClass->GetRealData(dataName); + if (stagingRd && stagingRd->TestBit(TRealData::kTransient)) + stagingRd = nullptr; + } if (rd && (!rd->TestBit(TRealData::kTransient) || alternateElement)) { // -- Data member exists in the dictionary meta info, get the offset. // If we are using an alternateElement, it is the target of a rule @@ -3704,6 +3716,17 @@ void TBranchElement::InitializeOffsets() // We are a rule with no specific target, it applies to the whole // object, let's set the offset to zero offset = 0; + } else if (stagingRd) { + // -- Staging redirect: read into the parent's on-file object. + offset = stagingRd->GetThisOffset(); + subBranch->fOnfileObject = fOnfileObject; + subBranch->SetBit(kReadFromStagingArray); + // The sub-branch's read-action sequence may have been built + // earlier (before we had a chance to set the bit and the + // on-file object), so re-build it now to pick up the staging + // redirect. We defer this until after the per-sub-branch + // SetOffset call below to make sure the action offsets are + // computed against the staging element layout. } else { // -- No dictionary meta info for this data member, it must no // longer exist @@ -3767,6 +3790,15 @@ void TBranchElement::InitializeOffsets() // 'localOffset', we need to remove it explicitly. subBranch->SetOffset(offset - localOffset); } + if (subBranch->TestBit(kReadFromStagingArray) + && subBranch->fReadActionSequence) + { + // The sub-branch's read action sequence may have been + // built before the staging-redirect bit was set above, + // so re-build it now to install the UseCacheVectorLoop + // wrappers (see TBranchElement::SetReadActionSequence). + subBranch->SetReadActionSequence(); + } } } else { // -- Set fBranchOffset for sub-branch. @@ -5764,6 +5796,21 @@ void TBranchElement::SetReadActionSequence() if (create) { SetActionSequence(originalClass, localInfo, create, fReadActionSequence); } + + if (TestBit(kReadFromStagingArray) && fReadActionSequence && fOnfileObject) { + // The on-disk data for this split sub-branch needs to land in the + // parent's on-file staging area (e.g. it is the source of an I/O rule + // whose source member is a nested struct that has been split on disk). + // Replace each action with a UseCacheVectorLoop wrapping the original, + // so the action iterates over the staging area rather than the user + // collection. The element offsets configured on the inner actions are + // already expressed relative to the staging element layout. + TVirtualStreamerInfo *stagingInfo = fOnfileObject->fClass + ? fOnfileObject->fClass->GetStreamerInfo() + : nullptr; + if (stagingInfo) + fReadActionSequence->WrapAllActionsWithUseCacheVectorLoop(stagingInfo); + } } //////////////////////////////////////////////////////////////////////////////// From bf98b74423b306cdd878bdb4e6960b6b7d4f2bba Mon Sep 17 00:00:00 2001 From: Josh Bendavid Date: Thu, 21 May 2026 13:18:37 +0000 Subject: [PATCH 2/3] [tree] apply clang-format Co-Authored-By: Claude Opus 4.7 (1M context) --- tree/tree/inc/TBranchElement.h | 3 ++- tree/tree/src/TBranchElement.cxx | 10 +++------- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/tree/tree/inc/TBranchElement.h b/tree/tree/inc/TBranchElement.h index 262725fd7c47d..0d00ffd1879f7 100644 --- a/tree/tree/inc/TBranchElement.h +++ b/tree/tree/inc/TBranchElement.h @@ -52,7 +52,8 @@ class TBranchElement : public TBranch { kAddressSet = BIT(20), ///< The addressing set have been called for this branch kMakeClass = BIT(21), ///< This branch has been switched to using the MakeClass Mode kDecomposedObj = BIT(21), ///< More explicit alias for kMakeClass. - kReadFromStagingArray = BIT(23) ///< This split sub-branch must read its data into the parent's on-file staging area (e.g. for a schema-evolution rule with a nested split source). + kReadFromStagingArray = BIT(23) ///< This split sub-branch must read its data into the parent's on-file staging + ///< area (e.g. for a schema-evolution rule with a nested split source). }; diff --git a/tree/tree/src/TBranchElement.cxx b/tree/tree/src/TBranchElement.cxx index e3ab14d9ff8e1..d02b0d3c16baa 100644 --- a/tree/tree/src/TBranchElement.cxx +++ b/tree/tree/src/TBranchElement.cxx @@ -3695,7 +3695,7 @@ void TBranchElement::InitializeOffsets() dataName.Replace(dotpos,endpos-dotpos,subBranchElement->GetFullName()); } TRealData* rd = pClass->GetRealData(dataName); - TRealData* stagingRd = nullptr; + TRealData *stagingRd = nullptr; if (!rd && fOnfileObject && fOnfileObject->fClass) { // The data member does not exist in the user (target) class. // If the parent owns an on-file staging area (e.g. for an I/O @@ -3790,9 +3790,7 @@ void TBranchElement::InitializeOffsets() // 'localOffset', we need to remove it explicitly. subBranch->SetOffset(offset - localOffset); } - if (subBranch->TestBit(kReadFromStagingArray) - && subBranch->fReadActionSequence) - { + if (subBranch->TestBit(kReadFromStagingArray) && subBranch->fReadActionSequence) { // The sub-branch's read action sequence may have been // built before the staging-redirect bit was set above, // so re-build it now to install the UseCacheVectorLoop @@ -5805,9 +5803,7 @@ void TBranchElement::SetReadActionSequence() // so the action iterates over the staging area rather than the user // collection. The element offsets configured on the inner actions are // already expressed relative to the staging element layout. - TVirtualStreamerInfo *stagingInfo = fOnfileObject->fClass - ? fOnfileObject->fClass->GetStreamerInfo() - : nullptr; + TVirtualStreamerInfo *stagingInfo = fOnfileObject->fClass ? fOnfileObject->fClass->GetStreamerInfo() : nullptr; if (stagingInfo) fReadActionSequence->WrapAllActionsWithUseCacheVectorLoop(stagingInfo); } From 21f60cf35661447a58786dd5a0419aeb792a4146 Mon Sep 17 00:00:00 2001 From: Josh Bendavid Date: Fri, 4 Sep 2026 20:05:54 +0000 Subject: [PATCH 3/3] [roottest] Test I/O rule for vector with nested split source Add a data model evolution test where an I/O rule for the value type of a vector sources a member nested inside a struct. The file is written with the old model (values inside the nested `Deep` struct) as a free-standing object, as a free-standing vector and as TTree branches with split levels 0 and 99; it is read back with a model where those values live at the top level of `Event` and are filled by the rule. Before the accompanying fix, only the elements read from the split TTree branch were left with their default values, because the on-file staging area the rule reads from was never filled. Test for #19773. Co-Authored-By: Claude Opus 5 --- .../io/datamodelevolution/stl/CMakeLists.txt | 1 + .../stl/nestedsource/CMakeLists.txt | 23 ++++++ .../stl/nestedsource/execReadNestedSource.cxx | 82 +++++++++++++++++++ .../stl/nestedsource/execReadNestedSource.ref | 12 +++ .../nestedsource/execWriteNestedSource.cxx | 65 +++++++++++++++ .../nestedsource/execWriteNestedSource.ref | 3 + 6 files changed, 186 insertions(+) create mode 100644 roottest/root/io/datamodelevolution/stl/nestedsource/CMakeLists.txt create mode 100644 roottest/root/io/datamodelevolution/stl/nestedsource/execReadNestedSource.cxx create mode 100644 roottest/root/io/datamodelevolution/stl/nestedsource/execReadNestedSource.ref create mode 100644 roottest/root/io/datamodelevolution/stl/nestedsource/execWriteNestedSource.cxx create mode 100644 roottest/root/io/datamodelevolution/stl/nestedsource/execWriteNestedSource.ref diff --git a/roottest/root/io/datamodelevolution/stl/CMakeLists.txt b/roottest/root/io/datamodelevolution/stl/CMakeLists.txt index fa8d98ffca31f..3faf8d2b44778 100644 --- a/roottest/root/io/datamodelevolution/stl/CMakeLists.txt +++ b/roottest/root/io/datamodelevolution/stl/CMakeLists.txt @@ -39,3 +39,4 @@ ROOTTEST_ADD_TEST(ReadFile FIXTURES_REQUIRED root-io-datamodelevolution-stl-WriteFile-fixture root-io-datamodelevolution-stl-readFile-compile-fixture) +ROOTTEST_ADD_TESTDIRS() diff --git a/roottest/root/io/datamodelevolution/stl/nestedsource/CMakeLists.txt b/roottest/root/io/datamodelevolution/stl/nestedsource/CMakeLists.txt new file mode 100644 index 0000000000000..1d06f995b60bc --- /dev/null +++ b/roottest/root/io/datamodelevolution/stl/nestedsource/CMakeLists.txt @@ -0,0 +1,23 @@ +# Reading a split TTree branch holding a vector, where an I/O rule for T +# sources a member nested inside a struct. +# See https://github.com/root-project/root/issues/19773 + +ROOTTEST_COMPILE_MACRO(execWriteNestedSource.cxx + FIXTURES_SETUP root-io-datamodelevolution-stl-nestedsource-WriteNestedSource-compile-fixture) + +ROOTTEST_COMPILE_MACRO(execReadNestedSource.cxx + FIXTURES_SETUP root-io-datamodelevolution-stl-nestedsource-ReadNestedSource-compile-fixture) + +ROOTTEST_ADD_TEST(WriteNestedSource + MACRO execWriteNestedSource.cxx+ + OUTREF execWriteNestedSource.ref + LABELS longtest + FIXTURES_REQUIRED root-io-datamodelevolution-stl-nestedsource-WriteNestedSource-compile-fixture + FIXTURES_SETUP root-io-datamodelevolution-stl-nestedsource-WriteNestedSource-fixture) + +ROOTTEST_ADD_TEST(ReadNestedSource + MACRO execReadNestedSource.cxx+ + OUTREF execReadNestedSource.ref + LABELS longtest + FIXTURES_REQUIRED root-io-datamodelevolution-stl-nestedsource-ReadNestedSource-compile-fixture + root-io-datamodelevolution-stl-nestedsource-WriteNestedSource-fixture) diff --git a/roottest/root/io/datamodelevolution/stl/nestedsource/execReadNestedSource.cxx b/roottest/root/io/datamodelevolution/stl/nestedsource/execReadNestedSource.cxx new file mode 100644 index 0000000000000..caef06fbf3709 --- /dev/null +++ b/roottest/root/io/datamodelevolution/stl/nestedsource/execReadNestedSource.cxx @@ -0,0 +1,82 @@ +// Read the file written by execWriteNestedSource.cxx with a data model in +// which the members of the nested `Deep` struct have been moved to the top +// level of `Event`, where they are filled by an I/O rule whose source is the +// nested struct. +// +// All four cases below must report the same values; before the fix for issue +// https://github.com/root-project/root/issues/19773 the elements read from the +// split TTree branch were left with the default values because the on-file +// staging area the rule reads from was never filled. + +#include +#include +#include + +#include +#include + +struct Deep { + float fDeepMember = 0.; + int fDeepInt = 0; + + ClassDefNV(Deep, 1) +}; + +struct Event { + int fId = 0; + float fMember = 0.; + int fIntMember = 0; + + ClassDefNV(Event, 2) +}; + +#ifdef __ROOTCLING__ +#pragma link C++ class Deep+; +#pragma link C++ class Event+; +#pragma link C++ class std::vector+; + +#pragma read sourceClass = "Event" source = "Deep fDeep" version = "[1]" targetClass = "Event" \ + target = "fMember,fIntMember" code = "{ fMember = onfile.fDeep.fDeepMember; fIntMember = onfile.fDeep.fDeepInt; }" +#endif + +void Print(const char *what, const Event &event) +{ + std::cout << what << ": fId=" << event.fId << " fMember=" << event.fMember << " fIntMember=" << event.fIntMember + << "\n"; +} + +void Print(const char *what, const std::vector &events) +{ + for (unsigned int i = 0; i < events.size(); ++i) { + std::cout << what << " i=" << i; + std::cout << ": fId=" << events[i].fId; + std::cout << " fMember=" << events[i].fMember; + std::cout << " fIntMember=" << events[i].fIntMember; + std::cout << "\n"; + } +} + +int execReadNestedSource() +{ + TFile file("nestedsource.root", "READ"); + + auto *single = file.Get("e"); + Print("Plain object", *single); + + auto *vec = file.Get>("ve"); + Print("Plain vector", *vec); + + auto *tree = file.Get("t"); + + std::vector *ve0 = nullptr; + std::vector *ve99 = nullptr; + tree->SetBranchAddress("ve0", &ve0); + tree->SetBranchAddress("ve99", &ve99); + + tree->GetEntry(0); + + Print("Tree (splitlevel 0)", *ve0); + Print("Tree (splitlevel 99)", *ve99); + + return 0; +} diff --git a/roottest/root/io/datamodelevolution/stl/nestedsource/execReadNestedSource.ref b/roottest/root/io/datamodelevolution/stl/nestedsource/execReadNestedSource.ref new file mode 100644 index 0000000000000..73662df1c8a3a --- /dev/null +++ b/roottest/root/io/datamodelevolution/stl/nestedsource/execReadNestedSource.ref @@ -0,0 +1,12 @@ +Processing execReadNestedSource.cxx+... +Plain object: fId=1 fMember=10 fIntMember=100 +Plain vector i=0: fId=1 fMember=10 fIntMember=100 +Plain vector i=1: fId=2 fMember=20 fIntMember=200 +Plain vector i=2: fId=3 fMember=30 fIntMember=300 +Tree (splitlevel 0) i=0: fId=1 fMember=10 fIntMember=100 +Tree (splitlevel 0) i=1: fId=2 fMember=20 fIntMember=200 +Tree (splitlevel 0) i=2: fId=3 fMember=30 fIntMember=300 +Tree (splitlevel 99) i=0: fId=1 fMember=10 fIntMember=100 +Tree (splitlevel 99) i=1: fId=2 fMember=20 fIntMember=200 +Tree (splitlevel 99) i=2: fId=3 fMember=30 fIntMember=300 +(int) 0 diff --git a/roottest/root/io/datamodelevolution/stl/nestedsource/execWriteNestedSource.cxx b/roottest/root/io/datamodelevolution/stl/nestedsource/execWriteNestedSource.cxx new file mode 100644 index 0000000000000..2b32dd722a51b --- /dev/null +++ b/roottest/root/io/datamodelevolution/stl/nestedsource/execWriteNestedSource.cxx @@ -0,0 +1,65 @@ +// Write a file with the old data model, where the interesting values are +// stored inside the nested `Deep` struct. They are read back by +// execReadNestedSource.cxx with a data model in which those values have been +// moved to the top level of `Event`. +// See https://github.com/root-project/root/issues/19773 + +#include +#include +#include + +#include +#include + +struct Deep { + float fDeepMember = 0.; + int fDeepInt = 0; + + ClassDefNV(Deep, 1) +}; + +struct Event { + int fId = 0; + Deep fDeep; + + ClassDefNV(Event, 1) +}; + +#ifdef __ROOTCLING__ +#pragma link C++ class Deep+; +#pragma link C++ class Event+; +#pragma link C++ class std::vector+; +#endif + +int execWriteNestedSource() +{ + TFile file("nestedsource.root", "RECREATE"); + + Event single; + single.fId = 1; + single.fDeep.fDeepMember = 10.; + single.fDeep.fDeepInt = 100; + file.WriteObject(&single, "e"); + + std::vector vec; + for (int i = 1; i <= 3; ++i) { + Event event; + event.fId = i; + event.fDeep.fDeepMember = 10. * i; + event.fDeep.fDeepInt = 100 * i; + vec.push_back(event); + } + auto *vecPtr = &vec; + file.WriteObject(vecPtr, "ve"); + + TTree tree("t", ""); + tree.Branch("ve0", "std::vector", &vecPtr, 32000, 0); + tree.Branch("ve99", "std::vector", &vecPtr, 32000, 99); + tree.Fill(); + + file.Write(); + + std::cout << "Wrote " << vec.size() << " events.\n"; + + return 0; +} diff --git a/roottest/root/io/datamodelevolution/stl/nestedsource/execWriteNestedSource.ref b/roottest/root/io/datamodelevolution/stl/nestedsource/execWriteNestedSource.ref new file mode 100644 index 0000000000000..0f940033ad20d --- /dev/null +++ b/roottest/root/io/datamodelevolution/stl/nestedsource/execWriteNestedSource.ref @@ -0,0 +1,3 @@ +Processing execWriteNestedSource.cxx+... +Wrote 3 events. +(int) 0