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/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 diff --git a/tree/tree/inc/TBranchElement.h b/tree/tree/inc/TBranchElement.h index 39c762041c414..0d00ffd1879f7 100644 --- a/tree/tree/inc/TBranchElement.h +++ b/tree/tree/inc/TBranchElement.h @@ -51,7 +51,9 @@ 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..d02b0d3c16baa 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,13 @@ 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 +5794,19 @@ 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); + } } ////////////////////////////////////////////////////////////////////////////////