Leak when reading std::vector

ROOT
1

Dear expert,

I was convinced to be able to read a std::vector from a TTree in the correct way, but maybe I am not. So I have copied the code from the example: root.cern.ch/root/html/tutorial … tor.C.html

Here my code:

int main()
{
  TFile output("output.root");          // line 8
  std::vector<float>* v = nullptr;

  TTree* tree = static_cast<TTree*>(output.Get("tree"));
  TBranch *bvpx = 0;
  tree->SetBranchAddress("vector", &v, &bvpx);
  tree->GetEntry(0);
  tree->ResetBranchAddresses();
}

when running with valgrind I get many definitely lost from line 8. See the attachement. I am using ROOT 6.02/05.

By the way: is this the correct way? Do I really need the TBranch?
vp4.gz (47.8 KB)

2

Hello Wiso,

thanks for the report. I understand two questions in the post:

  1. Is there a memory leak when opening a TFile?
  2. Is the reported snippet the right one to read a tree?
    The answers:
  3. We are not aware of any leak. This could well be a false positive: on which platform are you running?
  4. The snippet is correct. You could also adopt this pattern to access the tree root.cern.ch/root/htmldoc/TTreeReader.html). The latter method is probably the simplest (and typesafe :slight_smile:) to access a branch.

Cheers,
Danilo

3

Hi Wiso,

Note that the semantic of:std::vector<float>* v = nullptr; tree->SetBranchAddress("vector", &v, &bvpx);is that the TTree will allocate the object but the caller (your code) is the owner of the vector.
So to avoid the leak you ought to do: tree->GetEntry(0); tree->ResetBranchAddresses(); // TTree forgets about 'v' and (in this case) does not delete the vector. delete v;

Cheers,
Philippe.

4

[quote=“pcanal”]Hi Wiso,

Note that the semantic of:std::vector<float>* v = nullptr; tree->SetBranchAddress("vector", &v, &bvpx);is that the TTree will allocate the object but the caller (your code) is the owner of the vector.
So to avoid the leak you ought to do: tree->GetEntry(0); tree->ResetBranchAddresses(); // TTree forgets about 'v' and (in this case) does not delete the vector. delete v;

Cheers,
Philippe.[/quote]

Thank you. I have to call delete just at the end or for every event (every time I do GetEntry)?

I guess you are missing the delete in the example too.

5

I have to call delete just at the end or for every event (every time I do GetEntry)?Absolutely not (well you could if you wanted to but then you would be wasting resources).

By design, you want to allocate the object as few time as possible and reuse it for all the loop and then delete it only after you are completely done reading the file (Similarly, you should not call SetBranchAddress nor ResetBranchAddreses within the loop).

You original code snippet does not include any loop. Tweak to have a loop and delete the object correctly here is the example:[code] TFile output(“output.root”); // line 8
std::vector* v = nullptr;

TTree* tree = static_cast<TTree*>(output.Get(“tree”));
TBranch *bvpx = 0;
tree->SetBranchAddress(“vector”, &v, &bvpx);
for(Long64_t e = 0; e < tree->GetEntries(); ++e) {
tree->GetEntry(e);
// use v
}
tree->ResetBranchAddresses();
delete v;
}[/code]

Cheers,
Philippe.

6

Don’t you think that instead of: TTree* tree = static_cast<TTree*>(output.Get("tree")); is would be better to use: TTree *tree; output.GetObject("tree", tree); if (!tree) return -1; // just a precaution Also the “bvpx” seems to be completely redundant in this code.

7

Hi Wile

Don’t you think that instead of:

Yes, I agree. The original code snippet can be improved by using the more modern and safer interface. I am guessing that the intent is to read just the vector in which case the pointer would be useful. Finally the code can also be slightly updated to support also TChain.

[code]{
TFile output(“output.root”);
std::vector* v = nullptr;

TTree *tree; output.GetObject(“tree”, tree);
if (!tree) return -1; // just a precaution
TBranch *bvpx = 0;
tree->SetBranchAddress(“vector”, &v, &bvpx);
for(Long64_t e = 0; e < tree->GetEntriesFast(); ++e) {
Long64_t local = tree->LoadTree(e);
bvpx->GetEntry(local);
// use v
}
tree->ResetBranchAddresses();
delete v;
}[/code]

Cheers,
Philippe.

8

In the original post above I still find the question “Do I really need the TBranch?”.
I think if one wanted to read individual branches, the code could simply contain something like: tree->SetBranchStatus("*", 0); // disable all branches tree->SetBranchStatus("vector", 1); // activate the "vector" branch // ... tree->GetEntry(...);

9

I think if one wanted to read individual branches, the code would contain something like:

Both the technique I shown and the technique using BranchStatus will work. Using BranchStatus is slightly slower (especially for TTree with large number of branches) as each GetEntry needs to always ask which branches are enabled (and the selection using a sorta regex is a bit more cumbersome).

So the answer to

[quote]“Do I really need the TBranch?”.[/quote]Yes if you want to read only that branch (and want more performance).

Cheers,
Philippe.