Merge pull request #29553 from makortel/deleteNonconsumedModules

Delete unscheduled modules that are not consumed by any scheduled module
cms-sw · Dec 2, 2020 · 9be9e4c · 9be9e4c
2 parents 719f135 + c38022d
commit 9be9e4c
Show file tree

Hide file tree

Showing 81 changed files with 2,274 additions and 141 deletions.
diff --git a/FWCore/Framework/interface/EDConsumerBase.h b/FWCore/Framework/interface/EDConsumerBase.h
@@ -51,6 +51,7 @@
 
 namespace edm {
   class ModuleDescription;
+  class ModuleProcessName;
   class ProductResolverIndexHelper;
   class ProductRegistry;
   class ConsumesCollector;
@@ -105,7 +106,8 @@ namespace edm {
     typedef ProductLabels Labels;
     void labelsForToken(EDGetToken iToken, Labels& oLabels) const;
 
-    void modulesWhoseProductsAreConsumed(std::vector<ModuleDescription const*>& modules,
+    void modulesWhoseProductsAreConsumed(std::array<std::vector<ModuleDescription const*>*, NumBranchTypes>& modulesAll,
+                                         std::vector<ModuleProcessName>& modulesInPreviousProcesses,
                                          ProductRegistry const& preg,
                                          std::map<std::string, ModuleDescription const*> const& labelsToDesc,
                                          std::string const& processName) const;

diff --git a/FWCore/Framework/interface/EventProcessor.h b/FWCore/Framework/interface/EventProcessor.h
@@ -364,6 +364,7 @@ namespace edm {
     ExcludedDataMap eventSetupDataToExcludeFromPrefetching_;
 
     bool printDependencies_ = false;
+    bool deleteNonConsumedUnscheduledModules_ = true;
   };  // class EventProcessor
 
   //--------------------------------------------------------------------

diff --git a/FWCore/Framework/interface/ModuleProcessName.h b/FWCore/Framework/interface/ModuleProcessName.h
@@ -0,0 +1,30 @@
+#ifndef FWCore_Framework_ModuleProcessName_h
+#define FWCore_Framework_ModuleProcessName_h
+
+#include <string_view>
+
+namespace edm {
+  /**
+   * Helper class to hold a module label and a process name
+   *
+   * Note: does NOT own the string storage, be careful to use.
+   */
+  class ModuleProcessName {
+  public:
+    explicit ModuleProcessName(std::string_view module, std::string_view process)
+        : moduleLabel_{module}, processName_{process} {}
+
+    std::string_view moduleLabel() const { return moduleLabel_; }
+    std::string_view processName() const { return processName_; }
+
+  private:
+    std::string_view moduleLabel_;
+    std::string_view processName_;
+  };
+
+  inline bool operator<(ModuleProcessName const& a, ModuleProcessName const& b) {
+    return a.processName() == b.processName() ? a.moduleLabel() < b.moduleLabel() : a.processName() < b.processName();
+  }
+}  // namespace edm
+
+#endif
diff --git a/FWCore/Framework/interface/PathsAndConsumesOfModules.h b/FWCore/Framework/interface/PathsAndConsumesOfModules.h
@@ -15,6 +15,9 @@
 #include "FWCore/ServiceRegistry/interface/ConsumesInfo.h"
 #include "FWCore/ServiceRegistry/interface/PathsAndConsumesOfModulesBase.h"
 
+#include "FWCore/Framework/interface/ModuleProcessName.h"
+#include "FWCore/Utilities/interface/BranchType.h"
+
 #include <memory>
 #include <string>
 #include <utility>
@@ -28,10 +31,16 @@ namespace edm {
 
   class PathsAndConsumesOfModules : public PathsAndConsumesOfModulesBase {
   public:
+    PathsAndConsumesOfModules();
     ~PathsAndConsumesOfModules() override;
 
     void initialize(Schedule const*, std::shared_ptr<ProductRegistry const>);
 
+    void removeModules(std::vector<ModuleDescription const*> const& modules);
+
+    std::vector<ModuleProcessName> const& modulesInPreviousProcessesWhoseProductsAreConsumedBy(
+        unsigned int moduleID) const;
+
   private:
     std::vector<std::string> const& doPaths() const override { return paths_; }
     std::vector<std::string> const& doEndPaths() const override { return endPaths_; }
@@ -42,10 +51,12 @@ namespace edm {
     std::vector<ModuleDescription const*> const& doModulesOnPath(unsigned int pathIndex) const override;
     std::vector<ModuleDescription const*> const& doModulesOnEndPath(unsigned int endPathIndex) const override;
     std::vector<ModuleDescription const*> const& doModulesWhoseProductsAreConsumedBy(
-        unsigned int moduleID) const override;
+        unsigned int moduleID, BranchType branchType = InEvent) const override;
 
     std::vector<ConsumesInfo> doConsumesInfo(unsigned int moduleID) const override;
 
+    unsigned int doLargestModuleID() const override;
+
     unsigned int moduleIndex(unsigned int moduleID) const;
 
     // data members
@@ -62,12 +73,16 @@ namespace edm {
     // following data member
     std::vector<std::pair<unsigned int, unsigned int> > moduleIDToIndex_;
 
-    std::vector<std::vector<ModuleDescription const*> > modulesWhoseProductsAreConsumedBy_;
+    std::array<std::vector<std::vector<ModuleDescription const*> >, NumBranchTypes> modulesWhoseProductsAreConsumedBy_;
+    std::vector<std::vector<ModuleProcessName> > modulesInPreviousProcessesWhoseProductsAreConsumedBy_;
 
     Schedule const* schedule_;
     std::shared_ptr<ProductRegistry const> preg_;
   };
 
+  std::vector<ModuleDescription const*> nonConsumedUnscheduledModules(
+      edm::PathsAndConsumesOfModulesBase const& iPnC, std::vector<ModuleProcessName>& consumedByChildren);
+
   void checkForModuleDependencyCorrectness(edm::PathsAndConsumesOfModulesBase const& iPnC, bool iPrintDependencies);
 }  // namespace edm
 #endif
diff --git a/FWCore/Framework/interface/Schedule.h b/FWCore/Framework/interface/Schedule.h
@@ -230,10 +230,13 @@ namespace edm {
                                      std::vector<ModuleDescription const*>& descriptions,
                                      unsigned int hint) const;
 
-    void fillModuleAndConsumesInfo(std::vector<ModuleDescription const*>& allModuleDescriptions,
-                                   std::vector<std::pair<unsigned int, unsigned int>>& moduleIDToIndex,
-                                   std::vector<std::vector<ModuleDescription const*>>& modulesWhoseProductsAreConsumedBy,
-                                   ProductRegistry const& preg) const;
+    void fillModuleAndConsumesInfo(
+        std::vector<ModuleDescription const*>& allModuleDescriptions,
+        std::vector<std::pair<unsigned int, unsigned int>>& moduleIDToIndex,
+        std::array<std::vector<std::vector<ModuleDescription const*>>, NumBranchTypes>&
+            modulesWhoseProductsAreConsumedBy,
+        std::vector<std::vector<ModuleProcessName>>& modulesInPreviousProcessesWhoseProductsAreConsumedBy,
+        ProductRegistry const& preg) const;
 
     /// Return the number of events this Schedule has tried to process
     /// (inclues both successes and failures, including failures due
@@ -277,6 +280,9 @@ namespace edm {
                       const ProductRegistry& iRegistry,
                       eventsetup::ESRecordsToProxyIndices const&);
 
+    /// Deletes module with label iLabel
+    void deleteModule(std::string const& iLabel, ActivityRegistry* areg);
+
     /// returns the collection of pointers to workers
     AllWorkers const& allWorkers() const;
 

diff --git a/FWCore/Framework/interface/UnscheduledCallProducer.h b/FWCore/Framework/interface/UnscheduledCallProducer.h
@@ -51,6 +51,13 @@ namespace edm {
       }
     }
 
+    void removeWorker(Worker const* worker) {
+      unscheduledWorkers_.erase(std::remove(unscheduledWorkers_.begin(), unscheduledWorkers_.end(), worker),
+                                unscheduledWorkers_.end());
+      accumulatorWorkers_.erase(std::remove(accumulatorWorkers_.begin(), accumulatorWorkers_.end(), worker),
+                                accumulatorWorkers_.end());
+    }
+
     void setEventTransitionInfo(EventTransitionInfo const& info) { aux_.setEventTransitionInfo(info); }
 
     UnscheduledAuxiliary const& auxiliary() const { return aux_; }

diff --git a/FWCore/Framework/interface/WorkerManager.h b/FWCore/Framework/interface/WorkerManager.h
@@ -41,6 +41,9 @@ namespace edm {
     WorkerManager(std::shared_ptr<ModuleRegistry> modReg,
                   std::shared_ptr<ActivityRegistry> actReg,
                   ExceptionToActionTable const& actions);
+
+    void deleteModuleIfExists(std::string const& moduleLabel);
+
     void addToUnscheduledWorkers(ParameterSet& pset,
                                  ProductRegistry& preg,
                                  PreallocationConfiguration const* prealloc,

diff --git a/FWCore/Framework/interface/stream/EDAnalyzerAdaptorBase.h b/FWCore/Framework/interface/stream/EDAnalyzerAdaptorBase.h
@@ -40,6 +40,7 @@
 
 namespace edm {
   class ModuleCallingContext;
+  class ModuleProcessName;
   class ProductResolverIndexHelper;
   class EDConsumerBase;
   class PreallocationConfiguration;
@@ -111,7 +112,8 @@ namespace edm {
 
       const EDConsumerBase* consumer() const;
 
-      void modulesWhoseProductsAreConsumed(std::vector<ModuleDescription const*>& modules,
+      void modulesWhoseProductsAreConsumed(std::array<std::vector<ModuleDescription const*>*, NumBranchTypes>& modules,
+                                           std::vector<ModuleProcessName>& modulesInPreviousProcesses,
                                            ProductRegistry const& preg,
                                            std::map<std::string, ModuleDescription const*> const& labelsToDesc,
                                            std::string const& processName) const;

diff --git a/FWCore/Framework/interface/stream/ProducingModuleAdaptorBase.h b/FWCore/Framework/interface/stream/ProducingModuleAdaptorBase.h
@@ -45,6 +45,7 @@
 namespace edm {
   class Event;
   class ModuleCallingContext;
+  class ModuleProcessName;
   class ProductResolverIndexHelper;
   class EDConsumerBase;
   class PreallocationConfiguration;
@@ -102,7 +103,8 @@ namespace edm {
       void updateLookup(BranchType iBranchType, ProductResolverIndexHelper const&, bool iPrefetchMayGet);
       void updateLookup(eventsetup::ESRecordsToProxyIndices const&);
 
-      void modulesWhoseProductsAreConsumed(std::vector<ModuleDescription const*>& modules,
+      void modulesWhoseProductsAreConsumed(std::array<std::vector<ModuleDescription const*>*, NumBranchTypes>& modules,
+                                           std::vector<ModuleProcessName>& modulesInPreviousProcesses,
                                            ProductRegistry const& preg,
                                            std::map<std::string, ModuleDescription const*> const& labelsToDesc,
                                            std::string const& processName) const;

diff --git a/FWCore/Framework/src/EDConsumerBase.cc b/FWCore/Framework/src/EDConsumerBase.cc
@@ -22,6 +22,7 @@
 #include "FWCore/Framework/interface/ConsumesCollector.h"
 #include "FWCore/Framework/interface/ESRecordsToProxyIndices.h"
 #include "FWCore/Framework/interface/ComponentDescription.h"
+#include "FWCore/Framework/interface/ModuleProcessName.h"
 #include "FWCore/MessageLogger/interface/MessageLogger.h"
 #include "FWCore/Utilities/interface/BranchType.h"
 #include "FWCore/Utilities/interface/Likely.h"
@@ -467,41 +468,55 @@ namespace {
   }
 }  // namespace
 
-void EDConsumerBase::modulesWhoseProductsAreConsumed(std::vector<ModuleDescription const*>& modules,
-                                                     ProductRegistry const& preg,
-                                                     std::map<std::string, ModuleDescription const*> const& labelsToDesc,
-                                                     std::string const& processName) const {
-  ProductResolverIndexHelper const& iHelper = *preg.productLookup(InEvent);
-
+void EDConsumerBase::modulesWhoseProductsAreConsumed(
+    std::array<std::vector<ModuleDescription const*>*, NumBranchTypes>& modulesAll,
+    std::vector<ModuleProcessName>& modulesInPreviousProcesses,
+    ProductRegistry const& preg,
+    std::map<std::string, ModuleDescription const*> const& labelsToDesc,
+    std::string const& processName) const {
   std::set<std::string> alreadyFound;
 
+  auto modulesInPreviousProcessesEmplace = [&modulesInPreviousProcesses](std::string_view module,
+                                                                         std::string_view process) {
+    auto it = std::lower_bound(
+        modulesInPreviousProcesses.begin(), modulesInPreviousProcesses.end(), ModuleProcessName(module, process));
+    modulesInPreviousProcesses.emplace(it, module, process);
+  };
+
   auto itKind = m_tokenInfo.begin<kKind>();
   auto itLabels = m_tokenInfo.begin<kLabels>();
   for (auto itInfo = m_tokenInfo.begin<kLookupInfo>(), itEnd = m_tokenInfo.end<kLookupInfo>(); itInfo != itEnd;
        ++itInfo, ++itKind, ++itLabels) {
-    if (itInfo->m_branchType == InEvent and (not itInfo->m_index.skipCurrentProcess())) {
-      const unsigned int labelStart = itLabels->m_startOfModuleLabel;
-      const char* const consumedModuleLabel = &(m_tokenLabels[labelStart]);
-      const char* const consumedProductInstance = consumedModuleLabel + itLabels->m_deltaToProductInstance;
-      const char* const consumedProcessName = consumedModuleLabel + itLabels->m_deltaToProcessName;
+    ProductResolverIndexHelper const& helper = *preg.productLookup(itInfo->m_branchType);
+    std::vector<ModuleDescription const*>& modules = *modulesAll[itInfo->m_branchType];
 
+    const unsigned int labelStart = itLabels->m_startOfModuleLabel;
+    const char* const consumedModuleLabel = &(m_tokenLabels[labelStart]);
+    const char* const consumedProductInstance = consumedModuleLabel + itLabels->m_deltaToProductInstance;
+    const char* const consumedProcessName = consumedModuleLabel + itLabels->m_deltaToProcessName;
+
+    if (not itInfo->m_index.skipCurrentProcess()) {
       if (*consumedModuleLabel != '\0') {    // not a consumesMany
         if (*consumedProcessName != '\0') {  // process name is specified in consumes call
-          if (processName == consumedProcessName &&
-              iHelper.index(
+          if (helper.index(
                   *itKind, itInfo->m_type, consumedModuleLabel, consumedProductInstance, consumedProcessName) !=
-                  ProductResolverIndexInvalid) {
-            insertFoundModuleLabel(*itKind,
-                                   itInfo->m_type,
-                                   consumedModuleLabel,
-                                   consumedProductInstance,
-                                   modules,
-                                   alreadyFound,
-                                   labelsToDesc,
-                                   preg);
+              ProductResolverIndexInvalid) {
+            if (processName == consumedProcessName) {
+              insertFoundModuleLabel(*itKind,
+                                     itInfo->m_type,
+                                     consumedModuleLabel,
+                                     consumedProductInstance,
+                                     modules,
+                                     alreadyFound,
+                                     labelsToDesc,
+                                     preg);
+            } else {
+              // Product explicitly from different process than the current process, so must refer to an earlier process (unless it ends up "not found")
+              modulesInPreviousProcessesEmplace(consumedModuleLabel, consumedProcessName);
+            }
           }
         } else {  // process name was empty
-          auto matches = iHelper.relatedIndexes(*itKind, itInfo->m_type, consumedModuleLabel, consumedProductInstance);
+          auto matches = helper.relatedIndexes(*itKind, itInfo->m_type, consumedModuleLabel, consumedProductInstance);
           for (unsigned int j = 0; j < matches.numberOfMatches(); ++j) {
             if (processName == matches.processName(j)) {
               insertFoundModuleLabel(*itKind,
@@ -512,12 +527,16 @@ void EDConsumerBase::modulesWhoseProductsAreConsumed(std::vector<ModuleDescripti
                                      alreadyFound,
                                      labelsToDesc,
                                      preg);
+            } else {
+              // Product did not match to current process, so must refer to an earlier process (unless it ends up "not found")
+              // Recall that empty process name means "in the latest process" that can change event-by-event
+              modulesInPreviousProcessesEmplace(consumedModuleLabel, matches.processName(j));
             }
           }
         }
         // consumesMany case
       } else if (itInfo->m_index.productResolverIndex() == ProductResolverIndexInvalid) {
-        auto matches = iHelper.relatedIndexes(*itKind, itInfo->m_type);
+        auto matches = helper.relatedIndexes(*itKind, itInfo->m_type);
         for (unsigned int j = 0; j < matches.numberOfMatches(); ++j) {
           if (processName == matches.processName(j)) {
             insertFoundModuleLabel(*itKind,
@@ -528,9 +547,22 @@ void EDConsumerBase::modulesWhoseProductsAreConsumed(std::vector<ModuleDescripti
                                    alreadyFound,
                                    labelsToDesc,
                                    preg);
+          } else {
+            modulesInPreviousProcessesEmplace(matches.moduleLabel(j), matches.processName(j));
           }
         }
       }
+    } else {
+      // The skipCurrentProcess means the same as empty process name,
+      // except the current process is skipped. Therefore need to do
+      // the same matching as above. There is no consumesMany branch
+      // in this case.
+      auto matches = helper.relatedIndexes(*itKind, itInfo->m_type, consumedModuleLabel, consumedProductInstance);
+      for (unsigned int j = 0; j < matches.numberOfMatches(); ++j) {
+        if (processName != matches.processName(j)) {
+          modulesInPreviousProcessesEmplace(matches.moduleLabel(j), matches.processName(j));
+        }
+      }
     }
   }
 }