Skip to content

Commit c446f2f

Browse files
Bugfix: recursing to parent object was invoking copy constructor of store object
Using static_cast to type was invoking copy constructor because a new object was created. This was in particular a problem if the store object is a std::vector because new memory is allocated, while the store pointer is connected to the branch. Not yet debugged to full extend but this is very likely related to the fact, that the branch creates sub-branches for the underlying type of the vector. This might also be the reason for the problem earlier experienced with an attempt to use the extracted object directly via SetBranchAddress. In summary, static cast to type reference has to be used static_cast<T>.method(...) -> static_cast<T&>.method(...) -> T::method(...) Filling now the branches individually, so the bookkeeping for the extracted object has been removed.
1 parent e7aa961 commit c446f2f

File tree

1 file changed

+10
-23
lines changed

1 file changed

+10
-23
lines changed

Framework/Utils/include/Utils/RootTreeWriter.h

Lines changed: 10 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -107,24 +107,18 @@ class RootTreeWriter
107107

108108
/// process functor
109109
/// It expects a context which is used by lambda capture in the snapshot function.
110-
/// Recursively process all inputs and set the branch address to extracted objects.
111-
/// Release function is called on the object after filling the tree.
110+
/// Recursively process all inputs and fill branches individually from extracted
111+
/// objects.
112112
template <typename ContextType>
113113
void operator()(ContextType&& context)
114114
{
115115
if (!mTree || !mFile || mFile->IsZombie()) {
116116
throw std::runtime_error("Writer is invalid state, probably closed previously");
117117
}
118+
// execute tree structure handlers and fill the individual branches
118119
mTreeStructure->exec(std::forward<ContextType>(context), mBranchSpecs);
119-
mTree->Fill();
120-
for (auto& spec : mBranchSpecs) {
121-
if (!spec.releaseFct) {
122-
continue;
123-
}
124-
spec.releaseFct(spec.data);
125-
spec.data = nullptr;
126-
spec.releaseFct = nullptr;
127-
}
120+
// set the number of elements according to branch content
121+
mTree->SetEntries();
128122
}
129123

130124
/// write the tree and close the file
@@ -148,8 +142,6 @@ class RootTreeWriter
148142
struct BranchSpec {
149143
key_type key;
150144
std::string name;
151-
void* data = nullptr;
152-
std::function<void(void*)> releaseFct = nullptr;
153145
TBranch* branch = nullptr;
154146
TClass* classinfo = nullptr;
155147
};
@@ -201,7 +193,7 @@ class RootTreeWriter
201193
/// setup this instance and recurse to the parent one
202194
void setupInstance(std::vector<BranchSpec>& specs, TTree* tree)
203195
{
204-
static_cast<PrevT>(*this).setupInstance(specs, tree);
196+
PrevT::setupInstance(specs, tree);
205197
constexpr size_t Index = STAGE - 1;
206198
specs[Index].branch = tree->Branch(specs[Index].name.c_str(), &mStore);
207199
specs[Index].classinfo = TClass::GetClass(typeid(type));
@@ -211,7 +203,7 @@ class RootTreeWriter
211203
// process previous stage and this stage
212204
void process(InputContext& context, std::vector<BranchSpec>& specs)
213205
{
214-
static_cast<PrevT>(*this).process(context, specs);
206+
PrevT::process(context, specs);
215207
constexpr size_t Index = STAGE - 1;
216208
auto data = context.get<type*>(specs[Index].key.c_str());
217209
// could either copy to the corresponding store variable or use the object
@@ -220,20 +212,15 @@ class RootTreeWriter
220212
// FIXME: get rid of the const_cast
221213
// FIXME: using object directly results in a segfault in the Streamer when
222214
// using std::vector<o2::test::Polymorphic> in test_RootTreeWriter.cxx
215+
// for std::vector, the branch has sub-branches so maybe the address can not
216+
// simply be set
223217
//specs[Index].branch->SetAddress(const_cast<type*>(data.get()));
224218
// handling copy-or-move is also more complicated because the returned smart
225219
// pointer wraps a const buffer which might reside in the input queue and
226220
// thus can not be moved.
227221
//copyOrMove(mStore, (type&)*data);
228222
mStore = *data;
229-
// the data object is a smart pointer and we have to keep the data alieve
230-
// until the tree is actually filled after the recursive processing of inputs
231-
// release the base pointer from the smart pointer instance and keep the
232-
// daleter to be called after tree fill.
233-
// Note: the deleter might not be existent if the content was moved before
234-
specs[Index].data = const_cast<type*>(data.release());
235-
auto deleter = data.get_deleter();
236-
specs[Index].releaseFct = [deleter](void* buffer) { deleter(reinterpret_cast<type*>(buffer)); };
223+
specs[Index].branch->Fill();
237224
}
238225

239226
private:

0 commit comments

Comments
 (0)