Skip to content

Commit 94123ac

Browse files
Dr15JonesDr15Jones
committed
Treat Path/Trigger Inserters like other modules
Now added to all the relevant registries which also handle all the necessary initialization.
1 parent 3ac5df3 commit 94123ac

File tree

13 files changed

+133
-91
lines changed

13 files changed

+133
-91
lines changed

‎FWCore/Framework/interface/ModuleRegistry.h‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,9 @@
2626
// user include files
2727
#include "FWCore/ServiceRegistry/interface/ActivityRegistry.h"
2828
#include "FWCore/Utilities/interface/propagate_const.h"
29+
#include "FWCore/Utilities/interface/Exception.h"
2930
#include "FWCore/Framework/interface/ModuleTypeResolverMaker.h"
31+
#include "FWCore/Framework/interface/maker/ModuleHolder.h"
3032

3133
// forward declarations
3234
namespace edm {
@@ -46,6 +48,8 @@ namespace edm {
4648
std::string const& moduleLabel,
4749
signalslot::Signal<void(ModuleDescription const&)>& iPre,
4850
signalslot::Signal<void(ModuleDescription const&)>& iPost);
51+
//returns a null if module not found
52+
std::shared_ptr<maker::ModuleHolder> getExistingModule(std::string const& moduleLabel);
4953

5054
maker::ModuleHolder* replaceModule(std::string const& iModuleLabel,
5155
edm::ParameterSet const& iPSet,
@@ -55,6 +59,43 @@ namespace edm {
5559
signalslot::Signal<void(ModuleDescription const&)>& iPre,
5660
signalslot::Signal<void(ModuleDescription const&)>& iPost);
5761

62+
template <typename T, typename... Args>
63+
std::shared_ptr<T> makeExplicitModule(ModuleDescription const& md,
64+
PreallocationConfiguration const& iPrealloc,
65+
SignallingProductRegistryFiller* iReg,
66+
signalslot::Signal<void(ModuleDescription const&)>& iPre,
67+
signalslot::Signal<void(ModuleDescription const&)>& iPost,
68+
Args&&... args) {
69+
bool postCalled = false;
70+
if (labelToModule_.find(md.moduleLabel()) != labelToModule_.end()) {
71+
throw cms::Exception("InsertError") << "Module with label '" << md.moduleLabel() << "' already exists.";
72+
}
73+
74+
try {
75+
std::shared_ptr<T> module;
76+
convertException::wrap([&]() {
77+
iPre(md);
78+
module = std::make_shared<T>(std::forward<Args>(args)...);
79+
80+
auto holder = std::make_shared<maker::ModuleHolderT<typename T::ModuleType>>(module);
81+
holder->finishModuleInitialization(md, iPrealloc, iReg);
82+
labelToModule_.emplace(md.moduleLabel(), std::move(holder));
83+
84+
// if exception then post will be called in the catch block
85+
postCalled = true;
86+
iPost(md);
87+
});
88+
return module;
89+
} catch (cms::Exception& iException) {
90+
if (!postCalled) {
91+
CMS_SA_ALLOW try { iPost(md); } catch (...) {
92+
// If post throws an exception ignore it because we are already handling another exception
93+
}
94+
}
95+
throw;
96+
}
97+
}
98+
5899
template <typename F>
59100
void forAllModuleHolders(F iFunc) {
60101
for (auto& labelMod : labelToModule_) {

‎FWCore/Framework/interface/Schedule.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -304,10 +304,10 @@ namespace edm {
304304
std::shared_ptr<ModuleRegistry const> moduleRegistry() const { return get_underlying_safe(moduleRegistry_); }
305305
std::shared_ptr<ModuleRegistry>& moduleRegistry() { return get_underlying_safe(moduleRegistry_); }
306306

307+
edm::propagate_const<std::shared_ptr<ModuleRegistry>> moduleRegistry_;
307308
edm::propagate_const<std::shared_ptr<TriggerResultInserter>> resultsInserter_;
308309
std::vector<edm::propagate_const<std::shared_ptr<PathStatusInserter>>> pathStatusInserters_;
309310
std::vector<edm::propagate_const<std::shared_ptr<EndPathStatusInserter>>> endPathStatusInserters_;
310-
edm::propagate_const<std::shared_ptr<ModuleRegistry>> moduleRegistry_;
311311
std::vector<edm::propagate_const<std::shared_ptr<StreamSchedule>>> streamSchedules_;
312312
//In the future, we will have one GlobalSchedule per simultaneous transition
313313
edm::propagate_const<std::unique_ptr<GlobalSchedule>> globalSchedule_;

‎FWCore/Framework/interface/StreamSchedule.h‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,6 @@ namespace edm {
128128
typedef std::vector<Path> TrigPaths;
129129
typedef std::shared_ptr<HLTGlobalStatus> TrigResPtr;
130130
typedef std::shared_ptr<HLTGlobalStatus const> TrigResConstPtr;
131-
typedef std::shared_ptr<Worker> WorkerPtr;
132131
typedef std::vector<Worker*> AllWorkers;
133132

134133
typedef std::vector<Worker*> Workers;
@@ -318,9 +317,9 @@ namespace edm {
318317

319318
edm::propagate_const<TrigResPtr> results_;
320319

321-
edm::propagate_const<WorkerPtr> results_inserter_;
322-
std::vector<edm::propagate_const<WorkerPtr>> pathStatusInserterWorkers_;
323-
std::vector<edm::propagate_const<WorkerPtr>> endPathStatusInserterWorkers_;
320+
edm::propagate_const<Worker*> results_inserter_;
321+
std::vector<edm::propagate_const<Worker*>> pathStatusInserterWorkers_;
322+
std::vector<edm::propagate_const<Worker*>> endPathStatusInserterWorkers_;
324323

325324
TrigPaths trig_paths_;
326325
TrigPaths end_paths_;

‎FWCore/Framework/interface/WorkerManager.h‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,11 +90,20 @@ namespace edm {
9090
std::string const& label,
9191
bool addToAllWorkers = true);
9292

93+
template <typename T>
94+
Worker* getWorkerForModule(T const& module) {
95+
auto* worker = getWorkerForExistingModule(module.moduleDescription().moduleLabel());
96+
assert(worker != nullptr);
97+
assert(worker->matchesBaseClassPointer(static_cast<typename T::ModuleType const*>(&module)));
98+
return worker;
99+
}
93100
void resetAll();
94101

95102
void releaseMemoryPostLookupSignal();
96103

97104
private:
105+
Worker* getWorkerForExistingModule(std::string const& label);
106+
98107
WorkerRegistry workerReg_;
99108
ExceptionToActionTable const* actionTable_;
100109
AllWorkers allWorkers_;

‎FWCore/Framework/interface/WorkerRegistry.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ namespace edm {
2424
class ModuleRegistry;
2525
class ModuleTypeResolverMaker;
2626
class ParameterSet;
27+
class ExceptionToActionTable;
2728
namespace maker {
2829
class ModuleHolder;
2930
}
@@ -55,6 +56,9 @@ namespace edm {
5556
/// If one doesn't exist, returns nullptr
5657
Worker const* get(std::string const& moduleLabel) const;
5758

59+
//Creates worker if needed
60+
Worker* getWorkerFromExistingModule(std::string const& moduleLabel, ExceptionToActionTable const* actions);
61+
5862
/// Deletes the module of the Worker, but the Worker continues to exist.
5963
void deleteModule(std::string const& moduleLabel);
6064

‎FWCore/Framework/interface/maker/Worker.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -248,6 +248,7 @@ namespace edm {
248248

249249
virtual bool hasAccumulator() const noexcept = 0;
250250

251+
virtual bool matchesBaseClassPointer(void const* iPtr) const noexcept = 0;
251252
// Used in PuttableProductResolver
252253
edm::WaitingTaskList& waitingTaskList() noexcept { return waitingTasks_; }
253254

‎FWCore/Framework/interface/maker/WorkerT.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,8 @@ namespace edm {
8282
template <typename D>
8383
void callWorkerStreamEnd(D, StreamID, LumiTransitionInfo const&, ModuleCallingContext const*);
8484

85+
bool matchesBaseClassPointer(void const* iPtr) const noexcept final { return &(*module_) == iPtr; }
86+
8587
protected:
8688
T& module() { return *module_; }
8789
T const& module() const { return *module_; }

‎FWCore/Framework/src/GlobalSchedule.cc‎

Lines changed: 3 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -58,38 +58,23 @@ namespace edm {
5858
}
5959
}
6060
if (inserter) {
61-
inserter->doPreallocate(prealloc);
6261
for (auto& wm : workerManagers_) {
63-
auto results_inserter = WorkerPtr(new edm::WorkerT<TriggerResultInserter::ModuleType>(
64-
inserter, inserter->moduleDescription(), &actions)); // propagate_const<T> has no reset() function
65-
results_inserter->setActivityRegistry(actReg_);
66-
wm.addToAllWorkers(results_inserter.get());
67-
extraWorkers_.emplace_back(std::move(results_inserter));
62+
(void)wm.getWorkerForModule(*inserter);
6863
}
6964
}
7065

7166
for (auto& pathStatusInserter : pathStatusInserters) {
7267
std::shared_ptr<PathStatusInserter> inserterPtr = get_underlying(pathStatusInserter);
73-
inserterPtr->doPreallocate(prealloc);
7468

7569
for (auto& wm : workerManagers_) {
76-
WorkerPtr workerPtr(
77-
new edm::WorkerT<PathStatusInserter::ModuleType>(inserterPtr, inserterPtr->moduleDescription(), &actions));
78-
workerPtr->setActivityRegistry(actReg_);
79-
wm.addToAllWorkers(workerPtr.get());
80-
extraWorkers_.emplace_back(std::move(workerPtr));
70+
(void)wm.getWorkerForModule(*inserterPtr);
8171
}
8272
}
8373

8474
for (auto& endPathStatusInserter : endPathStatusInserters) {
8575
std::shared_ptr<EndPathStatusInserter> inserterPtr = get_underlying(endPathStatusInserter);
86-
inserterPtr->doPreallocate(prealloc);
8776
for (auto& wm : workerManagers_) {
88-
WorkerPtr workerPtr(new edm::WorkerT<EndPathStatusInserter::ModuleType>(
89-
inserterPtr, inserterPtr->moduleDescription(), &actions));
90-
workerPtr->setActivityRegistry(actReg_);
91-
wm.addToAllWorkers(workerPtr.get());
92-
extraWorkers_.emplace_back(std::move(workerPtr));
77+
(void)wm.getWorkerForModule(*inserterPtr);
9378
}
9479
}
9580

‎FWCore/Framework/src/ModuleRegistry.cc‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,14 @@ namespace edm {
3333
return get_underlying_safe(modItr->second);
3434
}
3535

36+
std::shared_ptr<maker::ModuleHolder> ModuleRegistry::getExistingModule(std::string const& moduleLabel) {
37+
auto modItr = labelToModule_.find(moduleLabel);
38+
if (modItr == labelToModule_.end()) {
39+
return {};
40+
}
41+
return get_underlying_safe(modItr->second);
42+
}
43+
3644
maker::ModuleHolder* ModuleRegistry::replaceModule(std::string const& iModuleLabel,
3745
edm::ParameterSet const& iPSet,
3846
edm::PreallocationConfiguration const& iPrealloc) {

‎FWCore/Framework/src/Schedule.cc‎

Lines changed: 28 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -68,13 +68,13 @@ namespace edm {
6868
// Here we make the trigger results inserter directly. This should
6969
// probably be a utility in the WorkerRegistry or elsewhere.
7070

71-
std::shared_ptr<TriggerResultInserter> makeInserter(
72-
ParameterSet& proc_pset,
73-
PreallocationConfiguration const& iPrealloc,
74-
SignallingProductRegistryFiller& preg,
75-
ExceptionToActionTable const& actions,
76-
std::shared_ptr<ActivityRegistry> areg,
77-
std::shared_ptr<ProcessConfiguration const> processConfiguration) {
71+
std::shared_ptr<TriggerResultInserter> makeInserter(ParameterSet& proc_pset,
72+
PreallocationConfiguration const& iPrealloc,
73+
SignallingProductRegistryFiller& preg,
74+
ExceptionToActionTable const& actions,
75+
std::shared_ptr<ActivityRegistry> areg,
76+
std::shared_ptr<ProcessConfiguration const> processConfiguration,
77+
ModuleRegistry& moduleRegistry) {
7878
ParameterSet* trig_pset = proc_pset.getPSetForUpdate("@trigger_paths");
7979
trig_pset->registerIt();
8080

@@ -85,27 +85,13 @@ namespace edm {
8585
processConfiguration.get(),
8686
ModuleDescription::getUniqueID());
8787

88-
areg->preModuleConstructionSignal_(md);
89-
bool postCalled = false;
90-
std::shared_ptr<TriggerResultInserter> returnValue;
91-
// Caught exception is rethrown
92-
CMS_SA_ALLOW try {
93-
auto module = make_shared_noexcept_false<TriggerResultInserter>(*trig_pset, iPrealloc.numberOfStreams());
94-
maker::ModuleHolderT<TriggerResultInserter::ModuleType>::finishModuleInitialization(
95-
*module, md, iPrealloc, &preg);
96-
returnValue = module;
97-
postCalled = true;
98-
// if exception then post will be called in the catch block
99-
areg->postModuleConstructionSignal_(md);
100-
} catch (...) {
101-
if (!postCalled) {
102-
CMS_SA_ALLOW try { areg->postModuleConstructionSignal_(md); } catch (...) {
103-
// If post throws an exception ignore it because we are already handling another exception
104-
}
105-
}
106-
throw;
107-
}
108-
return returnValue;
88+
return moduleRegistry.makeExplicitModule<TriggerResultInserter>(md,
89+
iPrealloc,
90+
&preg,
91+
areg->preModuleConstructionSignal_,
92+
areg->postModuleConstructionSignal_,
93+
*trig_pset,
94+
iPrealloc.numberOfStreams());
10995
}
11096

11197
template <typename T>
@@ -115,6 +101,7 @@ namespace edm {
115101
SignallingProductRegistryFiller& preg,
116102
std::shared_ptr<ActivityRegistry> areg,
117103
std::shared_ptr<ProcessConfiguration const> processConfiguration,
104+
ModuleRegistry& moduleRegistry,
118105
std::string const& moduleTypeName) {
119106
ParameterSet pset;
120107
pset.addParameter<std::string>("@module_type", moduleTypeName);
@@ -126,25 +113,13 @@ namespace edm {
126113
for (auto const& pathName : pathNames) {
127114
ModuleDescription md(
128115
pset.id(), moduleTypeName, pathName, processConfiguration.get(), ModuleDescription::getUniqueID());
129-
130-
areg->preModuleConstructionSignal_(md);
131-
bool postCalled = false;
132-
// Caught exception is rethrown
133-
CMS_SA_ALLOW try {
134-
auto module = make_shared_noexcept_false<T>(iPrealloc.numberOfStreams());
135-
maker::ModuleHolderT<typename T::ModuleType>::finishModuleInitialization(*module, md, iPrealloc, &preg);
136-
pathStatusInserters.emplace_back(module);
137-
postCalled = true;
138-
// if exception then post will be called in the catch block
139-
areg->postModuleConstructionSignal_(md);
140-
} catch (...) {
141-
if (!postCalled) {
142-
CMS_SA_ALLOW try { areg->postModuleConstructionSignal_(md); } catch (...) {
143-
// If post throws an exception ignore it because we are already handling another exception
144-
}
145-
}
146-
throw;
147-
}
116+
auto module = moduleRegistry.makeExplicitModule<T>(md,
117+
iPrealloc,
118+
&preg,
119+
areg->preModuleConstructionSignal_,
120+
areg->postModuleConstructionSignal_,
121+
iPrealloc.numberOfStreams());
122+
pathStatusInserters.emplace_back(std::move(module));
148123
}
149124
}
150125

@@ -494,10 +469,11 @@ namespace edm {
494469
ProcessContext const* processContext,
495470
ModuleTypeResolverMaker const* resolverMaker)
496471
: //Only create a resultsInserter if there is a trigger path
497-
resultsInserter_{tns.getTrigPaths().empty()
498-
? std::shared_ptr<TriggerResultInserter>{}
499-
: makeInserter(proc_pset, prealloc, preg, actions, areg, processConfiguration)},
500472
moduleRegistry_(std::make_shared<ModuleRegistry>(resolverMaker)),
473+
resultsInserter_{
474+
tns.getTrigPaths().empty()
475+
? std::shared_ptr<TriggerResultInserter>{}
476+
: makeInserter(proc_pset, prealloc, preg, actions, areg, processConfiguration, *moduleRegistry_)},
501477
all_output_communicators_(),
502478
preallocConfig_(prealloc),
503479
pathNames_(&tns.getTrigPaths()),
@@ -509,6 +485,7 @@ namespace edm {
509485
preg,
510486
areg,
511487
processConfiguration,
488+
*moduleRegistry_,
512489
std::string("PathStatusInserter"));
513490

514491
if (endPathNames_->size() > 1) {
@@ -518,6 +495,7 @@ namespace edm {
518495
preg,
519496
areg,
520497
processConfiguration,
498+
*moduleRegistry_,
521499
std::string("EndPathStatusInserter"));
522500
}
523501

@@ -539,8 +517,6 @@ namespace edm {
539517
processContext));
540518
}
541519

542-
//TriggerResults are injected automatically by StreamSchedules and are
543-
// unknown to the ModuleRegistry
544520
const std::string kTriggerResults("TriggerResults");
545521
std::vector<std::string> modulesToUse;
546522
modulesToUse.reserve(streamSchedules_[0]->allWorkersLumisAndEvents().size());

0 commit comments

Comments
 (0)