Make use of ProcessHistoryRegistry thread safe

FWCore/Framework/interface/EventPrincipal.h

@@ -57,7 +57,7 @@ namespace edm {
     ~EventPrincipal() {}
 
     void fillEventPrincipal(EventAuxiliary const& aux,
-        ProcessHistoryRegistry& processHistoryRegistry,
+        ProcessHistoryRegistry const& processHistoryRegistry,
         boost::shared_ptr<EventSelectionIDVector> eventSelectionIDs = boost::shared_ptr<EventSelectionIDVector>(),
         boost::shared_ptr<BranchListIndexes> branchListIndexes = boost::shared_ptr<BranchListIndexes>(),
         boost::shared_ptr<BranchMapper> mapper = boost::shared_ptr<BranchMapper>(new BranchMapper),

FWCore/Framework/interface/HistoryAppender.h

@@ -4,37 +4,13 @@
 #include "DataFormats/Provenance/interface/ProcessHistory.h"
 #include "DataFormats/Provenance/interface/ProcessHistoryID.h"
 
-#include <map>
+#include "boost/shared_ptr.hpp"
 
 namespace edm {
 
   class ProcessConfiguration;
   class ProcessHistoryRegistry;
 
-  class CachedHistory {
-  public:
-
-    CachedHistory(ProcessHistory const* inputProcessHistory,
-                  ProcessHistory const* processHistory,
-                  ProcessHistoryID const& processHistoryID) :
-      inputProcessHistory_(inputProcessHistory),
-      processHistory_(processHistory),
-      processHistoryID_(processHistoryID) {
-    }
-
-    ProcessHistory const* inputProcessHistory() const { return inputProcessHistory_; }
-    ProcessHistory const* processHistory() const { return processHistory_; }
-    ProcessHistoryID const& processHistoryID () const { return processHistoryID_; }
-
-  private:
-
-    // This class does not own the memory
-    ProcessHistory const* inputProcessHistory_;
-    ProcessHistory const* processHistory_;
-
-    ProcessHistoryID processHistoryID_;
-  };
-
   class HistoryAppender {
   public:
 
@@ -43,36 +19,22 @@ namespace edm {
     // Used to append the current process to the process history
     // when necessary. Optimized to cache the results so it
     // does not need to repeat the same calculations many times.
-    CachedHistory const&
+    boost::shared_ptr<ProcessHistory const>
     appendToProcessHistory(ProcessHistoryID const& inputPHID,
-                           ProcessConfiguration const& pc,
-                           ProcessHistoryRegistry& processHistoryRegistry);
+                           ProcessHistory const* inputProcessHistory,
+                           ProcessConfiguration const& pc);
 
   private:
-    HistoryAppender(HistoryAppender const&);
-    HistoryAppender& operator=(HistoryAppender const&);
+    HistoryAppender(HistoryAppender const&) = delete;
+    HistoryAppender& operator=(HistoryAppender const&) = delete;
 
     // Throws if the new process name is already in the process
     // process history
     void checkProcessHistory(ProcessHistory const& ph,
                              ProcessConfiguration const& pc) const;
 
-    // The map is intended to have the key be the ProcessHistoryID
-    // read from the input file in one of the Auxiliary objects.
-    // The CachedHistory has the ProcessHistoryID after adding
-    // the current process and the two pointers to the corresponding
-    // ProcessHistory objects in the registry, except if the history
-    // is empty then the pointer is to the data member of this class
-    // because the empty one is never in the registry.
-    typedef std::map<ProcessHistoryID, CachedHistory> HistoryMap;
-    HistoryMap historyMap_;
-
-    // We cache iterator to the previous element for
-    // performance. We expect the IDs to repeat many times
-    // and this avoids the lookup in that case.
-    HistoryMap::const_iterator previous_;
-
-    ProcessHistory emptyHistory_;
+    ProcessHistoryID m_cachedInputPHID;
+    boost::shared_ptr<ProcessHistory const> m_cachedHistory;
   };
 }
 #endif

FWCore/Framework/interface/LuminosityBlockPrincipal.h

@@ -43,7 +43,7 @@ namespace edm {
 
     ~LuminosityBlockPrincipal() {}
 
-    void fillLuminosityBlockPrincipal(ProcessHistoryRegistry& processHistoryRegistry, DelayedReader* reader = 0);
+    void fillLuminosityBlockPrincipal(ProcessHistoryRegistry const& processHistoryRegistry, DelayedReader* reader = 0);
 
     RunPrincipal const& runPrincipal() const {
       return *runPrincipal_;

FWCore/Framework/interface/Principal.h

@@ -86,7 +86,7 @@ namespace edm {
 
     void addAliasedProduct(boost::shared_ptr<BranchDescription const> bd);
 
-    void fillPrincipal(ProcessHistoryID const& hist, ProcessHistoryRegistry& phr, DelayedReader* reader);
+    void fillPrincipal(ProcessHistoryID const& hist, ProcessHistoryRegistry const& phr, DelayedReader* reader);
 
     void clearPrincipal();
 
@@ -235,9 +235,7 @@ namespace edm {
 
     virtual bool isComplete_() const {return true;}
 
-    ProcessHistoryRegistry* processHistoryRegistry_; 
-
-    ProcessHistory const* processHistoryPtr_;
+    boost::shared_ptr<ProcessHistory const> processHistoryPtr_;
 
     ProcessHistoryID processHistoryID_;
 
@@ -268,7 +266,6 @@ namespace edm {
     // The Principal does not own this object.
     HistoryAppender* historyAppender_;
 
-    static const ProcessHistory emptyProcessHistory_;
   };
 
   template <typename PROD>

FWCore/Framework/interface/RunPrincipal.h

@@ -18,6 +18,7 @@ is the DataBlock.
 #include "boost/shared_ptr.hpp"
 
 #include "DataFormats/Provenance/interface/RunAuxiliary.h"
+#include "DataFormats/Provenance/interface/ProcessHistoryID.h"
 #include "FWCore/Utilities/interface/RunIndex.h"
 #include "FWCore/Framework/interface/Principal.h"
 
@@ -40,7 +41,7 @@ namespace edm {
         unsigned int iRunIndex);
     ~RunPrincipal() {}
 
-    void fillRunPrincipal(ProcessHistoryRegistry& processHistoryRegistry, DelayedReader* reader = 0);
+    void fillRunPrincipal(ProcessHistoryRegistry const& processHistoryRegistry, DelayedReader* reader = 0);
 
     /** Multiple Runs may be processed simultaneously. The
      return value can be used to identify a particular Run.
@@ -60,6 +61,10 @@ namespace edm {
     RunNumber_t run() const {
       return aux().run();
     }
+
+    ProcessHistoryID const& reducedProcessHistoryID() const {
+      return m_reducedHistoryID;
+    }
 
     RunID const& id() const {
       return aux().id();
@@ -104,8 +109,8 @@ namespace edm {
 
     void resolveProductImmediate(ProductHolderBase const& phb) const;
 
-    // A vector of product holders.
     boost::shared_ptr<RunAuxiliary> aux_;
+    ProcessHistoryID m_reducedHistoryID;
     RunIndex index_;
 
     bool complete_;

FWCore/Framework/interface/SubProcess.h

@@ -37,7 +37,6 @@ namespace edm {
     SubProcess(ParameterSet& parameterSet,
                ParameterSet const& topLevelParameterSet,
                boost::shared_ptr<ProductRegistry const> parentProductRegistry,
-               ProcessHistoryRegistry& processHistoryRegistry,
                boost::shared_ptr<BranchIDListHelper const> parentBranchIDListHelper,
                eventsetup::EventSetupsController& esController,
                ActivityRegistry& parentActReg,
@@ -220,17 +219,21 @@ namespace edm {
 
     ServiceToken                                  serviceToken_;
     boost::shared_ptr<ProductRegistry const>      parentPreg_;
-    boost::shared_ptr<ProductRegistry const>	  preg_;
-    ProcessHistoryRegistry&                       processHistoryRegistry_;
+    boost::shared_ptr<ProductRegistry const>	    preg_;
     boost::shared_ptr<BranchIDListHelper>         branchIDListHelper_;
     std::unique_ptr<ExceptionToActionTable const> act_table_;
     boost::shared_ptr<ProcessConfiguration const> processConfiguration_;
     ProcessContext                                processContext_;
+    //We require 1 history for each Run, Lumi and Stream
+    // The vectors first hold Stream info, then Lumi then Run
+    unsigned int                                  historyLumiOffset_;
+    unsigned int                                  historyRunOffset_;
+    std::vector<ProcessHistoryRegistry>           processHistoryRegistries_;
+    std::vector<HistoryAppender>                  historyAppenders_;
     PrincipalCache                                principalCache_;
     boost::shared_ptr<eventsetup::EventSetupProvider> esp_;
     std::auto_ptr<Schedule>                       schedule_;
     std::map<ProcessHistoryID, ProcessHistoryID>  parentToChildPhID_;
-    std::unique_ptr<HistoryAppender>              historyAppender_;
     std::auto_ptr<SubProcess>                     subProcess_;
     std::unique_ptr<ParameterSet>                 processParameterSet_;
 

FWCore/Framework/src/EventPrincipal.cc

@@ -51,7 +51,7 @@ namespace edm {
 
   void
   EventPrincipal::fillEventPrincipal(EventAuxiliary const& aux,
-        ProcessHistoryRegistry& processHistoryRegistry,
+        ProcessHistoryRegistry const& processHistoryRegistry,
         boost::shared_ptr<EventSelectionIDVector> eventSelectionIDs,
         boost::shared_ptr<BranchListIndexes> branchListIndexes,
         boost::shared_ptr<BranchMapper> mapper,

FWCore/Framework/src/EventProcessor.cc

@@ -523,7 +523,6 @@ namespace edm {
       subProcess_.reset(new SubProcess(*subProcessParameterSet,
                                        *parameterSet,
                                        preg_,
-                                       input_->processHistoryRegistryForUpdate(),
                                        branchIDListHelper_,
                                        *espController_,
                                        *actReg_,
@@ -1706,14 +1705,17 @@ namespace edm {
                                                         historyAppender_.get(),
                                                         0));
     input_->readRun(*rp, *historyAppender_);
+    assert(input_->reducedProcessHistoryID() == rp->reducedProcessHistoryID());
     principalCache_.insert(rp);
-    return statemachine::Run(input_->reducedProcessHistoryID(), input_->run());
+    return statemachine::Run(rp->reducedProcessHistoryID(), input_->run());
   }
 
   statemachine::Run EventProcessor::readAndMergeRun() {
     principalCache_.merge(input_->runAuxiliary(), preg_);
-    input_->readAndMergeRun(*principalCache_.runPrincipalPtr());
-    return statemachine::Run(input_->reducedProcessHistoryID(), input_->run());
+    auto runPrincipal =principalCache_.runPrincipalPtr();
+    input_->readAndMergeRun(*runPrincipal);
+    assert(input_->reducedProcessHistoryID() == runPrincipal->reducedProcessHistoryID());
+    return statemachine::Run(runPrincipal->reducedProcessHistoryID(), input_->run());
   }
 
   int EventProcessor::readLuminosityBlock() {

FWCore/Framework/src/HistoryAppender.cc

@@ -4,48 +4,45 @@
 #include "FWCore/Utilities/interface/EDMException.h"
 
 #include <string>
+#include <cassert>
+
+static edm::ProcessHistory s_emptyHistory;
 
 namespace edm {
 
-  HistoryAppender::HistoryAppender() :
-    previous_(historyMap_.end()) {
+  HistoryAppender::HistoryAppender()
+  {
   }
 
-  CachedHistory const&
+  boost::shared_ptr<ProcessHistory const>
   HistoryAppender::appendToProcessHistory(ProcessHistoryID const& inputPHID,
-                                          ProcessConfiguration const& pc,
-                                          ProcessHistoryRegistry& processHistoryRegistry)  {
-
-    if (previous_ != historyMap_.end() && inputPHID == previous_->first) return previous_->second;
-
-    HistoryMap::iterator iter = historyMap_.find(inputPHID);
-    if (iter != historyMap_.end()) return iter->second;
+                                          ProcessHistory const* iInputProcessHistory,
+                                          ProcessConfiguration const& pc)  {
+    assert((iInputProcessHistory) == nullptr or (inputPHID == iInputProcessHistory->id()));
+    if (m_cachedHistory.get() != nullptr and inputPHID==m_cachedInputPHID) {
+      return m_cachedHistory;
+    }
 
-    ProcessHistory const* inputProcessHistory = &emptyHistory_;
+    ProcessHistory const* inputProcessHistory = iInputProcessHistory? iInputProcessHistory : &s_emptyHistory;
 
     if (inputPHID.isValid()) {
-      inputProcessHistory = processHistoryRegistry.getMapped(inputPHID);
-      if (inputProcessHistory == nullptr) {
+      if (iInputProcessHistory == nullptr) {
         throw Exception(errors::LogicError)
           << "HistoryAppender::appendToProcessHistory\n"
-          << "Input ProcessHistory not found in registry\n"
+          << "Input ProcessHistory has valid ID but is nullptr\n"
           << "Contact a Framework developer\n";
       }
     }
 
-    ProcessHistory newProcessHistory;
-    newProcessHistory = *inputProcessHistory;
-    checkProcessHistory(newProcessHistory, pc);
-    newProcessHistory.push_back(pc);
-    processHistoryRegistry.registerProcessHistory(newProcessHistory);
-    ProcessHistoryID newProcessHistoryID = newProcessHistory.setProcessHistoryID();
-    CachedHistory newValue(inputProcessHistory,
-                           processHistoryRegistry.getMapped(newProcessHistoryID),
-                           newProcessHistoryID);
-    std::pair<ProcessHistoryID, CachedHistory> newEntry(inputPHID, newValue);
-    std::pair<HistoryMap::iterator, bool> result = historyMap_.insert(newEntry);
-    previous_ = result.first;
-    return result.first->second;
+    boost::shared_ptr<ProcessHistory> newProcessHistory(new ProcessHistory);
+    *newProcessHistory = *inputProcessHistory;
+    checkProcessHistory(*newProcessHistory, pc);
+    newProcessHistory->push_back(pc);
+    //force it to create the ID
+    newProcessHistory->setProcessHistoryID();
+    m_cachedInputPHID =inputPHID;
+    m_cachedHistory = newProcessHistory;
+    return m_cachedHistory;
   }
 
   void

FWCore/Framework/src/InputSource.cc

@@ -318,12 +318,12 @@ namespace edm {
     // Note: For the moment, we do not support saving and restoring the state of the
     // random number generator if random numbers are generated during processing of runs
     // (e.g. beginRun(), endRun())
-    runPrincipal.fillRunPrincipal(processHistoryRegistryForUpdate());
+    runPrincipal.fillRunPrincipal(processHistoryRegistry());
   }
 
   void
   InputSource::readLuminosityBlock_(LuminosityBlockPrincipal& lumiPrincipal) {
-    lumiPrincipal.fillLuminosityBlockPrincipal(processHistoryRegistryForUpdate());
+    lumiPrincipal.fillLuminosityBlockPrincipal(processHistoryRegistry());
   }
 
   void

FWCore/Framework/src/LuminosityBlockPrincipal.cc

@@ -23,7 +23,7 @@ namespace edm {
 
   void
   LuminosityBlockPrincipal::fillLuminosityBlockPrincipal(
-      ProcessHistoryRegistry& processHistoryRegistry,
+      ProcessHistoryRegistry const& processHistoryRegistry,
       DelayedReader* reader) {
 
     complete_ = false;