What is the Correct Procedure for Using TClonesArray

Hello,

I am a bit confused by the documentation on how to correctly use TClones Array.
(1)In the instructions given in the User Guide (which is the same as on the TClonesArray html site), there are two examples given. As far as I can tell the only difference between the first (O(100000)xO(10000)) and the second (O(10000)) is the way in which a new object is added to the array. Is this really the case? From the text below the examples, I would have thought that the more important line for performance is “a.Delete()”. Should, in the second example, it be changed to “a.Clear()”.
(2) In the explanation of how to use TClonesArray a point is made that TCloneArray::Clear() should be used in place of TClonesArray::Delete(). It would be very useful to have an example of how to use Clear(), i.e. an example of the proper use of the array.
(3) My specific usage example:
I have memory leak I am trying to track down and I want to make sure it is not in my use of TClonesArray.
I define a class with a member:

TClonesArray *fList;

In the construction I initiate the array:

fList = new TClonesArray("MyClass", 10);

I then have an event loop and an object loop within it:

for(int iEvent=0;iEvent<nEvent;iEvent++){
  fList->Clear();
  int nSaved=0;
  for(int i=0;i<nObjects;i++){
    MyObject *o = new MyObject();
    bool pass = PassSelection(o);
    if(!pass){
      delete o;
      continue;
    }
    (*fList)[nSaved++] = MyObject;
  }
  //Use fList
}
fList->Delete();

Is this correct?

Thank you,

-Ben

code[nSaved++] = MyObject;[/code]This is the source of the leak. The left side also allocates a MyObject. Instead use:for(int iEvent=0;iEvent<nEvent;iEvent++){ fList->Delete(); // or Clear() if all the data member are only plain old data type. int nSaved=0; for(int i=0;i<nObjects;i++){ MyObject *o = new ( (*fList)[nSaved++] ) MyObject(); bool pass = PassSelection(o); if(!pass){ delete o; continue; } } //Use fList } fList->Delete(); // You could also do delete fList; if you no longer need the TClonesArray.

[quote]is the way in which a new object is added to the array. Is this really the case? [/quote]Yes it is. The goal is to avoid the memory allocation (included in the ‘usual’ “new MyObject”) and reuse the memory for each iteration.

[quote]from the text below the examples, I would have thought that the more important line for performance is “a.Delete()”. Should, in the second example, it be changed to “a.Clear()”.[/quote]Not yet. Currently, when you fill the TClonesArray directly (as opposed to when you read them from file), you have no good option except to call ‘new ( (*fList)[nSaved++] ) MyObject();’ which does execute the constructor. In the constructor you must assume that none of the memory as been initialized. If your object contains variable size sub object (for example an std::string), this also means that before re-using the memory you would need to ‘destroy’ the existing content (but not release the memory associate with the object itself). In the general case, this can only be done by calling the destructor (but not the operator delete). The member function TClonesArray::Delete does exactly that.

When reading the TClonesArray from file, instead of calling the constructors for each iteration they are called only once and the appropriate call to clean-up between iteration becomes the call to TClonesArray::Clear.

Shortly we will introduce a new interface which will simply this is a bit and your code will becomes:for(int iEvent=0;iEvent<nEvent;iEvent++){ fList->Clear(); int nSaved=0; for(int i=0;i<nObjects;i++){ MyObject *o = (MyObject*)fList->ConstructedAt(nSaved++); // This call the allocation and constructor only once per index. bool pass = PassSelection(o); if(!pass){ delete o; continue; } } //Use fList } fList->Delete();

Cheers,
Philippe.

Phillipe,

Thank you for your response.
In the code you quoted above, where you increment nSaved in the “new” command, what happens to the size of fList when you “delete o”? It seems that the fList will keep increasing in size when it shouldn’t. Also, doesn’t calling delete here default the purpose of a TClonesArray? Would it be better to do:

for(int iEvent=0;iEvent<nEvent;iEvent++){
  fList->Delete();  // or Clear() if all the data member are only plain old data type.
  int nSaved=0;
  for(int i=0;i<nObjects;i++){
    MyObject *o = new (  (*fList)[nSaved]  ) MyObject();
    bool pass = PassSelection(o);
    if(!pass){
      fList->Remove(o);
      continue;
    }
  nSaved++;
  }
  //Use fList
}
fList->Delete(); // You could also do delete fList; if you no longer need the TClonesArray.

Thanks gain,

-Ben

Hi,

Indeed the code I copied did not properly take into account the ‘if(!pass)’ case and as you mentioned ‘delete o’ was very bad (i.e. the object of a TClonesArray should never be deleted directly) and needs to be replaced by Remove. I.e. your latest version of the code seems correct.

Cheers,
Philippe.