Refactor how ED modules are constructed

- simplified calling sequence to make a Worker
- consolidated how modules are finalized
- renamed classes to better match their usage

Author: Dr15Jones

FWCore/Framework/interface/MakerMacros.h

@@ -1,8 +1,8 @@
 #ifndef Framework_MakerMacros_h
 #define Framework_MakerMacros_h
 
-#include "FWCore/Framework/interface/maker/MakerPluginFactory.h"
-#include "FWCore/Framework/interface/maker/WorkerMaker.h"
+#include "FWCore/Framework/interface/maker/ModuleMakerPluginFactory.h"
+#include "FWCore/Framework/interface/maker/ModuleMaker.h"
 #include "FWCore/ParameterSet/interface/ParameterSetDescriptionFillerPluginFactory.h"
 // The following includes are temporary until a better
 // solution can be found.  Placing these includes here
@@ -13,7 +13,7 @@
 // implementation file only.
 #include "FWCore/Framework/interface/maker/WorkerT.h"
 
-#define DEFINE_FWK_MODULE(type)                                              \
-  DEFINE_EDM_PLUGIN(edm::MakerPluginFactory, edm::WorkerMaker<type>, #type); \
+#define DEFINE_FWK_MODULE(type)                                                    \
+  DEFINE_EDM_PLUGIN(edm::ModuleMakerPluginFactory, edm::ModuleMaker<type>, #type); \
   DEFINE_FWK_PSET_DESC_FILLER(type)
 #endif

FWCore/Framework/interface/maker/MakerPluginFactory.h

@@ -27,7 +27,6 @@
 #include "FWCore/Framework/interface/SignallingProductRegistryFiller.h"
 // forward declarations
 namespace edm {
-  class Maker;
   class ModuleDescription;
   class SignallingProductRegistryFiller;
   class ExceptionToActionTable;
@@ -36,41 +35,51 @@ namespace edm {
   namespace maker {
     class ModuleHolder {
     public:
-      ModuleHolder(Maker const* iMaker) : m_maker(iMaker) {}
+      ModuleHolder() = default;
       virtual ~ModuleHolder() {}
-      std::unique_ptr<Worker> makeWorker(ExceptionToActionTable const* actions) const;
+      virtual std::unique_ptr<Worker> makeWorker(ExceptionToActionTable const* actions) const = 0;
 
       virtual ModuleDescription const& moduleDescription() const = 0;
-      virtual void setModuleDescription(ModuleDescription const& iDesc) = 0;
-      virtual void preallocate(PreallocationConfiguration const&) = 0;
-      virtual void registerProductsAndCallbacks(SignallingProductRegistryFiller*) = 0;
+      virtual void finishModuleInitialization(ModuleDescription const& iDesc,
+                                              PreallocationConfiguration const& iPrealloc,
+                                              SignallingProductRegistryFiller* iReg) = 0;
       virtual void replaceModuleFor(Worker*) const = 0;
 
       virtual std::unique_ptr<OutputModuleCommunicator> createOutputModuleCommunicator() = 0;
-
-    protected:
-      Maker const* m_maker;
     };
 
     template <typename T>
     class ModuleHolderT : public ModuleHolder {
     public:
-      ModuleHolderT(std::shared_ptr<T> iModule, Maker const* iMaker) : ModuleHolder(iMaker), m_mod(iModule) {}
+      ModuleHolderT(std::shared_ptr<T> iModule) : m_mod(iModule) {}
       ~ModuleHolderT() override {}
       std::shared_ptr<T> module() const { return m_mod; }
       void replaceModuleFor(Worker* iWorker) const override {
         auto w = dynamic_cast<WorkerT<T>*>(iWorker);
         assert(nullptr != w);
         w->setModule(m_mod);
       }
+      std::unique_ptr<Worker> makeWorker(ExceptionToActionTable const* actions) const override {
+        return std::make_unique<edm::WorkerT<T>>(module(), moduleDescription(), actions);
+      }
+
+      static void finishModuleInitialization(T& iModule,
+                                             ModuleDescription const& iDesc,
+                                             PreallocationConfiguration const& iPrealloc,
+                                             SignallingProductRegistryFiller* iReg) {
+        iModule.setModuleDescription(iDesc);
+        iModule.doPreallocate(iPrealloc);
+        if (iReg) {
+          iModule.registerProductsAndCallbacks(&iModule, iReg);
+        }
+      };
       ModuleDescription const& moduleDescription() const override { return m_mod->moduleDescription(); }
-      void setModuleDescription(ModuleDescription const& iDesc) override { m_mod->setModuleDescription(iDesc); }
-      void preallocate(PreallocationConfiguration const& iPrealloc) override { m_mod->doPreallocate(iPrealloc); }
 
-      void registerProductsAndCallbacks(SignallingProductRegistryFiller* iReg) override {
-        m_mod->registerProductsAndCallbacks(module().get(), iReg);
+      void finishModuleInitialization(ModuleDescription const& iDesc,
+                                      PreallocationConfiguration const& iPrealloc,
+                                      SignallingProductRegistryFiller* iReg) override {
+        finishModuleInitialization(*m_mod, iDesc, iPrealloc, iReg);
       }
-
       std::unique_ptr<OutputModuleCommunicator> createOutputModuleCommunicator() override {
         return OutputModuleCommunicatorT<T>::createIfNeeded(m_mod.get());
       }

FWCore/Framework/interface/maker/ModuleHolder.h

@@ -1,5 +1,5 @@
-#ifndef FWCore_Framework_WorkerMaker_h
-#define FWCore_Framework_WorkerMaker_h
+#ifndef FWCore_Framework_ModuleMaker_h
+#define FWCore_Framework_ModuleMaker_h
 
 #include <cassert>
 #include <memory>
@@ -18,13 +18,12 @@ namespace edm {
   class ParameterSet;
   class ExceptionToActionTable;
 
-  class Maker {
+  class ModuleMakerBase {
   public:
-    virtual ~Maker();
+    virtual ~ModuleMakerBase();
     std::shared_ptr<maker::ModuleHolder> makeModule(MakeModuleParams const&,
                                                     signalslot::Signal<void(ModuleDescription const&)>& iPre,
                                                     signalslot::Signal<void(ModuleDescription const&)>& iPost) const;
-    std::unique_ptr<Worker> makeWorker(ExceptionToActionTable const*, maker::ModuleHolder const*) const;
 
     std::shared_ptr<maker::ModuleHolder> makeReplacementModule(edm::ParameterSet const& p) const {
       return makeModule(p);
@@ -42,60 +41,42 @@ namespace edm {
   private:
     virtual void fillDescriptions(ConfigurationDescriptions& iDesc) const = 0;
     virtual std::shared_ptr<maker::ModuleHolder> makeModule(edm::ParameterSet const& p) const = 0;
-    virtual std::unique_ptr<Worker> makeWorker(ExceptionToActionTable const* actions,
-                                               ModuleDescription const& md,
-                                               maker::ModuleHolder const* mod) const = 0;
     virtual const std::string& baseType() const = 0;
   };
 
   template <class T>
-  class WorkerMaker : public Maker {
+  class ModuleMaker : public ModuleMakerBase {
   public:
     //typedef T worker_type;
-    explicit WorkerMaker();
+    explicit ModuleMaker();
 
   private:
     void fillDescriptions(ConfigurationDescriptions& iDesc) const override;
-    std::unique_ptr<Worker> makeWorker(ExceptionToActionTable const* actions,
-                                       ModuleDescription const& md,
-                                       maker::ModuleHolder const* mod) const override;
     std::shared_ptr<maker::ModuleHolder> makeModule(edm::ParameterSet const& p) const override;
     const std::string& baseType() const override;
   };
 
   template <class T>
-  WorkerMaker<T>::WorkerMaker() {}
+  ModuleMaker<T>::ModuleMaker() {}
 
   template <class T>
-  void WorkerMaker<T>::fillDescriptions(ConfigurationDescriptions& iDesc) const {
+  void ModuleMaker<T>::fillDescriptions(ConfigurationDescriptions& iDesc) const {
     T::fillDescriptions(iDesc);
     T::prevalidate(iDesc);
   }
 
   template <class T>
-  std::shared_ptr<maker::ModuleHolder> WorkerMaker<T>::makeModule(edm::ParameterSet const& p) const {
+  std::shared_ptr<maker::ModuleHolder> ModuleMaker<T>::makeModule(edm::ParameterSet const& p) const {
     typedef T UserType;
     typedef typename UserType::ModuleType ModuleType;
     typedef MakeModuleHelper<ModuleType> MakerHelperType;
 
     return std::shared_ptr<maker::ModuleHolder>(
-        std::make_shared<maker::ModuleHolderT<ModuleType> >(MakerHelperType::template makeModule<UserType>(p), this));
+        std::make_shared<maker::ModuleHolderT<ModuleType> >(MakerHelperType::template makeModule<UserType>(p)));
   }
 
   template <class T>
-  std::unique_ptr<Worker> WorkerMaker<T>::makeWorker(ExceptionToActionTable const* actions,
-                                                     ModuleDescription const& md,
-                                                     maker::ModuleHolder const* mod) const {
-    typedef T UserType;
-    typedef typename UserType::ModuleType ModuleType;
-    typedef edm::WorkerT<ModuleType> WorkerType;
-
-    maker::ModuleHolderT<ModuleType> const* h = dynamic_cast<maker::ModuleHolderT<ModuleType> const*>(mod);
-    return std::make_unique<WorkerType>(h->module(), md, actions);
-  }
-
-  template <class T>
-  const std::string& WorkerMaker<T>::baseType() const {
+  const std::string& ModuleMaker<T>::baseType() const {
     return T::baseType();
   }
 

…/Framework/interface/maker/WorkerMaker.h …/Framework/interface/maker/ModuleMaker.hFWCore/Framework/interface/maker/WorkerMaker.h renamed to FWCore/Framework/interface/maker/ModuleMaker.h FWCore/Framework/interface/maker/WorkerMaker.h renamed to FWCore/Framework/interface/maker/ModuleMaker.h

@@ -0,0 +1,12 @@
+#ifndef FWCore_Framework_ModuleMakerPluginFactory_h
+#define FWCore_Framework_ModuleMakerPluginFactory_h
+
+#include "FWCore/PluginManager/interface/PluginFactory.h"
+
+namespace edm {
+  class ModuleMakerBase;
+
+  using ModuleMakerPluginFactory = edmplugin::PluginFactory<ModuleMakerBase*()>;
+}  // namespace edm
+
+#endif

FWCore/Framework/interface/maker/ModuleMakerPluginFactory.h

@@ -1,5 +1,5 @@
 #include "FWCore/Framework/interface/GlobalSchedule.h"
-#include "FWCore/Framework/interface/maker/WorkerMaker.h"
+#include "FWCore/Framework/interface/maker/ModuleMaker.h"
 #include "FWCore/Framework/src/TriggerResultInserter.h"
 #include "FWCore/Framework/src/PathStatusInserter.h"
 #include "FWCore/Framework/src/EndPathStatusInserter.h"

FWCore/Framework/src/GlobalSchedule.cc

@@ -1,36 +1,35 @@
 
-#include "FWCore/Framework/src/Factory.h"
-#include "FWCore/Framework/interface/maker/MakerPluginFactory.h"
+#include "FWCore/Framework/src/ModuleHolderFactory.h"
+#include "FWCore/Framework/interface/maker/ModuleMakerPluginFactory.h"
 #include "FWCore/Framework/interface/ModuleTypeResolverMaker.h"
 #include "FWCore/Framework/interface/resolveMaker.h"
-#include "FWCore/Utilities/interface/DebugMacros.h"
 #include "FWCore/Utilities/interface/EDMException.h"
 #include "FWCore/Utilities/interface/Algorithms.h"
 
 #include <iostream>
 
-EDM_REGISTER_PLUGINFACTORY(edm::MakerPluginFactory, "CMS EDM Framework Module");
+EDM_REGISTER_PLUGINFACTORY(edm::ModuleMakerPluginFactory, "CMS EDM Framework Module");
 namespace edm {
 
-  Factory const Factory::singleInstance_;
+  ModuleHolderFactory const ModuleHolderFactory::singleInstance_;
 
-  Factory::~Factory() = default;
+  ModuleHolderFactory::~ModuleHolderFactory() = default;
 
-  Factory::Factory() = default;
+  ModuleHolderFactory::ModuleHolderFactory() = default;
 
-  Factory const* Factory::get() { return &singleInstance_; }
+  ModuleHolderFactory const* ModuleHolderFactory::get() { return &singleInstance_; }
 
-  Maker const* Factory::findMaker(const MakeModuleParams& p, ModuleTypeResolverMaker const* resolverMaker) const {
+  ModuleMakerBase const* ModuleHolderFactory::findMaker(const MakeModuleParams& p,
+                                                        ModuleTypeResolverMaker const* resolverMaker) const {
     std::string modtype = p.pset_->getParameter<std::string>("@module_type");
-    FDEBUG(1) << "Factory: module_type = " << modtype << std::endl;
     MakerMap::iterator it = makers_.find(modtype);
     if (it != makers_.end()) {
       return it->second.get();
     }
-    return detail::resolveMaker<MakerPluginFactory>(modtype, resolverMaker, *p.pset_, makers_);
+    return detail::resolveMaker<ModuleMakerPluginFactory>(modtype, resolverMaker, *p.pset_, makers_);
   }
 
-  std::shared_ptr<maker::ModuleHolder> Factory::makeModule(
+  std::shared_ptr<maker::ModuleHolder> ModuleHolderFactory::makeModule(
       const MakeModuleParams& p,
       const ModuleTypeResolverMaker* resolverMaker,
       signalslot::Signal<void(const ModuleDescription&)>& pre,
@@ -40,7 +39,7 @@ namespace edm {
     return mod;
   }
 
-  std::shared_ptr<maker::ModuleHolder> Factory::makeReplacementModule(const edm::ParameterSet& p) const {
+  std::shared_ptr<maker::ModuleHolder> ModuleHolderFactory::makeReplacementModule(const edm::ParameterSet& p) const {
     std::string modtype = p.getParameter<std::string>("@module_type");
     MakerMap::iterator it = makers_.find(modtype);
     if (it != makers_.end()) {

FWCore/Framework/src/ModuleHolder.cc

@@ -1,8 +1,8 @@
-#ifndef FWCore_Framework_Factory_h
-#define FWCore_Framework_Factory_h
+#ifndef FWCore_Framework_ModuleHolderFactory_h
+#define FWCore_Framework_ModuleHolderFactory_h
 
 #include "FWCore/Framework/interface/maker/Worker.h"
-#include "FWCore/Framework/interface/maker/WorkerMaker.h"
+#include "FWCore/Framework/interface/maker/ModuleMaker.h"
 #include "FWCore/Framework/interface/maker/MakeModuleParams.h"
 
 #include <map>
@@ -15,13 +15,13 @@
 namespace edm {
   class ModuleTypeResolverMaker;
 
-  class Factory {
+  class ModuleHolderFactory {
   public:
-    typedef std::map<std::string, std::unique_ptr<Maker const>> MakerMap;
+    typedef std::map<std::string, std::unique_ptr<ModuleMakerBase const>> MakerMap;
 
-    ~Factory();
+    ~ModuleHolderFactory();
 
-    static Factory const* get();
+    static ModuleHolderFactory const* get();
 
     //This function is not const-thread safe
     std::shared_ptr<maker::ModuleHolder> makeModule(const MakeModuleParams&,
@@ -32,9 +32,9 @@ namespace edm {
     std::shared_ptr<maker::ModuleHolder> makeReplacementModule(const edm::ParameterSet&) const;
 
   private:
-    Factory();
-    Maker const* findMaker(const MakeModuleParams& p, const ModuleTypeResolverMaker*) const;
-    static Factory const singleInstance_;
+    ModuleHolderFactory();
+    ModuleMakerBase const* findMaker(const MakeModuleParams& p, const ModuleTypeResolverMaker*) const;
+    static ModuleHolderFactory const singleInstance_;
     //It is not safe to create modules across threads
     CMS_SA_ALLOW mutable MakerMap makers_;
   };