Re: Suggestion for change of BrMultRdo (and BrTofRdo?)

From: Stephen J. Sanders (ssanders@ku.edu)
Date: Tue Jul 31 2001 - 14:48:13 EDT

  • Next message: Ian Bearden: "RE: Suggestion for change of BrMultRdo (and BrTofRdo?)"

    Hi Christian,  FYI...This change should not affect anything that I'm 
    doing since,
    so far, I have primarily worked from the raw data files.  However, since 
    the current
    organization lends itself to using the brat output files, from my own 
    perspective
    making this change sooner rather than later would be best.
    
    I'll respond to the latest messages concerning the brag-brat2 question after
    I have a change to check out a few more things, such as system dependence.
    So far, I have only been working on ppc's.
    
    Your "time" trending program should be quite usefull.  I hope to have 
    revised
    calibrations for some of the most recent runs by later this week, assuming I
    can figure out my brag-brat2 problem. 
    
    
    Regards, Steve
    
    Christian Holm Christensen wrote:
    
    > Hi all, 
    > 
    > Ok, after some long analysis of the a serious memory leak in a program
    > of Eriks, it's become apparent to us that the members
    > BrMultRdo::fSingles and BrMultRdo::fRings are at the root (no pun
    > intended) of this.  The problem is, if you store BrMultRdo objects on
    > a TBranch in a TTree, and then later read it back with 
    > 
    >   BrMultRdo* tmaRdo = 0; 
    >   tree->SetBranchAddress("TMA", &Rdo); 
    > 
    >   Ssiz_t n = tree->GetEntries(); 
    >   for (Int_t i = 0; i < n; i++) { 
    >     tree->GetEntry(i); 
    > 
    >     ...
    > 
    >     tmaRdo->Clear(); 
    >   }
    > 
    > the TTree will not see the members BrMultRdo::fSingles and
    > BrMultRdo::fRings as TClonesArrays, but rather as the contained
    > TObjArray.  Therefore, the TTree will allocate new memory for the
    > BrMultRdo::BrSingle and BrMultRdo::BrRing contained in these
    > collections, which leads to a very serious memory leak.  
    > 
    > We tried a quick work around this problem, by introduceing the method 
    > BrMultRdo::Delete like: 
    > 
    >   void BrMultRdo::Delete(Option_t* option) 
    >   { 
    >     fSingles.Draw(); 
    >     fRings.Draw(); 
    >   }
    > 
    > and then loop over the TTree like: 
    > 
    >   BrMultRdo* tmaRdo = 0; 
    >   tree->SetBranchAddress("TMA", &Rdo); 
    > 
    >   Ssiz_t n = tree->GetEntries(); 
    >   for (Int_t i = 0; i < n; i++) { 
    >     tree->GetEntry(i); 
    > 
    >     ...
    > 
    >     tmaRdo->Delete(); 
    >   }
    > 
    > However, this not only doesn't work, but it also defies the purpose of
    > having TClonesArray all together.  
    > 
    > So the way to fix this, is to make the internal TClonesArray objects
    > of BrMultRdo to pointers to TClonesArray objects.  Then the TTree will 
    > see these as TClonesArray and will not allocate more memory than
    > needed. 
    > 
    > However, this will cause some reading of data files generated with
    > BRAT upto version 2.0.18 to be impossible.  It seems that using a
    > TClonesArray object in the way that I did is not as well supported as
    > I initially believed.  This problem really came as a huge surprise to
    > me, 'cause I did all sorts of experiment with BrMultRdo before
    > settleing on the current design, including I/O tests and reading loads of
    > documentation, non of which indicated a problem in this respect tough,
    > I  just didn't see this problem. 
    > 
    > Because the proposed change to BrMultRdo (and BrTofRdo) will
    > explicitly break backward compatiblity I'm giving you a heads up on
    > this.  We'd really like to make this change, but if someone objects
    > loudly we must discuss this.   
    > 
    > I should say though, that I don't think this will be a major
    > showstopper since not that there are probably not that many files
    > around made with BRAT 2 and containing BrMultRdo and BrTofRdo objects,
    > and after all, BRAT 2 is not our production version yet.  
    > 
    > Having said that, I must stress that if we are to make this change,
    > it most be within a week or so, since the sooner we do it, the less
    > hassle. 
    > 
    > The ROOT people seems to be willing to make changes to support
    > TClonesArray object members rather than pointer to TClonesArray object 
    > members, but since I last stumbled across this problem,  I've seen
    > nothing to indicate any development in this field.  The problem I
    > orginally saw is not this, but I believe related, see 
    > 
    >     http://root.cern.ch/root/roottalk/roottalk01/2567.html
    > 
    > and follow-ups.  I wrote an email to Philippe Canal today requesting
    > this feature ASAP. I'm yet to hear anything from him.  I'll keep you
    > posted. 
    > 
    > So in conclussion, the sitaution is this: 
    > 
    > * BrMultRdo is ok for normal I/O via BrEventIO 
    > 
    > * BrMultRdo can not be browsed if put as a branch in a TTree 
    > 
    > * Looping over a TTree containing BrMultRdo objects as a branch will
    >   cause a severe memory leak. 
    > 
    > * Both problems can be fixed here and now by turning the TClonesArray
    >   object members in to pointers to TClonesArray object members,
    >   however at the cost of making it harder to read older files (not
    >   impossible - but harder). 
    > 
    > * ROOT will fix this "in the near future" which may mean anything from
    >   days to weeks to months.
    > 
    > Therefore, the questions are these: 
    > 
    > * Do we wait for ROOT?, 
    > 
    > * If not, can we live with the cost of making it impossible to read
    >   some (probably not that many) old files? 
    > 
    > Eagerly awaiting your response. 
    > 
    > Yours, 
    > 
    > Christian  -----------------------------------------------------------
    > Holm Christensen                             Phone:  (+45) 35 35 96 91 
    >   Sankt Hansgade 23, 1. th.                  Office: (+45) 353  25 305 
    >   DK-2200 Copenhagen N                       Web:    www.nbi.dk/~cholm    
    >   Denmark                                    Email:       cholm@nbi.dk
    > 
    > 
    



    This archive was generated by hypermail 2b30 : Tue Jul 31 2001 - 14:49:35 EDT