From c2038667a4f26635e723686da7d783551398aad9 Mon Sep 17 00:00:00 2001
From: Philippe Canal <pcanal@fnal.gov>
Date: Thu, 11 Aug 2011 16:05:17 +0000
Subject: [PATCH] From ideas provided by Mike Marino, introduce
 TClonesArray::ConstructedAt which always returns an already constructed
 object.   If the slot is being used for the first time, it calls the default
 constructor otherwise it returns the object as is (unless a string is passed
 as the 2nd argument to the function in which case, it also calls
 Clear(second_argument) on the object). This allows replace code like:    for
 (int i = 0; i < ev->Ntracks; i++) {        new(a[i]) TTrack(x,y,z,...);      
  ...        ...    }    ...    a.Delete(); // or a.Clear("C") with the
 simpler and more efficient:    for (int i = 0; i < ev->Ntracks; i++) {       
 TTrack *track = (TTrack*)a.ConstructedAt(i);        track->Set(x,y,z,....);  
      ...        ...    }    ...    a.Clear(); even in case where the TTrack
 class allocates memory.

git-svn-id: http://root.cern.ch/svn/root/trunk@40562 27541ba8-7e3a-0410-8455-c3a389f83636
---
 core/cont/inc/TClonesArray.h              |  4 +-
 core/cont/src/TClonesArray.cxx            | 90 ++++++++++++++++++++--
 docbook/users-guide/CollectionClasses.xml | 93 ++++++++++++-----------
 docbook/users-guide/InputOutput.xml       |  3 +-
 test/Event.cxx                            | 87 +++++++++++++++++++--
 test/Event.h                              |  3 +-
 test/EventMT.cxx                          |  9 ++-
 7 files changed, 221 insertions(+), 68 deletions(-)

diff --git a/core/cont/inc/TClonesArray.h b/core/cont/inc/TClonesArray.h
index fb0a4a93cb1..e6b8b0f5a06 100644
--- a/core/cont/inc/TClonesArray.h
+++ b/core/cont/inc/TClonesArray.h
@@ -18,7 +18,7 @@
 // TClonesArray                                                         //
 //                                                                      //
 // An array of clone TObjects. The array expands automatically when     //
-// adding elements (shrinking can be done by hand).                     //
+// adding elements (shrinking can be done explicitly).                  //
 //                                                                      //
 //////////////////////////////////////////////////////////////////////////
 
@@ -66,6 +66,8 @@ public:
    void             AddBefore(const TObject *, TObject *) { MayNotUse("AddBefore"); }
    void             BypassStreamer(Bool_t bypass=kTRUE);
    Bool_t           CanBypassStreamer() const { return TestBit(kBypassStreamer); }
+   TObject         *ConstructedAt(Int_t idx);
+   TObject         *ConstructedAt(Int_t idx, Option_t *clear_options);
    void             SetClass(const char *classname,Int_t size=1000);
    void             SetClass(const TClass *cl,Int_t size=1000);
 
diff --git a/core/cont/src/TClonesArray.cxx b/core/cont/src/TClonesArray.cxx
index d719ba569ca..f9ed65c251c 100644
--- a/core/cont/src/TClonesArray.cxx
+++ b/core/cont/src/TClonesArray.cxx
@@ -41,12 +41,30 @@
 //         ...                                                          //
 //      }                                                               //
 //      ...                                                             //
-//      a.Delete();                                                     //
+//      a.Delete(); // or a.Clear() or a.Clear("C")                     //
+//   }                                                                  //
+//                                                                      //
+// To reduce the number of call to the constructor (especially useful   //
+// if the user class requires memory allocation), the object can be     //
+// added (and constructed when needed) using ConstructedAt which only   //
+// calls the constructor once per slot.                                 //
+//                                                                      //
+//   TClonesArray a("TTrack", 10000);                                   //
+//   while (TEvent *ev = (TEvent *)next()) {      // O(100000) events   //
+//      for (int i = 0; i < ev->Ntracks; i++) {   // O(10000) tracks    //
+//         TTrack *track = (TTrack*)a.ConstructedAt(i);                 //
+//         track->Set(x,y,z,....);                                      //
+//         ...                                                          //
+//         ...                                                          //
+//      }                                                               //
+//      ...                                                             //
+//      a.Clear(); // or a.Clear("C");                                  //
 //   }                                                                  //
 //                                                                      //
 // Note: the only supported way to add objects to a TClonesArray is     //
