Merge pull request #123 from pcanal/612-backport-001

612 backport of I/O rules and splitting improvements.
cms-sw · Mar 25, 2019 · 832d6b2 · 832d6b2
2 parents 6296678 + a473580
commit 832d6b2
Show file tree

Hide file tree

Showing 5 changed files with 105 additions and 20 deletions.
diff --git a/io/io/inc/TStreamerInfoActions.h b/io/io/inc/TStreamerInfoActions.h
@@ -42,6 +42,7 @@ namespace TStreamerInfoActions {
       virtual ~TConfiguration() {};
 
       virtual void AddToOffset(Int_t delta);
+      virtual void SetMissing();
 
       virtual TConfiguration *Copy() { return new TConfiguration(*this); }
 
@@ -193,6 +194,7 @@ namespace TStreamerInfoActions {
       ActionContainer_t     fActions;
 
       void AddToOffset(Int_t delta);
+      void SetMissing();
 
       TActionSequence *CreateCopy();
       static TActionSequence *CreateReadMemberWiseActions(TVirtualStreamerInfo *info, TVirtualCollectionProxy &proxy);

diff --git a/io/io/src/TStreamerInfo.cxx b/io/io/src/TStreamerInfo.cxx
@@ -1976,7 +1976,6 @@ void TStreamerInfo::BuildOld()
       std::string typeNameBuf;
       const char* dmType = nullptr;
       Bool_t dmIsPtr = false;
-      Bool_t isUniquePtr = false;
       TDataType* dt(nullptr);
       Int_t ndim = 0 ; //dm->GetArrayDim();
       std::array<Int_t, 5> maxIndices; // 5 is the maximum supported in TStreamerElement::SetMaxIndex
@@ -2012,7 +2011,7 @@ void TStreamerInfo::BuildOld()
             Bool_t nameChanged;
             typeNameBuf = TClassEdit::GetNameForIO(dmType, TClassEdit::EModType::kNone, &nameChanged);
             if (nameChanged) {
-               isUniquePtr = dmIsPtr = TClassEdit::IsUniquePtr(dmType);
+               dmIsPtr = TClassEdit::IsUniquePtr(dmType);
                dmType = typeNameBuf.c_str();
             }
             if ((isStdArray = TClassEdit::IsStdArray(dmType))){ // We tackle the std array case
@@ -2257,7 +2256,7 @@ void TStreamerInfo::BuildOld()
          if (element->GetNewType() != -2) {
             if (dm) {
                if (dmIsPtr) {
-                  if (isUniquePtr || strncmp(dm->GetTitle(),"->",2)==0) {
+                  if (strncmp(dm->GetTitle(),"->",2)==0) {
                      // We are fine, nothing to do.
                      if (newClass->IsTObject()) {
                         newType = kObjectp;

diff --git a/io/io/src/TStreamerInfoActions.cxx b/io/io/src/TStreamerInfoActions.cxx
@@ -63,7 +63,16 @@ namespace TStreamerInfoActions
       // Add the (potentially negative) delta to all the configuration's offset.  This is used by
       // TBranchElement in the case of split sub-object.
 
-      fOffset += delta;
+      if (fOffset != TVirtualStreamerInfo::kMissing)
+         fOffset += delta;
+   }
+
+  void TConfiguration::SetMissing()
+   {
+      // Add the (potentially negative) delta to all the configuration's offset.  This is used by
+      // TBranchElement in the case of split sub-object.
+
+      fOffset = TVirtualStreamerInfo::kMissing;
    }
 
    void TConfiguredAction::PrintDebug(TBuffer &buf, void *addr) const
@@ -153,7 +162,14 @@ namespace TStreamerInfoActions
          // Add the (potentially negative) delta to all the configuration's offset.  This is used by
          // TBranchElement in the case of split sub-object.
 
-         fOffset += delta;
+         if (fOffset != TVirtualStreamerInfo::kMissing)
+            fOffset += delta;
+         fObjectOffset = 0;
+      }
+
+      void SetMissing()
+      {
+         fOffset = TVirtualStreamerInfo::kMissing;
          fObjectOffset = 0;
       }
 
@@ -3588,6 +3604,7 @@ TStreamerInfoActions::TActionSequence *TStreamerInfoActions::TActionSequence::Cr
       }
       return sequence;
 }
+
 void TStreamerInfoActions::TActionSequence::AddToOffset(Int_t delta)
 {
    // Add the (potentially negative) delta to all the configuration's offset.  This is used by
@@ -3603,6 +3620,21 @@ void TStreamerInfoActions::TActionSequence::AddToOffset(Int_t delta)
    }
 }
 
+void TStreamerInfoActions::TActionSequence::SetMissing()
+{
+   // Add the (potentially negative) delta to all the configuration's offset.  This is used by
+   // TBranchElement in the case of split sub-object.
+
+   TStreamerInfoActions::ActionContainer_t::iterator end = fActions.end();
+   for(TStreamerInfoActions::ActionContainer_t::iterator iter = fActions.begin();
+       iter != end;
+       ++iter)
+   {
+      if (!iter->fConfiguration->fInfo->GetElements()->At(iter->fConfiguration->fElemId)->TestBit(TStreamerElement::kCache))
+         iter->fConfiguration->SetMissing();
+   }
+}
+
 TStreamerInfoActions::TActionSequence *TStreamerInfoActions::TActionSequence::CreateCopy()
 {
    // Create a copy of this sequence.

diff --git a/tree/tree/inc/TBranchElement.h b/tree/tree/inc/TBranchElement.h
@@ -228,6 +228,7 @@ class TBranchElement : public TBranch {
    virtual void             SetBranchFolder() { SetBit(kBranchFolder); }
    virtual void             SetClassName(const char* name) { fClassName = name; }
    virtual void             SetOffset(Int_t offset);
+   virtual void             SetMissing();
    inline  void             SetParentClass(TClass* clparent);
    virtual void             SetParentName(const char* name) { fParentName = name; }
    virtual void             SetTargetClass(const char *name);

diff --git a/tree/tree/src/TBranchElement.cxx b/tree/tree/src/TBranchElement.cxx
@@ -2015,7 +2015,15 @@ static void GatherArtificialElements(const TObjArray &branches, TStreamerInfoAct
             ids.back().fNestedIDs->fOwnOnfileObject = kTRUE;
          }
          ids.back().fNestedIDs->fOnfileObject = onfileObject;
-         GatherArtificialElements(branches, ids.back().fNestedIDs->fIDs, ename + ".", nextinfo, offset + nextel->GetOffset());
+         TString subprefix;
+         if (prefix.Length() && nextel->IsA() == TStreamerBase::Class()) {
+             // We skip the name of the base class if there is already a prefix.
+             // See TBranchElement::Unroll
+             subprefix = prefix;
+         } else {
+             subprefix = ename + ".";
+         }
+         GatherArtificialElements(branches, ids.back().fNestedIDs->fIDs, subprefix, nextinfo, offset + nextel->GetOffset());
          if (ids.back().fNestedIDs->fIDs.empty())
             ids.pop_back();
       }
@@ -2395,6 +2403,14 @@ TVirtualCollectionProxy* TBranchElement::GetCollectionProxy()
       } else {
          // We are not a top-level branch.
          TVirtualStreamerInfo* si = thiscast->GetInfoImp();
+         if (fCollProxy) {
+            // The GetInfo set fProxy for us, let's not
+            // redo it; the value of fCollProxy is possibly
+            // used/recorded is the actions sequences, so
+            // if we change it here, we would need to propagate
+            // the change.
+            return fCollProxy;
+         }
          TStreamerElement* se = si->GetElement(fID);
          cl = se->GetClassPointer();
       }
@@ -3515,7 +3531,7 @@ void TBranchElement::InitializeOffsets()
             //
             // Compensate for the i/o routines adding our local offset later.
             if (subBranch->fObject == 0 && localOffset == TStreamerInfo::kMissing) {
-               subBranch->SetOffset(TStreamerInfo::kMissing);
+               subBranch->SetMissing();
                // We stil need to set fBranchOffset in the case of a missing
                // element so that SetAddress is (as expected) not called
                // recursively in this case.
@@ -3649,9 +3665,17 @@ static void PrintElements(const TStreamerInfo *info, const TStreamerInfoActions:
 {
    for(auto &cursor : ids) {
       auto id = cursor.fElemID;
-      if (id >= 0)
-         info->GetElement(id)->ls();
-      else if (cursor.fNestedIDs) {
+      if (id >= 0) {
+         auto el = info->GetElement(id);
+         if (el)
+            el->ls();
+         else {
+            Error("TBranchElement::Print", "Element for id #%d not found in StreamerInfo for %s",
+                  id, info->GetName());
+            info->ls();
+            TClass::GetClass("PFTauWith")->GetStreamerInfos()->ls();
+         }
+      } else if (cursor.fNestedIDs) {
          Printf("      Within subobject of type %s offset = %d", cursor.fNestedIDs->fInfo->GetName(), cursor.fNestedIDs->fOffset);
          PrintElements(cursor.fNestedIDs->fInfo, cursor.fNestedIDs->fIDs);
       }
@@ -3707,7 +3731,8 @@ void TBranchElement::Print(Option_t* option) const
       } else if (!fNewIDs.empty() && GetInfoImp()) {
          TStreamerInfo *localInfo = GetInfoImp();
          if (fType == 3 || fType == 4) {
-            localInfo = (TStreamerInfo *)fClonesClass->GetStreamerInfo();
+            // Search for the correct version.
+            localInfo = FindOnfileInfo(fClonesClass, fBranches);
          }
          PrintElements(localInfo, fNewIDs);
          Printf("   with read actions:");
@@ -5287,7 +5312,6 @@ void TBranchElement::SetAddress(void* addr)
    // We do this only once because it depends only on
    // the type of our object, not on its address.
    if (!fInitOffsets) {
-      // R__LOCKGUARD(gInterpreterMutex);
       InitializeOffsets();
    }
 
@@ -5391,6 +5415,11 @@ void TBranchElement::SetOffset(Int_t offset)
    // We need to make sure that the Read and Write action's configuration
    // properly reflect this value.
 
+   if (offset == TVirtualStreamerInfo::kMissing) {
+       SetMissing();
+       return;
+   }
+
    if (fReadActionSequence) {
       fReadActionSequence->AddToOffset(offset - fOffset);
    }
@@ -5400,6 +5429,24 @@ void TBranchElement::SetOffset(Int_t offset)
    fOffset = offset;
 }
 
+////////////////////////////////////////////////////////////////////////////////
+/// Set offset of the object (to which the data member represented by this
+/// branch belongs) inside its containing object (if any) to mark it as missing.
+
+void TBranchElement::SetMissing()
+{
+   // We need to make sure that the Read and Write action's configuration
+   // properly reflect this value.
+
+   if (fReadActionSequence) {
+      fReadActionSequence->SetMissing();
+   }
+   if (fFillActionSequence) {
+      fFillActionSequence->SetMissing();
+   }
+   fOffset = TVirtualStreamerInfo::kMissing;
+}
+
 
 ////////////////////////////////////////////////////////////////////////////////
 /// Set the sequence of actions needed to read the data out of the buffer.
@@ -5419,17 +5466,21 @@ void TBranchElement::SetActionSequence(TClass *originalClass, TStreamerInfo *loc
 
    if (!isSplitNode)
       fNewIDs.erase(fNewIDs.begin());
-   else {
+
+   else if (fInitOffsets && fType != 3 && fType != 4) {
       // fObject has the address of the sub-object but the streamer action have
       // offset relative to the parent.
+
+      // Note: We skipped this for the top node of split collection because the
+      // sequence is about the content, we need to review what happens where an
+      // action related to the collection itself will land.
       TBranchElement *parent = dynamic_cast<TBranchElement*>(GetMother()->GetSubBranch(this));
-      if (fInitOffsets) {
-         auto index = parent->fBranches.IndexOf(this);
-         if (index >= 0) {
-            fReadActionSequence->AddToOffset( - parent->fBranchOffset[index] );
-         }
-      } // else it will be done by InitOffsets
-   }
+
+      auto index = parent->fBranches.IndexOf(this);
+      if (index >= 0) {
+         actionSequence->AddToOffset( - parent->fBranchOffset[index] );
+      }
+   } // else it will be done by InitOffsets
 }
 
 ////////////////////////////////////////////////////////////////////////////////