Merge pull request #47533 from Dr15Jones/limitAddingToProductRegistry

Compartmentalize how ProductRegistry is filled

Author: cmsbuild

DQM/SiStripMonitorHardware/src/SiStripSpyEventMatcher.cc

@@ -6,7 +6,7 @@
 #include "FWCore/Sources/interface/VectorInputSource.h"
 #include "FWCore/Sources/interface/VectorInputSourceDescription.h"
 #include "FWCore/Sources/interface/VectorInputSourceFactory.h"
-#include "FWCore/Framework/interface/SignallingProductRegistry.h"
+#include "DataFormats/Provenance/interface/ProductRegistry.h"
 #include "DataFormats/FEDRawData/interface/FEDRawDataCollection.h"
 #include "DataFormats/SiStripDigi/interface/SiStripRawDigi.h"
 #include "DataFormats/Common/interface/DetSetVector.h"
@@ -54,7 +54,7 @@ namespace sistrip {
         reorderedDigisTag_(config.getParameter<edm::InputTag>("SpyReorderedDigisTag")),
         virginRawDigisTag_(config.getParameter<edm::InputTag>("SpyVirginRawDigisTag")),
         counterDiffMax_(config.getParameter<uint32_t>("CounterDiffMaxAllowed")),
-        productRegistry_(new edm::SignallingProductRegistry),
+        productRegistry_(new edm::ProductRegistry),
         source_(constructSource(config.getParameter<edm::ParameterSet>("SpySource"))),
         // hardware information is not needed for the "overlay"
         processConfiguration_(std::make_unique<edm::ProcessConfiguration>(

DataFormats/Provenance/interface/ProductRegistry.h

@@ -29,31 +29,29 @@
 #include <vector>
 
 namespace edm {
-
+  class SignallingProductRegistryFiller;
   class ProductResolverIndexHelper;
   class TypeID;
 
   class ProductRegistry {
   public:
-    typedef std::map<BranchKey, ProductDescription> ProductList;
+    friend class SignallingProductRegistryFiller;
 
-    ProductRegistry();
+    typedef std::map<BranchKey, ProductDescription> ProductList;
 
+    ProductRegistry() = default;
+    ProductRegistry(const ProductRegistry&) = default;
+    ProductRegistry(ProductRegistry&&) = default;
+    ProductRegistry& operator=(ProductRegistry&&) = default;
     // A constructor from the persistent data members from another product registry.
     // saves time by not copying the transient components.
     // The constructed registry will be frozen by default.
     explicit ProductRegistry(ProductList const& productList, bool toBeFrozen = true);
 
-    virtual ~ProductRegistry() {}
+    ~ProductRegistry() = default;
 
     typedef std::map<BranchKey, ProductDescription const> ConstProductList;
 
-    void addProduct(ProductDescription const& productdesc, bool iFromListener = false);
-
-    void addLabelAlias(ProductDescription const& productdesc,
-                       std::string const& labelAlias,
-                       std::string const& instanceAlias);
-
     void copyProduct(ProductDescription const& productdesc);
 
     void setFrozen(bool initializeLookupInfo = true);
@@ -69,8 +67,6 @@ namespace edm {
                       ProductDescription::MatchMode branchesMustMatch = ProductDescription::Permissive);
 
     void updateFromInput(ProductList const& other);
-    // triggers callbacks for modules watching registration
-    void addFromInput(edm::ProductRegistry const&);
 
     void updateFromInput(std::vector<ProductDescription> const& other);
 
@@ -95,29 +91,13 @@ namespace edm {
     // colon-initialization list.
     std::vector<ProductDescription const*> allProductDescriptions() const;
 
-    //NOTE: this is not const since we only want items that have non-const access to this class to be
-    // able to call this internal iteration
-    // Called only for branches that are present (part of avoiding creating type information for dropped branches)
-    template <typename T>
-    void callForEachBranch(T const& iFunc) {
-      //NOTE: If implementation changes from a map, need to check that iterators are still valid
-      // after an insert with the new container, else need to copy the container and iterate over the copy
-      for (ProductRegistry::ProductList::const_iterator itEntry = productList_.begin(), itEntryEnd = productList_.end();
-           itEntry != itEntryEnd;
-           ++itEntry) {
-        if (itEntry->second.present()) {
-          iFunc(itEntry->second);
-        }
-      }
-    }
     ProductList::size_type size() const { return productList_.size(); }
 
     void print(std::ostream& os) const;
 
     bool anyProducts(BranchType const brType) const;
 
     std::shared_ptr<ProductResolverIndexHelper const> productLookup(BranchType branchType) const;
-    std::shared_ptr<ProductResolverIndexHelper> productLookup(BranchType branchType);
 
     // returns the appropriate ProductResolverIndex else ProductResolverIndexInvalid if no BranchID is available
     ProductResolverIndex indexFrom(BranchID const& iID) const;
@@ -152,7 +132,7 @@ namespace edm {
       std::array<bool, NumBranchTypes> productProduced_;
       bool anyProductProduced_;
 
-      std::array<edm::propagate_const<std::shared_ptr<ProductResolverIndexHelper>>, NumBranchTypes> productLookups_;
+      std::array<std::shared_ptr<const ProductResolverIndexHelper>, NumBranchTypes> productLookups_;
 
       std::array<ProductResolverIndex, NumBranchTypes> nextIndexValues_;
 
@@ -163,6 +143,30 @@ namespace edm {
       AliasToOriginalVector aliasToOriginal_;
     };
 
+  private:
+    //The following three routines are only called by SignallingProductRegistryFiller
+    void addProduct_(ProductDescription const& productdesc);
+
+    ProductDescription const& addLabelAlias_(ProductDescription const& productdesc,
+                                             std::string const& labelAlias,
+                                             std::string const& instanceAlias);
+
+    // triggers callbacks for modules watching registration
+    template <typename F>
+    void addFromInput_(edm::ProductRegistry const& iReg, F&& iCallback) {
+      throwIfFrozen();
+      for (auto const& prod : iReg.productList_) {
+        ProductList::iterator iter = productList_.find(prod.first);
+        if (iter == productList_.end()) {
+          productList_.insert(std::make_pair(prod.first, prod.second));
+          iCallback(prod.second);
+        } else {
+          assert(combinable(iter->second, prod.second));
+          iter->second.merge(prod.second);
+        }
+      }
+    }
+
   private:
     void setProductProduced(BranchType branchType) {
       transient_.productProduced_[branchType] = true;
@@ -185,7 +189,6 @@ namespace edm {
 
     void checkForDuplicateProcessName(ProductDescription const& desc, std::string const* processName) const;
 
-    virtual void addCalled(ProductDescription const&, bool iFromListener);
     void throwIfNotFrozen() const;
     void throwIfFrozen() const;
 

DataFormats/Provenance/src/ProductRegistry.cc

@@ -29,8 +29,6 @@
 
 namespace edm {
 
-  ProductRegistry::ProductRegistry() : productList_(), transient_() {}
-
   ProductRegistry::Transients::Transients()
       : frozen_(false),
         productProduced_(),
@@ -65,7 +63,7 @@ namespace edm {
     freezeIt(toBeFrozen);
   }
 
-  void ProductRegistry::addProduct(ProductDescription const& productDesc, bool fromListener) {
+  void ProductRegistry::addProduct_(ProductDescription const& productDesc) {
     assert(productDesc.produced());
     throwIfFrozen();
     std::pair<ProductList::iterator, bool> ret =
@@ -102,12 +100,11 @@ namespace edm {
             << "descendants of those products.\n";
       }
     }
-    addCalled(productDesc, fromListener);
   }
 
-  void ProductRegistry::addLabelAlias(ProductDescription const& productDesc,
-                                      std::string const& labelAlias,
-                                      std::string const& instanceAlias) {
+  ProductDescription const& ProductRegistry::addLabelAlias_(ProductDescription const& productDesc,
+                                                            std::string const& labelAlias,
+                                                            std::string const& instanceAlias) {
     assert(productDesc.produced());
     assert(productDesc.branchID().isValid());
     throwIfFrozen();
@@ -116,7 +113,7 @@ namespace edm {
     assert(ret.second);
     transient_.aliasToOriginal_.emplace_back(
         PRODUCT_TYPE, productDesc.unwrappedTypeID(), labelAlias, instanceAlias, productDesc.moduleLabel());
-    addCalled(bd, false);
+    return ret.first->second;
   }
 
   void ProductRegistry::copyProduct(ProductDescription const& productDesc) {
@@ -143,11 +140,7 @@ namespace edm {
   }
 
   std::shared_ptr<ProductResolverIndexHelper const> ProductRegistry::productLookup(BranchType branchType) const {
-    return get_underlying_safe(transient_.productLookups_[branchType]);
-  }
-
-  std::shared_ptr<ProductResolverIndexHelper> ProductRegistry::productLookup(BranchType branchType) {
-    return get_underlying_safe(transient_.productLookups_[branchType]);
+    return transient_.productLookups_[branchType];
   }
 
   void ProductRegistry::setFrozen(bool initializeLookupInfo) {
@@ -184,8 +177,6 @@ namespace edm {
     }
   }
 
-  void ProductRegistry::addCalled(ProductDescription const&, bool) {}
-
   std::vector<std::string> ProductRegistry::allBranchNames() const {
     std::vector<std::string> result;
     result.reserve(productList().size());
@@ -218,20 +209,6 @@ namespace edm {
     }
   }
 
-  void ProductRegistry::addFromInput(edm::ProductRegistry const& other) {
-    throwIfFrozen();
-    for (auto const& prod : other.productList_) {
-      ProductList::iterator iter = productList_.find(prod.first);
-      if (iter == productList_.end()) {
-        productList_.insert(std::make_pair(prod.first, prod.second));
-        addCalled(prod.second, false);
-      } else {
-        assert(combinable(iter->second, prod.second));
-        iter->second.merge(prod.second);
-      }
-    }
-  }
-
   void ProductRegistry::setUnscheduledProducts(std::set<std::string> const& unscheduledLabels) {
     throwIfFrozen();
 
@@ -319,6 +296,11 @@ namespace edm {
 
     transient_.branchIDToIndex_.clear();
 
+    std::array<std::shared_ptr<ProductResolverIndexHelper>, NumBranchTypes> new_productLookups{
+        {std::make_shared<ProductResolverIndexHelper>(),
+         std::make_shared<ProductResolverIndexHelper>(),
+         std::make_shared<ProductResolverIndexHelper>(),
+         std::make_shared<ProductResolverIndexHelper>()}};
     for (auto const& product : productList_) {
       auto const& desc = product.second;
 
@@ -438,13 +420,12 @@ namespace edm {
             }
           }
         }
-        ProductResolverIndex index = productLookup(desc.branchType())
-                                         ->insert(typeID,
-                                                  desc.moduleLabel().c_str(),
-                                                  desc.productInstanceName().c_str(),
-                                                  desc.processName().c_str(),
-                                                  containedTypeID,
-                                                  baseTypesOfContainedType);
+        ProductResolverIndex index = new_productLookups[desc.branchType()]->insert(typeID,
+                                                                                   desc.moduleLabel().c_str(),
+                                                                                   desc.productInstanceName().c_str(),
+                                                                                   desc.processName().c_str(),
+                                                                                   containedTypeID,
+                                                                                   baseTypesOfContainedType);
 
         transient_.branchIDToIndex_[desc.branchID()] = index;
       }
@@ -454,9 +435,12 @@ namespace edm {
       throwMissingDictionariesException(missingDictionaries, context, producedTypes, branchNamesForMissing);
     }
 
-    for (auto& iterProductLookup : transient_.productLookups_) {
+    for (auto& iterProductLookup : new_productLookups) {
       iterProductLookup->setFrozen();
     }
+    for (size_t i = 0; i < new_productLookups.size(); ++i) {
+      transient_.productLookups_[i] = std::move(new_productLookups[i]);
+    }
 
     unsigned int indexIntoNextIndexValue = 0;
     for (auto const& iterProductLookup : transient_.productLookups_) {

FWCore/Framework/interface/EventProcessor.h

@@ -18,7 +18,7 @@ configured in the user's main() function, and is set running.
 #include "FWCore/Framework/interface/MergeableRunProductProcesses.h"
 #include "FWCore/Framework/interface/SharedResourcesAcquirer.h"
 #include "FWCore/Framework/interface/PrincipalCache.h"
-#include "FWCore/Framework/interface/SignallingProductRegistry.h"
+#include "FWCore/Framework/interface/SignallingProductRegistryFiller.h"
 #include "FWCore/Framework/interface/PreallocationConfiguration.h"
 
 #include "FWCore/ParameterSet/interface/ParameterSet.h"
@@ -280,8 +280,7 @@ namespace edm {
 
     void processEventWithLooper(EventPrincipal&, unsigned int iStreamIndex);
 
-    std::shared_ptr<SignallingProductRegistry const> preg() const { return get_underlying_safe(preg_); }
-    std::shared_ptr<SignallingProductRegistry>& preg() { return get_underlying_safe(preg_); }
+    std::shared_ptr<ProductRegistry const> preg() const { return get_underlying_safe(preg_); }
     std::shared_ptr<BranchIDListHelper const> branchIDListHelper() const {
       return get_underlying_safe(branchIDListHelper_);
     }
@@ -312,7 +311,7 @@ namespace edm {
     oneapi::tbb::task_group taskGroup_;
 
     std::shared_ptr<ActivityRegistry> actReg_;  // We do not use propagate_const because the registry itself is mutable.
-    edm::propagate_const<std::shared_ptr<SignallingProductRegistry>> preg_;
+    edm::propagate_const<std::shared_ptr<ProductRegistry>> preg_;
     edm::propagate_const<std::shared_ptr<BranchIDListHelper>> branchIDListHelper_;
     edm::propagate_const<std::shared_ptr<ProcessBlockHelper>> processBlockHelper_;
     edm::propagate_const<std::shared_ptr<ThinnedAssociationsHelper>> thinnedAssociationsHelper_;

FWCore/Framework/interface/GlobalSchedule.h

@@ -14,7 +14,7 @@
 #include "FWCore/Framework/interface/WorkerManager.h"
 #include "FWCore/Framework/interface/maker/Worker.h"
 #include "FWCore/Framework/interface/WorkerRegistry.h"
-#include "FWCore/Framework/interface/SignallingProductRegistry.h"
+#include "FWCore/Framework/interface/SignallingProductRegistryFiller.h"
 #include "FWCore/MessageLogger/interface/ExceptionMessages.h"
 #include "FWCore/ServiceRegistry/interface/GlobalContext.h"
 #include "FWCore/ServiceRegistry/interface/ServiceRegistry.h"
@@ -60,7 +60,7 @@ namespace edm {
                    std::shared_ptr<ModuleRegistry> modReg,
                    std::vector<std::string> const& modulesToUse,
                    ParameterSet& proc_pset,
-                   SignallingProductRegistry& pregistry,
+                   SignallingProductRegistryFiller& pregistry,
                    PreallocationConfiguration const& prealloc,
                    ExceptionToActionTable const& actions,
                    std::shared_ptr<ActivityRegistry> areg,

FWCore/Framework/interface/InputSource.h

@@ -44,7 +44,6 @@ namespace edm {
   class ParameterSetDescription;
   class ProcessContext;
   class ProcessHistoryRegistry;
-  class SignallingProductRegistry;
   class StreamContext;
   class ModuleCallingContext;
   class SharedResourcesAcquirer;

FWCore/Framework/interface/ModuleChanger.h

@@ -31,11 +31,13 @@
 namespace edm {
   class ParameterSet;
   class Schedule;
-  class SignallingProductRegistry;
+  class SignallingProductRegistryFiller;
 
   class ModuleChanger {
   public:
-    ModuleChanger(Schedule*, SignallingProductRegistry const* iReg, eventsetup::ESRecordsToProductResolverIndices);
+    ModuleChanger(Schedule*,
+                  SignallingProductRegistryFiller const* iReg,
+                  eventsetup::ESRecordsToProductResolverIndices);
     ModuleChanger(const ModuleChanger&) = delete;                   // stop default
     const ModuleChanger& operator=(const ModuleChanger&) = delete;  // stop default
     virtual ~ModuleChanger();
@@ -50,7 +52,7 @@ namespace edm {
   private:
     // ---------- member data --------------------------------
     edm::propagate_const<Schedule*> schedule_;
-    SignallingProductRegistry const* registry_;
+    SignallingProductRegistryFiller const* registry_;
     eventsetup::ESRecordsToProductResolverIndices indices_;
   };
 }  // namespace edm

FWCore/Framework/interface/OutputModuleCore.h

@@ -55,7 +55,7 @@ namespace edm {
   class PreallocationConfiguration;
   class ActivityRegistry;
   class ThinnedAssociationsHelper;
-  class SignallingProductRegistry;
+  class SignallingProductRegistryFiller;
 
   template <typename T>
   class OutputModuleCommunicatorT;
@@ -214,7 +214,7 @@ namespace edm {
       /// Tell the OutputModule that is must end the current file.
       void doCloseFile();
 
-      void registerProductsAndCallbacks(OutputModuleCore const*, SignallingProductRegistry*);
+      void registerProductsAndCallbacks(OutputModuleCore const*, SignallingProductRegistryFiller*);
 
       bool needToRunSelection() const noexcept;
       std::vector<ProductResolverIndexAndSkipBit> productsUsedBySelection() const noexcept;

FWCore/Framework/interface/ProducerBase.h

@@ -22,7 +22,7 @@ EDProducts into an Event.
 namespace edm {
   class ModuleDescription;
   class ProducesCollector;
-  class SignallingProductRegistry;
+  class SignallingProductRegistryFiller;
   class Event;
   class LuminosityBlock;
   class ProcessBlock;
@@ -76,7 +76,7 @@ namespace edm {
     /// used by the fwk to register list of products
     std::function<void(ProductDescription const&)> registrationCallback() const;
 
-    void registerProducts(ProducerBase*, SignallingProductRegistry*, ModuleDescription const&);
+    void registerProducts(ProducerBase*, SignallingProductRegistryFiller*, ModuleDescription const&);
 
     using ProductRegistryHelper::recordProvenanceList;
     using ProductRegistryHelper::typeLabelList;