Memory leak with custom object branches

ROOT
1

Hello,

this concerns input and output of custom classes in root files.

Over the last few years I had used the dictionary generator and your guides to create custom classes and save run time instances of objects in root files to make them persistent. I had been mainly successful.

In my last attempt, I made a custom class (which does not inherit from TObject and I did not use CladdDef):

class MyClass{
	
public:
	MyClass();
	MyClass(float e, float t, uint16_t q, uint16_t pr,CaloGain::CaloGain g,Double_t x, Double_t y, Double_t z):
	energy(e),time(t),quality(q),provenance(pr),gain(g){
		position.SetXYZ(x,y,z);
		sampling = -1;
		id=0;
	};
	virtual ~MyClass();
	
	float energy;
	float time;
	uint16_t quality;
	uint16_t provenance;
	int sampling;
 
	TVector3 position;
	TVector3 correctedPosition;

	ULong64_t id;
	
	
};
#endif

This worked, I created many root files on the grid (where I did not monitor performance and memory consumption) and the objects are stored successfully even at branches with the quite complicate type:

std::vector<std::vector<MyClass*> >

The latest addition in the class was the two TVector3 objects which I somehow selected to create on the stack.

The root files are created successfully as i said, but when I run a TChain of all the files (combining to 1.2GB), the 3-4GB of RAM available on my system are very quickly depleted and cint (I use a “MakeClass” instance and call Loop() ) exits without a warning.

I don’t think this happened before I added the new TVector3 but I had been noticing the memory filling very quickly. I believe it’s just because of the creation of many MyClass object (each jentry has tens of thousands of entries of type MyClass in the std::vector<std::vector<MyClass*> > structure)

How do I make sure the objects created on access of the std::vector<std::vector<MyClass*> > branch, are deleted? Where is it supposed to be handled?

I have disabled all code in the “Loop” function leaving it like so:

void TimingStudy::Loop()
{
	
	if (fChain == 0) return;
	
	Long64_t nentries = fChain->GetEntriesFast();
	
	Long64_t nbytes = 0, nb = 0;
	for (Long64_t jentry=0; jentry<nentries;jentry++) {
		Long64_t ientry = LoadTree(jentry);
		if (ientry < 0) break;
		nb = fChain->GetEntry(jentry);   nbytes += nb;
        }//end of for

}//Loop

so I am sure I don’t have any obvious memory leaks. The memory still gets depleted equally quickly.

Is it because I did not inherit from TObject? I thought that was not needed any more.

I am trying now to modify my class to inherit from TObject but I am having other problems, but maybe this is another issue.

2

[quote]branch, are deleted? Where is it supposed to be handled?[/quote]A priori the deletion is supposed to happen (as needed) during the call to TTree::GetEntry. At least with v5.30/02 this is indeed exactly what I get.

In your case, it might be due to the fact that the TTree is set in MakeClass mode (SetMakeClass(1)) which tells it not to use objects.

Another solution might be (and the layout of your class suggest this might indeed be an option) to use a vector<vector > rather than containing pointers.

You can also explicitly delete the object at the end of you reading loop.

Is it because I did not inherit from TObject? I thought that was not needed any more.

No, that should have no effect either way as far as the issue you are describing is concerned.

Cheers,
Philippe.

3

Thank you Philippe.

The SetMakeClass(0) did not help at all with the memory leak, but manually deleting the objects made a dramatic change, since in a vector<vector<MyClass*>*> there can be tens of thousands of objects.

But there is still a memory leak. I tried to delete first all the MyClass instances in the inner loop, then the vector<MyClass*> instance in the outer loop. This works and removes a lot of leaks. At the end of the event loop though (over jentries) if I try to delete the object pointed by the full vector<vector<MyClass*>> (that is the one branched to the relevant TTree branch using SetBranchAddress) I get a segmentation fault. Here are the code excerpts so you can understand what I mean:

//Declaration of variables

 vector<vector<MyClass*>*> *ClusterCells;
   UInt_t          ele_N;
   vector<float>   *ele_phi;
   vector<float>   *ele_eta;
//others omitted 
//Declaration of branches
   TBranch        *b_ClusterCells;   //!
   TBranch        *b_ele_N;   //!
   TBranch        *b_ele_phi;   //!
//.......

In TimingStudy::Init(TTree *tree){
//....
   ClusterCells = 0;
   ele_phi = 0;
   ele_eta = 0;
//.....
   fChain->SetBranchAddress("ClusterCells", &ClusterCells, &b_ClusterCells);
   fChain->SetBranchAddress("ele_N", &ele_N, &b_ele_N);
   fChain->SetBranchAddress("ele_phi", &ele_phi, &b_ele_phi);
   fChain->SetBranchAddress("ele_eta", &ele_eta, &b_ele_eta);
///...
}