-// via the new with placement method. The diffrent Add() methods of     //
-// TObjArray and its base classes are not allowed.                      //
+// via the new with placement method or the ConstructedAt method.       //
+// The other Add() methods ofTObjArray and its base classes are not     //
+// allowed.                                                             //
 //                                                                      //
 // Considering that a new/delete costs about 70 mus on a 300 MHz HP,    //
 // O(10^9) new/deletes will save about 19 hours.                        //
@@ -317,6 +335,52 @@ void TClonesArray::Compress()
    R__ASSERT(je == jf);
 }
 
+//______________________________________________________________________________
+TObject *TClonesArray::ConstructedAt(Int_t idx)
+{
+   // Get an object at index 'idx' that is guaranteed to have been constructed.
+   // It might be either a freshly allocated object or one that had already been
+   // allocated (and assumingly used).  In the later case, it is the callers 
+   // responsability to insure that the object is returned to a known state,
+   // usually by calling the Clear method on the TClonesArray.
+   //
+   // Tests to see if the destructor has been called on the object.  
+   // If so, or if the object has never been constructed the class constructor is called using
+   // New().  If not, return a pointer to the correct memory location.
+   // This explicitly to deal with TObject classes that allocate memory
+   // which will be reset (but not deallocated) in their Clear()
+   // functions.
+   
+   TObject *obj = (*this)[idx];
+   if ( obj && obj->TestBit(TObject::kNotDeleted) ) {
+      return obj;
+   }
+   return (fClass) ? static_cast<TObject*>(fClass->New(obj)) : 0;
+}
+   
+//______________________________________________________________________________
+TObject *TClonesArray::ConstructedAt(Int_t idx, Option_t *clear_options)
+{
+   // Get an object at index 'idx' that is guaranteed to have been constructed.
+   // It might be either a freshly allocated object or one that had already been
+   // allocated (and assumingly used).  In the later case, the function Clear
+   // will be called and passed the value of 'clear_options'
+   //
+   // Tests to see if the destructor has been called on the object.  
+   // If so, or if the object has never been constructed the class constructor is called using
+   // New().  If not, return a pointer to the correct memory location.
+   // This explicitly to deal with TObject classes that allocate memory
+   // which will be reset (but not deallocated) in their Clear()
+   // functions.
+   
+   TObject *obj = (*this)[idx];
+   if ( obj && obj->TestBit(TObject::kNotDeleted) ) {
+      obj->Clear(clear_options);
+      return obj;
+   }
+   return (fClass) ? static_cast<TObject*>(fClass->New(obj)) : 0;
+}
+
 //______________________________________________________________________________
 void TClonesArray::Clear(Option_t *option)
 {
@@ -332,12 +396,19 @@ void TClonesArray::Clear(Option_t *option)
 
    if (option && option[0] == 'C') {
       const char *cplus = strstr(option,"+");
+      if (cplus) {
+         cplus = cplus + 1;
+      } else {
+         cplus = "";
+      }
       Int_t n = GetEntriesFast();
       for (Int_t i = 0; i < n; i++) {
          TObject *obj = UncheckedAt(i);
          if (obj) {
-            if (cplus) obj->Clear(cplus+1);
-            else obj->Clear();
+            obj->Clear(cplus);
+            obj->ResetBit( kHasUUID ); 
+            obj->ResetBit( kIsReferenced ); 
+            obj->SetUniqueID( 0 );
          }
       }
    }
@@ -829,9 +900,14 @@ TObject *&TClonesArray::operator[](Int_t idx)
    if (idx >= fSize)
       Expand(TMath::Max(idx+1, GrowBy(fSize)));
 
-   if (!fKeep->fCont[idx])
+   if (!fKeep->fCont[idx]) {
       fKeep->fCont[idx] = (TObject*) TStorage::ObjectAlloc(fClass->Size());
-
+      // Reset the bit so that:
+      //    obj = myClonesArray[i];
+      //    obj->TestBit(TObject::kNotDeleted)
+      // will behave correctly.
+      memset(fKeep->fCont[idx], 0, sizeof(TObject));  // TClonesArray requires the class to start with the TObject part.
+   }
    fCont[idx] = fKeep->fCont[idx];
 
    fLast = TMath::Max(idx, GetAbsLast());
diff --git a/docbook/users-guide/CollectionClasses.xml b/docbook/users-guide/CollectionClasses.xml
index 49b587b70df..ca8afa71725 100644
--- a/docbook/users-guide/CollectionClasses.xml
+++ b/docbook/users-guide/CollectionClasses.xml
@@ -41,7 +41,7 @@ Collections act as flexible alternatives to traditional data structures of compu
 
 <programlisting language="c++">
 if (myObject-&gt;InheritsFrom("TParticle") {
-printf("myObject is a TParticlen");
+   printf("myObject is a TParticlen");
 }
 </programlisting>
 
@@ -118,17 +118,17 @@ These include:</para>
 
 <programlisting language="c++">
 class TEvent : public TObject {
-private:
-TList *fTracks;  <code>//list of all tracks</code>
-TList *fVertex1; <code>//subset of tracks part of vertex1</code>
-TList *fVertex2; <code>//subset of tracks part of vertex2</code>
+   private:
+   TList *fTracks;  <code>//list of all tracks</code>
+   TList *fVertex1; <code>//subset of tracks part of vertex1</code>
+   TList *fVertex2; <code>//subset of tracks part of vertex2</code>
 };
 TEvent::~TEvent()
 {
-fTracks-&gt;Delete();
-delete fTracks;
-delete fVertex1;
-delete fVertex2;
+   fTracks-&gt;Delete();
+   delete fTracks;
+   delete fVertex1;
+   delete fVertex2;
 }
 </programlisting>
 
@@ -152,23 +152,23 @@ delete fVertex2;
 
 <programlisting language="c++">
 class TObjNum : public TObject {
-private:
-Int_t  num; <code> // TObjNum is a simple container for an integer.</code>
+   private:
+   Int_t  num; <code> // TObjNum is a simple container for an integer.</code>
 public:
-TObjNum(Int_t i = 0) : num(i) { }
-~TObjNum() { }
-void     SetNum(Int_t i) { num = i; }
-Int_t    GetNum() const { return num; }
-void     Print(Option_t *) const
-{ printf("num = %dn", num); }
-Bool_t   IsEqual(TObject *obj) const
-{ return num == ((TObjNum*)obj)-&gt;num; }
-Bool_t   IsSortable() const { return kTRUE; }
-Int_t    Compare(const TObject *obj) const
-{ if (num &lt; ((TObjNum*)obj)-&gt;num) return -1;
-else if (num &gt; ((TObjNum*)obj)-&gt;num) return 1;
-else return 0; }
-ULong_t  Hash() const { return num; }
+   TObjNum(Int_t i = 0) : num(i) { }
+   ~TObjNum() { }
+   void     SetNum(Int_t i) { num = i; }
+   Int_t    GetNum() const { return num; }
+   void     Print(Option_t *) const
+      { printf("num = %dn", num); }
+   Bool_t   IsEqual(TObject *obj) const
+      { return num == ((TObjNum*)obj)-&gt;num; }
+   Bool_t   IsSortable() const { return kTRUE; }
+   Int_t    Compare(const TObject *obj) const
+      { if (num &lt; ((TObjNum*)obj)-&gt;num) return -1;
+        else if (num &gt; ((TObjNum*)obj)-&gt;num) return 1;
+        else return 0; }
+   ULong_t  Hash() const { return num; }
 };
 </programlisting>
 
@@ -264,7 +264,7 @@ GetListOfPrimitives()-&gt;ForEach(TObject,Draw)();
 <programlisting language="c++">
 TIter next(GetListOfTracks());
 while ((TTrack *obj = (TTrack *)next()))
-obj-&gt;Draw();
+   obj-&gt;Draw();
 </programlisting>
 <itemizedlist>
 <listitem><para>Using the <emphasis role="bold"><code>TObjLink</code></emphasis> list entries (that wrap the <emphasis role="bold"><code>TObject</code></emphasis>*):</para></listitem>
@@ -272,8 +272,8 @@ obj-&gt;Draw();
 <programlisting language="c++">
 TObjLink *lnk = GetListOfPrimitives()-&gt;FirstLink();
 while (lnk) {
-lnk-&gt;GetObject()-&gt;Draw();
-lnk = lnk-&gt;Next();
+   lnk-&gt;GetObject()-&gt;Draw();
+   lnk = lnk-&gt;Next();
 }
 </programlisting>
 <itemizedlist>
@@ -282,8 +282,8 @@ lnk = lnk-&gt;Next();
 <programlisting language="c++">
 TFree *idcur = this;
 while (idcur) {
-...
-idcur = (TFree*)GetListOfFree()-&gt;After(idcur);
+   ...
+   idcur = (TFree*)GetListOfFree()-&gt;After(idcur);
 }
 </programlisting>
 
@@ -309,7 +309,7 @@ idcur = (TFree*)GetListOfFree()-&gt;After(idcur);
 <programlisting language="c++">
 for (int i = 0; i &lt;= fArr.GetLast(); i++)
 if ((track = (TTrack*)fArr[i]))     <emphasis role="italic"><code>// or fArr.At(i)</code></emphasis>
-track-&gt;Draw();
+   track-&gt;Draw();
 </programlisting>
 
 <para>Main features of <emphasis role="bold"><code>TObjArray</code></emphasis> are simple, well-known array semantics. <emphasis role="bold">Overhead per element</emphasis>: none, except possible over sizing of <code>fCont</code>.</para>
@@ -334,12 +334,12 @@ track-&gt;Draw();
 <programlisting language="c++">
 TObjArray a(10000);
 while (TEvent *ev = (TEvent *)next()) {        <emphasis role="italic"><code>// O(100000)</code></emphasis>
-for (int i = 0; i &lt; ev-&gt;Ntracks; i++) {     <emphasis role="italic"><code>// O(10000)</code></emphasis>
-a[i] = new TTrack(x,y,z,...);
-...
-}
-...
-a.Delete();
+   for (int i = 0; i &lt; ev-&gt;Ntracks; i++) {     <emphasis role="italic"><code>// O(10000)</code></emphasis>
+      a[i] = new TTrack(x,y,z,...);
+      ...
+   }
+   ...
+   a.Delete();
 }
 </programlisting>
 
@@ -349,12 +349,13 @@ a.Delete();
 <programlisting language="c++">
 TClonesArray a("TTrack", 10000);
 while (TEvent *ev = (TEvent *)next()) {      <code>// O(100000</code><emphasis role="italic">)</emphasis>
-for (int i = 0; i &lt; ev-&gt;Ntracks; i++) {   <code>// O(10000)</code>
-new(a[i]) TTrack(x,y,z,...);
-...
-}
-...
-a.Delete();
+   for (int i = 0; i &lt; ev-&gt;Ntracks; i++) {   <code>// O(10000)</code>
+      TTrack *track = (Track*)a.ConstructedAt(i);
+      track->Set(x,y,z,...);
+      ...
+   }
+   ...
+   a.Clear(); // Or Clear("C") if the track objects must be returned (via Track::Clear) to a default state.
 }
 </programlisting>
 
@@ -370,9 +371,9 @@ a.Delete();
 <programlisting language="c++">
 template&lt;class T&gt;
 class ArrayContainer {
-private:
-T *member[10];
-...
+   private:
+   T *member[10];
+   ...
 };
 </programlisting>
 
diff --git a/docbook/users-guide/InputOutput.xml b/docbook/users-guide/InputOutput.xml
index 9094f07363a..03608b69d47 100644
--- a/docbook/users-guide/InputOutput.xml
+++ b/docbook/users-guide/InputOutput.xml
@@ -1467,7 +1467,8 @@ fTracks-&gt;BypassStreamer(kFALSE);    <emphasis role="italic"><code>// use the
 <programlisting language="c++">
 <code>TRef   fLastTrack;         </code>    <emphasis role="italic"><code>//pointer to last track</code></emphasis>
 <code>…</code>
-<code>Track *track = new(tracks[fNtrack++])Track(random);</code>
+<code>Track *track = (Track*)fTracks->ConstructedAt(fNtrack++);
+track->Set(random);</code>
 <emphasis role="italic"><code>// Save reference to last Track in the collection of Tracks</code></emphasis>
 <code>fLastTrack = track;</code>
 </programlisting>
diff --git a/test/Event.cxx b/test/Event.cxx
index def16822501..f7fd4ac1a36 100644
--- a/test/Event.cxx
+++ b/test/Event.cxx
@@ -192,8 +192,8 @@ Track *Event::AddTrack(Float_t random, Float_t ptmin)
    // is called. If tracks[i] is 0, a new Track object will be created
    // otherwise the previous Track[i] will be overwritten.
 
-   TClonesArray &tracks = *fTracks;
-   Track *track = new(tracks[fNtrack++]) Track(random);
+   Track *track = (Track*)fTracks->ConstructedAt(fNtrack++);
+   track->Set(random);
    //Save reference to last Track in the collection of Tracks
    fLastTrack = track;
    //Save reference in fHighPt if track is a high Pt track
@@ -252,7 +252,7 @@ void Event::SetRandomVertex() {
 }
 
 //______________________________________________________________________________
-Track::Track(const Track &orig) : TObject(orig)
+Track::Track(const Track &orig) : TObject(orig),fTriggerBits(orig.fTriggerBits)
 {
    // Copy a track object
 
@@ -286,9 +286,6 @@ Track::Track(const Track &orig) : TObject(orig)
       fPointValue = 0;
    }
    fValid  = orig.fValid;
-
-   fTriggerBits = orig.fTriggerBits;
-
 }
 
 //______________________________________________________________________________
@@ -373,12 +370,16 @@ Track &Track::operator=(const Track &orig)
       fNsp = orig.fNsp;
       if (fNsp == 0) {
          delete [] fPointValue;
+         fPointValue = 0;
       } else {
          for(int i=0; i<fNsp; i++) {
             fPointValue[i] = orig.fPointValue[i];
          }         
       }
    } else {
+      if (fNsp) {
+         delete [] fPointValue;
+      }
       fNsp = orig.fNsp;
       if (fNsp) {
          fPointValue = new Double32_t[fNsp];
@@ -399,9 +400,79 @@ Track &Track::operator=(const Track &orig)
 //______________________________________________________________________________
 void Track::Clear(Option_t * /*option*/)
 {
+   // Note that we intend on using TClonesArray::ConstructedAt, so we do not
+   // need to delete any of the arrays.
+
+   TObject::Clear();
    fTriggerBits.Clear(); 
-   delete [] fPointValue; 
-   fPointValue = 0; 
+}
+
+//______________________________________________________________________________
+void Track::Set(Float_t random)
+{
+   // Set the values of the Track data members.
+   
+   Float_t a,b,px,py;
+   gRandom->Rannor(px,py);
+   fPx = px;
+   fPy = py;
+   fPz = TMath::Sqrt(px*px+py*py);
+   fRandom = 1000*random;
+   if (fRandom < 10) fMass2 = 0.106;
+   else if (fRandom < 100) fMass2 = 0.8;
+   else if (fRandom < 500) fMass2 = 4.5;
+   else if (fRandom < 900) fMass2 = 8.9;
+   else  fMass2 = 9.8;
+   gRandom->Rannor(a,b);
+   fBx = 0.1*a;
+   fBy = 0.1*b;
+   fMeanCharge = 0.01*gRandom->Rndm(1);
+   gRandom->Rannor(a,b);
+   fXfirst = a*10;
+   fXlast  = b*10;
+   gRandom->Rannor(a,b);
+   fYfirst = a*12;
+   fYlast  = b*16;
+   gRandom->Rannor(a,b);
+   fZfirst = 50 + 5*a;
+   fZlast  = 200 + 10*b;
+   fCharge = Double32_t(Int_t(3*gRandom->Rndm(1)) - 1);
+   
+   fTriggerBits.SetBitNumber((UInt_t)(64*gRandom->Rndm(1)));
+   fTriggerBits.SetBitNumber((UInt_t)(64*gRandom->Rndm(1)));
+   fTriggerBits.SetBitNumber((UInt_t)(64*gRandom->Rndm(1)));
+   
+   fVertex[0] = gRandom->Gaus(0,0.1);
+   fVertex[1] = gRandom->Gaus(0,0.2);
+   fVertex[2] = gRandom->Gaus(0,10);
+   fNpoint = Int_t(60+10*gRandom->Rndm(1));
+   Int_t newNsp = Int_t(3*gRandom->Rndm(1));
+   if (fNsp > newNsp) {
+      fNsp = newNsp;
+      if (fNsp == 0) {
+         delete [] fPointValue;
+         fPointValue = 0;
+      } else {
+         for(int i=0; i<fNsp; i++) {
+            fPointValue[i] = i+1;
+         }         
+      }
+      
+   } else {
+      if (fNsp) {
+         delete [] fPointValue;
+      }
+      fNsp = newNsp;
+      if (fNsp) {
+         fPointValue = new Double32_t[fNsp];
+         for(int i=0; i<fNsp; i++) {
+            fPointValue[i] = i+1;
+         }
+      } else {
+         fPointValue = 0;
+      }
+   }
+   fValid  = Int_t(0.6+gRandom->Rndm(1));
 }
 
 //______________________________________________________________________________
diff --git a/test/Event.h b/test/Event.h
index d03e8373674..3b34bb132ad 100644
--- a/test/Event.h
+++ b/test/Event.h
@@ -46,12 +46,13 @@ private:
    TBits        fTriggerBits;  //Bits triggered by this track.
 
 public:
-   Track() { fPointValue = 0; }
+   Track() : fTriggerBits(64) { fNsp = 0; fPointValue = 0; }
    Track(const Track& orig);
    Track(Float_t random);
    virtual ~Track() {Clear();}
    Track &operator=(const Track &orig);
 
+   void          Set(Float_t random);
    void          Clear(Option_t *option="");
    Float_t       GetPx() const { return fPx; }
    Float_t       GetPy() const { return fPy; }
diff --git a/test/EventMT.cxx b/test/EventMT.cxx
index f74d53455ab..61739258724 100644
--- a/test/EventMT.cxx
+++ b/test/EventMT.cxx
@@ -232,7 +232,7 @@ void Event::SetRandomVertex() {
 }
 
 //______________________________________________________________________________
-Track::Track(const Track &orig) : TObject(orig)
+Track::Track(const Track &orig) : TObject(orig),fTriggerBits(orig.fTriggerBits)
 {
    // Copy a track object
 
@@ -266,9 +266,6 @@ Track::Track(const Track &orig) : TObject(orig)
       fPointValue = 0;
    }
    fValid  = orig.fValid;
-
-   fTriggerBits = orig.fTriggerBits;
-
 }
 
 //______________________________________________________________________________
@@ -353,12 +350,16 @@ Track &Track::operator=(const Track &orig)
       fNsp = orig.fNsp;
       if (fNsp == 0) {
          delete [] fPointValue;
+         fPointValue = 0;
       } else {
          for(int i=0; i<fNsp; i++) {
             fPointValue[i] = orig.fPointValue[i];
          }         
       }
    } else {
+      if (fNsp) {
+         delete [] fPointValue;
+      }
       fNsp = orig.fNsp;
       if (fNsp) {
          fPointValue = new Double32_t[fNsp];
-- 
GitLab