void TimingStudy::Loop()
{
	//...

	Long64_t nentries = fChain->GetEntriesFast();
	
	Long64_t nbytes = 0, nb = 0;
	for (Long64_t jentry=0; jentry<nentries;jentry++) {
		Long64_t ientry = LoadTree(jentry);
		if (ientry < 0) break;
		nb = fChain->GetEntry(jentry);   nbytes += nb;
	        std::vector<MyCaloCell *> * cellVector;
				for(int idx=0; idx<2; idx++){
					int i=minPair[idx];
					cellVector= ClusterCells->at(i);
					for(uint k=0; k<cellVector->size(); k++){
						MyCaloCell * curCaloCell = cellVector->at(k);
						//end of inner loop
						delete curCaloCell; //Ok
					}//loop over cells
					if(cellVector !=0){
						cellVector->clear();
					} 
					delete cellVector; //Ok
				}//loop over clusters
		if(ClusterCells !=0){
			ClusterCells->clear(); //Ok
                        //delete ClusterCells leads to seg fault
		} 
      }
}

So the question is, when a branch holds a vector, how do we delete the resulting instance?

I compiled the code and ran Valgrind (see attached)and it seams that there are memory leaks even if I delete the MyClass objects and their parent vectors. I think there are even leaks from the complicated MyClass vectors but even from simpler vectors of floats (valgrind output for malloc through new but without delete)

==7104==    by 0x8069170: ROOT::vectorlEfloatgR_Dictionary() (dict.cxx:240)
...
==7104==    by 0x8068CBB: ROOT::vectorlEMyClassmUgR_Dictionary() (dict.cxx:182)
...
==7104==    by 0x8069625: ROOT::vectorlEvectorlEMyClassmUgRmUgR_Dictionary() (dict.cxx:298)

FYI, dict.cxx is the rootcint dictionary for the vector structures, main.cxx is the code that includes the main function that owns the TimingStudy object.

I notice that most of the “new” allocations are done by the Load/LoadTree functions, but there is no “Unload” function to clear the created objects.

If I can recap (sorry for the long post):

A. How can we delete objects directly assigned to branch variables at the end of the loop step (over jentries)? In this case, ClusterCells of type vector<vector<MyClass*>> or even simpler like ele_phi of type vector *. Since it’s a pointer to a vector, and it can be a fairly large vector, at the end of the loop step, if it is not cleared it can lead to memory leaks.

B. is SetMakeClass(0) supposed to be used in the creation of the tree as well? Because setting it at the chain during the read does not work (unless it is not enforced at the change of files) at least under 5.30 on ubuntu.

C. At write time, using objects on the stack to branch, many times leads to seg fault, in my experience. Is branching of “more transient” objects supported (for example on an inner loop where pushing_back objects created on the stack may not guarantee that they survive till the time root writes the vector into the tree)?

Thank you very much,
Nikiforos
valout.txt (239 KB)

4

[quote]At write time, using objects on the stack to branch, many times leads to seg fault, in my experience.[/quote]If you use an object created on the stack, you must make sure that this object’s life time is such that it is not deleted until you have executed all the Fill that you need.

[quote] (for example on an inner loop where pushing_back objects created on the stack may not guarantee that they survive till the time root writes the vector into the tree)[/quote]That depends what you mean … if you mean that you pushing on the vector the address of stack object, this would be a (C++) problem … if you mean that you are pushing a copy of those object, you would be fine. Without actually seen the code, I can not be more precise.

[quote] is SetMakeClass(0) supposed to be used in the creation of the tree as well?[/quote]Yes, but it is the default so it does not need to be set explicitly.

[quote]A. How can we delete objects directly assigned to branch variables at the end of the loop step (over jentries)? In this case, ClusterCells of type vector<vector<MyClass*>> or even simpler like ele_phi of type vector *. Since it’s a pointer to a vector, and it can be a fairly large vector, at the end of the loop step, if it is not cleared it can lead to memory leaks.[/quote] … humm do you really need a vector of pointer of vector to pointer of object … wouldn’t a vector<vector > be simpler to handle?

[quote] How can we delete objects directly assigned to branch variables at the end of the loop step (over jentries)? [/quote]This is technically a C++ question, in your case you need to loop over each of the elements of the vector<vector<MyClass*> >, for each of them you need to delete all their MyClass and then delete the vector themselves.

The I/O by default properly handle the case vector<vector > and the case vector<vector<MyClass*> > in most cases but (unintentionally) fails to delete the content of the inner vector in vector<vector<MyClass*> *>.

Cheers,
Philippe.

5

Thank you Philippe,

I used pointers trying to extend somehow common practices I saw being used.

With the framework I was using I was not sure I could fill the tree before the objects would fall out of scope. So I wasn’t comfortable branching stack objects. But I guess more careful planning and structuring should do it and is indeed much more clean.

My uneducated interpretation of the valgrind output was that it also fails to delete the inner vector itself and the outer vector. I try to delete the inner most contents (MyClass objects) myself in a loop, as I show in an earlier post.

In my case for some reason it appears that SetMakeClass(0) does not do the trick even for the “simpler” vector branches. But, as I said, I really am not an expert.

Thanks for all the advice!

Cheers,
Nikiforos

6

[quote]In my case for some reason it appears that SetMakeClass(0) does not do the trick even for the “simpler” vector branches. [/quote]I can not reproduce this part. Maybe you are using an older version of ROOT?

Philippe.

7

Hi,

The memory leaks coming from nested collection of pointers are fixed in trunk by revision 41025.

Cheers,
Philippe.

PS. With the trunk valgrind does not report any memory leak in root.cern.ch/viewvc/trunk/root/i … t=roottest (they are still memory that is not leaked but is also not deleted at the end, use --suppressions=$ROOTSYS/etc/valgrind-root.supp to hide those)