From 902039b48bd29086aa9667785ca721933aa650ad Mon Sep 17 00:00:00 2001 From: Matti Kortelainen Date: Wed, 1 Apr 2020 16:29:14 +0200 Subject: [PATCH 01/26] Make Worker::description() to return a pointer to ModuleDescription --- FWCore/Framework/src/EventProcessor.cc | 2 +- FWCore/Framework/src/GlobalSchedule.cc | 4 ++-- FWCore/Framework/src/Path.cc | 4 +++- FWCore/Framework/src/ProductResolvers.cc | 4 ++-- FWCore/Framework/src/Schedule.cc | 18 +++++++-------- FWCore/Framework/src/StreamSchedule.cc | 22 +++++++++---------- .../Framework/src/UnscheduledConfigurator.h | 2 +- FWCore/Framework/src/Worker.cc | 19 +++++++++------- FWCore/Framework/src/Worker.h | 3 +-- FWCore/Framework/src/WorkerManager.cc | 2 +- FWCore/Framework/src/WorkerT.cc | 2 +- 11 files changed, 43 insertions(+), 39 deletions(-) diff --git a/FWCore/Framework/src/EventProcessor.cc b/FWCore/Framework/src/EventProcessor.cc index 5fbdafc3d0fd4..5b9efdc9ea533 100644 --- a/FWCore/Framework/src/EventProcessor.cc +++ b/FWCore/Framework/src/EventProcessor.cc @@ -1914,7 +1914,7 @@ namespace edm { s = std::make_unique("ModulesSynchingOnLumis"); (*s) << "The following modules require synchronizing on LuminosityBlock boundaries:"; } - (*s) << "\n " << worker->description().moduleName() << " " << worker->description().moduleLabel(); + (*s) << "\n " << worker->description()->moduleName() << " " << worker->description()->moduleLabel(); } } } diff --git a/FWCore/Framework/src/GlobalSchedule.cc b/FWCore/Framework/src/GlobalSchedule.cc index 501f4443011e9..4de2436fd817f 100644 --- a/FWCore/Framework/src/GlobalSchedule.cc +++ b/FWCore/Framework/src/GlobalSchedule.cc @@ -99,7 +99,7 @@ namespace edm { Worker* found = nullptr; for (auto& wm : workerManagers_) { for (auto const& worker : wm.allWorkers()) { - if (worker->description().moduleLabel() == iLabel) { + if (worker->description()->moduleLabel() == iLabel) { found = worker; break; } @@ -118,7 +118,7 @@ namespace edm { result.reserve(allWorkers().size()); for (auto const& worker : allWorkers()) { - ModuleDescription const* p = worker->descPtr(); + ModuleDescription const* p = worker->description(); result.push_back(p); } return result; diff --git a/FWCore/Framework/src/Path.cc b/FWCore/Framework/src/Path.cc index fcf01ad587c75..7c7b40fb37341 100644 --- a/FWCore/Framework/src/Path.cc +++ b/FWCore/Framework/src/Path.cc @@ -266,12 +266,14 @@ namespace edm { CMS_SA_ALLOW try { std::ostringstream ost; ost << iEP.id(); + ModuleDescription const* desc = worker.getWorker()->description(); + assert(desc != nullptr); shouldContinue = handleWorkerFailure(*pEx, iModuleIndex, /*isEvent*/ true, /*isBegin*/ true, InEvent, - worker.getWorker()->description(), + *desc, ost.str()); //If we didn't rethrow, then we effectively skipped worker.skipWorker(iEP); diff --git a/FWCore/Framework/src/ProductResolvers.cc b/FWCore/Framework/src/ProductResolvers.cc index 50ae96afbbc13..d072f3b9e5078 100644 --- a/FWCore/Framework/src/ProductResolvers.cc +++ b/FWCore/Framework/src/ProductResolvers.cc @@ -412,8 +412,8 @@ namespace edm { } catch (cms::Exception& ex) { std::ostringstream ost; - ost << "Calling produce method for unscheduled module " << worker_->description().moduleName() << "/'" - << worker_->description().moduleLabel() << "'"; + ost << "Calling produce method for unscheduled module " << worker_->description()->moduleName() << "/'" + << worker_->description()->moduleLabel() << "'"; ex.addContext(ost.str()); throw; } diff --git a/FWCore/Framework/src/Schedule.cc b/FWCore/Framework/src/Schedule.cc index e98b82952dced..acb10cdb91a34 100644 --- a/FWCore/Framework/src/Schedule.cc +++ b/FWCore/Framework/src/Schedule.cc @@ -719,8 +719,8 @@ namespace edm { std::vector modulesToUse; modulesToUse.reserve(streamSchedules_[0]->allWorkers().size()); for (auto const& worker : streamSchedules_[0]->allWorkers()) { - if (worker->description().moduleLabel() != kTriggerResults) { - modulesToUse.push_back(worker->description().moduleLabel()); + if (worker->description()->moduleLabel() != kTriggerResults) { + modulesToUse.push_back(worker->description()->moduleLabel()); } } //The unscheduled modules are at the end of the list, but we want them at the front @@ -752,8 +752,8 @@ namespace edm { // reduceParameterSet would fail to find it. Just remove it up front. std::set usedModuleLabels; for (auto const& worker : allWorkers()) { - if (worker->description().moduleLabel() != kTriggerResults) { - usedModuleLabels.insert(worker->description().moduleLabel()); + if (worker->description()->moduleLabel() != kTriggerResults) { + usedModuleLabels.insert(worker->description()->moduleLabel()); } } std::vector modulesInConfig(proc_pset.getParameter>("@all_modules")); @@ -841,7 +841,7 @@ namespace edm { std::transform(workers.begin(), workers.end(), std::back_inserter(modDesc), - [](const Worker* iWorker) -> const ModuleDescription* { return iWorker->descPtr(); }); + [](const Worker* iWorker) -> const ModuleDescription* { return iWorker->description(); }); // propagate_const has no reset() function summaryTimeKeeper_ = std::make_unique(prealloc.numberOfStreams(), modDesc, tns, processContext); @@ -1379,7 +1379,7 @@ namespace edm { eventsetup::ESRecordsToProxyIndices const& iIndices) { Worker* found = nullptr; for (auto const& worker : allWorkers()) { - if (worker->description().moduleLabel() == iLabel) { + if (worker->description()->moduleLabel() == iLabel) { found = worker; break; } @@ -1427,7 +1427,7 @@ namespace edm { result.reserve(allWorkers().size()); for (auto const& worker : allWorkers()) { - ModuleDescription const* p = worker->descPtr(); + ModuleDescription const* p = worker->description(); result.push_back(p); } return result; @@ -1481,7 +1481,7 @@ namespace edm { std::map labelToDesc; unsigned int i = 0; for (auto const& worker : allWorkers()) { - ModuleDescription const* p = worker->descPtr(); + ModuleDescription const* p = worker->description(); allModuleDescriptions.push_back(p); moduleIDToIndex.push_back(std::pair(p->id(), i)); labelToDesc[p->moduleLabel()] = p; @@ -1496,7 +1496,7 @@ namespace edm { worker->modulesWhoseProductsAreConsumed(modules, preg, labelToDesc); } catch (cms::Exception& ex) { ex.addContext("Calling Worker::modulesWhoseProductsAreConsumed() for module " + - worker->description().moduleLabel()); + worker->description()->moduleLabel()); throw; } ++i; diff --git a/FWCore/Framework/src/StreamSchedule.cc b/FWCore/Framework/src/StreamSchedule.cc index 05da5e216a757..475664cd08499 100644 --- a/FWCore/Framework/src/StreamSchedule.cc +++ b/FWCore/Framework/src/StreamSchedule.cc @@ -198,7 +198,7 @@ namespace edm { //See if all modules were used std::set usedWorkerLabels; for (auto const& worker : allWorkers()) { - usedWorkerLabels.insert(worker->description().moduleLabel()); + usedWorkerLabels.insert(worker->description()->moduleLabel()); } std::vector modulesInConfig(proc_pset.getParameter>("@all_modules")); std::set modulesInConfigSet(modulesInConfig.begin(), modulesInConfig.end()); @@ -291,7 +291,7 @@ namespace edm { for (auto w : allWorkers()) { //determine if this module could read a branch we want to delete early - auto pset = pset::Registry::instance()->getMapped(w->description().parameterSetID()); + auto pset = pset::Registry::instance()->getMapped(w->description()->parameterSetID()); if (nullptr != pset) { auto branches = pset->getUntrackedParameter>("mightGet", kEmpty); if (not branches.empty()) { @@ -446,10 +446,10 @@ namespace edm { // We have a filter on an end path, and the filter is not explicitly ignored. // See if the filter is allowed. std::vector allowed_filters = proc_pset.getUntrackedParameter("@filters_on_endpaths"); - if (!search_all(allowed_filters, worker->description().moduleName())) { + if (!search_all(allowed_filters, worker->description()->moduleName())) { // Filter is not allowed. Ignore the result, and issue a warning. filterAction = WorkerInPath::Ignore; - LogWarning("FilterOnEndPath") << "The EDFilter '" << worker->description().moduleName() + LogWarning("FilterOnEndPath") << "The EDFilter '" << worker->description()->moduleName() << "' with module label '" << moduleLabel << "' appears on EndPath '" << pathName << "'.\n" << "The return value of the filter will be ignored.\n" @@ -534,7 +534,7 @@ namespace edm { void StreamSchedule::replaceModule(maker::ModuleHolder* iMod, std::string const& iLabel) { Worker* found = nullptr; for (auto const& worker : allWorkers()) { - if (worker->description().moduleLabel() == iLabel) { + if (worker->description()->moduleLabel() == iLabel) { found = worker; break; } @@ -552,7 +552,7 @@ namespace edm { result.reserve(allWorkers().size()); for (auto const& worker : allWorkers()) { - ModuleDescription const* p = worker->descPtr(); + ModuleDescription const* p = worker->description(); result.push_back(p); } return result; @@ -764,7 +764,7 @@ namespace edm { if (itFound != trig_paths_.end()) { oLabelsToFill.reserve(itFound->size()); for (size_t i = 0; i < itFound->size(); ++i) { - oLabelsToFill.push_back(itFound->getWorker(i)->description().moduleLabel()); + oLabelsToFill.push_back(itFound->getWorker(i)->description()->moduleLabel()); } } } @@ -793,7 +793,7 @@ namespace edm { if (found) { descriptions.reserve(itFound->size()); for (size_t i = 0; i < itFound->size(); ++i) { - descriptions.push_back(itFound->getWorker(i)->descPtr()); + descriptions.push_back(itFound->getWorker(i)->description()); } } } @@ -822,7 +822,7 @@ namespace edm { if (found) { descriptions.reserve(itFound->size()); for (size_t i = 0; i < itFound->size(); ++i) { - descriptions.push_back(itFound->getWorker(i)->descPtr()); + descriptions.push_back(itFound->getWorker(i)->description()); } } } @@ -836,7 +836,7 @@ namespace edm { sum.timesPassed += path.timesPassed(which); sum.timesFailed += path.timesFailed(which); sum.timesExcept += path.timesExcept(which); - sum.moduleLabel = path.getWorker(which)->description().moduleLabel(); + sum.moduleLabel = path.getWorker(which)->description()->moduleLabel(); } static void fillPathSummary(Path const& path, PathSummary& sum) { @@ -868,7 +868,7 @@ namespace edm { sum.timesPassed += w.timesPassed(); sum.timesFailed += w.timesFailed(); sum.timesExcept += w.timesExcept(); - sum.moduleLabel = w.description().moduleLabel(); + sum.moduleLabel = w.description()->moduleLabel(); } static void fillWorkerSummary(Worker const* pw, WorkerSummary& sum) { fillWorkerSummaryAux(*pw, sum); } diff --git a/FWCore/Framework/src/UnscheduledConfigurator.h b/FWCore/Framework/src/UnscheduledConfigurator.h index bc75404270a10..c7f6dfa31f9df 100644 --- a/FWCore/Framework/src/UnscheduledConfigurator.h +++ b/FWCore/Framework/src/UnscheduledConfigurator.h @@ -34,7 +34,7 @@ namespace edm { template UnscheduledConfigurator(IT iBegin, IT iEnd, UnscheduledAuxiliary const* iAux) : m_aux(iAux) { for (auto it = iBegin; it != iEnd; ++it) { - m_labelToWorker.emplace((*it)->description().moduleLabel(), *it); + m_labelToWorker.emplace((*it)->description()->moduleLabel(), *it); } } diff --git a/FWCore/Framework/src/Worker.cc b/FWCore/Framework/src/Worker.cc index 207917880c151..853a61a338349 100644 --- a/FWCore/Framework/src/Worker.cc +++ b/FWCore/Framework/src/Worker.cc @@ -180,7 +180,7 @@ namespace edm { return true; } - ModuleCallingContext tempContext(&description(), ModuleCallingContext::State::kInvalid, parentContext, nullptr); + ModuleCallingContext tempContext(description(), ModuleCallingContext::State::kInvalid, parentContext, nullptr); // If we are processing an endpath and the module was scheduled, treat SkipEvent or FailPath // as IgnoreCompletely, so any subsequent OutputModules are still run. @@ -349,13 +349,14 @@ namespace edm { void Worker::beginJob() { try { convertException::wrap([&]() { - ModuleBeginJobSignalSentry cpp(actReg_.get(), description()); + ModuleBeginJobSignalSentry cpp(actReg_.get(), *description()); implBeginJob(); }); } catch (cms::Exception& ex) { state_ = Exception; std::ostringstream ost; - ost << "Calling beginJob for module " << description().moduleName() << "/'" << description().moduleLabel() << "'"; + ost << "Calling beginJob for module " << description()->moduleName() << "/'" << description()->moduleLabel() + << "'"; ex.addContext(ost.str()); throw; } @@ -364,13 +365,15 @@ namespace edm { void Worker::endJob() { try { convertException::wrap([&]() { - ModuleEndJobSignalSentry cpp(actReg_.get(), description()); + ModuleDescription const* desc = description(); + assert(desc != nullptr); + ModuleEndJobSignalSentry cpp(actReg_.get(), *desc); implEndJob(); }); } catch (cms::Exception& ex) { state_ = Exception; std::ostringstream ost; - ost << "Calling endJob for module " << description().moduleName() << "/'" << description().moduleLabel() << "'"; + ost << "Calling endJob for module " << description()->moduleName() << "/'" << description()->moduleLabel() << "'"; ex.addContext(ost.str()); throw; } @@ -393,7 +396,7 @@ namespace edm { } catch (cms::Exception& ex) { state_ = Exception; std::ostringstream ost; - ost << "Calling beginStream for module " << description().moduleName() << "/'" << description().moduleLabel() + ost << "Calling beginStream for module " << description()->moduleName() << "/'" << description()->moduleLabel() << "'"; ex.addContext(ost.str()); throw; @@ -417,7 +420,7 @@ namespace edm { } catch (cms::Exception& ex) { state_ = Exception; std::ostringstream ost; - ost << "Calling endStream for module " << description().moduleName() << "/'" << description().moduleLabel() + ost << "Calling endStream for module " << description()->moduleName() << "/'" << description()->moduleLabel() << "'"; ex.addContext(ost.str()); throw; @@ -428,7 +431,7 @@ namespace edm { try { implRegisterThinnedAssociations(registry, helper); } catch (cms::Exception& ex) { - ex.addContext("Calling registerThinnedAssociations() for module " + description().moduleLabel()); + ex.addContext("Calling registerThinnedAssociations() for module " + description()->moduleLabel()); throw ex; } } diff --git a/FWCore/Framework/src/Worker.h b/FWCore/Framework/src/Worker.h index b236508048277..7ca119e34d471 100644 --- a/FWCore/Framework/src/Worker.h +++ b/FWCore/Framework/src/Worker.h @@ -185,8 +185,7 @@ namespace edm { void postDoEvent(EventPrincipal const&); - ModuleDescription const& description() const { return *(moduleCallingContext_.moduleDescription()); } - ModuleDescription const* descPtr() const { return moduleCallingContext_.moduleDescription(); } + ModuleDescription const* description() const { return moduleCallingContext_.moduleDescription(); } ///The signals are required to live longer than the last call to 'doWork' /// this was done to improve performance based on profiling void setActivityRegistry(std::shared_ptr areg); diff --git a/FWCore/Framework/src/WorkerManager.cc b/FWCore/Framework/src/WorkerManager.cc index 5987c44fbbfd0..621f011292259 100644 --- a/FWCore/Framework/src/WorkerManager.cc +++ b/FWCore/Framework/src/WorkerManager.cc @@ -85,7 +85,7 @@ namespace edm { auto const lumiLookup = iRegistry.productLookup(InLumi); auto const eventLookup = iRegistry.productLookup(InEvent); if (!allWorkers_.empty()) { - auto const& processName = allWorkers_[0]->description().processName(); + auto const& processName = allWorkers_[0]->description()->processName(); auto processBlockModuleToIndicies = processBlockLookup->indiciesForModulesInProcess(processName); auto runModuleToIndicies = runLookup->indiciesForModulesInProcess(processName); auto lumiModuleToIndicies = lumiLookup->indiciesForModulesInProcess(processName); diff --git a/FWCore/Framework/src/WorkerT.cc b/FWCore/Framework/src/WorkerT.cc index 8885ff2ee1aed..3978441cce32c 100644 --- a/FWCore/Framework/src/WorkerT.cc +++ b/FWCore/Framework/src/WorkerT.cc @@ -685,7 +685,7 @@ namespace edm { BranchType iBranchType, std::unordered_multimap> const& iIndicies) { - resolvePutIndiciesImpl(&module(), iBranchType, iIndicies, description().moduleLabel()); + resolvePutIndiciesImpl(&module(), iBranchType, iIndicies, description()->moduleLabel()); } template From d16fc6af4f39e277ce44b7ce8cf5d6eb3aaf6cac Mon Sep 17 00:00:00 2001 From: Matti Kortelainen Date: Mon, 30 Mar 2020 17:13:31 +0200 Subject: [PATCH 02/26] Delete unscheduled modules whose products are not consumed by anything The corresponding WorkerT continues to exist, but is removed from all WorkerManagers so that it does not get called for any transitions. The deleted modules are removed from PathsAndConsumesOfModules. Allow worker_ to be nullptr in UnscheduledProductResolver, can happen if a module is deleted early. The Worker::description() returns now nullptr if module was deleted. The description() is not expected to be called when the module is deleted, so this change is mostly a safety check in the hope of getting a segfault if the nullptr is accessed. --- FWCore/Framework/interface/EDConsumerBase.h | 3 +- .../interface/PathsAndConsumesOfModules.h | 9 +- FWCore/Framework/interface/Schedule.h | 13 +- .../interface/UnscheduledCallProducer.h | 7 + FWCore/Framework/interface/WorkerManager.h | 3 + .../interface/stream/EDAnalyzerAdaptorBase.h | 3 +- .../stream/ProducingModuleAdaptorBase.h | 3 +- FWCore/Framework/src/EDConsumerBase.cc | 37 +++-- FWCore/Framework/src/EventProcessor.cc | 18 ++- FWCore/Framework/src/GlobalSchedule.cc | 6 + FWCore/Framework/src/GlobalSchedule.h | 3 + FWCore/Framework/src/ModuleRegistry.cc | 10 ++ FWCore/Framework/src/ModuleRegistry.h | 2 + .../src/PathsAndConsumesOfModules.cc | 105 +++++++++++- FWCore/Framework/src/ProductResolvers.cc | 5 +- FWCore/Framework/src/ProductResolvers.h | 8 +- FWCore/Framework/src/Schedule.cc | 22 ++- FWCore/Framework/src/StreamSchedule.cc | 2 + FWCore/Framework/src/StreamSchedule.h | 3 + FWCore/Framework/src/Worker.h | 19 ++- FWCore/Framework/src/WorkerManager.cc | 10 ++ FWCore/Framework/src/WorkerRegistry.cc | 19 +++ FWCore/Framework/src/WorkerRegistry.h | 8 + FWCore/Framework/src/WorkerT.h | 8 +- .../src/stream/EDAnalyzerAdaptorBase.cc | 6 +- .../src/stream/ProducingModuleAdaptorBase.cc | 6 +- FWCore/Framework/test/BuildFile.xml | 8 + .../Framework/test/run_module_delete_tests.sh | 9 ++ .../Framework/test/stubs/TestModuleDelete.cc | 149 ++++++++++++++++++ FWCore/Framework/test/stubs/ToyAnalyzers.cc | 69 ++++++++ .../Framework/test/stubs/ToyIntProducers.cc | 46 ++++++ .../Framework/test/test_module_delete_cfg.py | 92 +++++++++++ .../interface/PathsAndConsumesOfModulesBase.h | 6 + 33 files changed, 676 insertions(+), 41 deletions(-) create mode 100755 FWCore/Framework/test/run_module_delete_tests.sh create mode 100644 FWCore/Framework/test/stubs/TestModuleDelete.cc create mode 100644 FWCore/Framework/test/test_module_delete_cfg.py diff --git a/FWCore/Framework/interface/EDConsumerBase.h b/FWCore/Framework/interface/EDConsumerBase.h index 040c5ab67a767..7757889d8a567 100644 --- a/FWCore/Framework/interface/EDConsumerBase.h +++ b/FWCore/Framework/interface/EDConsumerBase.h @@ -104,7 +104,8 @@ namespace edm { typedef ProductLabels Labels; void labelsForToken(EDGetToken iToken, Labels& oLabels) const; - void modulesWhoseProductsAreConsumed(std::vector& modules, + void modulesWhoseProductsAreConsumed(std::vector& modulesEvent, + std::vector& modulesLumiRun, ProductRegistry const& preg, std::map const& labelsToDesc, std::string const& processName) const; diff --git a/FWCore/Framework/interface/PathsAndConsumesOfModules.h b/FWCore/Framework/interface/PathsAndConsumesOfModules.h index 0cfcbcc5517e6..1053a2244d8c6 100644 --- a/FWCore/Framework/interface/PathsAndConsumesOfModules.h +++ b/FWCore/Framework/interface/PathsAndConsumesOfModules.h @@ -32,6 +32,8 @@ namespace edm { void initialize(Schedule const*, std::shared_ptr); + void removeModules(std::vector const& modules); + private: std::vector const& doPaths() const override { return paths_; } std::vector const& doEndPaths() const override { return endPaths_; } @@ -43,6 +45,8 @@ namespace edm { std::vector const& doModulesOnEndPath(unsigned int endPathIndex) const override; std::vector const& doModulesWhoseProductsAreConsumedBy( unsigned int moduleID) const override; + std::vector const& doModulesWhoseProductsAreConsumedByLumiRun( + unsigned int moduleID) const override; std::vector doConsumesInfo(unsigned int moduleID) const override; @@ -62,12 +66,15 @@ namespace edm { // following data member std::vector > moduleIDToIndex_; - std::vector > modulesWhoseProductsAreConsumedBy_; + std::vector > modulesWhoseProductsAreConsumedByEvent_; + std::vector > modulesWhoseProductsAreConsumedByLumiRun_; Schedule const* schedule_; std::shared_ptr preg_; }; + std::vector nonConsumedUnscheduledModules(edm::PathsAndConsumesOfModulesBase const& iPnC); + 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 index 7f8a4c033aaac..677420d679545 100644 --- a/FWCore/Framework/interface/Schedule.h +++ b/FWCore/Framework/interface/Schedule.h @@ -230,10 +230,12 @@ namespace edm { std::vector& descriptions, unsigned int hint) const; - void fillModuleAndConsumesInfo(std::vector& allModuleDescriptions, - std::vector>& moduleIDToIndex, - std::vector>& modulesWhoseProductsAreConsumedBy, - ProductRegistry const& preg) const; + void fillModuleAndConsumesInfo( + std::vector& allModuleDescriptions, + std::vector>& moduleIDToIndex, + std::vector>& modulesWhoseProductsAreConsumedByEvent, + std::vector>& modulesWhoseProductsAreConsumedByLumiRun, + 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 +279,9 @@ namespace edm { const ProductRegistry& iRegistry, eventsetup::ESRecordsToProxyIndices const&); + /// Deletes module with label iLabel + void deleteModule(std::string const& iLabel); + /// 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 index 9c94536ec16a3..748c03cf52e0f 100644 --- 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 index 11322ad9c269a..45adb0ea43621 100644 --- a/FWCore/Framework/interface/WorkerManager.h +++ b/FWCore/Framework/interface/WorkerManager.h @@ -41,6 +41,9 @@ namespace edm { WorkerManager(std::shared_ptr modReg, std::shared_ptr 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 index f5ac6157da996..78bfe98c16caa 100644 --- a/FWCore/Framework/interface/stream/EDAnalyzerAdaptorBase.h +++ b/FWCore/Framework/interface/stream/EDAnalyzerAdaptorBase.h @@ -109,7 +109,8 @@ namespace edm { const EDConsumerBase* consumer() const; - void modulesWhoseProductsAreConsumed(std::vector& modules, + void modulesWhoseProductsAreConsumed(std::vector& modulesEvent, + std::vector& modulesLumiRun, ProductRegistry const& preg, std::map const& labelsToDesc, std::string const& processName) const; diff --git a/FWCore/Framework/interface/stream/ProducingModuleAdaptorBase.h b/FWCore/Framework/interface/stream/ProducingModuleAdaptorBase.h index bca7bee2433fb..4962a386b4d08 100644 --- a/FWCore/Framework/interface/stream/ProducingModuleAdaptorBase.h +++ b/FWCore/Framework/interface/stream/ProducingModuleAdaptorBase.h @@ -100,7 +100,8 @@ namespace edm { void updateLookup(BranchType iBranchType, ProductResolverIndexHelper const&, bool iPrefetchMayGet); void updateLookup(eventsetup::ESRecordsToProxyIndices const&); - void modulesWhoseProductsAreConsumed(std::vector& modules, + void modulesWhoseProductsAreConsumed(std::vector& modulesEvent, + std::vector& modulesLumiRun, ProductRegistry const& preg, std::map const& labelsToDesc, std::string const& processName) const; diff --git a/FWCore/Framework/src/EDConsumerBase.cc b/FWCore/Framework/src/EDConsumerBase.cc index fcf4d53f0f31f..00298c2836baa 100644 --- a/FWCore/Framework/src/EDConsumerBase.cc +++ b/FWCore/Framework/src/EDConsumerBase.cc @@ -473,11 +473,14 @@ namespace { } } // namespace -void EDConsumerBase::modulesWhoseProductsAreConsumed(std::vector& modules, +void EDConsumerBase::modulesWhoseProductsAreConsumed(std::vector& modulesEvent, + std::vector& modulesLumiRun, ProductRegistry const& preg, std::map const& labelsToDesc, std::string const& processName) const { - ProductResolverIndexHelper const& iHelper = *preg.productLookup(InEvent); + ProductResolverIndexHelper const& helperEvent = *preg.productLookup(InEvent); + ProductResolverIndexHelper const& helperLumi = *preg.productLookup(InLumi); + ProductResolverIndexHelper const& helperRun = *preg.productLookup(InRun); std::set alreadyFound; @@ -485,7 +488,23 @@ void EDConsumerBase::modulesWhoseProductsAreConsumed(std::vector(); for (auto itInfo = m_tokenInfo.begin(), itEnd = m_tokenInfo.end(); itInfo != itEnd; ++itInfo, ++itKind, ++itLabels) { - if (itInfo->m_branchType == InEvent and (not itInfo->m_index.skipCurrentProcess())) { + if (not itInfo->m_index.skipCurrentProcess()) { + ProductResolverIndexHelper const* helper = nullptr; + std::vector* modules = nullptr; + if (itInfo->m_branchType == InEvent) { + helper = &helperEvent; + modules = &modulesEvent; + } else if (itInfo->m_branchType == InLumi) { + helper = &helperLumi; + modules = &modulesLumiRun; + } else if (itInfo->m_branchType == InRun) { + helper = &helperRun; + modules = &modulesLumiRun; + } else { + throw cms::Exception("LogicError") + << "EDConsumerBase::modulesWhoseProductsAreConsumed(): unknown branch type " << 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; @@ -494,27 +513,27 @@ void EDConsumerBase::modulesWhoseProductsAreConsumed(std::vectorindex( *itKind, itInfo->m_type, consumedModuleLabel, consumedProductInstance, consumedProcessName) != ProductResolverIndexInvalid) { insertFoundModuleLabel(*itKind, itInfo->m_type, consumedModuleLabel, consumedProductInstance, - modules, + *modules, alreadyFound, labelsToDesc, preg); } } 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, itInfo->m_type, consumedModuleLabel, consumedProductInstance, - modules, + *modules, alreadyFound, labelsToDesc, preg); @@ -523,14 +542,14 @@ void EDConsumerBase::modulesWhoseProductsAreConsumed(std::vectorm_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, itInfo->m_type, matches.moduleLabel(j), matches.productInstanceName(j), - modules, + *modules, alreadyFound, labelsToDesc, preg); diff --git a/FWCore/Framework/src/EventProcessor.cc b/FWCore/Framework/src/EventProcessor.cc index 5b9efdc9ea533..d8ed2036c8bb8 100644 --- a/FWCore/Framework/src/EventProcessor.cc +++ b/FWCore/Framework/src/EventProcessor.cc @@ -548,8 +548,24 @@ namespace edm { schedule_->convertCurrentProcessAlias(processConfiguration_->processName()); pathsAndConsumesOfModules_.initialize(schedule_.get(), preg()); - //NOTE: this may throw + // Note: all these may throw checkForModuleDependencyCorrectness(pathsAndConsumesOfModules_, printDependencies_); + if (auto const unusedModules = nonConsumedUnscheduledModules(pathsAndConsumesOfModules_); + not unusedModules.empty()) { + pathsAndConsumesOfModules_.removeModules(unusedModules); + + edm::LogWarning("DeleteModules").log([&unusedModules](auto& l) { + l << "Following modules are not in any Path or EndPath, nor is their output consumed by any other module, and " + "therefore they are deleted before beginJob transition."; + for (auto const& description : unusedModules) { + l << "\n " << description->moduleLabel(); + } + }); + for (auto const& description : unusedModules) { + schedule_->deleteModule(description->moduleLabel()); + } + } + actReg_->preBeginJobSignal_(pathsAndConsumesOfModules_, processContext_); if (preallocations_.numberOfLuminosityBlocks() > 1) { diff --git a/FWCore/Framework/src/GlobalSchedule.cc b/FWCore/Framework/src/GlobalSchedule.cc index 4de2436fd817f..177272192850a 100644 --- a/FWCore/Framework/src/GlobalSchedule.cc +++ b/FWCore/Framework/src/GlobalSchedule.cc @@ -113,6 +113,12 @@ namespace edm { } } + void GlobalSchedule::deleteModule(std::string const& iLabel) { + for (auto& wm : workerManagers_) { + wm.deleteModuleIfExists(iLabel); + } + } + std::vector GlobalSchedule::getAllModuleDescriptions() const { std::vector result; result.reserve(allWorkers().size()); diff --git a/FWCore/Framework/src/GlobalSchedule.h b/FWCore/Framework/src/GlobalSchedule.h index 2c66a6b915b14..335577a19838d 100644 --- a/FWCore/Framework/src/GlobalSchedule.h +++ b/FWCore/Framework/src/GlobalSchedule.h @@ -121,6 +121,9 @@ namespace edm { /// clone the type of module with label iLabel but configure with iPSet. void replaceModule(maker::ModuleHolder* iMod, std::string const& iLabel); + /// Delete the module with label iLabel + void deleteModule(std::string const& iLabel); + /// returns the collection of pointers to workers AllWorkers const& allWorkers() const { return workerManagers_[0].allWorkers(); } diff --git a/FWCore/Framework/src/ModuleRegistry.cc b/FWCore/Framework/src/ModuleRegistry.cc index 289cd55ff0235..f37ca0ba74de6 100644 --- a/FWCore/Framework/src/ModuleRegistry.cc +++ b/FWCore/Framework/src/ModuleRegistry.cc @@ -49,4 +49,14 @@ namespace edm { modItr->second = modPtr; return modItr->second.get(); } + + void ModuleRegistry::deleteModule(std::string const& iModuleLabel /* TODO: signals */) { + auto modItr = labelToModule_.find(iModuleLabel); + if (modItr == labelToModule_.end()) { + throw cms::Exception("LogicError") + << "Trying to delete module " << iModuleLabel + << " but it does not exist in the ModuleRegistry. Please contact framework developers."; + } + labelToModule_.erase(modItr); + } } // namespace edm diff --git a/FWCore/Framework/src/ModuleRegistry.h b/FWCore/Framework/src/ModuleRegistry.h index 2f9aa80d3bf29..d54d7272df723 100644 --- a/FWCore/Framework/src/ModuleRegistry.h +++ b/FWCore/Framework/src/ModuleRegistry.h @@ -48,6 +48,8 @@ namespace edm { edm::ParameterSet const& iPSet, edm::PreallocationConfiguration const&); + void deleteModule(std::string const& iModuleLabel /* TODO: signals */); + template void forAllModuleHolders(F iFunc) { for (auto& labelMod : labelToModule_) { diff --git a/FWCore/Framework/src/PathsAndConsumesOfModules.cc b/FWCore/Framework/src/PathsAndConsumesOfModules.cc index eafc1fd7550cf..5ffe0a4906c01 100644 --- a/FWCore/Framework/src/PathsAndConsumesOfModules.cc +++ b/FWCore/Framework/src/PathsAndConsumesOfModules.cc @@ -41,8 +41,44 @@ namespace edm { ++i; } - schedule->fillModuleAndConsumesInfo( - allModuleDescriptions_, moduleIDToIndex_, modulesWhoseProductsAreConsumedBy_, *preg); + schedule->fillModuleAndConsumesInfo(allModuleDescriptions_, + moduleIDToIndex_, + modulesWhoseProductsAreConsumedByEvent_, + modulesWhoseProductsAreConsumedByLumiRun_, + *preg); + } + + void PathsAndConsumesOfModules::removeModules(std::vector const& modules) { + // First check that no modules on Paths are removed + auto checkPath = [&modules](auto const& paths) { + for (auto const& path : paths) { + for (auto const& description : path) { + if (std::find(modules.begin(), modules.end(), description) != modules.end()) { + throw cms::Exception("Assert") + << "PathsAndConsumesOfModules::removeModules() is trying to remove a module with label " + << description->moduleLabel() << " id " << description->id() << " from a Path, this should not happen."; + } + } + } + }; + checkPath(modulesOnPaths_); + checkPath(modulesOnEndPaths_); + + // Remove the modules and adjust the indices in idToIndex map + for (auto iModule = 0U; iModule != allModuleDescriptions_.size(); ++iModule) { + auto found = std::find(modules.begin(), modules.end(), allModuleDescriptions_[iModule]); + if (found != modules.end()) { + allModuleDescriptions_.erase(allModuleDescriptions_.begin() + iModule); + modulesWhoseProductsAreConsumedByEvent_.erase(modulesWhoseProductsAreConsumedByEvent_.begin() + iModule); + modulesWhoseProductsAreConsumedByLumiRun_.erase(modulesWhoseProductsAreConsumedByLumiRun_.begin() + iModule); + for (auto& idToIndex : moduleIDToIndex_) { + if (idToIndex.second >= iModule) { + idToIndex.second--; + } + } + --iModule; + } + } } ModuleDescription const* PathsAndConsumesOfModules::doModuleDescription(unsigned int moduleID) const { @@ -51,7 +87,8 @@ namespace edm { std::vector>::const_iterator iter = std::lower_bound(moduleIDToIndex_.begin(), moduleIDToIndex_.end(), target); if (iter == moduleIDToIndex_.end() || iter->first != moduleID) { - throw Exception(errors::LogicError) << "PathsAndConsumesOfModules::moduleDescription: Unknown moduleID\n"; + throw Exception(errors::LogicError) + << "PathsAndConsumesOfModules::moduleDescription: Unknown moduleID " << moduleID << "\n"; } return allModuleDescriptions_.at(iter->second); } @@ -67,7 +104,12 @@ namespace edm { std::vector const& PathsAndConsumesOfModules::doModulesWhoseProductsAreConsumedBy( unsigned int moduleID) const { - return modulesWhoseProductsAreConsumedBy_.at(moduleIndex(moduleID)); + return modulesWhoseProductsAreConsumedByEvent_.at(moduleIndex(moduleID)); + } + + std::vector const& PathsAndConsumesOfModules::doModulesWhoseProductsAreConsumedByLumiRun( + unsigned int moduleID) const { + return modulesWhoseProductsAreConsumedByLumiRun_.at(moduleIndex(moduleID)); } std::vector PathsAndConsumesOfModules::doConsumesInfo(unsigned int moduleID) const { @@ -81,11 +123,64 @@ namespace edm { std::vector>::const_iterator iter = std::lower_bound(moduleIDToIndex_.begin(), moduleIDToIndex_.end(), target); if (iter == moduleIDToIndex_.end() || iter->first != moduleID) { - throw Exception(errors::LogicError) << "PathsAndConsumesOfModules::moduleIndex: Unknown moduleID\n"; + throw Exception(errors::LogicError) + << "PathsAndConsumesOfModules::moduleIndex: Unknown moduleID " << moduleID << "\n"; } return iter->second; } + std::vector nonConsumedUnscheduledModules(edm::PathsAndConsumesOfModulesBase const& iPnC) { + const std::string kTriggerResults("TriggerResults"); + + std::vector pathNames = iPnC.paths(); + const unsigned int kFirstEndPathIndex = pathNames.size(); + pathNames.insert(pathNames.end(), iPnC.endPaths().begin(), iPnC.endPaths().end()); + + std::unordered_set moduleIndicesInPaths; + + // Exctract all modules in Paths and EndPaths (need to be kept) to + // get the list of unscheduled modules + for (unsigned int pathIndex = 0; pathIndex != pathNames.size(); ++pathIndex) { + std::vector const* moduleDescriptions; + if (pathIndex < kFirstEndPathIndex) { + moduleDescriptions = &(iPnC.modulesOnPath(pathIndex)); + } else { + moduleDescriptions = &(iPnC.modulesOnEndPath(pathIndex - kFirstEndPathIndex)); + } + for (auto const& description : *moduleDescriptions) { + moduleIndicesInPaths.insert(description->id()); + } + } + + // Mark modules whose any output is consumed + // Mark TriggerResults and all Paths/EndPaths as consumed + auto const& allModules = iPnC.allModules(); + std::unordered_set consumedModules; + for (auto const& description : allModules) { + for (auto const& c : iPnC.modulesWhoseProductsAreConsumedBy(description->id())) { + consumedModules.insert(c->id()); + } + for (auto const& c : iPnC.modulesWhoseProductsAreConsumedByLumiRun(description->id())) { + consumedModules.insert(c->id()); + } + if (description->moduleLabel() == kTriggerResults or + std::find(pathNames.begin(), pathNames.end(), description->moduleLabel()) != pathNames.end()) { + consumedModules.insert(description->id()); + } + } + + // Keep modules that are in Paths/EndPaths or whose output is consumed + std::vector unusedModules; + std::copy_if(allModules.begin(), + allModules.end(), + std::back_inserter(unusedModules), + [&moduleIndicesInPaths, &consumedModules](ModuleDescription const* description) { + return moduleIndicesInPaths.find(description->id()) == moduleIndicesInPaths.end() and + consumedModules.find(description->id()) == consumedModules.end(); + }); + return unusedModules; + } + //==================================== // checkForCorrectness algorithm // diff --git a/FWCore/Framework/src/ProductResolvers.cc b/FWCore/Framework/src/ProductResolvers.cc index d072f3b9e5078..a5674afe4bdee 100644 --- a/FWCore/Framework/src/ProductResolvers.cc +++ b/FWCore/Framework/src/ProductResolvers.cc @@ -381,7 +381,6 @@ namespace edm { void UnscheduledProductResolver::setupUnscheduled(UnscheduledConfigurator const& iConfigure) { aux_ = iConfigure.auxiliary(); worker_ = iConfigure.findWorker(branchDescription().moduleLabel()); - assert(worker_ != nullptr); } ProductResolverBase::Resolution UnscheduledProductResolver::resolveProduct_(Principal const&, @@ -431,6 +430,10 @@ namespace edm { if (skipCurrentProcess) { return; } + if (worker_ == nullptr) { + throw cms::Exception("LogicError") << "UnscheduledProductResolver::prefetchAsync_() called with null worker_. " + "This should not happen, please contact framework developers."; + } //need to try changing prefetchRequested_ before adding to waitingTasks_ bool expected = false; bool prefetchRequested = prefetchRequested_.compare_exchange_strong(expected, true); diff --git a/FWCore/Framework/src/ProductResolvers.h b/FWCore/Framework/src/ProductResolvers.h index 095618149f86a..e1697e6d19c59 100644 --- a/FWCore/Framework/src/ProductResolvers.h +++ b/FWCore/Framework/src/ProductResolvers.h @@ -182,7 +182,7 @@ namespace edm { class UnscheduledProductResolver : public ProducedProductResolver { public: explicit UnscheduledProductResolver(std::shared_ptr bd) - : ProducedProductResolver(bd, ProductStatus::ResolveNotRun), aux_(nullptr), prefetchRequested_(false) {} + : ProducedProductResolver(bd, ProductStatus::ResolveNotRun) {} void setupUnscheduled(UnscheduledConfigurator const&) final; @@ -202,9 +202,9 @@ namespace edm { void resetProductData_(bool deleteEarly) override; CMS_THREAD_SAFE mutable WaitingTaskList waitingTasks_; - UnscheduledAuxiliary const* aux_; - Worker* worker_; - mutable std::atomic prefetchRequested_; + UnscheduledAuxiliary const* aux_ = nullptr; + Worker* worker_ = nullptr; + mutable std::atomic prefetchRequested_ = false; }; class AliasProductResolver : public DataManagingOrAliasProductResolver { diff --git a/FWCore/Framework/src/Schedule.cc b/FWCore/Framework/src/Schedule.cc index acb10cdb91a34..2467f5aeaaea3 100644 --- a/FWCore/Framework/src/Schedule.cc +++ b/FWCore/Framework/src/Schedule.cc @@ -1422,6 +1422,14 @@ namespace edm { return true; } + void Schedule::deleteModule(std::string const& iLabel) { + globalSchedule_->deleteModule(iLabel); + for (auto& stream : streamSchedules_) { + stream->deleteModule(iLabel); + } + moduleRegistry_->deleteModule(iLabel); + } + std::vector Schedule::getAllModuleDescriptions() const { std::vector result; result.reserve(allWorkers().size()); @@ -1468,15 +1476,18 @@ namespace edm { void Schedule::fillModuleAndConsumesInfo( std::vector& allModuleDescriptions, std::vector>& moduleIDToIndex, - std::vector>& modulesWhoseProductsAreConsumedBy, + std::vector>& modulesWhoseProductsAreConsumedByEvent, + std::vector>& modulesWhoseProductsAreConsumedByLumiRun, ProductRegistry const& preg) const { allModuleDescriptions.clear(); moduleIDToIndex.clear(); - modulesWhoseProductsAreConsumedBy.clear(); + modulesWhoseProductsAreConsumedByEvent.clear(); + modulesWhoseProductsAreConsumedByLumiRun.clear(); allModuleDescriptions.reserve(allWorkers().size()); moduleIDToIndex.reserve(allWorkers().size()); - modulesWhoseProductsAreConsumedBy.resize(allWorkers().size()); + modulesWhoseProductsAreConsumedByEvent.resize(allWorkers().size()); + modulesWhoseProductsAreConsumedByLumiRun.resize(allWorkers().size()); std::map labelToDesc; unsigned int i = 0; @@ -1491,9 +1502,10 @@ namespace edm { i = 0; for (auto const& worker : allWorkers()) { - std::vector& modules = modulesWhoseProductsAreConsumedBy.at(i); + std::vector& modulesEvent = modulesWhoseProductsAreConsumedByEvent.at(i); + std::vector& modulesLumiRun = modulesWhoseProductsAreConsumedByLumiRun.at(i); try { - worker->modulesWhoseProductsAreConsumed(modules, preg, labelToDesc); + worker->modulesWhoseProductsAreConsumed(modulesEvent, modulesLumiRun, preg, labelToDesc); } catch (cms::Exception& ex) { ex.addContext("Calling Worker::modulesWhoseProductsAreConsumed() for module " + worker->description()->moduleLabel()); diff --git a/FWCore/Framework/src/StreamSchedule.cc b/FWCore/Framework/src/StreamSchedule.cc index 475664cd08499..c2a9dd94e4300 100644 --- a/FWCore/Framework/src/StreamSchedule.cc +++ b/FWCore/Framework/src/StreamSchedule.cc @@ -547,6 +547,8 @@ namespace edm { found->beginStream(streamID_, streamContext_); } + void StreamSchedule::deleteModule(std::string const& iLabel) { workerManager_.deleteModuleIfExists(iLabel); } + std::vector StreamSchedule::getAllModuleDescriptions() const { std::vector result; result.reserve(allWorkers().size()); diff --git a/FWCore/Framework/src/StreamSchedule.h b/FWCore/Framework/src/StreamSchedule.h index 2157ec9d543ab..2aae80ce68f13 100644 --- a/FWCore/Framework/src/StreamSchedule.h +++ b/FWCore/Framework/src/StreamSchedule.h @@ -251,6 +251,9 @@ namespace edm { /// clone the type of module with label iLabel but configure with iPSet. void replaceModule(maker::ModuleHolder* iMod, std::string const& iLabel); + /// Delete the module with label iLabel + void deleteModule(std::string const& iLabel); + /// returns the collection of pointers to workers AllWorkers const& allWorkers() const { return workerManager_.allWorkers(); } diff --git a/FWCore/Framework/src/Worker.h b/FWCore/Framework/src/Worker.h index 7ca119e34d471..aa006606f5202 100644 --- a/FWCore/Framework/src/Worker.h +++ b/FWCore/Framework/src/Worker.h @@ -123,6 +123,11 @@ namespace edm { Worker(Worker const&) = delete; // Disallow copying and moving Worker& operator=(Worker const&) = delete; // Disallow copying and moving + void clearModule() { + moduleValid_ = false; + doClearModule(); + } + virtual bool wantsProcessBlocks() const = 0; virtual bool wantsInputProcessBlocks() const = 0; virtual bool wantsGlobalRuns() const = 0; @@ -185,7 +190,12 @@ namespace edm { void postDoEvent(EventPrincipal const&); - ModuleDescription const* description() const { return moduleCallingContext_.moduleDescription(); } + ModuleDescription const* description() const { + if (moduleValid_) { + return moduleCallingContext_.moduleDescription(); + } + return nullptr; + } ///The signals are required to live longer than the last call to 'doWork' /// this was done to improve performance based on profiling void setActivityRegistry(std::shared_ptr areg); @@ -201,7 +211,8 @@ namespace edm { iIndicies) = 0; virtual void modulesWhoseProductsAreConsumed( - std::vector& modules, + std::vector& modulesEvent, + std::vector& modulesLumiRun, ProductRegistry const& preg, std::map const& labelsToDesc) const = 0; @@ -235,6 +246,9 @@ namespace edm { protected: template friend class workerhelper::CallImpl; + + virtual void doClearModule() = 0; + virtual std::string workerType() const = 0; virtual bool implDo(EventTransitionInfo const&, ModuleCallingContext const*) = 0; @@ -566,6 +580,7 @@ namespace edm { edm::WaitingTaskList waitingTasks_; std::atomic workStarted_; bool ranAcquireWithoutException_; + bool moduleValid_ = true; }; namespace { diff --git a/FWCore/Framework/src/WorkerManager.cc b/FWCore/Framework/src/WorkerManager.cc index 621f011292259..2932813deb00d 100644 --- a/FWCore/Framework/src/WorkerManager.cc +++ b/FWCore/Framework/src/WorkerManager.cc @@ -30,6 +30,16 @@ namespace edm { unscheduled_(*areg), lastSetupEventPrincipal_(nullptr) {} // WorkerManager::WorkerManager + void WorkerManager::deleteModuleIfExists(std::string const& moduleLabel) { + auto worker = workerReg_.get(moduleLabel); + if (worker != nullptr) { + auto eraseBeg = std::remove(allWorkers_.begin(), allWorkers_.end(), worker); + allWorkers_.erase(eraseBeg, allWorkers_.end()); + unscheduled_.removeWorker(worker); + workerReg_.deleteModule(moduleLabel); + } + } + Worker* WorkerManager::getWorker(ParameterSet& pset, ProductRegistry& preg, PreallocationConfiguration const* prealloc, diff --git a/FWCore/Framework/src/WorkerRegistry.cc b/FWCore/Framework/src/WorkerRegistry.cc index 285e149c238f5..8c24d3a3df2cf 100644 --- a/FWCore/Framework/src/WorkerRegistry.cc +++ b/FWCore/Framework/src/WorkerRegistry.cc @@ -44,4 +44,23 @@ namespace edm { } return (workerIt->second.get()); } + + Worker const* WorkerRegistry::get(std::string const& moduleLabel) const { + WorkerMap::const_iterator workerIt = m_workerMap.find(moduleLabel); + if (workerIt != m_workerMap.end()) { + return workerIt->second; + } + return nullptr; + } + + void WorkerRegistry::deleteModule(std::string const& moduleLabel) { + WorkerMap::iterator workerIt = m_workerMap.find(moduleLabel); + if (workerIt == m_workerMap.end()) { + throw cms::Exception("LogicError") + << "WorkerRegistry::deleteModule() Trying to delete the module of a Worker with label " << moduleLabel + << " but no such Worker exists in the WorkerRegistry. Please contact framework developers."; + } + workerIt->second->clearModule(); + } + } // namespace edm diff --git a/FWCore/Framework/src/WorkerRegistry.h b/FWCore/Framework/src/WorkerRegistry.h index 94ede53871a85..d87b6d9d9c039 100644 --- a/FWCore/Framework/src/WorkerRegistry.h +++ b/FWCore/Framework/src/WorkerRegistry.h @@ -50,6 +50,14 @@ namespace edm { create it @note Workers are owned by this class, do not delete them*/ Worker* getWorker(WorkerParams const& p, std::string const& moduleLabel); + + /// Retrieve particular instance of the worker without creating it + /// If one doesn't exist, returns nullptr + Worker const* get(std::string const& moduleLabel) const; + + /// Deletes the module of the Worker, but the Worker continues to exist. + void deleteModule(std::string const& moduleLabel); + void clear(); private: diff --git a/FWCore/Framework/src/WorkerT.h b/FWCore/Framework/src/WorkerT.h index 4c8284c959d44..dacef33b551d2 100644 --- a/FWCore/Framework/src/WorkerT.h +++ b/FWCore/Framework/src/WorkerT.h @@ -77,6 +77,8 @@ namespace edm { T const& module() const { return *module_; } private: + void doClearModule() override { get_underlying_safe(module_).reset(); } + bool implDo(EventTransitionInfo const&, ModuleCallingContext const*) override; void itemsToGetForSelection(std::vector&) const final; @@ -107,10 +109,12 @@ namespace edm { TaskQueueAdaptor serializeRunModule() override; void modulesWhoseProductsAreConsumed( - std::vector& modules, + std::vector& modulesEvent, + std::vector& modulesLumiRun, ProductRegistry const& preg, std::map const& labelsToDesc) const override { - module_->modulesWhoseProductsAreConsumed(modules, preg, labelsToDesc, module_->moduleDescription().processName()); + module_->modulesWhoseProductsAreConsumed( + modulesEvent, modulesLumiRun, preg, labelsToDesc, module_->moduleDescription().processName()); } void convertCurrentProcessAlias(std::string const& processName) override { diff --git a/FWCore/Framework/src/stream/EDAnalyzerAdaptorBase.cc b/FWCore/Framework/src/stream/EDAnalyzerAdaptorBase.cc index b88733bc0bbc5..95fb8eb4388e8 100644 --- a/FWCore/Framework/src/stream/EDAnalyzerAdaptorBase.cc +++ b/FWCore/Framework/src/stream/EDAnalyzerAdaptorBase.cc @@ -122,12 +122,14 @@ void EDAnalyzerAdaptorBase::updateLookup(eventsetup::ESRecordsToProxyIndices con const edm::EDConsumerBase* EDAnalyzerAdaptorBase::consumer() const { return m_streamModules[0]; } void EDAnalyzerAdaptorBase::modulesWhoseProductsAreConsumed( - std::vector& modules, + std::vector& modulesEvent, + std::vector& modulesLumiRun, ProductRegistry const& preg, std::map const& labelsToDesc, std::string const& processName) const { assert(not m_streamModules.empty()); - return m_streamModules[0]->modulesWhoseProductsAreConsumed(modules, preg, labelsToDesc, processName); + return m_streamModules[0]->modulesWhoseProductsAreConsumed( + modulesEvent, modulesLumiRun, preg, labelsToDesc, processName); } void EDAnalyzerAdaptorBase::convertCurrentProcessAlias(std::string const& processName) { diff --git a/FWCore/Framework/src/stream/ProducingModuleAdaptorBase.cc b/FWCore/Framework/src/stream/ProducingModuleAdaptorBase.cc index accfb10c45514..87296f745d0f0 100644 --- a/FWCore/Framework/src/stream/ProducingModuleAdaptorBase.cc +++ b/FWCore/Framework/src/stream/ProducingModuleAdaptorBase.cc @@ -118,12 +118,14 @@ namespace edm { template void ProducingModuleAdaptorBase::modulesWhoseProductsAreConsumed( - std::vector& modules, + std::vector& modulesEvent, + std::vector& modulesLumiRun, ProductRegistry const& preg, std::map const& labelsToDesc, std::string const& processName) const { assert(not m_streamModules.empty()); - return m_streamModules[0]->modulesWhoseProductsAreConsumed(modules, preg, labelsToDesc, processName); + return m_streamModules[0]->modulesWhoseProductsAreConsumed( + modulesEvent, modulesLumiRun, preg, labelsToDesc, processName); } template diff --git a/FWCore/Framework/test/BuildFile.xml b/FWCore/Framework/test/BuildFile.xml index a217cad0a9ed9..785e7581cb6b4 100644 --- a/FWCore/Framework/test/BuildFile.xml +++ b/FWCore/Framework/test/BuildFile.xml @@ -180,6 +180,13 @@ + + + + + + + @@ -385,3 +392,4 @@ + diff --git a/FWCore/Framework/test/run_module_delete_tests.sh b/FWCore/Framework/test/run_module_delete_tests.sh new file mode 100755 index 0000000000000..ddd0070119290 --- /dev/null +++ b/FWCore/Framework/test/run_module_delete_tests.sh @@ -0,0 +1,9 @@ +#!/bin/bash + +# Pass in name and status +function die { echo $1: status $2 ; exit $2; } + +TEST_DIR=src/FWCore/Framework/test + +cmsRun $TEST_DIR/test_module_delete_cfg.py || die "module deletion test failed" $? +echo "module deletion test succeeded" diff --git a/FWCore/Framework/test/stubs/TestModuleDelete.cc b/FWCore/Framework/test/stubs/TestModuleDelete.cc new file mode 100644 index 0000000000000..76e96af35e8e3 --- /dev/null +++ b/FWCore/Framework/test/stubs/TestModuleDelete.cc @@ -0,0 +1,149 @@ +#include "DataFormats/TestObjects/interface/ToyProducts.h" +#include "FWCore/Framework/interface/global/EDProducer.h" +#include "FWCore/Framework/interface/global/EDAnalyzer.h" +#include "FWCore/Framework/interface/stream/EDProducer.h" +#include "FWCore/Framework/interface/stream/EDAnalyzer.h" +#include "FWCore/Framework/interface/MakerMacros.h" +#include "FWCore/MessageLogger/interface/MessageLogger.h" +#include "FWCore/Utilities/interface/EDMException.h" + +#include +#include +#include + +namespace { + // No need to protect, all access is serialized by the framework + std::vector g_constructed; + std::vector g_destructed; + + std::vector g_lumi_constructed; + std::vector g_lumi_destructed; + + std::vector g_run_constructed; + std::vector g_run_destructed; +} // namespace + +namespace edmtest { + // Similar to FailingProducer, but set a flag when deleted + class TestModuleDeleteProducer : public edm::global::EDProducer<> { + public: + explicit TestModuleDeleteProducer(edm::ParameterSet const& p) + : moduleLabel_{p.getParameter("@module_label")} { + produces(); + g_constructed.push_back(moduleLabel_); + } + ~TestModuleDeleteProducer() override { g_destructed.push_back(moduleLabel_); } + void produce(edm::StreamID, edm::Event& e, edm::EventSetup const& c) const override { + throw edm::Exception(edm::errors::NotFound) << "Intentional 'NotFound' exception for testing purposes\n"; + } + + private: + std::string moduleLabel_; + }; + + class TestModuleDeleteInLumiProducer : public edm::global::EDProducer { + public: + explicit TestModuleDeleteInLumiProducer(edm::ParameterSet const& p) + : moduleLabel_{p.getParameter("@module_label")} { + produces(); + g_lumi_constructed.push_back(moduleLabel_); + } + ~TestModuleDeleteInLumiProducer() override { g_lumi_destructed.push_back(moduleLabel_); } + void globalBeginLuminosityBlockProduce(edm::LuminosityBlock&, edm::EventSetup const&) const override { + throw edm::Exception(edm::errors::NotFound) << "Intentional 'NotFound' exception for testing purposes\n"; + } + void produce(edm::StreamID, edm::Event& e, edm::EventSetup const& c) const override { + throw edm::Exception(edm::errors::NotFound) << "Intentional 'NotFound' exception for testing purposes\n"; + } + + private: + std::string moduleLabel_; + }; + + class TestModuleDeleteInRunProducer : public edm::global::EDProducer { + public: + explicit TestModuleDeleteInRunProducer(edm::ParameterSet const& p) + : moduleLabel_{p.getParameter("@module_label")} { + produces(); + g_run_constructed.push_back(moduleLabel_); + } + ~TestModuleDeleteInRunProducer() override { g_run_destructed.push_back(moduleLabel_); } + void globalBeginRunProduce(edm::Run&, edm::EventSetup const&) const override { + throw edm::Exception(edm::errors::NotFound) << "Intentional 'NotFound' exception for testing purposes\n"; + } + void produce(edm::StreamID, edm::Event& e, edm::EventSetup const& c) const override { + throw edm::Exception(edm::errors::NotFound) << "Intentional 'NotFound' exception for testing purposes\n"; + } + + private: + std::string moduleLabel_; + }; + + class TestModuleDeleteAnalyzer : public edm::global::EDAnalyzer<> { + public: + explicit TestModuleDeleteAnalyzer(edm::ParameterSet const& /*p*/){}; + + void beginJob() override { + if (g_constructed.size() == 0) { + throw cms::Exception("LogicError") + << "No TestModuleDeleteProducer modules constructed in this job, the test is meaningless"; + } + + auto formatException = + [](std::vector& constructed, std::vector& destructed, cms::Exception& ex) { + ex << " Modules constructed but not destructed:\n"; + auto newEnd = std::remove_if(constructed.begin(), constructed.end(), [&](const std::string& label) { + auto found = std::find(destructed.begin(), destructed.end(), label); + if (found != destructed.end()) { + destructed.erase(found); + return true; + } + return false; + }); + constructed.erase(newEnd, constructed.end()); + for (const auto& lab : constructed) { + ex << " " << lab << "\n"; + } + }; + + if (g_constructed.size() != g_destructed.size()) { + cms::Exception ex("Assert"); + ex << "Number of TestModuleDeleteProducer constructors " << g_constructed.size() + << " differs from the number of destructors " << g_destructed.size() << "."; + formatException(g_constructed, g_destructed, ex); + throw ex; + } + + if (g_lumi_constructed.size() == 0) { + throw cms::Exception("LogicError") + << "No TestModuleDeleteInLumiProducer modules constructed in this job, the test is meaningless"; + } + if (g_lumi_constructed.size() != g_lumi_destructed.size()) { + cms::Exception ex("Assert"); + ex << "Number of TestModuleDeleteInLumiProducer constructors " << g_lumi_constructed.size() + << " differs from the number of destructors " << g_lumi_destructed.size() << "."; + formatException(g_lumi_constructed, g_lumi_destructed, ex); + throw ex; + } + + if (g_run_constructed.size() == 0) { + throw cms::Exception("LogicError") + << "No TestModuleDeleteInRunProducer modules constructed in this job, the test is meaningless"; + } + if (g_run_constructed.size() != g_run_destructed.size()) { + cms::Exception ex("Assert"); + ex << "Number of TestModuleDeleteInRunProducer constructors " << g_run_constructed.size() + << " differs from the number of destructors " << g_run_destructed.size() << "."; + formatException(g_run_constructed, g_run_destructed, ex); + throw ex; + } + } + + void analyze(edm::StreamID, edm::Event const& e, edm::EventSetup const& c) const override {} + }; +} // namespace edmtest + +DEFINE_FWK_MODULE(edmtest::TestModuleDeleteProducer); +DEFINE_FWK_MODULE(edmtest::TestModuleDeleteInLumiProducer); +DEFINE_FWK_MODULE(edmtest::TestModuleDeleteInRunProducer); +DEFINE_FWK_MODULE(edmtest::TestModuleDeleteAnalyzer); diff --git a/FWCore/Framework/test/stubs/ToyAnalyzers.cc b/FWCore/Framework/test/stubs/ToyAnalyzers.cc index d01ed1893d3f5..e54aeabe3cdca 100644 --- a/FWCore/Framework/test/stubs/ToyAnalyzers.cc +++ b/FWCore/Framework/test/stubs/ToyAnalyzers.cc @@ -18,6 +18,7 @@ Toy EDAnalyzers for testing purposes only. #include "FWCore/ParameterSet/interface/ParameterSet.h" #include "FWCore/Utilities/interface/Exception.h" #include "FWCore/Utilities/interface/InputTag.h" +#include "FWCore/Utilities/interface/transform.h" // #include #include @@ -102,6 +103,73 @@ namespace edmtest { std::vector> m_tokens; }; + //-------------------------------------------------------------------- + // + class GenericIntsAnalyzer : public edm::global::EDAnalyzer<> { + public: + GenericIntsAnalyzer(edm::ParameterSet const& iPSet) + : tokensBeginRun_(edm::vector_transform( + iPSet.getUntrackedParameter>("srcBeginRun"), + [this](edm::InputTag const& tag) { return this->consumes(tag); })), + tokensBeginLumi_(edm::vector_transform( + iPSet.getUntrackedParameter>("srcBeginLumi"), + [this](edm::InputTag const& tag) { return this->consumes(tag); })), + tokensEvent_( + edm::vector_transform(iPSet.getUntrackedParameter>("srcEvent"), + [this](edm::InputTag const& tag) { return this->consumes(tag); })), + tokensEndLumi_(edm::vector_transform( + iPSet.getUntrackedParameter>("srcEndLumi"), + [this](edm::InputTag const& tag) { return this->consumes(tag); })), + tokensEndRun_(edm::vector_transform( + iPSet.getUntrackedParameter>("srcEndRun"), + [this](edm::InputTag const& tag) { return this->consumes(tag); })), + shouldExist_(iPSet.getUntrackedParameter("inputShouldExist")), + shouldBeMissing_(iPSet.getUntrackedParameter("inputShouldBeMissing")) { + if (shouldExist_ and shouldBeMissing_) { + throw cms::Exception("Configuration") + << "inputShouldExist and inputShouldBeMissing both can not be set to True"; + } + } + + void analyze(edm::StreamID, edm::Event const& iEvent, edm::EventSetup const&) const override { + for (auto const& token : tokensEvent_) { + if (shouldExist_) { + (void)iEvent.get(token); + } else if (shouldBeMissing_) { + auto h = iEvent.getHandle(token); + if (h.isValid()) { + edm::EDConsumerBase::Labels labels; + labelsForToken(token, labels); + throw cms::Exception("ProductExists") + << "Product " << labels.module << ":" << labels.productInstance << ":" << labels.process + << " exists, but expectation was it should be missing"; + } + } + } + } + + static void fillDescriptions(edm::ConfigurationDescriptions& descriptions) { + edm::ParameterSetDescription desc; + desc.addUntracked>("srcBeginRun", std::vector{}); + desc.addUntracked>("srcBeginLumi", std::vector{}); + desc.addUntracked>("srcEvent", std::vector{}); + desc.addUntracked>("srcEndLumi", std::vector{}); + desc.addUntracked>("srcEndRun", std::vector{}); + desc.addUntracked("inputShouldExist", false); + desc.addUntracked("inputShouldBeMissing", false); + descriptions.addDefault(desc); + } + + private: + const std::vector> tokensBeginRun_; + const std::vector> tokensBeginLumi_; + const std::vector> tokensEvent_; + const std::vector> tokensEndLumi_; + const std::vector> tokensEndRun_; + const bool shouldExist_; + const bool shouldBeMissing_; + }; + //-------------------------------------------------------------------- // class IntConsumingAnalyzer : public edm::global::EDAnalyzer<> { @@ -327,6 +395,7 @@ using edmtest::SimpleViewAnalyzer; DEFINE_FWK_MODULE(NonAnalyzer); DEFINE_FWK_MODULE(IntTestAnalyzer); DEFINE_FWK_MODULE(MultipleIntsAnalyzer); +DEFINE_FWK_MODULE(edmtest::GenericIntsAnalyzer); DEFINE_FWK_MODULE(IntConsumingAnalyzer); DEFINE_FWK_MODULE(edmtest::IntFromRunConsumingAnalyzer); DEFINE_FWK_MODULE(ConsumingStreamAnalyzer); diff --git a/FWCore/Framework/test/stubs/ToyIntProducers.cc b/FWCore/Framework/test/stubs/ToyIntProducers.cc index ea8367597674d..6898d1443a03a 100644 --- a/FWCore/Framework/test/stubs/ToyIntProducers.cc +++ b/FWCore/Framework/test/stubs/ToyIntProducers.cc @@ -53,6 +53,27 @@ namespace edmtest { // We throw an edm exception with a configurable action. throw edm::Exception(edm::errors::NotFound) << "Intentional 'NotFound' exception for testing purposes\n"; } + //-------------------------------------------------------------------- + // + // throws an exception. + // Announces an IntProduct but does not produce one; + // every call to FailingInLumiProducer::produce throws a cms exception + // + class FailingInLumiProducer : public edm::global::EDProducer { + public: + explicit FailingInLumiProducer(edm::ParameterSet const& /*p*/) { + produces(); + } + void produce(edm::StreamID, edm::Event& e, edm::EventSetup const& c) const override; + + void globalBeginLuminosityBlockProduce(edm::LuminosityBlock&, edm::EventSetup const&) const override; + }; + + void FailingInLumiProducer::produce(edm::StreamID, edm::Event&, edm::EventSetup const&) const {} + void FailingInLumiProducer::globalBeginLuminosityBlockProduce(edm::LuminosityBlock&, edm::EventSetup const&) const { + // We throw an edm exception with a configurable action. + throw edm::Exception(edm::errors::NotFound) << "Intentional 'NotFound' exception for testing purposes\n"; + } //-------------------------------------------------------------------- // @@ -716,6 +737,29 @@ namespace edmtest { processBlock.put(token3_, std::make_unique(value_ + 3)); processBlock.put(token4_, std::make_unique(value_ + 4)); } + + //-------------------------------------------------------------------- + // + // Produces an IntProduct instance, the module must get run, otherwise an exception is thrown + class MustRunIntProducer : public edm::global::EDProducer<> { + public: + explicit MustRunIntProducer(edm::ParameterSet const& p) + : token_{produces()}, value_(p.getParameter("ivalue")) {} + void produce(edm::StreamID, edm::Event& e, edm::EventSetup const& c) const override { + wasRun_ = true; + e.emplace(token_, value_); + } + void endJob() override { + if (not wasRun_) { + throw cms::Exception("NotRun") << "This module should have run for event transitions, but it did not"; + } + } + + private: + const edm::EDPutTokenT token_; + const int value_; + mutable std::atomic wasRun_ = false; + }; } // namespace edmtest using edmtest::AddIntsProducer; @@ -737,6 +781,7 @@ using edmtest::NonEventIntProducer; using edmtest::NonProducer; using edmtest::TransientIntProducer; DEFINE_FWK_MODULE(FailingProducer); +DEFINE_FWK_MODULE(edmtest::FailingInLumiProducer); DEFINE_FWK_MODULE(edmtest::FailingInRunProducer); DEFINE_FWK_MODULE(NonProducer); DEFINE_FWK_MODULE(IntProducer); @@ -755,3 +800,4 @@ DEFINE_FWK_MODULE(ManyIntWhenRegisteredProducer); DEFINE_FWK_MODULE(NonEventIntProducer); DEFINE_FWK_MODULE(IntProducerBeginProcessBlock); DEFINE_FWK_MODULE(IntProducerEndProcessBlock); +DEFINE_FWK_MODULE(edmtest::MustRunIntProducer); diff --git a/FWCore/Framework/test/test_module_delete_cfg.py b/FWCore/Framework/test/test_module_delete_cfg.py new file mode 100644 index 0000000000000..ecfd4ab16ca6d --- /dev/null +++ b/FWCore/Framework/test/test_module_delete_cfg.py @@ -0,0 +1,92 @@ +import FWCore.ParameterSet.Config as cms + +process = cms.Process("TESTMODULEDELETE") + +process.maxEvents.input = 8 +process.source = cms.Source("EmptySource") + +#process.Tracer = cms.Service("Tracer") + +# Cases to test +# - event/lumi/run product consumed, module kept +# - event/lumi/run product not consumed, module deleted +# - event product not consumed but module in Path, module kept +# - event/lumi/run product with the same module label but different instance name, module deleted +# - event/lumi/run product with the same module label and instance name but with skipCurrentProcess, module deleted + +intEventProducer = cms.EDProducer("IntProducer", ivalue = cms.int32(1)) +intNonEventProducer = cms.EDProducer("NonEventIntProducer", ivalue = cms.int32(1)) +intEventProducerMustRun = cms.EDProducer("edmtest::MustRunIntProducer", ivalue = cms.int32(1)) +intEventConsumer = cms.EDAnalyzer("IntTestAnalyzer", + moduleLabel = cms.untracked.InputTag("producerEventConsumed"), + valueMustMatch = cms.untracked.int32(1) +) +def nonEventConsumer(transition, sourcePattern, expected): + transCap = transition[0].upper() + transition[1:] + runOrLumiBlock = transCap + if "Lumi" in runOrLumiBlock: + runOrLumiBlock = runOrLumiBlock+"nosityBlock" + ret = intNonEventProducer.clone() + setattr(ret, "consumes%s"%runOrLumiBlock, cms.InputTag(sourcePattern%transCap, transition)) + setattr(ret, "expect%s"%runOrLumiBlock, cms.untracked.int32(expected)) + return ret + +process.producerEventConsumed = intEventProducer.clone(ivalue = 1) +process.producerBeginLumiConsumed = intNonEventProducer.clone(ivalue = 2) +process.producerEndLumiConsumed = intNonEventProducer.clone(ivalue = 3) +process.producerBeginRunConsumed = intNonEventProducer.clone(ivalue = 4) +process.producerEndRunConsumed = intNonEventProducer.clone(ivalue = 5) + +process.producerEventNotConsumedInPath = intEventProducerMustRun.clone() + +process.consumerEvent = intEventConsumer.clone(moduleLabel = "producerEventConsumed", valueMustMatch = 1) +process.consumerBeginLumi = nonEventConsumer("beginLumi", "producer%sConsumed", 2) +process.consumerEndLumi = nonEventConsumer("endLumi", "producer%sConsumed", 3) +process.consumerBeginRun = nonEventConsumer("beginRun", "producer%sConsumed", 4) +process.consumerEndRun = nonEventConsumer("endRun", "producer%sConsumed", 5) + +process.producerEventNotConsumed = cms.EDProducer("edmtest::TestModuleDeleteProducer") +process.producerBeginLumiNotConsumed = cms.EDProducer("edmtest::TestModuleDeleteInLumiProducer") +process.producerBeginRunNotConsumed = cms.EDProducer("edmtest::TestModuleDeleteInRunProducer") + +process.consumerNotExist = cms.EDAnalyzer("edmtest::GenericIntsAnalyzer", + inputShouldBeMissing = cms.untracked.bool(True), + srcBeginRun = cms.untracked.VInputTag( + "producerBeginRunNotConsumed:doesNotExist", + cms.InputTag("producerBeginRunNotConsumed", "", cms.InputTag.skipCurrentProcess()) + ), + srcBeginLumi = cms.untracked.VInputTag( + "producerBeginLumiNotConsumed:doesNotExist", + cms.InputTag("producerBeginLumiNotConsumed", "", cms.InputTag.skipCurrentProcess()) + ), + srcEvent = cms.untracked.VInputTag( + "producerEventNotConsumed:doesNotExist", + cms.InputTag("producerEventNotConsumed", "", cms.InputTag.skipCurrentProcess()) + ), +) + +process.intAnalyzerDelete = cms.EDAnalyzer("edmtest::TestModuleDeleteAnalyzer") + +process.t = cms.Task( + process.producerEventConsumed, + process.producerBeginLumiConsumed, + process.producerEndLumiConsumed, + process.producerBeginRunConsumed, + process.producerEndRunConsumed, + process.producerEventNotConsumed, + process.producerBeginLumiNotConsumed, + process.producerBeginRunNotConsumed, +) + +process.p = cms.Path( + process.producerEventNotConsumedInPath+ + process.consumerEvent+ + process.consumerBeginLumi+ + process.consumerEndLumi+ + process.consumerBeginRun+ + process.consumerEndRun+ + process.consumerNotExist+ + process.intAnalyzerDelete + , + process.t +) diff --git a/FWCore/ServiceRegistry/interface/PathsAndConsumesOfModulesBase.h b/FWCore/ServiceRegistry/interface/PathsAndConsumesOfModulesBase.h index c37eba496d2c1..ef9fa62d29dec 100644 --- a/FWCore/ServiceRegistry/interface/PathsAndConsumesOfModulesBase.h +++ b/FWCore/ServiceRegistry/interface/PathsAndConsumesOfModulesBase.h @@ -64,6 +64,10 @@ namespace edm { return doModulesWhoseProductsAreConsumedBy(moduleID); } + std::vector const& modulesWhoseProductsAreConsumedByLumiRun(unsigned int moduleID) const { + return doModulesWhoseProductsAreConsumedByLumiRun(moduleID); + } + // This returns the declared consumes information for a module. // Note the other functions above return a reference to an object // that is held in memory throughout the job, while the following @@ -82,6 +86,8 @@ namespace edm { virtual std::vector const& doModulesOnEndPath(unsigned int endPathIndex) const = 0; virtual std::vector const& doModulesWhoseProductsAreConsumedBy( unsigned int moduleID) const = 0; + virtual std::vector const& doModulesWhoseProductsAreConsumedByLumiRun( + unsigned int moduleID) const = 0; virtual std::vector doConsumesInfo(unsigned int moduleID) const = 0; }; } // namespace edm From 0c888f644fa1684f154bb2189bffb69c9a7ae2ef Mon Sep 17 00:00:00 2001 From: Matti Kortelainen Date: Thu, 2 Apr 2020 02:05:08 +0200 Subject: [PATCH 03/26] Delete non-consumed unscheduled modules in SubProcessses Need to pass module label and process name for consumes of products of parent (sub)process(es). --- FWCore/Framework/interface/EDConsumerBase.h | 3 + .../Framework/interface/ModuleProcessName.h | 30 ++ .../interface/PathsAndConsumesOfModules.h | 10 +- FWCore/Framework/interface/Schedule.h | 1 + .../interface/stream/EDAnalyzerAdaptorBase.h | 3 + .../stream/ProducingModuleAdaptorBase.h | 2 + FWCore/Framework/src/EDConsumerBase.cc | 85 ++-- FWCore/Framework/src/EventProcessor.cc | 7 +- .../src/PathsAndConsumesOfModules.cc | 21 +- FWCore/Framework/src/Schedule.cc | 8 +- FWCore/Framework/src/SubProcess.cc | 47 +- FWCore/Framework/src/SubProcess.h | 4 + FWCore/Framework/src/Worker.h | 2 + FWCore/Framework/src/WorkerT.h | 10 +- .../src/stream/EDAnalyzerAdaptorBase.cc | 3 +- .../src/stream/ProducingModuleAdaptorBase.cc | 3 +- .../Framework/test/run_module_delete_tests.sh | 2 + FWCore/Framework/test/stubs/ToyAnalyzers.cc | 47 +- .../Framework/test/stubs/ToyIntProducers.cc | 37 +- .../test/test_module_delete_subprocess_cfg.py | 443 ++++++++++++++++++ 20 files changed, 697 insertions(+), 71 deletions(-) create mode 100644 FWCore/Framework/interface/ModuleProcessName.h create mode 100644 FWCore/Framework/test/test_module_delete_subprocess_cfg.py diff --git a/FWCore/Framework/interface/EDConsumerBase.h b/FWCore/Framework/interface/EDConsumerBase.h index 7757889d8a567..79d5c9a198e46 100644 --- a/FWCore/Framework/interface/EDConsumerBase.h +++ b/FWCore/Framework/interface/EDConsumerBase.h @@ -20,6 +20,7 @@ // system include files #include +#include #include #include #include @@ -50,6 +51,7 @@ namespace edm { class ModuleDescription; + class ModuleProcessName; class ProductResolverIndexHelper; class ProductRegistry; class ConsumesCollector; @@ -106,6 +108,7 @@ namespace edm { void modulesWhoseProductsAreConsumed(std::vector& modulesEvent, std::vector& modulesLumiRun, + std::set& modulesInPreviousProcesses, ProductRegistry const& preg, std::map const& labelsToDesc, std::string const& processName) const; diff --git a/FWCore/Framework/interface/ModuleProcessName.h b/FWCore/Framework/interface/ModuleProcessName.h new file mode 100644 index 0000000000000..0b8b451b07ab7 --- /dev/null +++ b/FWCore/Framework/interface/ModuleProcessName.h @@ -0,0 +1,30 @@ +#ifndef FWCore_Framework_ModuleProcessName_h +#define FWCore_Framework_ModuleProcessName_h + +#include + +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 index 1053a2244d8c6..e853c53258e6e 100644 --- a/FWCore/Framework/interface/PathsAndConsumesOfModules.h +++ b/FWCore/Framework/interface/PathsAndConsumesOfModules.h @@ -15,7 +15,10 @@ #include "FWCore/ServiceRegistry/interface/ConsumesInfo.h" #include "FWCore/ServiceRegistry/interface/PathsAndConsumesOfModulesBase.h" +#include "FWCore/Framework/interface/ModuleProcessName.h" + #include +#include #include #include #include @@ -28,12 +31,15 @@ namespace edm { class PathsAndConsumesOfModules : public PathsAndConsumesOfModulesBase { public: + PathsAndConsumesOfModules(); ~PathsAndConsumesOfModules() override; void initialize(Schedule const*, std::shared_ptr); void removeModules(std::vector const& modules); + std::set const& modulesInPreviousProcessesWhoseProductsAreConsumedBy(unsigned int moduleID) const; + private: std::vector const& doPaths() const override { return paths_; } std::vector const& doEndPaths() const override { return endPaths_; } @@ -68,12 +74,14 @@ namespace edm { std::vector > modulesWhoseProductsAreConsumedByEvent_; std::vector > modulesWhoseProductsAreConsumedByLumiRun_; + std::vector > modulesInPreviousProcessesWhoseProductsAreConsumedBy_; Schedule const* schedule_; std::shared_ptr preg_; }; - std::vector nonConsumedUnscheduledModules(edm::PathsAndConsumesOfModulesBase const& iPnC); + std::vector nonConsumedUnscheduledModules(edm::PathsAndConsumesOfModulesBase const& iPnC, + std::set& consumedByChildren); void checkForModuleDependencyCorrectness(edm::PathsAndConsumesOfModulesBase const& iPnC, bool iPrintDependencies); } // namespace edm diff --git a/FWCore/Framework/interface/Schedule.h b/FWCore/Framework/interface/Schedule.h index 677420d679545..5d552ecd38a78 100644 --- a/FWCore/Framework/interface/Schedule.h +++ b/FWCore/Framework/interface/Schedule.h @@ -235,6 +235,7 @@ namespace edm { std::vector>& moduleIDToIndex, std::vector>& modulesWhoseProductsAreConsumedByEvent, std::vector>& modulesWhoseProductsAreConsumedByLumiRun, + std::vector>& modulesInPreviousProcessesWhoseProductsAreConsumedBy, ProductRegistry const& preg) const; /// Return the number of events this Schedule has tried to process diff --git a/FWCore/Framework/interface/stream/EDAnalyzerAdaptorBase.h b/FWCore/Framework/interface/stream/EDAnalyzerAdaptorBase.h index 78bfe98c16caa..7a8e8252096b9 100644 --- a/FWCore/Framework/interface/stream/EDAnalyzerAdaptorBase.h +++ b/FWCore/Framework/interface/stream/EDAnalyzerAdaptorBase.h @@ -20,6 +20,7 @@ // system include files #include +#include #include #include @@ -40,6 +41,7 @@ namespace edm { class ModuleCallingContext; + class ModuleProcessName; class ProductResolverIndexHelper; class EDConsumerBase; class PreallocationConfiguration; @@ -111,6 +113,7 @@ namespace edm { void modulesWhoseProductsAreConsumed(std::vector& modulesEvent, std::vector& modulesLumiRun, + std::set& modulesInPreviousProcesses, ProductRegistry const& preg, std::map const& labelsToDesc, std::string const& processName) const; diff --git a/FWCore/Framework/interface/stream/ProducingModuleAdaptorBase.h b/FWCore/Framework/interface/stream/ProducingModuleAdaptorBase.h index 4962a386b4d08..ee14b5974618b 100644 --- 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,6 +103,7 @@ namespace edm { void modulesWhoseProductsAreConsumed(std::vector& modulesEvent, std::vector& modulesLumiRun, + std::set& modulesInPreviousProcesses, ProductRegistry const& preg, std::map const& labelsToDesc, std::string const& processName) const; diff --git a/FWCore/Framework/src/EDConsumerBase.cc b/FWCore/Framework/src/EDConsumerBase.cc index 00298c2836baa..616772af7b367 100644 --- 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" @@ -475,6 +476,7 @@ namespace { void EDConsumerBase::modulesWhoseProductsAreConsumed(std::vector& modulesEvent, std::vector& modulesLumiRun, + std::set& modulesInPreviousProcesses, ProductRegistry const& preg, std::map const& labelsToDesc, std::string const& processName) const { @@ -488,42 +490,46 @@ void EDConsumerBase::modulesWhoseProductsAreConsumed(std::vector(); for (auto itInfo = m_tokenInfo.begin(), itEnd = m_tokenInfo.end(); itInfo != itEnd; ++itInfo, ++itKind, ++itLabels) { - if (not itInfo->m_index.skipCurrentProcess()) { - ProductResolverIndexHelper const* helper = nullptr; - std::vector* modules = nullptr; - if (itInfo->m_branchType == InEvent) { - helper = &helperEvent; - modules = &modulesEvent; - } else if (itInfo->m_branchType == InLumi) { - helper = &helperLumi; - modules = &modulesLumiRun; - } else if (itInfo->m_branchType == InRun) { - helper = &helperRun; - modules = &modulesLumiRun; - } else { - throw cms::Exception("LogicError") - << "EDConsumerBase::modulesWhoseProductsAreConsumed(): unknown branch type " << itInfo->m_branchType; - } + ProductResolverIndexHelper const* helper = nullptr; + std::vector* modules = nullptr; + if (itInfo->m_branchType == InEvent) { + helper = &helperEvent; + modules = &modulesEvent; + } else if (itInfo->m_branchType == InLumi) { + helper = &helperLumi; + modules = &modulesLumiRun; + } else if (itInfo->m_branchType == InRun) { + helper = &helperRun; + modules = &modulesLumiRun; + } else { + throw cms::Exception("LogicError") << "EDConsumerBase::modulesWhoseProductsAreConsumed(): unknown branch type " + << 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; + 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 && - helper->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") + modulesInPreviousProcesses.emplace(consumedModuleLabel, consumedProcessName); + } } } else { // process name was empty auto matches = helper->relatedIndexes(*itKind, itInfo->m_type, consumedModuleLabel, consumedProductInstance); @@ -537,6 +543,10 @@ void EDConsumerBase::modulesWhoseProductsAreConsumed(std::vectorrelatedIndexes(*itKind, itInfo->m_type, consumedModuleLabel, consumedProductInstance); + for (unsigned int j = 0; j < matches.numberOfMatches(); ++j) { + if (processName != matches.processName(j)) { + modulesInPreviousProcesses.emplace(matches.moduleLabel(j), matches.processName(j)); + } + } } } } diff --git a/FWCore/Framework/src/EventProcessor.cc b/FWCore/Framework/src/EventProcessor.cc index d8ed2036c8bb8..6a7236057c616 100644 --- a/FWCore/Framework/src/EventProcessor.cc +++ b/FWCore/Framework/src/EventProcessor.cc @@ -547,10 +547,15 @@ namespace edm { actReg_->preallocateSignal_(bounds); schedule_->convertCurrentProcessAlias(processConfiguration_->processName()); pathsAndConsumesOfModules_.initialize(schedule_.get(), preg()); + std::set consumedBySubProcesses; + for_all(subProcesses_, [&consumedBySubProcesses](auto& subProcess) { + auto c = subProcess.keepOnlyConsumedUnscheduledModules(); + consumedBySubProcesses.insert(c.begin(), c.end()); + }); // Note: all these may throw checkForModuleDependencyCorrectness(pathsAndConsumesOfModules_, printDependencies_); - if (auto const unusedModules = nonConsumedUnscheduledModules(pathsAndConsumesOfModules_); + if (auto const unusedModules = nonConsumedUnscheduledModules(pathsAndConsumesOfModules_, consumedBySubProcesses); not unusedModules.empty()) { pathsAndConsumesOfModules_.removeModules(unusedModules); diff --git a/FWCore/Framework/src/PathsAndConsumesOfModules.cc b/FWCore/Framework/src/PathsAndConsumesOfModules.cc index 5ffe0a4906c01..fb46cff6186e2 100644 --- a/FWCore/Framework/src/PathsAndConsumesOfModules.cc +++ b/FWCore/Framework/src/PathsAndConsumesOfModules.cc @@ -1,6 +1,7 @@ #include "FWCore/Framework/interface/PathsAndConsumesOfModules.h" #include "FWCore/Framework/interface/Schedule.h" +#include "FWCore/Framework/interface/ModuleProcessName.h" #include "FWCore/Framework/src/Worker.h" #include "throwIfImproperDependencies.h" @@ -9,7 +10,8 @@ #include namespace edm { - PathsAndConsumesOfModules::~PathsAndConsumesOfModules() {} + PathsAndConsumesOfModules::PathsAndConsumesOfModules() = default; + PathsAndConsumesOfModules::~PathsAndConsumesOfModules() = default; void PathsAndConsumesOfModules::initialize(Schedule const* schedule, std::shared_ptr preg) { schedule_ = schedule; @@ -45,6 +47,7 @@ namespace edm { moduleIDToIndex_, modulesWhoseProductsAreConsumedByEvent_, modulesWhoseProductsAreConsumedByLumiRun_, + modulesInPreviousProcessesWhoseProductsAreConsumedBy_, *preg); } @@ -71,6 +74,8 @@ namespace edm { allModuleDescriptions_.erase(allModuleDescriptions_.begin() + iModule); modulesWhoseProductsAreConsumedByEvent_.erase(modulesWhoseProductsAreConsumedByEvent_.begin() + iModule); modulesWhoseProductsAreConsumedByLumiRun_.erase(modulesWhoseProductsAreConsumedByLumiRun_.begin() + iModule); + modulesInPreviousProcessesWhoseProductsAreConsumedBy_.erase( + modulesInPreviousProcessesWhoseProductsAreConsumedBy_.begin() + iModule); for (auto& idToIndex : moduleIDToIndex_) { if (idToIndex.second >= iModule) { idToIndex.second--; @@ -81,6 +86,11 @@ namespace edm { } } + std::set const& PathsAndConsumesOfModules::modulesInPreviousProcessesWhoseProductsAreConsumedBy( + unsigned int moduleID) const { + return modulesInPreviousProcessesWhoseProductsAreConsumedBy_.at(moduleIndex(moduleID)); + } + ModuleDescription const* PathsAndConsumesOfModules::doModuleDescription(unsigned int moduleID) const { unsigned int dummy = 0; auto target = std::make_pair(moduleID, dummy); @@ -129,7 +139,8 @@ namespace edm { return iter->second; } - std::vector nonConsumedUnscheduledModules(edm::PathsAndConsumesOfModulesBase const& iPnC) { + std::vector nonConsumedUnscheduledModules(edm::PathsAndConsumesOfModulesBase const& iPnC, + std::set& consumedByChildren) { const std::string kTriggerResults("TriggerResults"); std::vector pathNames = iPnC.paths(); @@ -154,6 +165,7 @@ namespace edm { // Mark modules whose any output is consumed // Mark TriggerResults and all Paths/EndPaths as consumed + // Mark anything possibly consumed by child SubProcesses auto const& allModules = iPnC.allModules(); std::unordered_set consumedModules; for (auto const& description : allModules) { @@ -166,6 +178,11 @@ namespace edm { if (description->moduleLabel() == kTriggerResults or std::find(pathNames.begin(), pathNames.end(), description->moduleLabel()) != pathNames.end()) { consumedModules.insert(description->id()); + } else if (consumedByChildren.find(ModuleProcessName{description->moduleLabel(), description->processName()}) != + consumedByChildren.end() or + consumedByChildren.find(ModuleProcessName{description->moduleLabel(), ""}) != + consumedByChildren.end()) { + consumedModules.insert(description->id()); } } diff --git a/FWCore/Framework/src/Schedule.cc b/FWCore/Framework/src/Schedule.cc index 2467f5aeaaea3..a1a1f8cc559f4 100644 --- a/FWCore/Framework/src/Schedule.cc +++ b/FWCore/Framework/src/Schedule.cc @@ -1478,16 +1478,19 @@ namespace edm { std::vector>& moduleIDToIndex, std::vector>& modulesWhoseProductsAreConsumedByEvent, std::vector>& modulesWhoseProductsAreConsumedByLumiRun, + std::vector>& modulesInPreviousProcessesWhoseProductsAreConsumedBy, ProductRegistry const& preg) const { allModuleDescriptions.clear(); moduleIDToIndex.clear(); modulesWhoseProductsAreConsumedByEvent.clear(); modulesWhoseProductsAreConsumedByLumiRun.clear(); + modulesInPreviousProcessesWhoseProductsAreConsumedBy.clear(); allModuleDescriptions.reserve(allWorkers().size()); moduleIDToIndex.reserve(allWorkers().size()); modulesWhoseProductsAreConsumedByEvent.resize(allWorkers().size()); modulesWhoseProductsAreConsumedByLumiRun.resize(allWorkers().size()); + modulesInPreviousProcessesWhoseProductsAreConsumedBy.resize(allWorkers().size()); std::map labelToDesc; unsigned int i = 0; @@ -1504,8 +1507,11 @@ namespace edm { for (auto const& worker : allWorkers()) { std::vector& modulesEvent = modulesWhoseProductsAreConsumedByEvent.at(i); std::vector& modulesLumiRun = modulesWhoseProductsAreConsumedByLumiRun.at(i); + std::set& modulesInPreviousProcesses = + modulesInPreviousProcessesWhoseProductsAreConsumedBy.at(i); try { - worker->modulesWhoseProductsAreConsumed(modulesEvent, modulesLumiRun, preg, labelToDesc); + worker->modulesWhoseProductsAreConsumed( + modulesEvent, modulesLumiRun, modulesInPreviousProcesses, preg, labelToDesc); } catch (cms::Exception& ex) { ex.addContext("Calling Worker::modulesWhoseProductsAreConsumed() for module " + worker->description()->moduleLabel()); diff --git a/FWCore/Framework/src/SubProcess.cc b/FWCore/Framework/src/SubProcess.cc index a15d9d6f57326..cd3fbec4f027c 100644 --- a/FWCore/Framework/src/SubProcess.cc +++ b/FWCore/Framework/src/SubProcess.cc @@ -220,6 +220,49 @@ namespace edm { SubProcess::~SubProcess() {} + std::set SubProcess::keepOnlyConsumedUnscheduledModules() { + ServiceRegistry::Operate operate(serviceToken_); + schedule_->convertCurrentProcessAlias(processConfiguration_->processName()); + pathsAndConsumesOfModules_.initialize(schedule_.get(), preg_); + + // Note: all these may throw + checkForModuleDependencyCorrectness(pathsAndConsumesOfModules_, false); + + // Consumes information from the child SubProcesses + std::set consumedByChildren; + for_all(subProcesses_, [&consumedByChildren](auto& subProcess) { + auto c = subProcess.keepOnlyConsumedUnscheduledModules(); + consumedByChildren.insert(c.begin(), c.end()); + }); + + // Non-consumed unscheduled modules in this SubProcess, take into account of the consumes from child SubProcesses + if (auto const unusedModules = nonConsumedUnscheduledModules(pathsAndConsumesOfModules_, consumedByChildren); + not unusedModules.empty()) { + pathsAndConsumesOfModules_.removeModules(unusedModules); + + edm::LogWarning("DeleteModules").log([&unusedModules, this](auto& l) { + l << "Following modules are not in any Path or EndPath, nor is their output consumed by any other module, and " + "therefore they are deleted from SubProcess " + << processConfiguration_->processName() << " before beginJob transition."; + for (auto const& description : unusedModules) { + l << "\n " << description->moduleLabel(); + } + }); + for (auto const& description : unusedModules) { + schedule_->deleteModule(description->moduleLabel()); + } + } + + // Products possibly consumed from the parent (Sub)Process + for (auto const& description : pathsAndConsumesOfModules_.allModules()) { + for (auto const& dep : + pathsAndConsumesOfModules_.modulesInPreviousProcessesWhoseProductsAreConsumedBy(description->id())) { + consumedByChildren.emplace(dep.moduleLabel(), dep.processName()); + } + } + return consumedByChildren; + } + void SubProcess::doBeginJob() { this->beginJob(); } void SubProcess::doEndJob() { endJob(); } @@ -235,10 +278,6 @@ namespace edm { fixBranchIDListsForEDAliases(droppedBranchIDToKeptBranchID()); } ServiceRegistry::Operate operate(serviceToken_); - schedule_->convertCurrentProcessAlias(processConfiguration_->processName()); - pathsAndConsumesOfModules_.initialize(schedule_.get(), preg_); - //NOTE: this may throw - checkForModuleDependencyCorrectness(pathsAndConsumesOfModules_, false); actReg_->preBeginJobSignal_(pathsAndConsumesOfModules_, processContext_); schedule_->beginJob(*preg_, esp_->recordsToProxyIndices()); for_all(subProcesses_, [](auto& subProcess) { subProcess.doBeginJob(); }); diff --git a/FWCore/Framework/src/SubProcess.h b/FWCore/Framework/src/SubProcess.h index a4b1236e60152..2d61e8c1c416d 100644 --- a/FWCore/Framework/src/SubProcess.h +++ b/FWCore/Framework/src/SubProcess.h @@ -76,6 +76,10 @@ namespace edm { SelectedProductsForBranchType const& keptProducts() const { return keptProducts_; } + // Returns the set of modules whose products may be consumed by + // modules in this SubProcess or its child SubProcesses + std::set keepOnlyConsumedUnscheduledModules(); + void doBeginJob(); void doEndJob(); diff --git a/FWCore/Framework/src/Worker.h b/FWCore/Framework/src/Worker.h index aa006606f5202..df823842d586a 100644 --- a/FWCore/Framework/src/Worker.h +++ b/FWCore/Framework/src/Worker.h @@ -70,6 +70,7 @@ namespace edm { class EventPrincipal; class EventSetupImpl; class EarlyDeleteHelper; + class ModuleProcessName; class ProductResolverIndexHelper; class ProductResolverIndexAndSkipBit; class StreamID; @@ -213,6 +214,7 @@ namespace edm { virtual void modulesWhoseProductsAreConsumed( std::vector& modulesEvent, std::vector& modulesLumiRun, + std::set& modulesInPreviousProcesses, ProductRegistry const& preg, std::map const& labelsToDesc) const = 0; diff --git a/FWCore/Framework/src/WorkerT.h b/FWCore/Framework/src/WorkerT.h index dacef33b551d2..a70cd2b40b767 100644 --- a/FWCore/Framework/src/WorkerT.h +++ b/FWCore/Framework/src/WorkerT.h @@ -22,6 +22,7 @@ WorkerT: Code common to all workers. namespace edm { class ModuleCallingContext; + class ModuleProcessName; class ProductResolverIndexAndSkipBit; class ThinnedAssociationsHelper; class WaitingTaskWithArenaHolder; @@ -111,10 +112,15 @@ namespace edm { void modulesWhoseProductsAreConsumed( std::vector& modulesEvent, std::vector& modulesLumiRun, + std::set& modulesInPreviousProcesses, ProductRegistry const& preg, std::map const& labelsToDesc) const override { - module_->modulesWhoseProductsAreConsumed( - modulesEvent, modulesLumiRun, preg, labelsToDesc, module_->moduleDescription().processName()); + module_->modulesWhoseProductsAreConsumed(modulesEvent, + modulesLumiRun, + modulesInPreviousProcesses, + preg, + labelsToDesc, + module_->moduleDescription().processName()); } void convertCurrentProcessAlias(std::string const& processName) override { diff --git a/FWCore/Framework/src/stream/EDAnalyzerAdaptorBase.cc b/FWCore/Framework/src/stream/EDAnalyzerAdaptorBase.cc index 95fb8eb4388e8..84f64db561b99 100644 --- a/FWCore/Framework/src/stream/EDAnalyzerAdaptorBase.cc +++ b/FWCore/Framework/src/stream/EDAnalyzerAdaptorBase.cc @@ -124,12 +124,13 @@ const edm::EDConsumerBase* EDAnalyzerAdaptorBase::consumer() const { return m_st void EDAnalyzerAdaptorBase::modulesWhoseProductsAreConsumed( std::vector& modulesEvent, std::vector& modulesLumiRun, + std::set& modulesInPreviousProcesses, ProductRegistry const& preg, std::map const& labelsToDesc, std::string const& processName) const { assert(not m_streamModules.empty()); return m_streamModules[0]->modulesWhoseProductsAreConsumed( - modulesEvent, modulesLumiRun, preg, labelsToDesc, processName); + modulesEvent, modulesLumiRun, modulesInPreviousProcesses, preg, labelsToDesc, processName); } void EDAnalyzerAdaptorBase::convertCurrentProcessAlias(std::string const& processName) { diff --git a/FWCore/Framework/src/stream/ProducingModuleAdaptorBase.cc b/FWCore/Framework/src/stream/ProducingModuleAdaptorBase.cc index 87296f745d0f0..aae8b899c6d37 100644 --- a/FWCore/Framework/src/stream/ProducingModuleAdaptorBase.cc +++ b/FWCore/Framework/src/stream/ProducingModuleAdaptorBase.cc @@ -120,12 +120,13 @@ namespace edm { void ProducingModuleAdaptorBase::modulesWhoseProductsAreConsumed( std::vector& modulesEvent, std::vector& modulesLumiRun, + std::set& modulesInPreviousProcesses, ProductRegistry const& preg, std::map const& labelsToDesc, std::string const& processName) const { assert(not m_streamModules.empty()); return m_streamModules[0]->modulesWhoseProductsAreConsumed( - modulesEvent, modulesLumiRun, preg, labelsToDesc, processName); + modulesEvent, modulesLumiRun, modulesInPreviousProcesses, preg, labelsToDesc, processName); } template diff --git a/FWCore/Framework/test/run_module_delete_tests.sh b/FWCore/Framework/test/run_module_delete_tests.sh index ddd0070119290..f2eb01e565382 100755 --- a/FWCore/Framework/test/run_module_delete_tests.sh +++ b/FWCore/Framework/test/run_module_delete_tests.sh @@ -7,3 +7,5 @@ TEST_DIR=src/FWCore/Framework/test cmsRun $TEST_DIR/test_module_delete_cfg.py || die "module deletion test failed" $? echo "module deletion test succeeded" +cmsRun $TEST_DIR/test_module_delete_subprocess_cfg.py || die "module deletion test with subprocess failed" $? +echo "module deletion test with subprocess succeeded" diff --git a/FWCore/Framework/test/stubs/ToyAnalyzers.cc b/FWCore/Framework/test/stubs/ToyAnalyzers.cc index e54aeabe3cdca..1c76f1ae22494 100644 --- a/FWCore/Framework/test/stubs/ToyAnalyzers.cc +++ b/FWCore/Framework/test/stubs/ToyAnalyzers.cc @@ -105,24 +105,24 @@ namespace edmtest { //-------------------------------------------------------------------- // - class GenericIntsAnalyzer : public edm::global::EDAnalyzer<> { + template + class GenericAnalyzerT : public edm::global::EDAnalyzer<> { public: - GenericIntsAnalyzer(edm::ParameterSet const& iPSet) - : tokensBeginRun_(edm::vector_transform( - iPSet.getUntrackedParameter>("srcBeginRun"), - [this](edm::InputTag const& tag) { return this->consumes(tag); })), - tokensBeginLumi_(edm::vector_transform( - iPSet.getUntrackedParameter>("srcBeginLumi"), - [this](edm::InputTag const& tag) { return this->consumes(tag); })), - tokensEvent_( - edm::vector_transform(iPSet.getUntrackedParameter>("srcEvent"), - [this](edm::InputTag const& tag) { return this->consumes(tag); })), - tokensEndLumi_(edm::vector_transform( - iPSet.getUntrackedParameter>("srcEndLumi"), - [this](edm::InputTag const& tag) { return this->consumes(tag); })), - tokensEndRun_(edm::vector_transform( - iPSet.getUntrackedParameter>("srcEndRun"), - [this](edm::InputTag const& tag) { return this->consumes(tag); })), + GenericAnalyzerT(edm::ParameterSet const& iPSet) + : tokensBeginRun_( + edm::vector_transform(iPSet.getUntrackedParameter>("srcBeginRun"), + [this](edm::InputTag const& tag) { return this->consumes(tag); })), + tokensBeginLumi_( + edm::vector_transform(iPSet.getUntrackedParameter>("srcBeginLumi"), + [this](edm::InputTag const& tag) { return this->consumes(tag); })), + tokensEvent_(edm::vector_transform(iPSet.getUntrackedParameter>("srcEvent"), + [this](edm::InputTag const& tag) { return this->consumes(tag); })), + tokensEndLumi_( + edm::vector_transform(iPSet.getUntrackedParameter>("srcEndLumi"), + [this](edm::InputTag const& tag) { return this->consumes(tag); })), + tokensEndRun_( + edm::vector_transform(iPSet.getUntrackedParameter>("srcEndRun"), + [this](edm::InputTag const& tag) { return this->consumes(tag); })), shouldExist_(iPSet.getUntrackedParameter("inputShouldExist")), shouldBeMissing_(iPSet.getUntrackedParameter("inputShouldBeMissing")) { if (shouldExist_ and shouldBeMissing_) { @@ -161,14 +161,16 @@ namespace edmtest { } private: - const std::vector> tokensBeginRun_; - const std::vector> tokensBeginLumi_; - const std::vector> tokensEvent_; - const std::vector> tokensEndLumi_; - const std::vector> tokensEndRun_; + const std::vector> tokensBeginRun_; + const std::vector> tokensBeginLumi_; + const std::vector> tokensEvent_; + const std::vector> tokensEndLumi_; + const std::vector> tokensEndRun_; const bool shouldExist_; const bool shouldBeMissing_; }; + using GenericIntsAnalyzer = GenericAnalyzerT; + using GenericUInt64Analyzer = GenericAnalyzerT; //-------------------------------------------------------------------- // @@ -396,6 +398,7 @@ DEFINE_FWK_MODULE(NonAnalyzer); DEFINE_FWK_MODULE(IntTestAnalyzer); DEFINE_FWK_MODULE(MultipleIntsAnalyzer); DEFINE_FWK_MODULE(edmtest::GenericIntsAnalyzer); +DEFINE_FWK_MODULE(edmtest::GenericUInt64Analyzer); DEFINE_FWK_MODULE(IntConsumingAnalyzer); DEFINE_FWK_MODULE(edmtest::IntFromRunConsumingAnalyzer); DEFINE_FWK_MODULE(ConsumingStreamAnalyzer); diff --git a/FWCore/Framework/test/stubs/ToyIntProducers.cc b/FWCore/Framework/test/stubs/ToyIntProducers.cc index 6898d1443a03a..aa568acd426a7 100644 --- a/FWCore/Framework/test/stubs/ToyIntProducers.cc +++ b/FWCore/Framework/test/stubs/ToyIntProducers.cc @@ -744,21 +744,48 @@ namespace edmtest { class MustRunIntProducer : public edm::global::EDProducer<> { public: explicit MustRunIntProducer(edm::ParameterSet const& p) - : token_{produces()}, value_(p.getParameter("ivalue")) {} + : moduleLabel_{p.getParameter("@module_label")}, + token_{produces()}, + value_(p.getParameter("ivalue")), + produce_{p.getParameter("produce")}, + mustRunEvent_{p.getParameter("mustRunEvent")} {} + ~MustRunIntProducer() { + if (not wasRunEndJob_) { + throw cms::Exception("NotRun") << "This module (" << moduleLabel_ + << ") should have run for endJob transition, but it did not"; + } + } + static void fillDescriptions(edm::ConfigurationDescriptions& descriptions) { + edm::ParameterSetDescription desc; + desc.add("ivalue"); + desc.add("produce", true); + desc.add("mustRunEvent", true) + ->setComment( + "If set to false, the endJob() is still required to be called to check that the module was not deleted " + "early on"); + descriptions.addDefault(desc); + } void produce(edm::StreamID, edm::Event& e, edm::EventSetup const& c) const override { - wasRun_ = true; - e.emplace(token_, value_); + wasRunEvent_ = true; + if (produce_) { + e.emplace(token_, value_); + } } void endJob() override { - if (not wasRun_) { + wasRunEndJob_ = true; + if (mustRunEvent_ and not wasRunEvent_) { throw cms::Exception("NotRun") << "This module should have run for event transitions, but it did not"; } } private: + const std::string moduleLabel_; const edm::EDPutTokenT token_; const int value_; - mutable std::atomic wasRun_ = false; + const bool produce_; + const bool mustRunEvent_; + mutable std::atomic wasRunEndJob_ = false; + mutable std::atomic wasRunEvent_ = false; }; } // namespace edmtest diff --git a/FWCore/Framework/test/test_module_delete_subprocess_cfg.py b/FWCore/Framework/test/test_module_delete_subprocess_cfg.py new file mode 100644 index 0000000000000..e087e4b19aae0 --- /dev/null +++ b/FWCore/Framework/test/test_module_delete_subprocess_cfg.py @@ -0,0 +1,443 @@ +import FWCore.ParameterSet.Config as cms + +process = cms.Process("A") + +process.maxEvents.input = 8 +process.source = cms.Source("EmptySource") + +#process.Tracer = cms.Service("Tracer") + +# Process tree +# A (Process) +# ^ +# \- B +# | ^ +# | \- BA +# | +# \- C +# | +# \- D +# ^ +# \- DA + +# Cases to test +# - event/lumi/run product consumed in a B or C, module kept +# - event/lumi/run product consumed in BA, module kept +# - event/lumi/run product not consumed anywhere, module deleted +# - event(/lumi/run) product produced in A and any SubProcess, consumed with empty process name, module kept +# - event(/lumi/run) product produced in A and any SubProcess, consumed with SubProcess name, A module deleted +# - event(/lumi/run) product produced in A and any SubProcess, consumed with A name or skipProcess, SubProcess module deleted +# - event(/lumi/run) product produced in B and consumed in BA, module kept +# - event(/lumi/run) product produced in B and consumed in C, module deleted (and product not found) +# - event(/lumi/run) product producer in A and dropped for SubProcess, module deleted + +intEventProducer = cms.EDProducer("IntProducer", ivalue = cms.int32(1)) +intNonEventProducer = cms.EDProducer("NonEventIntProducer", ivalue = cms.int32(1)) +intEventProducerMustRun = cms.EDProducer("edmtest::MustRunIntProducer", ivalue = cms.int32(1), mustRunEvent = cms.bool(True)) +intEventConsumer = cms.EDAnalyzer("IntTestAnalyzer", + moduleLabel = cms.untracked.InputTag("producerEventConsumed"), + valueMustMatch = cms.untracked.int32(1) +) +intGenericConsumer = cms.EDAnalyzer("edmtest::GenericIntsAnalyzer", + srcEvent = cms.untracked.VInputTag(), + inputShouldExist = cms.untracked.bool(True) +) +uint64GenericConsumer = cms.EDAnalyzer("edmtest::GenericUInt64Analyzer", + srcEvent = cms.untracked.VInputTag(), + inputShouldExist = cms.untracked.bool(True) +) + +def nonEventConsumer(transition, sourcePattern, expected): + transCap = transition[0].upper() + transition[1:] + runOrLumiBlock = transCap + if "Lumi" in runOrLumiBlock: + runOrLumiBlock = runOrLumiBlock+"nosityBlock" + ret = intNonEventProducer.clone() + setattr(ret, "consumes%s"%runOrLumiBlock, cms.InputTag(sourcePattern%transCap, transition)) + setattr(ret, "expect%s"%runOrLumiBlock, cms.untracked.int32(expected)) + return ret + +process.producerAEventConsumedInB = intEventProducer.clone(ivalue = 1) +process.producerABeginLumiConsumedInB = intNonEventProducer.clone(ivalue = 2) +process.producerAEndRunConsumedInB = intNonEventProducer.clone(ivalue = 5) +process.producerAEndLumiConsumedInC = intNonEventProducer.clone(ivalue = 3) +process.producerABeginRunConsumedInC = intNonEventProducer.clone(ivalue = 4) + +process.producerAEventConsumedInBA = intEventProducer.clone(ivalue = 10) +process.producerABeginLumiConsumedInBA = intNonEventProducer.clone(ivalue = 20) +process.producerAEndLumiConsumedInBA = intNonEventProducer.clone(ivalue = 30) +process.producerABeginRunConsumedInBA = intNonEventProducer.clone(ivalue = 40) +process.producerAEndRunConsumedInBA = intNonEventProducer.clone(ivalue = 50) + +process.producerEventNotConsumed = cms.EDProducer("edmtest::TestModuleDeleteProducer") +process.producerBeginLumiNotConsumed = cms.EDProducer("edmtest::TestModuleDeleteInLumiProducer") +process.producerBeginRunNotConsumed = cms.EDProducer("edmtest::TestModuleDeleteInRunProducer") + +# These producers do not get the event transitions for the events +# where the same-name producers in the SubProcesses produce a product. +# Nevertheless, these producers must not be deleted early, because +# their event transitions might get called. +process.producerEventMaybeConsumedInB = intEventProducerMustRun.clone(mustRunEvent=False) +process.producerEventMaybeConsumedInBA = intEventProducerMustRun.clone(mustRunEvent=False) +process.producerEventMaybeConsumedInC = intEventProducerMustRun.clone(mustRunEvent=False) + +process.producerAEventNotConsumedInB = cms.EDProducer("edmtest::TestModuleDeleteProducer") +process.producerAEventNotConsumedInBA = cms.EDProducer("edmtest::TestModuleDeleteProducer") +process.producerAEventNotConsumedInC = cms.EDProducer("edmtest::TestModuleDeleteProducer") +process.producerAEventNotConsumedInD = cms.EDProducer("edmtest::TestModuleDeleteProducer") + +process.producerEventConsumedInB1 = intEventProducerMustRun.clone() +process.producerEventConsumedInB2 = intEventProducerMustRun.clone() +process.producerEventConsumedInBA1 = intEventProducerMustRun.clone() +process.producerEventConsumedInBA2 = intEventProducerMustRun.clone() +process.producerEventConsumedInC1 = intEventProducerMustRun.clone() +process.producerEventConsumedInC2 = intEventProducerMustRun.clone() + +process.intAnalyzerDelete = cms.EDAnalyzer("edmtest::TestModuleDeleteAnalyzer") + +process.t = cms.Task( + process.producerAEventConsumedInB, + process.producerABeginLumiConsumedInB, + process.producerAEndRunConsumedInB, + process.producerAEndLumiConsumedInC, + process.producerABeginRunConsumedInC, + # + process.producerAEventConsumedInBA, + process.producerABeginLumiConsumedInBA, + process.producerAEndLumiConsumedInBA, + process.producerABeginRunConsumedInBA, + process.producerAEndRunConsumedInBA, + # + process.producerEventNotConsumed, + process.producerBeginLumiNotConsumed, + process.producerBeginRunNotConsumed, + # + process.producerEventMaybeConsumedInB, + process.producerEventMaybeConsumedInBA, + process.producerEventMaybeConsumedInC, + # + process.producerAEventNotConsumedInB, + process.producerAEventNotConsumedInBA, + process.producerAEventNotConsumedInC, + # + process.producerEventConsumedInB1, + process.producerEventConsumedInB2, + process.producerEventConsumedInBA1, + process.producerEventConsumedInBA2, + process.producerEventConsumedInC1, + process.producerEventConsumedInC2, +) + +process.p = cms.Path( + process.intAnalyzerDelete + , + process.t +) + +#################### +subprocessB = cms.Process("B") +process.addSubProcess( cms.SubProcess( + process = subprocessB, + SelectEvents = cms.untracked.PSet(), + outputCommands = cms.untracked.vstring() +) ) + +subprocessB.consumerEventFromA = intEventConsumer.clone(moduleLabel = "producerAEventConsumedInB", valueMustMatch = 1) +subprocessB.consumerBeginLumiFromA = nonEventConsumer("beginLumi", "producerA%sConsumedInB", 2) +subprocessB.consumerEndRunFromA = nonEventConsumer("endRun", "producerA%sConsumedInB", 5) + +subprocessB.consumerAEventNotConsumed = intGenericConsumer.clone( + srcEvent = [ + "producerEventNotConsumed:doesNotExist", + "producerEventNotConsumed:doesNotExist:A", + cms.InputTag("producerEventNotConsumed", "doesNotExist", cms.InputTag.skipCurrentProcess()) + ], + inputShouldExist = False, +) +subprocessB.consumerAEventNotConsumed2 = uint64GenericConsumer.clone( + srcEvent = [ + "producerEventNotConsumed", + "producerEventNotConsumed::A", + cms.InputTag("producerEventNotConsumed", "", cms.InputTag.skipCurrentProcess()) + ], + inputShouldExist = False, +) +subprocessB.producerEventNotConsumed = cms.EDProducer("edmtest::TestModuleDeleteProducer") +subprocessB.producerBeginLumiNotConsumed = cms.EDProducer("edmtest::TestModuleDeleteInLumiProducer") +subprocessB.producerBeginRunNotConsumed = cms.EDProducer("edmtest::TestModuleDeleteInRunProducer") + +subprocessB.producerEventMaybeConsumedInB = intEventProducerMustRun.clone() +subprocessB.producerEventMaybeConsumedInBA = intEventProducerMustRun.clone(mustRunEvent=False) +subprocessB.consumerEventMaybeInB = intGenericConsumer.clone(srcEvent = ["producerEventMaybeConsumedInB"]) + +subprocessB.producerAEventNotConsumedInB = intEventProducerMustRun.clone() +subprocessB.producerAEventNotConsumedInBA = cms.EDProducer("edmtest::TestModuleDeleteProducer") +subprocessB.consumerAEventNotConsumedInB = intGenericConsumer.clone(srcEvent = ["producerAEventNotConsumedInB::B"]) + +subprocessB.producerEventConsumedInB1 = cms.EDProducer("edmtest::TestModuleDeleteProducer") +subprocessB.producerEventConsumedInB2 = cms.EDProducer("edmtest::TestModuleDeleteProducer") +subprocessB.consumerEventNotConsumedInB1 = intGenericConsumer.clone(srcEvent = ["producerEventConsumedInB1::A"]) +subprocessB.consumerEventNotConsumedInB2 = intGenericConsumer.clone(srcEvent = [cms.InputTag("producerEventConsumedInB2", "", cms.InputTag.skipCurrentProcess())]) +subprocessB.producerBEventConsumedInBA1 = intEventProducerMustRun.clone() +subprocessB.producerBEventConsumedInBA2 = intEventProducerMustRun.clone() + +subprocessB.producerBEventConsumedInC1 = cms.EDProducer("edmtest::TestModuleDeleteProducer") +subprocessB.producerBEventConsumedInC2 = cms.EDProducer("edmtest::TestModuleDeleteProducer") + +subprocessB.producerBEventConsumedInB1 = intEventProducerMustRun.clone() +subprocessB.producerBEventConsumedInB2 = intEventProducerMustRun.clone() +subprocessB.producerBEventConsumedInB3 = intEventProducerMustRun.clone() +subprocessB.consumerBEventConsumedInB1 = intGenericConsumer.clone(srcEvent = ["producerBEventConsumedInB1"]) +subprocessB.consumerBEventConsumedInB2 = intGenericConsumer.clone(srcEvent = ["producerBEventConsumedInB2::B"]) +subprocessB.consumerBEventConsumedInB3 = intGenericConsumer.clone(srcEvent = [cms.InputTag("producerBEventConsumedInB3", "", cms.InputTag.currentProcess())]) + +subprocessB.t = cms.Task( + subprocessB.producerEventNotConsumed, + subprocessB.producerBeginLumiNotConsumed, + subprocessB.producerBeginRunNotConsumed, + # + subprocessB.producerEventMaybeConsumedInB, + subprocessB.producerEventMaybeConsumedInBA, + # + subprocessB.producerAEventNotConsumedInB, + subprocessB.producerAEventNotConsumedInBA, + # + subprocessB.producerEventConsumedInB1, + subprocessB.producerEventConsumedInB2, + subprocessB.producerBEventConsumedInBA1, + subprocessB.producerBEventConsumedInBA2, + # + subprocessB.producerBEventConsumedInC1, + subprocessB.producerBEventConsumedInC2, + # + subprocessB.producerBEventConsumedInB1, + subprocessB.producerBEventConsumedInB2, + subprocessB.producerBEventConsumedInB3, +) +subprocessB.p = cms.Path( + subprocessB.consumerEventFromA+ + subprocessB.consumerBeginLumiFromA+ + subprocessB.consumerEndRunFromA+ + # + subprocessB.consumerAEventNotConsumed+ + subprocessB.consumerAEventNotConsumed2+ + # + subprocessB.consumerEventMaybeInB+ + # + subprocessB.consumerAEventNotConsumedInB+ + subprocessB.consumerEventNotConsumedInB1+ + subprocessB.consumerEventNotConsumedInB2+ + # + subprocessB.consumerBEventConsumedInB1+ + subprocessB.consumerBEventConsumedInB2+ + subprocessB.consumerBEventConsumedInB3 + ,subprocessB.t +) + +#################### +subprocessBA = cms.Process("BA") +subprocessB.addSubProcess( cms.SubProcess( + process = subprocessBA, + SelectEvents = cms.untracked.PSet(), + outputCommands = cms.untracked.vstring() +) ) + +subprocessBA.consumerEventFromA = intEventConsumer.clone(moduleLabel = "producerAEventConsumedInBA", valueMustMatch = 10) +subprocessBA.consumerBeginLumiFromA = nonEventConsumer("beginLumi", "producerA%sConsumedInBA", 20) +subprocessBA.consumerEndLumiFromA = nonEventConsumer("endLumi", "producerA%sConsumedInBA", 30) +subprocessBA.consumerBeginRunFromA = nonEventConsumer("beginRun", "producerA%sConsumedInBA", 40) +subprocessBA.consumerEndRunFromA = nonEventConsumer("endRun", "producerA%sConsumedInBA", 50) + +subprocessBA.consumerABEventNotConsumed = intGenericConsumer.clone( + srcEvent = [ + "producerEventNotConsumed:doesNotExist", + "producerEventNotConsumed:doesNotExist:A", + "producerEventNotConsumed:doesNotExist:B", + cms.InputTag("producerEventNotConsumed", "doesNotExist", cms.InputTag.skipCurrentProcess()) + ], + inputShouldExist = False, +) +subprocessBA.consumerABEventNotConsumed2 = uint64GenericConsumer.clone( + srcEvent = [ + "producerEventNotConsumed", + "producerEventNotConsumed::A", + "producerEventNotConsumed::B", + cms.InputTag("producerEventNotConsumed", "", cms.InputTag.skipCurrentProcess()) + ], + inputShouldExist = False, +) + +subprocessBA.producerEventMaybeConsumedInBA = intEventProducerMustRun.clone() +subprocessBA.consumerEventMaybeInBA = intGenericConsumer.clone(srcEvent = ["producerEventMaybeConsumedInBA"]) + +subprocessBA.producerAEventNotConsumedInBA = intEventProducerMustRun.clone() +subprocessBA.consumerAEventNotConsumedInBA = intGenericConsumer.clone(srcEvent = ["producerAEventNotConsumedInBA::BA"]) + +subprocessBA.producerEventConsumedInBA1 = cms.EDProducer("edmtest::TestModuleDeleteProducer") +subprocessBA.producerEventConsumedInBA2 = cms.EDProducer("edmtest::TestModuleDeleteProducer") +subprocessBA.producerBEventConsumedInBA1 = cms.EDProducer("edmtest::TestModuleDeleteProducer") +subprocessBA.producerBEventConsumedInBA2 = cms.EDProducer("edmtest::TestModuleDeleteProducer") +subprocessBA.consumerEventNotConsumedInBA1 = intGenericConsumer.clone(srcEvent = ["producerEventConsumedInBA1::A", + "producerBEventConsumedInBA1::B"]) +subprocessBA.consumerEventNotConsumedInBA2 = intGenericConsumer.clone(srcEvent = [ + cms.InputTag("producerEventConsumedInBA2", "", cms.InputTag.skipCurrentProcess()), + cms.InputTag("producerBEventConsumedInBA2", "", cms.InputTag.skipCurrentProcess()) +]) + +subprocessBA.t = cms.Task( + subprocessBA.producerEventMaybeConsumedInBA, + # + subprocessBA.producerAEventNotConsumedInBA, + # + subprocessBA.producerEventConsumedInBA1, + subprocessBA.producerEventConsumedInBA2, + subprocessBA.producerBEventConsumedInBA1, + subprocessBA.producerBEventConsumedInBA2, +) +subprocessBA.p = cms.Path( + subprocessBA.consumerEventFromA+ + subprocessBA.consumerBeginLumiFromA+ + subprocessBA.consumerEndLumiFromA+ + subprocessBA.consumerBeginRunFromA+ + subprocessBA.consumerEndRunFromA+ + # + subprocessBA.consumerABEventNotConsumed+ + subprocessBA.consumerABEventNotConsumed2+ + # + subprocessBA.consumerEventMaybeInBA+ + # + subprocessBA.consumerAEventNotConsumedInBA+ + # + subprocessBA.consumerEventNotConsumedInBA1+ + subprocessBA.consumerEventNotConsumedInBA2 + , subprocessBA.t +) + +#################### +subprocessC = cms.Process("C") +process.addSubProcess( cms.SubProcess( + process = subprocessC, + SelectEvents = cms.untracked.PSet(), + outputCommands = cms.untracked.vstring() +) ) + +subprocessC.consumerEndLumiFromA = nonEventConsumer("endLumi", "producerA%sConsumedInC", 3) +subprocessC.consumerBeginRunFromA = nonEventConsumer("beginRun", "producerA%sConsumedInC", 4) + +subprocessC.consumerAEventNotConsumed = intGenericConsumer.clone( + srcEvent = [ + "producerEventNotConsumed:doesNotExist", + "producerEventNotConsumed:doesNotExist:A", + cms.InputTag("producerEventNotConsumed", "doesNotExist", cms.InputTag.skipCurrentProcess()) + ], + inputShouldExist = False, +) +subprocessC.consumerAEventNotConsumed2 = uint64GenericConsumer.clone( + srcEvent = [ + "producerEventNotConsumed", + "producerEventNotConsumed::A", + cms.InputTag("producerEventNotConsumed", "", cms.InputTag.skipCurrentProcess()) + ], + inputShouldExist = False, +) + +subprocessC.producerEventMaybeConsumedInC = intEventProducerMustRun.clone() +subprocessC.consumerEventMaybeInC = intGenericConsumer.clone(srcEvent = ["producerEventMaybeConsumedInC"]) + +subprocessC.producerAEventNotConsumedInC = intEventProducerMustRun.clone() +subprocessC.consumerAEventNotConsumedInC = intGenericConsumer.clone(srcEvent = ["producerAEventNotConsumedInC::C"]) + +subprocessC.producerEventConsumedInC1 = cms.EDProducer("edmtest::TestModuleDeleteProducer") +subprocessC.producerEventConsumedInC2 = cms.EDProducer("edmtest::TestModuleDeleteProducer") +subprocessC.consumerEventNotConsumedInC1 = intGenericConsumer.clone(srcEvent = ["producerEventConsumedInC1::A"]) +subprocessC.consumerEventNotConsumedInC2 = intGenericConsumer.clone(srcEvent = [cms.InputTag("producerEventConsumedInC2", "", cms.InputTag.skipCurrentProcess())]) + +subprocessC.consumerBEventConsumedInC1 = intGenericConsumer.clone(srcEvent = ["producerBEventConsumedInC1"], inputShouldExist=False) +subprocessC.consumerBEventConsumedInC2 = intGenericConsumer.clone(srcEvent = ["producerBEventConsumedInC1::B"], inputShouldExist=False) + +subprocessC.t = cms.Task( + subprocessC.producerEventMaybeConsumedInC, + # + subprocessC.producerAEventNotConsumedInC, + # + subprocessC.producerEventConsumedInC1, + subprocessC.producerEventConsumedInC2, +) +subprocessC.p = cms.Path( + subprocessC.consumerEndLumiFromA+ + subprocessC.consumerBeginRunFromA+ + # + subprocessC.consumerAEventNotConsumed+ + subprocessC.consumerAEventNotConsumed2+ + # + subprocessC.consumerEventMaybeInC+ + subprocessC.consumerAEventNotConsumedInC+ + # + subprocessC.consumerEventNotConsumedInC1+ + subprocessC.consumerEventNotConsumedInC2+ + # + subprocessC.consumerBEventConsumedInC1+ + subprocessC.consumerBEventConsumedInC2 + , subprocessC.t +) + +#################### +subprocessD = cms.Process("D") +process.addSubProcess( cms.SubProcess( + process = subprocessD, + SelectEvents = cms.untracked.PSet(), + outputCommands = cms.untracked.vstring( + "drop *_producerAEventNotConsumedInD_*_*", + ) +) ) + +subprocessD.consumerAEventNotConsumedInD = intGenericConsumer.clone( + srcEvent = [ + "producerAEvenNotConsumedInD", + "producerAEvenNotConsumedInD::A", + cms.InputTag("producerEventANotConsumedInD", "", cms.InputTag.skipCurrentProcess()) + ], + inputShouldExist = False, +) + +subprocessD.producerDEventNotConsumedInDA = cms.EDProducer("edmtest::TestModuleDeleteProducer") + +subprocessD.t = cms.Task( + subprocessD.producerDEventNotConsumedInDA +) +subprocessD.p = cms.Path( + subprocessD.consumerAEventNotConsumedInD, + subprocessD.t +) + +#################### +subprocessDA = cms.Process("BA") +subprocessD.addSubProcess( cms.SubProcess( + process = subprocessDA, + SelectEvents = cms.untracked.PSet(), + outputCommands = cms.untracked.vstring( + "drop *_producerDEventNotConsumedInDA_*_*", + ) +) ) + +subprocessDA.consumerAEventNotConsumedInD = intGenericConsumer.clone( + srcEvent = [ + "producerAEvenNotConsumedInD", + "producerAEvenNotConsumedInD::A", + cms.InputTag("producerEventANotConsumedInD", "", cms.InputTag.skipCurrentProcess()) + ], + inputShouldExist = False, +) +subprocessDA.consumerDEventNotConsumedInDA = intGenericConsumer.clone( + srcEvent = [ + "producerDEvenNotConsumedInDA", + "producerDEvenNotConsumedInDA::D", + cms.InputTag("producerEventDNotConsumedInDA", "", cms.InputTag.skipCurrentProcess()) + ], + inputShouldExist = False, +) + +subprocessDA.p = cms.Path( + subprocessDA.consumerAEventNotConsumedInD+ + subprocessDA.consumerDEventNotConsumedInDA +) From 3795ad1c1d1d0dd24f1bac41367d841c1313b133 Mon Sep 17 00:00:00 2001 From: Matti Kortelainen Date: Fri, 17 Apr 2020 21:06:21 +0200 Subject: [PATCH 04/26] Keep Accumulator modules running by adding data dependencies in global/stream/limited/one tests --- .../Framework/test/stubs/TestGlobalAnalyzers.cc | 8 +++++++- .../Framework/test/stubs/TestGlobalProducers.cc | 10 ++++++++-- .../Framework/test/stubs/TestLimitedAnalyzers.cc | 8 +++++++- .../Framework/test/stubs/TestLimitedProducers.cc | 12 +++++++++--- FWCore/Framework/test/stubs/TestOneAnalyzers.cc | 8 +++++++- FWCore/Framework/test/stubs/TestOneProducers.cc | 10 ++++++++-- .../Framework/test/stubs/TestStreamAnalyzers.cc | 5 +++++ .../Framework/test/stubs/TestStreamProducers.cc | 15 ++++++++++++++- FWCore/Framework/test/test_global_modules_cfg.py | 3 ++- FWCore/Framework/test/test_limited_modules_cfg.py | 2 ++ FWCore/Framework/test/test_one_modules_cfg.py | 2 ++ FWCore/Framework/test/test_stream_modules_cfg.py | 2 ++ 12 files changed, 73 insertions(+), 12 deletions(-) diff --git a/FWCore/Framework/test/stubs/TestGlobalAnalyzers.cc b/FWCore/Framework/test/stubs/TestGlobalAnalyzers.cc index 0c8a75e357997..06594397a930f 100644 --- a/FWCore/Framework/test/stubs/TestGlobalAnalyzers.cc +++ b/FWCore/Framework/test/stubs/TestGlobalAnalyzers.cc @@ -158,7 +158,13 @@ namespace edmtest { class LumiIntAnalyzer : public edm::global::EDAnalyzer> { public: explicit LumiIntAnalyzer(edm::ParameterSet const& p) - : trans_(p.getParameter("transitions")), cvalue_(p.getParameter("cachevalue")) {} + : trans_(p.getParameter("transitions")), cvalue_(p.getParameter("cachevalue")) { + // just to create a data dependence + auto const& tag = p.getParameter("moduleLabel"); + if (not tag.label().empty()) { + consumes(tag); + } + } const unsigned int trans_; const unsigned int cvalue_; mutable std::atomic m_count{0}; diff --git a/FWCore/Framework/test/stubs/TestGlobalProducers.cc b/FWCore/Framework/test/stubs/TestGlobalProducers.cc index 6b43b60808274..03fd019d42b83 100644 --- a/FWCore/Framework/test/stubs/TestGlobalProducers.cc +++ b/FWCore/Framework/test/stubs/TestGlobalProducers.cc @@ -786,13 +786,18 @@ namespace edmtest { } }; - class TestAccumulator : public edm::global::EDProducer { + class TestAccumulator : public edm::global::EDProducer { public: explicit TestAccumulator(edm::ParameterSet const& p) - : m_expectedCount(p.getParameter("expectedCount")) {} + : m_expectedCount(p.getParameter("expectedCount")), + m_putToken(produces()) {} void accumulate(edm::StreamID iID, edm::Event const&, edm::EventSetup const&) const override { ++m_count; } + void globalEndLuminosityBlockProduce(edm::LuminosityBlock& l, edm::EventSetup const&) const override { + l.emplace(m_putToken, m_count.load()); + } + ~TestAccumulator() { if (m_count.load() != m_expectedCount) { throw cms::Exception("TestCount") @@ -802,6 +807,7 @@ namespace edmtest { mutable std::atomic m_count{0}; const unsigned int m_expectedCount; + const edm::EDPutTokenT m_putToken; }; } // namespace global } // namespace edmtest diff --git a/FWCore/Framework/test/stubs/TestLimitedAnalyzers.cc b/FWCore/Framework/test/stubs/TestLimitedAnalyzers.cc index 0cdf8b14af93b..b702c93acc500 100644 --- a/FWCore/Framework/test/stubs/TestLimitedAnalyzers.cc +++ b/FWCore/Framework/test/stubs/TestLimitedAnalyzers.cc @@ -167,7 +167,13 @@ namespace edmtest { : edm::limited::EDAnalyzerBase(p), edm::limited::EDAnalyzer>(p), trans_(p.getParameter("transitions")), - cvalue_(p.getParameter("cachevalue")) {} + cvalue_(p.getParameter("cachevalue")) { + // just to create a data dependence + auto const& tag = p.getParameter("moduleLabel"); + if (not tag.label().empty()) { + consumes(tag); + } + } const unsigned int trans_; const unsigned int cvalue_; mutable std::atomic m_count{0}; diff --git a/FWCore/Framework/test/stubs/TestLimitedProducers.cc b/FWCore/Framework/test/stubs/TestLimitedProducers.cc index 916fe13cd3439..28d968715b86e 100644 --- a/FWCore/Framework/test/stubs/TestLimitedProducers.cc +++ b/FWCore/Framework/test/stubs/TestLimitedProducers.cc @@ -823,15 +823,20 @@ namespace edmtest { } }; - class TestAccumulator : public edm::limited::EDProducer { + class TestAccumulator : public edm::limited::EDProducer { public: explicit TestAccumulator(edm::ParameterSet const& p) : edm::limited::EDProducerBase(p), - edm::limited::EDProducer(p), - m_expectedCount(p.getParameter("expectedCount")) {} + edm::limited::EDProducer(p), + m_expectedCount(p.getParameter("expectedCount")), + m_putToken(produces()) {} void accumulate(edm::StreamID iID, edm::Event const&, edm::EventSetup const&) const override { ++m_count; } + void globalEndLuminosityBlockProduce(edm::LuminosityBlock& l, edm::EventSetup const&) const override { + l.emplace(m_putToken, m_count.load()); + } + ~TestAccumulator() { if (m_count.load() != m_expectedCount) { throw cms::Exception("TestCount") @@ -841,6 +846,7 @@ namespace edmtest { mutable std::atomic m_count{0}; const unsigned int m_expectedCount; + const edm::EDPutTokenT m_putToken; }; } // namespace limited } // namespace edmtest diff --git a/FWCore/Framework/test/stubs/TestOneAnalyzers.cc b/FWCore/Framework/test/stubs/TestOneAnalyzers.cc index 94043349a3e3c..29674a6749e4d 100644 --- a/FWCore/Framework/test/stubs/TestOneAnalyzers.cc +++ b/FWCore/Framework/test/stubs/TestOneAnalyzers.cc @@ -93,7 +93,13 @@ namespace edmtest { class WatchLumiBlocksAnalyzer : public edm::one::EDAnalyzer { public: - explicit WatchLumiBlocksAnalyzer(edm::ParameterSet const& p) : trans_(p.getParameter("transitions")) {} + explicit WatchLumiBlocksAnalyzer(edm::ParameterSet const& p) : trans_(p.getParameter("transitions")) { + // just to create a data dependence + auto const& tag = p.getParameter("moduleLabel"); + if (not tag.label().empty()) { + consumes(tag); + } + } const unsigned int trans_; bool bl = false; bool el = false; diff --git a/FWCore/Framework/test/stubs/TestOneProducers.cc b/FWCore/Framework/test/stubs/TestOneProducers.cc index 95c349048a54d..eef2ab1bf3f35 100644 --- a/FWCore/Framework/test/stubs/TestOneProducers.cc +++ b/FWCore/Framework/test/stubs/TestOneProducers.cc @@ -550,13 +550,18 @@ namespace edmtest { } }; - class TestAccumulator : public edm::one::EDProducer { + class TestAccumulator : public edm::one::EDProducer { public: explicit TestAccumulator(edm::ParameterSet const& p) - : m_expectedCount(p.getParameter("expectedCount")) {} + : m_expectedCount(p.getParameter("expectedCount")), + m_putToken(produces()) {} void accumulate(edm::Event const&, edm::EventSetup const&) override { ++m_count; } + void endLuminosityBlockProduce(edm::LuminosityBlock& l, edm::EventSetup const&) override { + l.emplace(m_putToken, m_count.load()); + } + ~TestAccumulator() { if (m_count.load() != m_expectedCount) { throw cms::Exception("TestCount") @@ -566,6 +571,7 @@ namespace edmtest { mutable std::atomic m_count{0}; const unsigned int m_expectedCount; + const edm::EDPutTokenT m_putToken; }; } // namespace one diff --git a/FWCore/Framework/test/stubs/TestStreamAnalyzers.cc b/FWCore/Framework/test/stubs/TestStreamAnalyzers.cc index d2bbbccfe247b..e13523b8bdbfd 100644 --- a/FWCore/Framework/test/stubs/TestStreamAnalyzers.cc +++ b/FWCore/Framework/test/stubs/TestStreamAnalyzers.cc @@ -177,6 +177,11 @@ namespace edmtest { trans_ = p.getParameter("transitions"); cvalue_ = p.getParameter("cachevalue"); m_count = 0; + // just to create a data dependence + auto const& tag = p.getParameter("moduleLabel"); + if (not tag.label().empty()) { + consumes(tag); + } } void analyze(edm::Event const&, edm::EventSetup const&) override { diff --git a/FWCore/Framework/test/stubs/TestStreamProducers.cc b/FWCore/Framework/test/stubs/TestStreamProducers.cc index 5c7f79d3069e2..8b97fd5c3fb58 100644 --- a/FWCore/Framework/test/stubs/TestStreamProducers.cc +++ b/FWCore/Framework/test/stubs/TestStreamProducers.cc @@ -921,14 +921,20 @@ namespace edmtest { //Using mutable since we want to update the value. mutable std::atomic m_value; mutable std::atomic m_expectedValue; + // Set only in constructor, framework runs constructors serially + CMS_THREAD_SAFE mutable edm::EDPutTokenT m_putToken; }; - class TestAccumulator : public edm::stream::EDProducer, edm::Accumulator> { + class TestAccumulator + : public edm::stream::EDProducer, edm::Accumulator, edm::EndLuminosityBlockProducer> { public: static std::atomic m_expectedCount; explicit TestAccumulator(edm::ParameterSet const& p, Count const* iCount) { iCount->m_expectedValue = p.getParameter("expectedCount"); + if (iCount->m_putToken.isUninitialized()) { + iCount->m_putToken = produces(); + } } static std::unique_ptr initializeGlobalCache(edm::ParameterSet const&) { @@ -937,6 +943,13 @@ namespace edmtest { void accumulate(edm::Event const&, edm::EventSetup const&) override { ++(globalCache()->m_value); } + static void globalEndLuminosityBlockProduce(edm::LuminosityBlock& l, + edm::EventSetup const&, + LuminosityBlockContext const* ctx) { + Count const* count = ctx->global(); + l.emplace(count->m_putToken, count->m_value.load()); + } + static void globalEndJob(Count const* iCount) { if (iCount->m_value != iCount->m_expectedValue) { throw cms::Exception("CountEvents") << "Number of events seen = " << iCount->m_value diff --git a/FWCore/Framework/test/test_global_modules_cfg.py b/FWCore/Framework/test/test_global_modules_cfg.py index 5a177d5deb58e..a3c13335b8e1f 100644 --- a/FWCore/Framework/test/test_global_modules_cfg.py +++ b/FWCore/Framework/test/test_global_modules_cfg.py @@ -15,7 +15,6 @@ numberOfThreads = cms.untracked.uint32(nStreams) ) - process.maxEvents = cms.untracked.PSet( input = cms.untracked.int32(nEvt) ) @@ -107,6 +106,8 @@ process.LumiIntAn = cms.EDAnalyzer("edmtest::global::LumiIntAnalyzer", transitions = cms.int32(int(nEvt+2*(nEvt/nEvtLumi))) ,cachevalue = cms.int32(nEvtLumi) + # needed to avoid deleting TestAccumulator1 + ,moduleLabel = cms.InputTag("TestAccumulator1") ) process.RunSumIntAn = cms.EDAnalyzer("edmtest::global::RunSummaryIntAnalyzer", diff --git a/FWCore/Framework/test/test_limited_modules_cfg.py b/FWCore/Framework/test/test_limited_modules_cfg.py index 9a05413e706c5..c4bc68fbcb3ec 100644 --- a/FWCore/Framework/test/test_limited_modules_cfg.py +++ b/FWCore/Framework/test/test_limited_modules_cfg.py @@ -124,6 +124,8 @@ concurrencyLimit = cms.untracked.uint32(1), transitions = cms.int32(nEvt+2*int(nEvt/nEvtLumi)) ,cachevalue = cms.int32(nEvtLumi) + # needed to avoid deleting TestAccumulator1 + ,moduleLabel = cms.InputTag("TestAccumulator1") ) process.RunSumIntAn = cms.EDAnalyzer("edmtest::limited::RunSummaryIntAnalyzer", diff --git a/FWCore/Framework/test/test_one_modules_cfg.py b/FWCore/Framework/test/test_one_modules_cfg.py index 6ea86ce9f9ef3..098e56299e5e8 100644 --- a/FWCore/Framework/test/test_one_modules_cfg.py +++ b/FWCore/Framework/test/test_one_modules_cfg.py @@ -101,6 +101,8 @@ process.WatchLumiBlockAn = cms.EDAnalyzer("edmtest::one::WatchLumiBlocksAnalyzer", transitions = cms.int32(nEvt+2*int(nEvt/nEvtLumi)) + # needed to avoid deleting TestAccumulator1 + ,moduleLabel = cms.InputTag("TestAccumulator1") ) process.RunCacheAn = cms.EDAnalyzer("edmtest::one::RunCacheAnalyzer", diff --git a/FWCore/Framework/test/test_stream_modules_cfg.py b/FWCore/Framework/test/test_stream_modules_cfg.py index a85cc778468cb..d0bce0d9afe0e 100644 --- a/FWCore/Framework/test/test_stream_modules_cfg.py +++ b/FWCore/Framework/test/test_stream_modules_cfg.py @@ -117,6 +117,8 @@ process.LumiIntAn = cms.EDAnalyzer("edmtest::stream::LumiIntAnalyzer", transitions = cms.int32(nEvt+2*int(nEvt/nEvtLumi)) ,cachevalue = cms.int32(nEvtLumi) + # needed to avoid deleting TestAccumulator1 + ,moduleLabel = cms.InputTag("TestAccumulator1") ) process.RunSumIntAn = cms.EDAnalyzer("edmtest::stream::RunSummaryIntAnalyzer", From 29d29dc10f965cd44fa2b9512c2acdd988c21e30 Mon Sep 17 00:00:00 2001 From: Matti Kortelainen Date: Sun, 19 Apr 2020 03:23:08 +0200 Subject: [PATCH 05/26] Add pre and post signals for early module deletion --- FWCore/Framework/interface/Schedule.h | 2 +- FWCore/Framework/src/EventProcessor.cc | 2 +- FWCore/Framework/src/ModuleRegistry.cc | 25 +++++++++++++++++-- FWCore/Framework/src/ModuleRegistry.h | 4 ++- FWCore/Framework/src/Schedule.cc | 4 +-- FWCore/Framework/src/SubProcess.cc | 2 +- .../interface/ActivityRegistry.h | 18 +++++++++++++ .../ServiceRegistry/src/ActivityRegistry.cc | 6 +++++ 8 files changed, 55 insertions(+), 8 deletions(-) diff --git a/FWCore/Framework/interface/Schedule.h b/FWCore/Framework/interface/Schedule.h index 5d552ecd38a78..cb104e69be082 100644 --- a/FWCore/Framework/interface/Schedule.h +++ b/FWCore/Framework/interface/Schedule.h @@ -281,7 +281,7 @@ namespace edm { eventsetup::ESRecordsToProxyIndices const&); /// Deletes module with label iLabel - void deleteModule(std::string const& 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/src/EventProcessor.cc b/FWCore/Framework/src/EventProcessor.cc index 6a7236057c616..30fa086a02575 100644 --- a/FWCore/Framework/src/EventProcessor.cc +++ b/FWCore/Framework/src/EventProcessor.cc @@ -567,7 +567,7 @@ namespace edm { } }); for (auto const& description : unusedModules) { - schedule_->deleteModule(description->moduleLabel()); + schedule_->deleteModule(description->moduleLabel(), actReg_.get()); } } diff --git a/FWCore/Framework/src/ModuleRegistry.cc b/FWCore/Framework/src/ModuleRegistry.cc index f37ca0ba74de6..c83762beb7640 100644 --- a/FWCore/Framework/src/ModuleRegistry.cc +++ b/FWCore/Framework/src/ModuleRegistry.cc @@ -50,13 +50,34 @@ namespace edm { return modItr->second.get(); } - void ModuleRegistry::deleteModule(std::string const& iModuleLabel /* TODO: signals */) { + void ModuleRegistry::deleteModule(std::string const& iModuleLabel, + signalslot::Signal& iPre, + signalslot::Signal& iPost) { auto modItr = labelToModule_.find(iModuleLabel); if (modItr == labelToModule_.end()) { throw cms::Exception("LogicError") << "Trying to delete module " << iModuleLabel << " but it does not exist in the ModuleRegistry. Please contact framework developers."; } - labelToModule_.erase(modItr); + // If iPost throws and exception, let it propagate + // If deletion throws an exception, capture it and call iPost before throwing an exception + // If iPost throws an exception, let it propagate + auto md = modItr->second->moduleDescription(); + iPre(modItr->second->moduleDescription()); + bool postCalled = false; + // exception is rethrown + CMS_SA_ALLOW try { + labelToModule_.erase(modItr); + // if exception then post will be called in the catch block + postCalled = true; + iPost(md); + } catch (...) { + if (not postCalled) { + // we're already handling exception, nothing we can do if iPost throws + CMS_SA_ALLOW try { iPost(md); } catch (...) { + } + } + throw; + } } } // namespace edm diff --git a/FWCore/Framework/src/ModuleRegistry.h b/FWCore/Framework/src/ModuleRegistry.h index d54d7272df723..4978ee6e2c6ee 100644 --- a/FWCore/Framework/src/ModuleRegistry.h +++ b/FWCore/Framework/src/ModuleRegistry.h @@ -48,7 +48,9 @@ namespace edm { edm::ParameterSet const& iPSet, edm::PreallocationConfiguration const&); - void deleteModule(std::string const& iModuleLabel /* TODO: signals */); + void deleteModule(std::string const& iModuleLabel, + signalslot::Signal& iPre, + signalslot::Signal& iPost); template void forAllModuleHolders(F iFunc) { diff --git a/FWCore/Framework/src/Schedule.cc b/FWCore/Framework/src/Schedule.cc index a1a1f8cc559f4..e81a6cedc1f5f 100644 --- a/FWCore/Framework/src/Schedule.cc +++ b/FWCore/Framework/src/Schedule.cc @@ -1422,12 +1422,12 @@ namespace edm { return true; } - void Schedule::deleteModule(std::string const& iLabel) { + void Schedule::deleteModule(std::string const& iLabel, ActivityRegistry* areg) { globalSchedule_->deleteModule(iLabel); for (auto& stream : streamSchedules_) { stream->deleteModule(iLabel); } - moduleRegistry_->deleteModule(iLabel); + moduleRegistry_->deleteModule(iLabel, areg->preModuleDestructionSignal_, areg->postModuleDestructionSignal_); } std::vector Schedule::getAllModuleDescriptions() const { diff --git a/FWCore/Framework/src/SubProcess.cc b/FWCore/Framework/src/SubProcess.cc index cd3fbec4f027c..8f899dea71c1e 100644 --- a/FWCore/Framework/src/SubProcess.cc +++ b/FWCore/Framework/src/SubProcess.cc @@ -249,7 +249,7 @@ namespace edm { } }); for (auto const& description : unusedModules) { - schedule_->deleteModule(description->moduleLabel()); + schedule_->deleteModule(description->moduleLabel(), actReg_.get()); } } diff --git a/FWCore/ServiceRegistry/interface/ActivityRegistry.h b/FWCore/ServiceRegistry/interface/ActivityRegistry.h index 8d689d7be6e85..b5327a4b34acd 100644 --- a/FWCore/ServiceRegistry/interface/ActivityRegistry.h +++ b/FWCore/ServiceRegistry/interface/ActivityRegistry.h @@ -678,6 +678,24 @@ namespace edm { // WARNING - ModuleDescription is not in fixed place. See note M above. AR_WATCH_USING_METHOD_1(watchPostModuleConstruction) + /// signal is emitted before the module is destructed, only for modules deleted before beginJob + typedef signalslot::Signal PreModuleDestruction; + PreModuleDestruction preModuleDestructionSignal_; + void watchPreModuleDestruction(PreModuleDestruction::slot_type const& iSlot) { + preModuleDestructionSignal_.connect(iSlot); + } + // note: ModuleDescription IS in the fixed place. See note M above. + AR_WATCH_USING_METHOD_1(watchPreModuleDestruction) + + /// signal is emitted after the module is destructed, only for modules deleted before beginJob + typedef signalslot::Signal PostModuleDestruction; + PostModuleDestruction postModuleDestructionSignal_; + void watchPostModuleDestruction(PostModuleDestruction::slot_type const& iSlot) { + postModuleDestructionSignal_.connect_front(iSlot); + } + // WARNING - ModuleDescription is not in fixed place. See note M above. + AR_WATCH_USING_METHOD_1(watchPostModuleDestruction) + /// signal is emitted before the module does beginJob typedef signalslot::Signal PreModuleBeginJob; PreModuleBeginJob preModuleBeginJobSignal_; diff --git a/FWCore/ServiceRegistry/src/ActivityRegistry.cc b/FWCore/ServiceRegistry/src/ActivityRegistry.cc index 0e3c764445afc..3aeb9e8d724f7 100644 --- a/FWCore/ServiceRegistry/src/ActivityRegistry.cc +++ b/FWCore/ServiceRegistry/src/ActivityRegistry.cc @@ -209,6 +209,9 @@ namespace edm { preModuleConstructionSignal_.connect(std::cref(iOther.preModuleConstructionSignal_)); postModuleConstructionSignal_.connect(std::cref(iOther.postModuleConstructionSignal_)); + preModuleDestructionSignal_.connect(std::cref(iOther.preModuleDestructionSignal_)); + postModuleDestructionSignal_.connect(std::cref(iOther.postModuleDestructionSignal_)); + preModuleBeginJobSignal_.connect(std::cref(iOther.preModuleBeginJobSignal_)); postModuleBeginJobSignal_.connect(std::cref(iOther.postModuleBeginJobSignal_)); @@ -414,6 +417,9 @@ namespace edm { copySlotsToFrom(preModuleConstructionSignal_, iOther.preModuleConstructionSignal_); copySlotsToFromReverse(postModuleConstructionSignal_, iOther.postModuleConstructionSignal_); + copySlotsToFrom(preModuleDestructionSignal_, iOther.preModuleDestructionSignal_); + copySlotsToFromReverse(postModuleDestructionSignal_, iOther.postModuleDestructionSignal_); + copySlotsToFrom(preModuleBeginJobSignal_, iOther.preModuleBeginJobSignal_); copySlotsToFromReverse(postModuleBeginJobSignal_, iOther.postModuleBeginJobSignal_); From e1c17dfa9edfb64e53fad12ee672e4895b508a9e Mon Sep 17 00:00:00 2001 From: Matti Kortelainen Date: Mon, 20 Apr 2020 01:34:52 +0200 Subject: [PATCH 06/26] Remove deleted modules from SystemTimeKeeper --- FWCore/Framework/src/Schedule.cc | 2 ++ FWCore/Framework/src/SystemTimeKeeper.cc | 20 ++++++++++++++++---- FWCore/Framework/src/SystemTimeKeeper.h | 2 ++ 3 files changed, 20 insertions(+), 4 deletions(-) diff --git a/FWCore/Framework/src/Schedule.cc b/FWCore/Framework/src/Schedule.cc index e81a6cedc1f5f..3d9db4d22b3ab 100644 --- a/FWCore/Framework/src/Schedule.cc +++ b/FWCore/Framework/src/Schedule.cc @@ -847,6 +847,8 @@ namespace edm { summaryTimeKeeper_ = std::make_unique(prealloc.numberOfStreams(), modDesc, tns, processContext); auto timeKeeperPtr = summaryTimeKeeper_.get(); + areg->watchPreModuleDestruction(timeKeeperPtr, &SystemTimeKeeper::removeModuleIfExists); + areg->watchPreModuleEvent(timeKeeperPtr, &SystemTimeKeeper::startModuleEvent); areg->watchPostModuleEvent(timeKeeperPtr, &SystemTimeKeeper::stopModuleEvent); areg->watchPreModuleEventAcquire(timeKeeperPtr, &SystemTimeKeeper::restartModuleEvent); diff --git a/FWCore/Framework/src/SystemTimeKeeper.cc b/FWCore/Framework/src/SystemTimeKeeper.cc index b33ce03198979..b50799f953658 100644 --- a/FWCore/Framework/src/SystemTimeKeeper.cc +++ b/FWCore/Framework/src/SystemTimeKeeper.cc @@ -27,6 +27,12 @@ #include "SystemTimeKeeper.h" using namespace edm; + +namespace { + bool lessModuleDescription(const ModuleDescription* iLHS, const ModuleDescription* iRHS) { + return iLHS->id() < iRHS->id(); + } +} // namespace // // constants, enums and typedefs // @@ -48,10 +54,7 @@ SystemTimeKeeper::SystemTimeKeeper(unsigned int iNumStreams, m_processContext(iProcessContext), m_minModuleID(0), m_numberOfEvents(0) { - std::sort( - m_modules.begin(), m_modules.end(), [](const ModuleDescription* iLHS, const ModuleDescription* iRHS) -> bool { - return iLHS->id() < iRHS->id(); - }); + std::sort(m_modules.begin(), m_modules.end(), lessModuleDescription); if (not m_modules.empty()) { m_minModuleID = m_modules.front()->id(); unsigned int numModuleSlots = m_modules.back()->id() - m_minModuleID + 1; @@ -103,6 +106,15 @@ SystemTimeKeeper::SystemTimeKeeper(unsigned int iNumStreams, // // member functions // +void SystemTimeKeeper::removeModuleIfExists(ModuleDescription const& module) { + // The deletion of a module is signaled to all (Sub)Processes, even + // though the module exists in only one of them. + auto found = std::lower_bound(m_modules.begin(), m_modules.end(), &module, lessModuleDescription); + if (*found == &module) { + m_modules.erase(found); + } +} + SystemTimeKeeper::PathTiming& SystemTimeKeeper::pathTiming(StreamContext const& iStream, PathContext const& iPath) { unsigned int offset = 0; if (iPath.isEndPath()) { diff --git a/FWCore/Framework/src/SystemTimeKeeper.h b/FWCore/Framework/src/SystemTimeKeeper.h index 2268ae64c4888..6361d72e728f1 100644 --- a/FWCore/Framework/src/SystemTimeKeeper.h +++ b/FWCore/Framework/src/SystemTimeKeeper.h @@ -58,6 +58,8 @@ namespace edm { // ---------- static member functions -------------------- // ---------- member functions --------------------------- + void removeModuleIfExists(ModuleDescription const& module); + void startProcessingLoop(); void stopProcessingLoop(); From 61dd9db02e8c8ae76127ceb3129a1805390db1b4 Mon Sep 17 00:00:00 2001 From: Matti Kortelainen Date: Mon, 20 Apr 2020 01:46:55 +0200 Subject: [PATCH 07/26] Keep modules in non-event ordering unit tests running by moving the last one in the chain to a Path Without a scheduled module consuming their output all modules would be deleted. --- FWCore/Framework/test/test_non_event_ordering_beginLumi_cfg.py | 3 ++- FWCore/Framework/test/test_non_event_ordering_beginRun_cfg.py | 3 ++- FWCore/Framework/test/test_non_event_ordering_endLumi_cfg.py | 3 ++- FWCore/Framework/test/test_non_event_ordering_endRun_cfg.py | 3 ++- 4 files changed, 8 insertions(+), 4 deletions(-) diff --git a/FWCore/Framework/test/test_non_event_ordering_beginLumi_cfg.py b/FWCore/Framework/test/test_non_event_ordering_beginLumi_cfg.py index 132af530c6c89..8f8df84c240bd 100644 --- a/FWCore/Framework/test/test_non_event_ordering_beginLumi_cfg.py +++ b/FWCore/Framework/test/test_non_event_ordering_beginLumi_cfg.py @@ -14,4 +14,5 @@ consumesBeginLuminosityBlock = cms.InputTag("a", "beginLumi"), expectBeginLuminosityBlock = cms.untracked.int32(1)) -process.schedule = cms.Schedule(tasks=cms.Task(process.a,process.b,process.c)) +process.t = cms.Task(process.a, process.c) +process.p = cms.Path(process.b, process.t) diff --git a/FWCore/Framework/test/test_non_event_ordering_beginRun_cfg.py b/FWCore/Framework/test/test_non_event_ordering_beginRun_cfg.py index c0a47616da351..a0e276fd42285 100644 --- a/FWCore/Framework/test/test_non_event_ordering_beginRun_cfg.py +++ b/FWCore/Framework/test/test_non_event_ordering_beginRun_cfg.py @@ -14,4 +14,5 @@ consumesBeginRun = cms.InputTag("a", "beginRun"), expectBeginRun = cms.untracked.int32(1)) -process.schedule = cms.Schedule(tasks=cms.Task(process.a,process.b,process.c)) +process.t = cms.Task(process.a, process.c) +process.p = cms.Path(process.b, process.t) diff --git a/FWCore/Framework/test/test_non_event_ordering_endLumi_cfg.py b/FWCore/Framework/test/test_non_event_ordering_endLumi_cfg.py index 51502288205fa..a7b6b50ce61f4 100644 --- a/FWCore/Framework/test/test_non_event_ordering_endLumi_cfg.py +++ b/FWCore/Framework/test/test_non_event_ordering_endLumi_cfg.py @@ -14,4 +14,5 @@ consumesEndLuminosityBlock = cms.InputTag("a", "endLumi"), expectEndLuminosityBlock = cms.untracked.int32(1)) -process.schedule = cms.Schedule(tasks=cms.Task(process.a,process.b,process.c)) +process.t = cms.Task(process.a, process.c) +process.p = cms.Path(process.b, process.t) diff --git a/FWCore/Framework/test/test_non_event_ordering_endRun_cfg.py b/FWCore/Framework/test/test_non_event_ordering_endRun_cfg.py index ed9c4b2482473..6bf298262e9e2 100644 --- a/FWCore/Framework/test/test_non_event_ordering_endRun_cfg.py +++ b/FWCore/Framework/test/test_non_event_ordering_endRun_cfg.py @@ -10,4 +10,5 @@ process.b = cms.EDProducer("NonEventIntProducer", ivalue = cms.int32(2), consumesEndRun = cms.InputTag("c","endRun"), expectEndRun = cms.untracked.int32(3) ) process.c = cms.EDProducer("NonEventIntProducer", ivalue = cms.int32(3), consumesEndRun = cms.InputTag("a", "endRun"), expectEndRun = cms.untracked.int32(1) ) -process.schedule = cms.Schedule(tasks=cms.Task(process.a,process.b,process.c)) +process.t = cms.Task(process.a, process.c) +process.p = cms.Path(process.b, process.t) From 5a41a17562d3f04065385dd0b436c74342bb075b Mon Sep 17 00:00:00 2001 From: Matti Kortelainen Date: Mon, 20 Apr 2020 01:52:00 +0200 Subject: [PATCH 08/26] Add comment about modules being deleted being ok in empty Paths tests --- FWCore/Framework/test/test_emptyEndPathWithTask_cfg.py | 1 + FWCore/Framework/test/test_emptyPathWithTask_cfg.py | 1 + 2 files changed, 2 insertions(+) diff --git a/FWCore/Framework/test/test_emptyEndPathWithTask_cfg.py b/FWCore/Framework/test/test_emptyEndPathWithTask_cfg.py index 5eb7ab27eea15..07c9cb1c314aa 100644 --- a/FWCore/Framework/test/test_emptyEndPathWithTask_cfg.py +++ b/FWCore/Framework/test/test_emptyEndPathWithTask_cfg.py @@ -7,6 +7,7 @@ input = cms.untracked.int32(3) ) +# note that these modules get deleted, but the module dependence check is made first process.intProducer1 = cms.EDProducer("IntProducer", ivalue = cms.int32(1)) process.intProducer2 = cms.EDProducer("AddIntsProducer", labels = cms.VInputTag("intProducer1")) diff --git a/FWCore/Framework/test/test_emptyPathWithTask_cfg.py b/FWCore/Framework/test/test_emptyPathWithTask_cfg.py index 35de48011f552..8827f99ce8834 100644 --- a/FWCore/Framework/test/test_emptyPathWithTask_cfg.py +++ b/FWCore/Framework/test/test_emptyPathWithTask_cfg.py @@ -7,6 +7,7 @@ input = cms.untracked.int32(3) ) +# note that these modules get deleted, but the module dependence check is made first process.intProducer1 = cms.EDProducer("IntProducer", ivalue = cms.int32(1)) process.intProducer2 = cms.EDProducer("AddIntsProducer", labels = cms.VInputTag("intProducer1")) From 1ab44495369c66e212d01cd04ca58a9e165976b0 Mon Sep 17 00:00:00 2001 From: Matti Kortelainen Date: Mon, 20 Apr 2020 01:54:25 +0200 Subject: [PATCH 09/26] Add data dependencies to modules that were running before in SubProcess unit test --- FWCore/Integration/test/testSubProcessUnscheduled_cfg.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/FWCore/Integration/test/testSubProcessUnscheduled_cfg.py b/FWCore/Integration/test/testSubProcessUnscheduled_cfg.py index fe686167f4852..40bc2fa33e9cd 100644 --- a/FWCore/Integration/test/testSubProcessUnscheduled_cfg.py +++ b/FWCore/Integration/test/testSubProcessUnscheduled_cfg.py @@ -17,6 +17,7 @@ # producer process.four = cms.EDProducer("BusyWaitIntProducer", ivalue = cms.int32(4), iterations=cms.uint32(10*1000)) +process.fourConsumer = cms.EDAnalyzer("MultipleIntsAnalyzer", getFromModules=cms.untracked.VInputTag("four")) # producer process.ten = cms.EDProducer("BusyWaitIntProducer", ivalue = cms.int32(10), iterations=cms.uint32(2*1000)) @@ -25,7 +26,7 @@ process.task = cms.Task(process.two, process.four, process.ten, process.adder) -process.path = cms.Path(process.task) +process.path = cms.Path(process.fourConsumer, process.task) subprocess = cms.Process("SUB") process.addSubProcess( cms.SubProcess( From 1c1a8a5fcd48e064af92087a3b9fa2bc0a155596 Mon Sep 17 00:00:00 2001 From: Matti Kortelainen Date: Mon, 20 Apr 2020 02:00:23 +0200 Subject: [PATCH 10/26] Find entire sub-DAGs of non-consumed modules and delete them --- .../src/PathsAndConsumesOfModules.cc | 70 ++++++++++++------ .../Framework/test/run_module_delete_tests.sh | 2 + .../Framework/test/stubs/TestModuleDelete.cc | 48 ++++++++++++ .../Framework/test/test_module_delete_cfg.py | 53 +++++++++++++ ..._module_delete_improperDependencies_cfg.py | 29 ++++++++ .../test/test_module_delete_subprocess_cfg.py | 74 ++++++++++++++++++- 6 files changed, 252 insertions(+), 24 deletions(-) create mode 100644 FWCore/Framework/test/test_module_delete_improperDependencies_cfg.py diff --git a/FWCore/Framework/src/PathsAndConsumesOfModules.cc b/FWCore/Framework/src/PathsAndConsumesOfModules.cc index fb46cff6186e2..d3fc0811cb801 100644 --- a/FWCore/Framework/src/PathsAndConsumesOfModules.cc +++ b/FWCore/Framework/src/PathsAndConsumesOfModules.cc @@ -138,7 +138,29 @@ namespace edm { } return iter->second; } +} // namespace edm + +namespace { + // helper function for nonConsumedUnscheduledModules, + void findAllConsumedModules(edm::PathsAndConsumesOfModulesBase const& iPnC, + edm::ModuleDescription const* module, + std::unordered_set& consumedModules) { + // If this node of the DAG has been processed already, no need to + // reprocess again + if (consumedModules.find(module->id()) != consumedModules.end()) { + return; + } + consumedModules.insert(module->id()); + for (auto const& c : iPnC.modulesWhoseProductsAreConsumedBy(module->id())) { + findAllConsumedModules(iPnC, c, consumedModules); + } + for (auto const& c : iPnC.modulesWhoseProductsAreConsumedByLumiRun(module->id())) { + findAllConsumedModules(iPnC, c, consumedModules); + } + } +} // namespace +namespace edm { std::vector nonConsumedUnscheduledModules(edm::PathsAndConsumesOfModulesBase const& iPnC, std::set& consumedByChildren) { const std::string kTriggerResults("TriggerResults"); @@ -147,10 +169,13 @@ namespace edm { const unsigned int kFirstEndPathIndex = pathNames.size(); pathNames.insert(pathNames.end(), iPnC.endPaths().begin(), iPnC.endPaths().end()); - std::unordered_set moduleIndicesInPaths; - - // Exctract all modules in Paths and EndPaths (need to be kept) to - // get the list of unscheduled modules + // The goal is to find modules that are not depended upon by + // scheduled modules. To do that, we identify all modules that are + // depended upon by scheduled modules, and do a set subtraction. + // + // First, denote all scheduled modules (i.e. in Paths and + // EndPaths) as "consumers". + std::vector consumerModules; for (unsigned int pathIndex = 0; pathIndex != pathNames.size(); ++pathIndex) { std::vector const* moduleDescriptions; if (pathIndex < kFirstEndPathIndex) { @@ -158,42 +183,41 @@ namespace edm { } else { moduleDescriptions = &(iPnC.modulesOnEndPath(pathIndex - kFirstEndPathIndex)); } - for (auto const& description : *moduleDescriptions) { - moduleIndicesInPaths.insert(description->id()); - } + std::copy(moduleDescriptions->begin(), moduleDescriptions->end(), std::back_inserter(consumerModules)); } - // Mark modules whose any output is consumed - // Mark TriggerResults and all Paths/EndPaths as consumed - // Mark anything possibly consumed by child SubProcesses + // Then add TriggerResults, and all Paths and EndPaths themselves + // to the set of "consumers" (even if they don't depend on any + // data products, they must not be deleted). Also add anything + // consumed by child SubProcesses to the set of "consumers". auto const& allModules = iPnC.allModules(); - std::unordered_set consumedModules; for (auto const& description : allModules) { - for (auto const& c : iPnC.modulesWhoseProductsAreConsumedBy(description->id())) { - consumedModules.insert(c->id()); - } - for (auto const& c : iPnC.modulesWhoseProductsAreConsumedByLumiRun(description->id())) { - consumedModules.insert(c->id()); - } if (description->moduleLabel() == kTriggerResults or std::find(pathNames.begin(), pathNames.end(), description->moduleLabel()) != pathNames.end()) { - consumedModules.insert(description->id()); + consumerModules.push_back(description); } else if (consumedByChildren.find(ModuleProcessName{description->moduleLabel(), description->processName()}) != consumedByChildren.end() or consumedByChildren.find(ModuleProcessName{description->moduleLabel(), ""}) != consumedByChildren.end()) { - consumedModules.insert(description->id()); + consumerModules.push_back(description); } } - // Keep modules that are in Paths/EndPaths or whose output is consumed + // Find modules that have any data dependence path to any module + // in consumerModules. + std::unordered_set consumedModules; + for (auto& description : consumerModules) { + findAllConsumedModules(iPnC, description, consumedModules); + } + + // All other modules will then be classified as non-consumed, even + // if they would have dependencies within them. std::vector unusedModules; std::copy_if(allModules.begin(), allModules.end(), std::back_inserter(unusedModules), - [&moduleIndicesInPaths, &consumedModules](ModuleDescription const* description) { - return moduleIndicesInPaths.find(description->id()) == moduleIndicesInPaths.end() and - consumedModules.find(description->id()) == consumedModules.end(); + [&consumedModules](ModuleDescription const* description) { + return consumedModules.find(description->id()) == consumedModules.end(); }); return unusedModules; } diff --git a/FWCore/Framework/test/run_module_delete_tests.sh b/FWCore/Framework/test/run_module_delete_tests.sh index f2eb01e565382..93a7c8dc11240 100755 --- a/FWCore/Framework/test/run_module_delete_tests.sh +++ b/FWCore/Framework/test/run_module_delete_tests.sh @@ -9,3 +9,5 @@ cmsRun $TEST_DIR/test_module_delete_cfg.py || die "module deletion test failed" echo "module deletion test succeeded" cmsRun $TEST_DIR/test_module_delete_subprocess_cfg.py || die "module deletion test with subprocess failed" $? echo "module deletion test with subprocess succeeded" +cmsRun $TEST_DIR/test_module_delete_improperDependencies_cfg.py && die "module deletion with improper module ordering test failed" $? +echo "module deletion test with improper module ordering succeeded" diff --git a/FWCore/Framework/test/stubs/TestModuleDelete.cc b/FWCore/Framework/test/stubs/TestModuleDelete.cc index 76e96af35e8e3..4a335058d25fc 100644 --- a/FWCore/Framework/test/stubs/TestModuleDelete.cc +++ b/FWCore/Framework/test/stubs/TestModuleDelete.cc @@ -29,10 +29,26 @@ namespace edmtest { public: explicit TestModuleDeleteProducer(edm::ParameterSet const& p) : moduleLabel_{p.getParameter("@module_label")} { + for (const auto& tag : p.getUntrackedParameter>("srcBeginRun")) { + consumes(tag); + } + for (const auto& tag : p.getUntrackedParameter>("srcBeginLumi")) { + consumes(tag); + } + for (const auto& tag : p.getUntrackedParameter>("srcEvent")) { + consumes(tag); + } produces(); g_constructed.push_back(moduleLabel_); } ~TestModuleDeleteProducer() override { g_destructed.push_back(moduleLabel_); } + static void fillDescriptions(edm::ConfigurationDescriptions& descriptions) { + edm::ParameterSetDescription desc; + desc.addUntracked>("srcBeginRun", std::vector{}); + desc.addUntracked>("srcBeginLumi", std::vector{}); + desc.addUntracked>("srcEvent", std::vector{}); + descriptions.addDefault(desc); + } void produce(edm::StreamID, edm::Event& e, edm::EventSetup const& c) const override { throw edm::Exception(edm::errors::NotFound) << "Intentional 'NotFound' exception for testing purposes\n"; } @@ -45,10 +61,26 @@ namespace edmtest { public: explicit TestModuleDeleteInLumiProducer(edm::ParameterSet const& p) : moduleLabel_{p.getParameter("@module_label")} { + for (const auto& tag : p.getUntrackedParameter>("srcBeginRun")) { + consumes(tag); + } + for (const auto& tag : p.getUntrackedParameter>("srcBeginLumi")) { + consumes(tag); + } + for (const auto& tag : p.getUntrackedParameter>("srcEvent")) { + consumes(tag); + } produces(); g_lumi_constructed.push_back(moduleLabel_); } ~TestModuleDeleteInLumiProducer() override { g_lumi_destructed.push_back(moduleLabel_); } + static void fillDescriptions(edm::ConfigurationDescriptions& descriptions) { + edm::ParameterSetDescription desc; + desc.addUntracked>("srcBeginRun", std::vector{}); + desc.addUntracked>("srcBeginLumi", std::vector{}); + desc.addUntracked>("srcEvent", std::vector{}); + descriptions.addDefault(desc); + } void globalBeginLuminosityBlockProduce(edm::LuminosityBlock&, edm::EventSetup const&) const override { throw edm::Exception(edm::errors::NotFound) << "Intentional 'NotFound' exception for testing purposes\n"; } @@ -64,10 +96,26 @@ namespace edmtest { public: explicit TestModuleDeleteInRunProducer(edm::ParameterSet const& p) : moduleLabel_{p.getParameter("@module_label")} { + for (const auto& tag : p.getUntrackedParameter>("srcBeginRun")) { + consumes(tag); + } + for (const auto& tag : p.getUntrackedParameter>("srcBeginLumi")) { + consumes(tag); + } + for (const auto& tag : p.getUntrackedParameter>("srcEvent")) { + consumes(tag); + } produces(); g_run_constructed.push_back(moduleLabel_); } ~TestModuleDeleteInRunProducer() override { g_run_destructed.push_back(moduleLabel_); } + static void fillDescriptions(edm::ConfigurationDescriptions& descriptions) { + edm::ParameterSetDescription desc; + desc.addUntracked>("srcBeginRun", std::vector{}); + desc.addUntracked>("srcBeginLumi", std::vector{}); + desc.addUntracked>("srcEvent", std::vector{}); + descriptions.addDefault(desc); + } void globalBeginRunProduce(edm::Run&, edm::EventSetup const&) const override { throw edm::Exception(edm::errors::NotFound) << "Intentional 'NotFound' exception for testing purposes\n"; } diff --git a/FWCore/Framework/test/test_module_delete_cfg.py b/FWCore/Framework/test/test_module_delete_cfg.py index ecfd4ab16ca6d..d88d287b920d4 100644 --- a/FWCore/Framework/test/test_module_delete_cfg.py +++ b/FWCore/Framework/test/test_module_delete_cfg.py @@ -13,6 +13,8 @@ # - event product not consumed but module in Path, module kept # - event/lumi/run product with the same module label but different instance name, module deleted # - event/lumi/run product with the same module label and instance name but with skipCurrentProcess, module deleted +# - DAG of event/lumi/run producers that are not consumed by an always-running module, whole DAG deleted +# - DAG of event(/lumi/run) producers that are partly consumed (kept) and partly non-consumed (delete) intEventProducer = cms.EDProducer("IntProducer", ivalue = cms.int32(1)) intNonEventProducer = cms.EDProducer("NonEventIntProducer", ivalue = cms.int32(1)) @@ -49,6 +51,38 @@ def nonEventConsumer(transition, sourcePattern, expected): process.producerBeginLumiNotConsumed = cms.EDProducer("edmtest::TestModuleDeleteInLumiProducer") process.producerBeginRunNotConsumed = cms.EDProducer("edmtest::TestModuleDeleteInRunProducer") +process.producerEventNotConsumedChain1 = cms.EDProducer("edmtest::TestModuleDeleteProducer") +process.producerEventNotConsumedChain2 = cms.EDProducer("edmtest::TestModuleDeleteProducer", + srcEvent = cms.untracked.VInputTag("producerEventNotConsumedChain1") +) +process.producerEventNotConsumedChain3 = cms.EDProducer("edmtest::TestModuleDeleteProducer", + srcEvent = cms.untracked.VInputTag("producerEventNotConsumedChain1") +) +process.producerEventNotConsumedChain4 = cms.EDProducer("edmtest::TestModuleDeleteProducer", + srcEvent = cms.untracked.VInputTag("producerEventNotConsumedChain2", "producerEventNotConsumedChain3") +) +process.producerEventNotConsumedChain5 = cms.EDProducer("edmtest::TestModuleDeleteInLumiProducer", + srcEvent = cms.untracked.VInputTag("producerEventNotConsumedChain1") +) +process.producerEventNotConsumedChain6 = cms.EDProducer("edmtest::TestModuleDeleteInRunProducer", + srcBeginLumi = cms.untracked.VInputTag("producerEventNotConsumedChain5") +) +process.producerEventNotConsumedChain7 = cms.EDProducer("edmtest::TestModuleDeleteInLumiProducer", + srcBeginRun = cms.untracked.VInputTag("producerEventNotConsumedChain6") +) +process.producerEventNotConsumedChain8 = cms.EDProducer("edmtest::TestModuleDeleteProducer", + srcBeginLumi = cms.untracked.VInputTag("producerEventNotConsumedChain7") +) + +process.producerEventPartiallyConsumedChain1 = intEventProducerMustRun.clone() +process.producerEventPartiallyConsumedChain2 = cms.EDProducer("AddIntsProducer", labels = cms.VInputTag("producerEventPartiallyConsumedChain1")) +process.producerEventPartiallyConsumedChain3 = cms.EDProducer("edmtest::TestModuleDeleteInLumiProducer", + srcEvent = cms.untracked.VInputTag("producerEventPartiallyConsumedChain1") +) +process.producerEventPartiallyConsumedChain4 = cms.EDProducer("edmtest::TestModuleDeleteInLumiProducer", + srcEvent = cms.untracked.VInputTag("producerEventPartiallyConsumedChain2", "producerEventPartiallyConsumedChain4") +) + process.consumerNotExist = cms.EDAnalyzer("edmtest::GenericIntsAnalyzer", inputShouldBeMissing = cms.untracked.bool(True), srcBeginRun = cms.untracked.VInputTag( @@ -73,19 +107,38 @@ def nonEventConsumer(transition, sourcePattern, expected): process.producerEndLumiConsumed, process.producerBeginRunConsumed, process.producerEndRunConsumed, + # process.producerEventNotConsumed, process.producerBeginLumiNotConsumed, process.producerBeginRunNotConsumed, + # + process.producerEventNotConsumedChain1, + process.producerEventNotConsumedChain2, + process.producerEventNotConsumedChain3, + process.producerEventNotConsumedChain4, + process.producerEventNotConsumedChain5, + process.producerEventNotConsumedChain6, + process.producerEventNotConsumedChain7, + process.producerEventNotConsumedChain8, + # + process.producerEventPartiallyConsumedChain1, + process.producerEventPartiallyConsumedChain3, + process.producerEventPartiallyConsumedChain4, ) process.p = cms.Path( process.producerEventNotConsumedInPath+ + # process.consumerEvent+ process.consumerBeginLumi+ process.consumerEndLumi+ process.consumerBeginRun+ process.consumerEndRun+ + # process.consumerNotExist+ + # + process.producerEventPartiallyConsumedChain2+ + # process.intAnalyzerDelete , process.t diff --git a/FWCore/Framework/test/test_module_delete_improperDependencies_cfg.py b/FWCore/Framework/test/test_module_delete_improperDependencies_cfg.py new file mode 100644 index 0000000000000..a627baa5414ac --- /dev/null +++ b/FWCore/Framework/test/test_module_delete_improperDependencies_cfg.py @@ -0,0 +1,29 @@ +import FWCore.ParameterSet.Config as cms + +process = cms.Process("TESTMODULEDELETE") + +process.maxEvents.input = 8 +process.source = cms.Source("EmptySource") + +process.producerEventNotConsumed1 = cms.EDProducer("edmtest::TestModuleDeleteProducer", + srcEvent = cms.untracked.VInputTag("producerEventNotConsumed2") +) +process.producerEventNotConsumed2 = cms.EDProducer("edmtest::TestModuleDeleteProducer", + srcEvent = cms.untracked.VInputTag("producerEventNotConsumed3") +) +process.producerEventNotConsumed3 = cms.EDProducer("edmtest::TestModuleDeleteProducer", + srcEvent = cms.untracked.VInputTag("producerEventNotConsumed1") +) +process.consumerEvent = cms.EDAnalyzer("edmtest::GenericIntsAnalyzer", + srcEvent = cms.untracked.VInputTag("producerEventNotConsumed3") +) + +process.t = cms.Task( + process.producerEventNotConsumed1, + process.producerEventNotConsumed2, + process.producerEventNotConsumed3, +) +process.p = cms.Path( + process.consumerEvent, + process.t +) diff --git a/FWCore/Framework/test/test_module_delete_subprocess_cfg.py b/FWCore/Framework/test/test_module_delete_subprocess_cfg.py index e087e4b19aae0..78486d89ee8de 100644 --- a/FWCore/Framework/test/test_module_delete_subprocess_cfg.py +++ b/FWCore/Framework/test/test_module_delete_subprocess_cfg.py @@ -93,6 +93,14 @@ def nonEventConsumer(transition, sourcePattern, expected): process.producerEventConsumedInC1 = intEventProducerMustRun.clone() process.producerEventConsumedInC2 = intEventProducerMustRun.clone() +process.producerANotConsumedChainEvent = cms.EDProducer("edmtest::TestModuleDeleteProducer") +process.producerANotConsumedChainLumi = cms.EDProducer("edmtest::TestModuleDeleteInLumiProducer", + srcEvent = cms.untracked.VInputTag("producerANotConsumedChainEvent") +) +process.producerANotConsumedChainRun = cms.EDProducer("edmtest::TestModuleDeleteInRunProducer", + srcEvent = cms.untracked.VInputTag("producerANotConsumedChainEvent") +) + process.intAnalyzerDelete = cms.EDAnalyzer("edmtest::TestModuleDeleteAnalyzer") process.t = cms.Task( @@ -126,6 +134,10 @@ def nonEventConsumer(transition, sourcePattern, expected): process.producerEventConsumedInBA2, process.producerEventConsumedInC1, process.producerEventConsumedInC2, + # + process.producerANotConsumedChainEvent, + process.producerANotConsumedChainLumi, + process.producerANotConsumedChainRun, ) process.p = cms.Path( @@ -191,6 +203,16 @@ def nonEventConsumer(transition, sourcePattern, expected): subprocessB.consumerBEventConsumedInB2 = intGenericConsumer.clone(srcEvent = ["producerBEventConsumedInB2::B"]) subprocessB.consumerBEventConsumedInB3 = intGenericConsumer.clone(srcEvent = [cms.InputTag("producerBEventConsumedInB3", "", cms.InputTag.currentProcess())]) +subprocessB.producerBNotConsumedChainEvent = cms.EDProducer("edmtest::TestModuleDeleteProducer", + srcBeginRun = cms.untracked.VInputTag("producerANotConsumedChainRun") +) +subprocessB.producerBNotConsumedChainLumi = cms.EDProducer("edmtest::TestModuleDeleteInLumiProducer", + srcEvent = cms.untracked.VInputTag("producerANotConsumedChainEvent") +) +subprocessB.producerBNotConsumedChainRun = cms.EDProducer("edmtest::TestModuleDeleteInRunProducer", + srcBeginLumi = cms.untracked.VInputTag("producerANotConsumedChainLumi") +) + subprocessB.t = cms.Task( subprocessB.producerEventNotConsumed, subprocessB.producerBeginLumiNotConsumed, @@ -213,6 +235,10 @@ def nonEventConsumer(transition, sourcePattern, expected): subprocessB.producerBEventConsumedInB1, subprocessB.producerBEventConsumedInB2, subprocessB.producerBEventConsumedInB3, + # + subprocessB.producerBNotConsumedChainEvent, + subprocessB.producerBNotConsumedChainLumi, + subprocessB.producerBNotConsumedChainRun, ) subprocessB.p = cms.Path( subprocessB.consumerEventFromA+ @@ -284,6 +310,22 @@ def nonEventConsumer(transition, sourcePattern, expected): cms.InputTag("producerBEventConsumedInBA2", "", cms.InputTag.skipCurrentProcess()) ]) +subprocessBA.producerBANotConsumedChainEvent = cms.EDProducer("edmtest::TestModuleDeleteProducer", + srcBeginLumi = cms.untracked.VInputTag("producerBNotConsumedChainLumi") +) +subprocessBA.producerBANotConsumedChainLumi = cms.EDProducer("edmtest::TestModuleDeleteInLumiProducer", + srcBeginRun = cms.untracked.VInputTag("producerBNotConsumedChainRun") +) +subprocessBA.producerBANotConsumedChainRun = cms.EDProducer("edmtest::TestModuleDeleteInRunProducer", + srcEvent = cms.untracked.VInputTag("producerBNotConsumedChainEvent") +) +subprocessBA.producerBANotConsumedChainEvent2 = cms.EDProducer("edmtest::TestModuleDeleteProducer", + srcBeginRun = cms.untracked.VInputTag("producerANotConsumedChainRun"), + srcBeginLumi = cms.untracked.VInputTag("producerANotConsumedChainLumi"), + srcEvent = cms.untracked.VInputTag("producerANotConsumedChainEvent"), +) + + subprocessBA.t = cms.Task( subprocessBA.producerEventMaybeConsumedInBA, # @@ -293,6 +335,11 @@ def nonEventConsumer(transition, sourcePattern, expected): subprocessBA.producerEventConsumedInBA2, subprocessBA.producerBEventConsumedInBA1, subprocessBA.producerBEventConsumedInBA2, + # + subprocessBA.producerBANotConsumedChainEvent, + subprocessBA.producerBANotConsumedChainLumi, + subprocessBA.producerBANotConsumedChainRun, + subprocessBA.producerBANotConsumedChainEvent2, ) subprocessBA.p = cms.Path( subprocessBA.consumerEventFromA+ @@ -355,6 +402,16 @@ def nonEventConsumer(transition, sourcePattern, expected): subprocessC.consumerBEventConsumedInC1 = intGenericConsumer.clone(srcEvent = ["producerBEventConsumedInC1"], inputShouldExist=False) subprocessC.consumerBEventConsumedInC2 = intGenericConsumer.clone(srcEvent = ["producerBEventConsumedInC1::B"], inputShouldExist=False) +subprocessC.producerCNotConsumedChainEvent = cms.EDProducer("edmtest::TestModuleDeleteProducer", + srcEvent = cms.untracked.VInputTag("producerANotConsumedChainEvent") +) +subprocessC.producerCNotConsumedChainLumi = cms.EDProducer("edmtest::TestModuleDeleteInLumiProducer", + srcBeginLumi = cms.untracked.VInputTag("producerANotConsumedChainLumi") +) +subprocessC.producerCNotConsumedChainRun = cms.EDProducer("edmtest::TestModuleDeleteInRunProducer", + srcBeginRun = cms.untracked.VInputTag("producerANotConsumedChainRun") +) + subprocessC.t = cms.Task( subprocessC.producerEventMaybeConsumedInC, # @@ -362,6 +419,10 @@ def nonEventConsumer(transition, sourcePattern, expected): # subprocessC.producerEventConsumedInC1, subprocessC.producerEventConsumedInC2, + # + subprocessC.producerCNotConsumedChainEvent, + subprocessC.producerCNotConsumedChainLumi, + subprocessC.producerCNotConsumedChainRun, ) subprocessC.p = cms.Path( subprocessC.consumerEndLumiFromA+ @@ -437,7 +498,18 @@ def nonEventConsumer(transition, sourcePattern, expected): inputShouldExist = False, ) +subprocessDA.producerDNotConsumedChainRun = cms.EDProducer("edmtest::TestModuleDeleteInRunProducer", + srcBeginRun = cms.untracked.VInputTag("producerANotConsumedChainRun"), + srcBeginLumi = cms.untracked.VInputTag("producerANotConsumedChainLumi"), + srcEvent = cms.untracked.VInputTag("producerANotConsumedChainEvent"), +) + +subprocessDA.t = cms.Task( + subprocessDA.producerDNotConsumedChainRun +) + subprocessDA.p = cms.Path( subprocessDA.consumerAEventNotConsumedInD+ - subprocessDA.consumerDEventNotConsumedInDA + subprocessDA.consumerDEventNotConsumedInDA, + subprocessDA.t ) From bf39115f57d5f5d0f52e9c6d2c3e9524f418b8b8 Mon Sep 17 00:00:00 2001 From: Matti Kortelainen Date: Wed, 22 Apr 2020 19:23:38 +0200 Subject: [PATCH 11/26] Watch {pre,post}ModuleDestruction signals in MessageLogger --- FWCore/MessageService/interface/MessageLogger.h | 3 +++ FWCore/MessageService/src/MessageLogger.cc | 14 +++++++++++--- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/FWCore/MessageService/interface/MessageLogger.h b/FWCore/MessageService/interface/MessageLogger.h index ff46741db63b2..371432953f2f6 100644 --- a/FWCore/MessageService/interface/MessageLogger.h +++ b/FWCore/MessageService/interface/MessageLogger.h @@ -73,6 +73,9 @@ namespace edm { void preModuleConstruction(ModuleDescription const&); void postModuleConstruction(ModuleDescription const&); + void preModuleDestruction(ModuleDescription const&); + void postModuleDestruction(ModuleDescription const&); + void preSourceConstruction(ModuleDescription const&); void postSourceConstruction(ModuleDescription const&); diff --git a/FWCore/MessageService/src/MessageLogger.cc b/FWCore/MessageService/src/MessageLogger.cc index 11839cbf8fcf1..bbac5c23f1077 100644 --- a/FWCore/MessageService/src/MessageLogger.cc +++ b/FWCore/MessageService/src/MessageLogger.cc @@ -283,6 +283,9 @@ namespace edm { iRegistry.watchPostSourceConstruction(this, &MessageLogger::postSourceConstruction); // change log 3 + iRegistry.watchPreModuleDestruction(this, &MessageLogger::preModuleDestruction); + iRegistry.watchPostModuleDestruction(this, &MessageLogger::postModuleDestruction); + iRegistry.watchPreModuleEvent(this, &MessageLogger::preModuleEvent); iRegistry.watchPostModuleEvent(this, &MessageLogger::postModuleEvent); @@ -543,12 +546,17 @@ namespace edm { } establishModule(desc, "@ctor"); // ChangeLog 16 } - void MessageLogger::postModuleConstruction( - const ModuleDescription& - iDescription) { //it is now guaranteed that this will be called even if the module throws + //it is now guaranteed that this will be called even if the module throws + void MessageLogger::postModuleConstruction(const ModuleDescription& iDescription) { unEstablishModule(iDescription, "AfterModConstruction"); } + void MessageLogger::preModuleDestruction(const ModuleDescription& desc) { establishModule(desc, "@dtor"); } + //it is guaranteed that this will be called even if the module throws + void MessageLogger::postModuleDestruction(const ModuleDescription& iDescription) { + unEstablishModule(iDescription, "AfterModDestruction"); + } + void MessageLogger::preModuleBeginJob(const ModuleDescription& desc) { establishModule(desc, "@beginJob"); // ChangeLog 13 } From 52799a9f34f7a8e938087de60064dc343223b1e4 Mon Sep 17 00:00:00 2001 From: Matti Kortelainen Date: Wed, 22 Apr 2020 19:34:18 +0200 Subject: [PATCH 12/26] Invalidate deleted modules in RandomNumberGeneratorService --- IOMC/RandomEngine/src/RandomNumberGeneratorService.cc | 8 ++++++++ IOMC/RandomEngine/src/RandomNumberGeneratorService.h | 5 ++++- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/IOMC/RandomEngine/src/RandomNumberGeneratorService.cc b/IOMC/RandomEngine/src/RandomNumberGeneratorService.cc index b9809ff3afad5..1ce292684ceb4 100644 --- a/IOMC/RandomEngine/src/RandomNumberGeneratorService.cc +++ b/IOMC/RandomEngine/src/RandomNumberGeneratorService.cc @@ -187,6 +187,7 @@ namespace edm { } } activityRegistry.watchPreModuleConstruction(this, &RandomNumberGeneratorService::preModuleConstruction); + activityRegistry.watchPreModuleDestruction(this, &RandomNumberGeneratorService::preModuleDestruction); activityRegistry.watchPreallocate(this, &RandomNumberGeneratorService::preallocate); @@ -382,6 +383,13 @@ namespace edm { } } + void RandomNumberGeneratorService::preModuleDestruction(ModuleDescription const& description) { + std::map::iterator iter = seedsAndNameMap_.find(description.moduleLabel()); + if (iter != seedsAndNameMap_.end()) { + iter->second.setModuleID(SeedsAndName::kInvalid); + } + } + void RandomNumberGeneratorService::preallocate(SystemBounds const& sb) { nStreams_ = sb.maxNumberOfStreams(); assert(nStreams_ >= 1); diff --git a/IOMC/RandomEngine/src/RandomNumberGeneratorService.h b/IOMC/RandomEngine/src/RandomNumberGeneratorService.h index 98de89c375be6..0139094a5e8bb 100644 --- a/IOMC/RandomEngine/src/RandomNumberGeneratorService.h +++ b/IOMC/RandomEngine/src/RandomNumberGeneratorService.h @@ -81,6 +81,7 @@ namespace edm { static void fillDescriptions(ConfigurationDescriptions& descriptions); void preModuleConstruction(ModuleDescription const& description); + void preModuleDestruction(ModuleDescription const& description); void preallocate(SystemBounds const&); void preBeginLumi(LuminosityBlock const& lumi) override; @@ -234,8 +235,10 @@ namespace edm { // It is left as max unsigned if the module is never constructed and not in the process class SeedsAndName { public: + static constexpr unsigned int kInvalid = std::numeric_limits::max(); + SeedsAndName(VUint32 const& theSeeds, std::string const& theEngineName) - : seeds_(theSeeds), engineName_(theEngineName), moduleID_(std::numeric_limits::max()) {} + : seeds_(theSeeds), engineName_(theEngineName), moduleID_(kInvalid) {} VUint32 const& seeds() const { return seeds_; } std::string const& engineName() const { return engineName_; } unsigned int moduleID() const { return moduleID_; } From 6ac7ac9f03e6043c350ca4eb2c818d12dc355695 Mon Sep 17 00:00:00 2001 From: Matti Kortelainen Date: Wed, 22 Apr 2020 19:46:02 +0200 Subject: [PATCH 13/26] Remove deleted modules from the excluded modules list in ConcurrentModuleTimer --- FWCore/Services/plugins/ConcurrentModuleTimer.cc | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/FWCore/Services/plugins/ConcurrentModuleTimer.cc b/FWCore/Services/plugins/ConcurrentModuleTimer.cc index 6588e6b6b5e72..aa4e168b9bcb4 100644 --- a/FWCore/Services/plugins/ConcurrentModuleTimer.cc +++ b/FWCore/Services/plugins/ConcurrentModuleTimer.cc @@ -86,6 +86,12 @@ ConcurrentModuleTimer::ConcurrentModuleTimer(edm::ParameterSet const& iConfig, e } } }); + iReg.watchPreModuleDestruction([this](ModuleDescription const& iMod) { + auto found = std::find(m_excludedModuleIds.begin(), m_excludedModuleIds.end(), iMod.id()); + if (found != m_excludedModuleIds.end()) { + m_excludedModuleIds.erase(found); + } + }); iReg.watchPreModuleEvent([this](StreamContext const&, ModuleCallingContext const& iContext) { if (trackModule(iContext)) { start(); From 381480e742b7f698557e983cd705a1ac13193ea7 Mon Sep 17 00:00:00 2001 From: Matti Kortelainen Date: Wed, 22 Apr 2020 19:59:09 +0200 Subject: [PATCH 14/26] Remove deleted modules in StallMonitor --- FWCore/Services/plugins/StallMonitor.cc | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/FWCore/Services/plugins/StallMonitor.cc b/FWCore/Services/plugins/StallMonitor.cc index 8cb7fa530aac3..a1ea21a03bc67 100644 --- a/FWCore/Services/plugins/StallMonitor.cc +++ b/FWCore/Services/plugins/StallMonitor.cc @@ -197,6 +197,7 @@ namespace edm { private: void preModuleConstruction(edm::ModuleDescription const&); + void preModuleDestruction(edm::ModuleDescription const&); void postBeginJob(); void preSourceEvent(StreamID); void postSourceEvent(StreamID); @@ -253,6 +254,7 @@ StallMonitor::StallMonitor(ParameterSet const& iPS, ActivityRegistry& iRegistry) stallThreshold_{ std::chrono::round(duration(iPS.getUntrackedParameter("stallThreshold")))} { iRegistry.watchPreModuleConstruction(this, &StallMonitor::preModuleConstruction); + iRegistry.watchPreModuleDestruction(this, &StallMonitor::preModuleDestruction); iRegistry.watchPostBeginJob(this, &StallMonitor::postBeginJob); iRegistry.watchPostModuleEventPrefetching(this, &StallMonitor::postModuleEventPrefetching); iRegistry.watchPreModuleEventAcquire(this, &StallMonitor::preModuleEventAcquire); @@ -373,6 +375,12 @@ void StallMonitor::preModuleConstruction(ModuleDescription const& md) { } } +void StallMonitor::preModuleDestruction(ModuleDescription const& md) { + // Reset the module label back if the module is deleted before + // beginJob() so that the entry is ignored in the summary printouts. + moduleLabels_[md.id()] = ""; +} + void StallMonitor::postBeginJob() { // Since a (push,emplace)_back cannot be called for a vector of a // type containing atomics (like 'StallStatistics')--i.e. atomics From add0c31bbca08dac778390ebe4d5c753bd251bca Mon Sep 17 00:00:00 2001 From: Matti Kortelainen Date: Wed, 22 Apr 2020 20:04:53 +0200 Subject: [PATCH 15/26] Watch {pre,post}ModuleDestruction signals in Timing --- FWCore/Services/plugins/Timing.cc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/FWCore/Services/plugins/Timing.cc b/FWCore/Services/plugins/Timing.cc index f459c1eb614e2..911ea28eec42f 100644 --- a/FWCore/Services/plugins/Timing.cc +++ b/FWCore/Services/plugins/Timing.cc @@ -263,6 +263,9 @@ namespace edm { iRegistry.watchPreModuleConstruction(this, &Timing::preModule); iRegistry.watchPostModuleConstruction(this, &Timing::postModule); + iRegistry.watchPreModuleDestruction(this, &Timing::preModule); + iRegistry.watchPostModuleDestruction(this, &Timing::postModule); + iRegistry.watchPreModuleBeginJob(this, &Timing::preModule); iRegistry.watchPostModuleBeginJob(this, &Timing::postModule); From a756679c7a1427a663e5bbd1c3941a85d9d00259 Mon Sep 17 00:00:00 2001 From: Matti Kortelainen Date: Thu, 23 Apr 2020 00:52:34 +0200 Subject: [PATCH 16/26] Watch {pre,post}ModuleDestruction signals in Tracer --- FWCore/Services/plugins/Tracer.cc | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/FWCore/Services/plugins/Tracer.cc b/FWCore/Services/plugins/Tracer.cc index 7d7c86ebd0fad..95b6bac3c54f4 100644 --- a/FWCore/Services/plugins/Tracer.cc +++ b/FWCore/Services/plugins/Tracer.cc @@ -147,6 +147,9 @@ namespace edm { void preModuleConstruction(ModuleDescription const& md); void postModuleConstruction(ModuleDescription const& md); + void preModuleDestruction(ModuleDescription const& md); + void postModuleDestruction(ModuleDescription const& md); + void preModuleBeginJob(ModuleDescription const& md); void postModuleBeginJob(ModuleDescription const& md); @@ -322,6 +325,9 @@ Tracer::Tracer(ParameterSet const& iPS, ActivityRegistry& iRegistry) iRegistry.watchPreModuleConstruction(this, &Tracer::preModuleConstruction); iRegistry.watchPostModuleConstruction(this, &Tracer::postModuleConstruction); + iRegistry.watchPreModuleDestruction(this, &Tracer::preModuleDestruction); + iRegistry.watchPostModuleDestruction(this, &Tracer::postModuleDestruction); + iRegistry.watchPreModuleBeginJob(this, &Tracer::preModuleBeginJob); iRegistry.watchPostModuleBeginJob(this, &Tracer::postModuleBeginJob); @@ -980,6 +986,26 @@ void Tracer::postModuleConstruction(ModuleDescription const& desc) { } } +void Tracer::preModuleDestruction(ModuleDescription const& desc) { + LogAbsolute out("Tracer"); + out << TimeStamper(printTimestamps_); + out << indention_ << indention_ << " starting: destructing module with label '" << desc.moduleLabel() + << "' id = " << desc.id(); + if (dumpContextForLabels_.find(desc.moduleLabel()) != dumpContextForLabels_.end()) { + out << "\n" << desc; + } +} + +void Tracer::postModuleDestruction(ModuleDescription const& desc) { + LogAbsolute out("Tracer"); + out << TimeStamper(printTimestamps_); + out << indention_ << indention_ << " finished: destructing module with label '" << desc.moduleLabel() + << "' id = " << desc.id(); + if (dumpContextForLabels_.find(desc.moduleLabel()) != dumpContextForLabels_.end()) { + out << "\n" << desc; + } +} + void Tracer::preModuleBeginJob(ModuleDescription const& desc) { LogAbsolute out("Tracer"); out << TimeStamper(printTimestamps_); From f26bf974a831d0e2d41cecfc619ee430e05e500f Mon Sep 17 00:00:00 2001 From: Matti Kortelainen Date: Thu, 23 Apr 2020 00:59:51 +0200 Subject: [PATCH 17/26] Watch {pre,post}ModuleDestruction signals in NVProfilerService --- .../CUDAServices/plugins/NVProfilerService.cc | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/HeterogeneousCore/CUDAServices/plugins/NVProfilerService.cc b/HeterogeneousCore/CUDAServices/plugins/NVProfilerService.cc index 65bb196ba0fa2..a1637305fc627 100644 --- a/HeterogeneousCore/CUDAServices/plugins/NVProfilerService.cc +++ b/HeterogeneousCore/CUDAServices/plugins/NVProfilerService.cc @@ -209,6 +209,10 @@ class NVProfilerService { void preModuleConstruction(edm::ModuleDescription const&); void postModuleConstruction(edm::ModuleDescription const&); + // these signal pair are guaranteed to be called by the same thread + void preModuleDestruction(edm::ModuleDescription const&); + void postModuleDestruction(edm::ModuleDescription const&); + // these signal pair are guaranteed to be called by the same thread void preModuleBeginJob(edm::ModuleDescription const&); void postModuleBeginJob(edm::ModuleDescription const&); @@ -399,6 +403,10 @@ NVProfilerService::NVProfilerService(edm::ParameterSet const& config, edm::Activ registry.watchPreModuleConstruction(this, &NVProfilerService::preModuleConstruction); registry.watchPostModuleConstruction(this, &NVProfilerService::postModuleConstruction); + // these signal pair are guaranteed to be called by the same thread + registry.watchPreModuleDestruction(this, &NVProfilerService::preModuleDestruction); + registry.watchPostModuleDestruction(this, &NVProfilerService::postModuleDestruction); + // these signal pair are guaranteed to be called by the same thread registry.watchPreModuleBeginJob(this, &NVProfilerService::preModuleBeginJob); registry.watchPostModuleBeginJob(this, &NVProfilerService::postModuleBeginJob); @@ -820,6 +828,24 @@ void NVProfilerService::postModuleConstruction(edm::ModuleDescription const& des } } +void NVProfilerService::preModuleDestruction(edm::ModuleDescription const& desc) { + if (not skipFirstEvent_) { + auto mid = desc.id(); + global_modules_.grow_to_at_least(mid + 1); + auto const& label = desc.moduleLabel(); + auto const& msg = label + " destruction"; + global_modules_[mid] = nvtxDomainRangeStartColor(global_domain_, msg.c_str(), labelColor(label)); + } +} + +void NVProfilerService::postModuleDestruction(edm::ModuleDescription const& desc) { + if (not skipFirstEvent_) { + auto mid = desc.id(); + nvtxDomainRangeEnd(global_domain_, global_modules_[mid]); + global_modules_[mid] = nvtxInvalidRangeId; + } +} + void NVProfilerService::preModuleBeginJob(edm::ModuleDescription const& desc) { if (not skipFirstEvent_) { auto mid = desc.id(); From cb79bdba5f58d73b6ebd181e1ef21543420c3b26 Mon Sep 17 00:00:00 2001 From: Matti Kortelainen Date: Thu, 23 Apr 2020 16:05:45 +0200 Subject: [PATCH 18/26] Include EDAlias and SwitchProducer in the tests --- .../Framework/test/test_module_delete_cfg.py | 48 +++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/FWCore/Framework/test/test_module_delete_cfg.py b/FWCore/Framework/test/test_module_delete_cfg.py index d88d287b920d4..ffc1875460eb3 100644 --- a/FWCore/Framework/test/test_module_delete_cfg.py +++ b/FWCore/Framework/test/test_module_delete_cfg.py @@ -15,6 +15,8 @@ # - event/lumi/run product with the same module label and instance name but with skipCurrentProcess, module deleted # - DAG of event/lumi/run producers that are not consumed by an always-running module, whole DAG deleted # - DAG of event(/lumi/run) producers that are partly consumed (kept) and partly non-consumed (delete) +# - EDAlias with one instance consumed (original producer kept) and another non-consumed (original producer deleted) +# - SwitchProducer non-chosen case deleted intEventProducer = cms.EDProducer("IntProducer", ivalue = cms.int32(1)) intNonEventProducer = cms.EDProducer("NonEventIntProducer", ivalue = cms.int32(1)) @@ -83,6 +85,42 @@ def nonEventConsumer(transition, sourcePattern, expected): srcEvent = cms.untracked.VInputTag("producerEventPartiallyConsumedChain2", "producerEventPartiallyConsumedChain4") ) +process.producerEventAliasNotConsumed = cms.EDProducer("edmtest::TestModuleDeleteProducer") +process.producerEventAliasConsumed = intEventProducerMustRun.clone() +process.producerEventAlias = cms.EDAlias( + producerEventAliasNotConsumed = cms.VPSet( + cms.PSet( + type = cms.string("edmtestIntProduct"), + fromProductInstance = cms.string(""), + toProductInstance = cms.string("notConsumed"), + ) + ), + producerEventAliasConsumed = cms.VPSet( + cms.PSet( + type = cms.string("edmtestIntProduct"), + fromProductInstance = cms.string(""), + toProductInstance = cms.string("consumed"), + ) + ) +) +process.consumerEventAlias = cms.EDAnalyzer("edmtest::GenericIntsAnalyzer", + srcEvent = cms.untracked.VInputTag("producerEventAlias:consumed") +) + +class SwitchProducerTest(cms.SwitchProducer): + def __init__(self, **kargs): + super(SwitchProducerTest,self).__init__( + dict( + test1 = lambda: (True, -10), + test2 = lambda: (True, -9) + ), **kargs) +process.producerEventSwitchProducerNotConsumed = cms.EDProducer("edmtest::TestModuleDeleteProducer") +process.producerEventSwitchProducerConsumed = intEventProducerMustRun.clone() +process.producerEventSwitchProducer = SwitchProducerTest( + test1 = cms.EDProducer("AddIntsProducer", labels = cms.VInputTag("producerEventSwitchProducerNotConsumed")), + test2 = cms.EDProducer("AddIntsProducer", labels = cms.VInputTag("producerEventSwitchProducerConsumed")), +) + process.consumerNotExist = cms.EDAnalyzer("edmtest::GenericIntsAnalyzer", inputShouldBeMissing = cms.untracked.bool(True), srcBeginRun = cms.untracked.VInputTag( @@ -124,6 +162,12 @@ def nonEventConsumer(transition, sourcePattern, expected): process.producerEventPartiallyConsumedChain1, process.producerEventPartiallyConsumedChain3, process.producerEventPartiallyConsumedChain4, + # + process.producerEventAliasNotConsumed, + process.producerEventAliasConsumed, + # + process.producerEventSwitchProducerNotConsumed, + process.producerEventSwitchProducerConsumed, ) process.p = cms.Path( @@ -139,6 +183,10 @@ def nonEventConsumer(transition, sourcePattern, expected): # process.producerEventPartiallyConsumedChain2+ # + process.consumerEventAlias+ + # + process.producerEventSwitchProducer+ + # process.intAnalyzerDelete , process.t From 1c41364ae1f7ec31138815cf0e22d5e5ac3a1585 Mon Sep 17 00:00:00 2001 From: Matti Kortelainen Date: Wed, 3 Jun 2020 02:04:25 +0200 Subject: [PATCH 19/26] Move to std::array --- FWCore/Framework/interface/EDConsumerBase.h | 3 +- .../interface/PathsAndConsumesOfModules.h | 8 ++-- FWCore/Framework/interface/Schedule.h | 4 +- .../interface/stream/EDAnalyzerAdaptorBase.h | 3 +- .../stream/ProducingModuleAdaptorBase.h | 3 +- FWCore/Framework/src/EDConsumerBase.cc | 47 ++++++------------- .../src/PathsAndConsumesOfModules.cc | 28 +++++------ FWCore/Framework/src/Schedule.cc | 23 +++++---- FWCore/Framework/src/Worker.h | 3 +- FWCore/Framework/src/WorkerT.h | 11 ++--- .../src/stream/EDAnalyzerAdaptorBase.cc | 5 +- .../src/stream/ProducingModuleAdaptorBase.cc | 5 +- .../interface/PathsAndConsumesOfModulesBase.h | 14 ++---- 13 files changed, 61 insertions(+), 96 deletions(-) diff --git a/FWCore/Framework/interface/EDConsumerBase.h b/FWCore/Framework/interface/EDConsumerBase.h index 79d5c9a198e46..654f8a7933d3c 100644 --- a/FWCore/Framework/interface/EDConsumerBase.h +++ b/FWCore/Framework/interface/EDConsumerBase.h @@ -106,8 +106,7 @@ namespace edm { typedef ProductLabels Labels; void labelsForToken(EDGetToken iToken, Labels& oLabels) const; - void modulesWhoseProductsAreConsumed(std::vector& modulesEvent, - std::vector& modulesLumiRun, + void modulesWhoseProductsAreConsumed(std::array*, NumBranchTypes>& modulesAll, std::set& modulesInPreviousProcesses, ProductRegistry const& preg, std::map const& labelsToDesc, diff --git a/FWCore/Framework/interface/PathsAndConsumesOfModules.h b/FWCore/Framework/interface/PathsAndConsumesOfModules.h index e853c53258e6e..0384101852b6f 100644 --- a/FWCore/Framework/interface/PathsAndConsumesOfModules.h +++ b/FWCore/Framework/interface/PathsAndConsumesOfModules.h @@ -16,6 +16,7 @@ #include "FWCore/ServiceRegistry/interface/PathsAndConsumesOfModulesBase.h" #include "FWCore/Framework/interface/ModuleProcessName.h" +#include "FWCore/Utilities/interface/BranchType.h" #include #include @@ -50,9 +51,7 @@ namespace edm { std::vector const& doModulesOnPath(unsigned int pathIndex) const override; std::vector const& doModulesOnEndPath(unsigned int endPathIndex) const override; std::vector const& doModulesWhoseProductsAreConsumedBy( - unsigned int moduleID) const override; - std::vector const& doModulesWhoseProductsAreConsumedByLumiRun( - unsigned int moduleID) const override; + unsigned int moduleID, BranchType branchType = InEvent) const override; std::vector doConsumesInfo(unsigned int moduleID) const override; @@ -72,8 +71,7 @@ namespace edm { // following data member std::vector > moduleIDToIndex_; - std::vector > modulesWhoseProductsAreConsumedByEvent_; - std::vector > modulesWhoseProductsAreConsumedByLumiRun_; + std::array >, NumBranchTypes> modulesWhoseProductsAreConsumedBy_; std::vector > modulesInPreviousProcessesWhoseProductsAreConsumedBy_; Schedule const* schedule_; diff --git a/FWCore/Framework/interface/Schedule.h b/FWCore/Framework/interface/Schedule.h index cb104e69be082..a5f8aa50e099a 100644 --- a/FWCore/Framework/interface/Schedule.h +++ b/FWCore/Framework/interface/Schedule.h @@ -233,8 +233,8 @@ namespace edm { void fillModuleAndConsumesInfo( std::vector& allModuleDescriptions, std::vector>& moduleIDToIndex, - std::vector>& modulesWhoseProductsAreConsumedByEvent, - std::vector>& modulesWhoseProductsAreConsumedByLumiRun, + std::array>, NumBranchTypes>& + modulesWhoseProductsAreConsumedBy, std::vector>& modulesInPreviousProcessesWhoseProductsAreConsumedBy, ProductRegistry const& preg) const; diff --git a/FWCore/Framework/interface/stream/EDAnalyzerAdaptorBase.h b/FWCore/Framework/interface/stream/EDAnalyzerAdaptorBase.h index 7a8e8252096b9..d0e807d8a0227 100644 --- a/FWCore/Framework/interface/stream/EDAnalyzerAdaptorBase.h +++ b/FWCore/Framework/interface/stream/EDAnalyzerAdaptorBase.h @@ -111,8 +111,7 @@ namespace edm { const EDConsumerBase* consumer() const; - void modulesWhoseProductsAreConsumed(std::vector& modulesEvent, - std::vector& modulesLumiRun, + void modulesWhoseProductsAreConsumed(std::array*, NumBranchTypes>& modules, std::set& modulesInPreviousProcesses, ProductRegistry const& preg, std::map const& labelsToDesc, diff --git a/FWCore/Framework/interface/stream/ProducingModuleAdaptorBase.h b/FWCore/Framework/interface/stream/ProducingModuleAdaptorBase.h index ee14b5974618b..1ec3d7c885223 100644 --- a/FWCore/Framework/interface/stream/ProducingModuleAdaptorBase.h +++ b/FWCore/Framework/interface/stream/ProducingModuleAdaptorBase.h @@ -101,8 +101,7 @@ namespace edm { void updateLookup(BranchType iBranchType, ProductResolverIndexHelper const&, bool iPrefetchMayGet); void updateLookup(eventsetup::ESRecordsToProxyIndices const&); - void modulesWhoseProductsAreConsumed(std::vector& modulesEvent, - std::vector& modulesLumiRun, + void modulesWhoseProductsAreConsumed(std::array*, NumBranchTypes>& modules, std::set& modulesInPreviousProcesses, ProductRegistry const& preg, std::map const& labelsToDesc, diff --git a/FWCore/Framework/src/EDConsumerBase.cc b/FWCore/Framework/src/EDConsumerBase.cc index 616772af7b367..ced9d9c7842db 100644 --- a/FWCore/Framework/src/EDConsumerBase.cc +++ b/FWCore/Framework/src/EDConsumerBase.cc @@ -474,37 +474,20 @@ namespace { } } // namespace -void EDConsumerBase::modulesWhoseProductsAreConsumed(std::vector& modulesEvent, - std::vector& modulesLumiRun, - std::set& modulesInPreviousProcesses, - ProductRegistry const& preg, - std::map const& labelsToDesc, - std::string const& processName) const { - ProductResolverIndexHelper const& helperEvent = *preg.productLookup(InEvent); - ProductResolverIndexHelper const& helperLumi = *preg.productLookup(InLumi); - ProductResolverIndexHelper const& helperRun = *preg.productLookup(InRun); - +void EDConsumerBase::modulesWhoseProductsAreConsumed( + std::array*, NumBranchTypes>& modulesAll, + std::set& modulesInPreviousProcesses, + ProductRegistry const& preg, + std::map const& labelsToDesc, + std::string const& processName) const { std::set alreadyFound; auto itKind = m_tokenInfo.begin(); auto itLabels = m_tokenInfo.begin(); for (auto itInfo = m_tokenInfo.begin(), itEnd = m_tokenInfo.end(); itInfo != itEnd; ++itInfo, ++itKind, ++itLabels) { - ProductResolverIndexHelper const* helper = nullptr; - std::vector* modules = nullptr; - if (itInfo->m_branchType == InEvent) { - helper = &helperEvent; - modules = &modulesEvent; - } else if (itInfo->m_branchType == InLumi) { - helper = &helperLumi; - modules = &modulesLumiRun; - } else if (itInfo->m_branchType == InRun) { - helper = &helperRun; - modules = &modulesLumiRun; - } else { - throw cms::Exception("LogicError") << "EDConsumerBase::modulesWhoseProductsAreConsumed(): unknown branch type " - << itInfo->m_branchType; - } + ProductResolverIndexHelper const& helper = *preg.productLookup(itInfo->m_branchType); + std::vector& modules = *modulesAll[itInfo->m_branchType]; const unsigned int labelStart = itLabels->m_startOfModuleLabel; const char* const consumedModuleLabel = &(m_tokenLabels[labelStart]); @@ -514,7 +497,7 @@ void EDConsumerBase::modulesWhoseProductsAreConsumed(std::vectorm_index.skipCurrentProcess()) { if (*consumedModuleLabel != '\0') { // not a consumesMany if (*consumedProcessName != '\0') { // process name is specified in consumes call - if (helper->index( + if (helper.index( *itKind, itInfo->m_type, consumedModuleLabel, consumedProductInstance, consumedProcessName) != ProductResolverIndexInvalid) { if (processName == consumedProcessName) { @@ -522,7 +505,7 @@ void EDConsumerBase::modulesWhoseProductsAreConsumed(std::vectorm_type, consumedModuleLabel, consumedProductInstance, - *modules, + modules, alreadyFound, labelsToDesc, preg); @@ -532,14 +515,14 @@ void EDConsumerBase::modulesWhoseProductsAreConsumed(std::vectorrelatedIndexes(*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, itInfo->m_type, consumedModuleLabel, consumedProductInstance, - *modules, + modules, alreadyFound, labelsToDesc, preg); @@ -552,14 +535,14 @@ void EDConsumerBase::modulesWhoseProductsAreConsumed(std::vectorm_index.productResolverIndex() == ProductResolverIndexInvalid) { - auto matches = helper->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, itInfo->m_type, matches.moduleLabel(j), matches.productInstanceName(j), - *modules, + modules, alreadyFound, labelsToDesc, preg); @@ -573,7 +556,7 @@ void EDConsumerBase::modulesWhoseProductsAreConsumed(std::vectorrelatedIndexes(*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)) { modulesInPreviousProcesses.emplace(matches.moduleLabel(j), matches.processName(j)); diff --git a/FWCore/Framework/src/PathsAndConsumesOfModules.cc b/FWCore/Framework/src/PathsAndConsumesOfModules.cc index d3fc0811cb801..e02c7e4ba42d9 100644 --- a/FWCore/Framework/src/PathsAndConsumesOfModules.cc +++ b/FWCore/Framework/src/PathsAndConsumesOfModules.cc @@ -45,8 +45,7 @@ namespace edm { schedule->fillModuleAndConsumesInfo(allModuleDescriptions_, moduleIDToIndex_, - modulesWhoseProductsAreConsumedByEvent_, - modulesWhoseProductsAreConsumedByLumiRun_, + modulesWhoseProductsAreConsumedBy_, modulesInPreviousProcessesWhoseProductsAreConsumedBy_, *preg); } @@ -72,8 +71,10 @@ namespace edm { auto found = std::find(modules.begin(), modules.end(), allModuleDescriptions_[iModule]); if (found != modules.end()) { allModuleDescriptions_.erase(allModuleDescriptions_.begin() + iModule); - modulesWhoseProductsAreConsumedByEvent_.erase(modulesWhoseProductsAreConsumedByEvent_.begin() + iModule); - modulesWhoseProductsAreConsumedByLumiRun_.erase(modulesWhoseProductsAreConsumedByLumiRun_.begin() + iModule); + for (auto iBranchType = 0U; iBranchType != NumBranchTypes; ++iBranchType) { + modulesWhoseProductsAreConsumedBy_[iBranchType].erase( + modulesWhoseProductsAreConsumedBy_[iBranchType].begin() + iModule); + } modulesInPreviousProcessesWhoseProductsAreConsumedBy_.erase( modulesInPreviousProcessesWhoseProductsAreConsumedBy_.begin() + iModule); for (auto& idToIndex : moduleIDToIndex_) { @@ -113,13 +114,8 @@ namespace edm { } std::vector const& PathsAndConsumesOfModules::doModulesWhoseProductsAreConsumedBy( - unsigned int moduleID) const { - return modulesWhoseProductsAreConsumedByEvent_.at(moduleIndex(moduleID)); - } - - std::vector const& PathsAndConsumesOfModules::doModulesWhoseProductsAreConsumedByLumiRun( - unsigned int moduleID) const { - return modulesWhoseProductsAreConsumedByLumiRun_.at(moduleIndex(moduleID)); + unsigned int moduleID, BranchType branchType) const { + return modulesWhoseProductsAreConsumedBy_[branchType].at(moduleIndex(moduleID)); } std::vector PathsAndConsumesOfModules::doConsumesInfo(unsigned int moduleID) const { @@ -151,11 +147,11 @@ namespace { return; } consumedModules.insert(module->id()); - for (auto const& c : iPnC.modulesWhoseProductsAreConsumedBy(module->id())) { - findAllConsumedModules(iPnC, c, consumedModules); - } - for (auto const& c : iPnC.modulesWhoseProductsAreConsumedByLumiRun(module->id())) { - findAllConsumedModules(iPnC, c, consumedModules); + for (auto iBranchType = 0U; iBranchType != edm::NumBranchTypes; ++iBranchType) { + for (auto const& c : + iPnC.modulesWhoseProductsAreConsumedBy(module->id(), static_cast(iBranchType))) { + findAllConsumedModules(iPnC, c, consumedModules); + } } } } // namespace diff --git a/FWCore/Framework/src/Schedule.cc b/FWCore/Framework/src/Schedule.cc index 3d9db4d22b3ab..8955cbeca6ea0 100644 --- a/FWCore/Framework/src/Schedule.cc +++ b/FWCore/Framework/src/Schedule.cc @@ -1478,20 +1478,21 @@ namespace edm { void Schedule::fillModuleAndConsumesInfo( std::vector& allModuleDescriptions, std::vector>& moduleIDToIndex, - std::vector>& modulesWhoseProductsAreConsumedByEvent, - std::vector>& modulesWhoseProductsAreConsumedByLumiRun, + std::array>, NumBranchTypes>& modulesWhoseProductsAreConsumedBy, std::vector>& modulesInPreviousProcessesWhoseProductsAreConsumedBy, ProductRegistry const& preg) const { allModuleDescriptions.clear(); moduleIDToIndex.clear(); - modulesWhoseProductsAreConsumedByEvent.clear(); - modulesWhoseProductsAreConsumedByLumiRun.clear(); + for (auto iBranchType = 0U; iBranchType < NumBranchTypes; ++iBranchType) { + modulesWhoseProductsAreConsumedBy[iBranchType].clear(); + } modulesInPreviousProcessesWhoseProductsAreConsumedBy.clear(); allModuleDescriptions.reserve(allWorkers().size()); moduleIDToIndex.reserve(allWorkers().size()); - modulesWhoseProductsAreConsumedByEvent.resize(allWorkers().size()); - modulesWhoseProductsAreConsumedByLumiRun.resize(allWorkers().size()); + for (auto iBranchType = 0U; iBranchType < NumBranchTypes; ++iBranchType) { + modulesWhoseProductsAreConsumedBy[iBranchType].resize(allWorkers().size()); + } modulesInPreviousProcessesWhoseProductsAreConsumedBy.resize(allWorkers().size()); std::map labelToDesc; @@ -1507,13 +1508,15 @@ namespace edm { i = 0; for (auto const& worker : allWorkers()) { - std::vector& modulesEvent = modulesWhoseProductsAreConsumedByEvent.at(i); - std::vector& modulesLumiRun = modulesWhoseProductsAreConsumedByLumiRun.at(i); + std::array*, NumBranchTypes> modules; + for (auto iBranchType = 0U; iBranchType < NumBranchTypes; ++iBranchType) { + modules[iBranchType] = &modulesWhoseProductsAreConsumedBy[iBranchType].at(i); + } + std::set& modulesInPreviousProcesses = modulesInPreviousProcessesWhoseProductsAreConsumedBy.at(i); try { - worker->modulesWhoseProductsAreConsumed( - modulesEvent, modulesLumiRun, modulesInPreviousProcesses, preg, labelToDesc); + worker->modulesWhoseProductsAreConsumed(modules, modulesInPreviousProcesses, preg, labelToDesc); } catch (cms::Exception& ex) { ex.addContext("Calling Worker::modulesWhoseProductsAreConsumed() for module " + worker->description()->moduleLabel()); diff --git a/FWCore/Framework/src/Worker.h b/FWCore/Framework/src/Worker.h index df823842d586a..8ba8879529a19 100644 --- a/FWCore/Framework/src/Worker.h +++ b/FWCore/Framework/src/Worker.h @@ -212,8 +212,7 @@ namespace edm { iIndicies) = 0; virtual void modulesWhoseProductsAreConsumed( - std::vector& modulesEvent, - std::vector& modulesLumiRun, + std::array*, NumBranchTypes>& modules, std::set& modulesInPreviousProcesses, ProductRegistry const& preg, std::map const& labelsToDesc) const = 0; diff --git a/FWCore/Framework/src/WorkerT.h b/FWCore/Framework/src/WorkerT.h index a70cd2b40b767..a87f3385bed4a 100644 --- a/FWCore/Framework/src/WorkerT.h +++ b/FWCore/Framework/src/WorkerT.h @@ -110,17 +110,12 @@ namespace edm { TaskQueueAdaptor serializeRunModule() override; void modulesWhoseProductsAreConsumed( - std::vector& modulesEvent, - std::vector& modulesLumiRun, + std::array*, NumBranchTypes>& modules, std::set& modulesInPreviousProcesses, ProductRegistry const& preg, std::map const& labelsToDesc) const override { - module_->modulesWhoseProductsAreConsumed(modulesEvent, - modulesLumiRun, - modulesInPreviousProcesses, - preg, - labelsToDesc, - module_->moduleDescription().processName()); + module_->modulesWhoseProductsAreConsumed( + modules, modulesInPreviousProcesses, preg, labelsToDesc, module_->moduleDescription().processName()); } void convertCurrentProcessAlias(std::string const& processName) override { diff --git a/FWCore/Framework/src/stream/EDAnalyzerAdaptorBase.cc b/FWCore/Framework/src/stream/EDAnalyzerAdaptorBase.cc index 84f64db561b99..dc762510f44ac 100644 --- a/FWCore/Framework/src/stream/EDAnalyzerAdaptorBase.cc +++ b/FWCore/Framework/src/stream/EDAnalyzerAdaptorBase.cc @@ -122,15 +122,14 @@ void EDAnalyzerAdaptorBase::updateLookup(eventsetup::ESRecordsToProxyIndices con const edm::EDConsumerBase* EDAnalyzerAdaptorBase::consumer() const { return m_streamModules[0]; } void EDAnalyzerAdaptorBase::modulesWhoseProductsAreConsumed( - std::vector& modulesEvent, - std::vector& modulesLumiRun, + std::array*, NumBranchTypes>& modules, std::set& modulesInPreviousProcesses, ProductRegistry const& preg, std::map const& labelsToDesc, std::string const& processName) const { assert(not m_streamModules.empty()); return m_streamModules[0]->modulesWhoseProductsAreConsumed( - modulesEvent, modulesLumiRun, modulesInPreviousProcesses, preg, labelsToDesc, processName); + modules, modulesInPreviousProcesses, preg, labelsToDesc, processName); } void EDAnalyzerAdaptorBase::convertCurrentProcessAlias(std::string const& processName) { diff --git a/FWCore/Framework/src/stream/ProducingModuleAdaptorBase.cc b/FWCore/Framework/src/stream/ProducingModuleAdaptorBase.cc index aae8b899c6d37..7b8ff2c777d5f 100644 --- a/FWCore/Framework/src/stream/ProducingModuleAdaptorBase.cc +++ b/FWCore/Framework/src/stream/ProducingModuleAdaptorBase.cc @@ -118,15 +118,14 @@ namespace edm { template void ProducingModuleAdaptorBase::modulesWhoseProductsAreConsumed( - std::vector& modulesEvent, - std::vector& modulesLumiRun, + std::array*, NumBranchTypes>& modules, std::set& modulesInPreviousProcesses, ProductRegistry const& preg, std::map const& labelsToDesc, std::string const& processName) const { assert(not m_streamModules.empty()); return m_streamModules[0]->modulesWhoseProductsAreConsumed( - modulesEvent, modulesLumiRun, modulesInPreviousProcesses, preg, labelsToDesc, processName); + modules, modulesInPreviousProcesses, preg, labelsToDesc, processName); } template diff --git a/FWCore/ServiceRegistry/interface/PathsAndConsumesOfModulesBase.h b/FWCore/ServiceRegistry/interface/PathsAndConsumesOfModulesBase.h index ef9fa62d29dec..99882765ab8c8 100644 --- a/FWCore/ServiceRegistry/interface/PathsAndConsumesOfModulesBase.h +++ b/FWCore/ServiceRegistry/interface/PathsAndConsumesOfModulesBase.h @@ -23,6 +23,7 @@ // Created: 11/5/2014 #include "FWCore/ServiceRegistry/interface/ConsumesInfo.h" +#include "FWCore/Utilities/interface/BranchType.h" #include #include @@ -60,12 +61,9 @@ namespace edm { // it consumes a module label that is an EDAlias, the corresponding module // description will be included in the returned vector (but the label in the // module description is not the EDAlias label). - std::vector const& modulesWhoseProductsAreConsumedBy(unsigned int moduleID) const { - return doModulesWhoseProductsAreConsumedBy(moduleID); - } - - std::vector const& modulesWhoseProductsAreConsumedByLumiRun(unsigned int moduleID) const { - return doModulesWhoseProductsAreConsumedByLumiRun(moduleID); + std::vector const& modulesWhoseProductsAreConsumedBy( + unsigned int moduleID, BranchType branchType = InEvent) const { + return doModulesWhoseProductsAreConsumedBy(moduleID, branchType); } // This returns the declared consumes information for a module. @@ -85,9 +83,7 @@ namespace edm { virtual std::vector const& doModulesOnPath(unsigned int pathIndex) const = 0; virtual std::vector const& doModulesOnEndPath(unsigned int endPathIndex) const = 0; virtual std::vector const& doModulesWhoseProductsAreConsumedBy( - unsigned int moduleID) const = 0; - virtual std::vector const& doModulesWhoseProductsAreConsumedByLumiRun( - unsigned int moduleID) const = 0; + unsigned int moduleID, BranchType branchType) const = 0; virtual std::vector doConsumesInfo(unsigned int moduleID) const = 0; }; } // namespace edm From bb852dd18ef98a25e757ff2a6a5b2c58bf7bde29 Mon Sep 17 00:00:00 2001 From: Matti Kortelainen Date: Wed, 3 Jun 2020 19:00:32 +0200 Subject: [PATCH 20/26] Replace set with sorted vector --- FWCore/Framework/interface/EDConsumerBase.h | 3 +-- .../interface/PathsAndConsumesOfModules.h | 10 +++++----- FWCore/Framework/interface/Schedule.h | 2 +- .../interface/stream/EDAnalyzerAdaptorBase.h | 3 +-- .../stream/ProducingModuleAdaptorBase.h | 2 +- FWCore/Framework/src/EDConsumerBase.cc | 17 ++++++++++++----- FWCore/Framework/src/EventProcessor.cc | 12 ++++++++++-- .../Framework/src/PathsAndConsumesOfModules.cc | 16 +++++++++------- FWCore/Framework/src/Schedule.cc | 4 ++-- FWCore/Framework/src/SubProcess.cc | 18 ++++++++++++++---- FWCore/Framework/src/SubProcess.h | 2 +- FWCore/Framework/src/Worker.h | 2 +- FWCore/Framework/src/WorkerT.h | 2 +- .../src/stream/EDAnalyzerAdaptorBase.cc | 2 +- .../src/stream/ProducingModuleAdaptorBase.cc | 2 +- 15 files changed, 61 insertions(+), 36 deletions(-) diff --git a/FWCore/Framework/interface/EDConsumerBase.h b/FWCore/Framework/interface/EDConsumerBase.h index 654f8a7933d3c..48e8684e37a13 100644 --- a/FWCore/Framework/interface/EDConsumerBase.h +++ b/FWCore/Framework/interface/EDConsumerBase.h @@ -20,7 +20,6 @@ // system include files #include -#include #include #include #include @@ -107,7 +106,7 @@ namespace edm { void labelsForToken(EDGetToken iToken, Labels& oLabels) const; void modulesWhoseProductsAreConsumed(std::array*, NumBranchTypes>& modulesAll, - std::set& modulesInPreviousProcesses, + std::vector& modulesInPreviousProcesses, ProductRegistry const& preg, std::map const& labelsToDesc, std::string const& processName) const; diff --git a/FWCore/Framework/interface/PathsAndConsumesOfModules.h b/FWCore/Framework/interface/PathsAndConsumesOfModules.h index 0384101852b6f..c642a17be5f7f 100644 --- a/FWCore/Framework/interface/PathsAndConsumesOfModules.h +++ b/FWCore/Framework/interface/PathsAndConsumesOfModules.h @@ -19,7 +19,6 @@ #include "FWCore/Utilities/interface/BranchType.h" #include -#include #include #include #include @@ -39,7 +38,8 @@ namespace edm { void removeModules(std::vector const& modules); - std::set const& modulesInPreviousProcessesWhoseProductsAreConsumedBy(unsigned int moduleID) const; + std::vector const& modulesInPreviousProcessesWhoseProductsAreConsumedBy( + unsigned int moduleID) const; private: std::vector const& doPaths() const override { return paths_; } @@ -72,14 +72,14 @@ namespace edm { std::vector > moduleIDToIndex_; std::array >, NumBranchTypes> modulesWhoseProductsAreConsumedBy_; - std::vector > modulesInPreviousProcessesWhoseProductsAreConsumedBy_; + std::vector > modulesInPreviousProcessesWhoseProductsAreConsumedBy_; Schedule const* schedule_; std::shared_ptr preg_; }; - std::vector nonConsumedUnscheduledModules(edm::PathsAndConsumesOfModulesBase const& iPnC, - std::set& consumedByChildren); + std::vector nonConsumedUnscheduledModules( + edm::PathsAndConsumesOfModulesBase const& iPnC, std::vector& consumedByChildren); void checkForModuleDependencyCorrectness(edm::PathsAndConsumesOfModulesBase const& iPnC, bool iPrintDependencies); } // namespace edm diff --git a/FWCore/Framework/interface/Schedule.h b/FWCore/Framework/interface/Schedule.h index a5f8aa50e099a..81d655368f5fa 100644 --- a/FWCore/Framework/interface/Schedule.h +++ b/FWCore/Framework/interface/Schedule.h @@ -235,7 +235,7 @@ namespace edm { std::vector>& moduleIDToIndex, std::array>, NumBranchTypes>& modulesWhoseProductsAreConsumedBy, - std::vector>& modulesInPreviousProcessesWhoseProductsAreConsumedBy, + std::vector>& modulesInPreviousProcessesWhoseProductsAreConsumedBy, ProductRegistry const& preg) const; /// Return the number of events this Schedule has tried to process diff --git a/FWCore/Framework/interface/stream/EDAnalyzerAdaptorBase.h b/FWCore/Framework/interface/stream/EDAnalyzerAdaptorBase.h index d0e807d8a0227..c2e2aa9ee9708 100644 --- a/FWCore/Framework/interface/stream/EDAnalyzerAdaptorBase.h +++ b/FWCore/Framework/interface/stream/EDAnalyzerAdaptorBase.h @@ -20,7 +20,6 @@ // system include files #include -#include #include #include @@ -112,7 +111,7 @@ namespace edm { const EDConsumerBase* consumer() const; void modulesWhoseProductsAreConsumed(std::array*, NumBranchTypes>& modules, - std::set& modulesInPreviousProcesses, + std::vector& modulesInPreviousProcesses, ProductRegistry const& preg, std::map const& labelsToDesc, std::string const& processName) const; diff --git a/FWCore/Framework/interface/stream/ProducingModuleAdaptorBase.h b/FWCore/Framework/interface/stream/ProducingModuleAdaptorBase.h index 1ec3d7c885223..0a95af8c53e69 100644 --- a/FWCore/Framework/interface/stream/ProducingModuleAdaptorBase.h +++ b/FWCore/Framework/interface/stream/ProducingModuleAdaptorBase.h @@ -102,7 +102,7 @@ namespace edm { void updateLookup(eventsetup::ESRecordsToProxyIndices const&); void modulesWhoseProductsAreConsumed(std::array*, NumBranchTypes>& modules, - std::set& modulesInPreviousProcesses, + std::vector& modulesInPreviousProcesses, ProductRegistry const& preg, std::map const& labelsToDesc, std::string const& processName) const; diff --git a/FWCore/Framework/src/EDConsumerBase.cc b/FWCore/Framework/src/EDConsumerBase.cc index ced9d9c7842db..6858370802620 100644 --- a/FWCore/Framework/src/EDConsumerBase.cc +++ b/FWCore/Framework/src/EDConsumerBase.cc @@ -476,12 +476,19 @@ namespace { void EDConsumerBase::modulesWhoseProductsAreConsumed( std::array*, NumBranchTypes>& modulesAll, - std::set& modulesInPreviousProcesses, + std::vector& modulesInPreviousProcesses, ProductRegistry const& preg, std::map const& labelsToDesc, std::string const& processName) const { std::set 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(); auto itLabels = m_tokenInfo.begin(); for (auto itInfo = m_tokenInfo.begin(), itEnd = m_tokenInfo.end(); itInfo != itEnd; @@ -511,7 +518,7 @@ void EDConsumerBase::modulesWhoseProductsAreConsumed( preg); } else { // Product explicitly from different process than the current process, so must refer to an earlier process (unless it ends up "not found") - modulesInPreviousProcesses.emplace(consumedModuleLabel, consumedProcessName); + modulesInPreviousProcessesEmplace(consumedModuleLabel, consumedProcessName); } } } else { // process name was empty @@ -529,7 +536,7 @@ void EDConsumerBase::modulesWhoseProductsAreConsumed( } 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 - modulesInPreviousProcesses.emplace(consumedModuleLabel, matches.processName(j)); + modulesInPreviousProcessesEmplace(consumedModuleLabel, matches.processName(j)); } } } @@ -547,7 +554,7 @@ void EDConsumerBase::modulesWhoseProductsAreConsumed( labelsToDesc, preg); } else { - modulesInPreviousProcesses.emplace(matches.moduleLabel(j), matches.processName(j)); + modulesInPreviousProcessesEmplace(matches.moduleLabel(j), matches.processName(j)); } } } @@ -559,7 +566,7 @@ void EDConsumerBase::modulesWhoseProductsAreConsumed( auto matches = helper.relatedIndexes(*itKind, itInfo->m_type, consumedModuleLabel, consumedProductInstance); for (unsigned int j = 0; j < matches.numberOfMatches(); ++j) { if (processName != matches.processName(j)) { - modulesInPreviousProcesses.emplace(matches.moduleLabel(j), matches.processName(j)); + modulesInPreviousProcessesEmplace(matches.moduleLabel(j), matches.processName(j)); } } } diff --git a/FWCore/Framework/src/EventProcessor.cc b/FWCore/Framework/src/EventProcessor.cc index 30fa086a02575..dae882afb392b 100644 --- a/FWCore/Framework/src/EventProcessor.cc +++ b/FWCore/Framework/src/EventProcessor.cc @@ -547,10 +547,18 @@ namespace edm { actReg_->preallocateSignal_(bounds); schedule_->convertCurrentProcessAlias(processConfiguration_->processName()); pathsAndConsumesOfModules_.initialize(schedule_.get(), preg()); - std::set consumedBySubProcesses; + std::vector consumedBySubProcesses; for_all(subProcesses_, [&consumedBySubProcesses](auto& subProcess) { auto c = subProcess.keepOnlyConsumedUnscheduledModules(); - consumedBySubProcesses.insert(c.begin(), c.end()); + if (consumedBySubProcesses.empty()) { + consumedBySubProcesses = std::move(c); + } else if (not c.empty()) { + std::vector tmp; + tmp.reserve(consumedBySubProcesses.size() + c.size()); + std::merge( + consumedBySubProcesses.begin(), consumedBySubProcesses.end(), c.begin(), c.end(), std::back_inserter(tmp)); + std::swap(consumedBySubProcesses, tmp); + } }); // Note: all these may throw diff --git a/FWCore/Framework/src/PathsAndConsumesOfModules.cc b/FWCore/Framework/src/PathsAndConsumesOfModules.cc index e02c7e4ba42d9..666253850dabe 100644 --- a/FWCore/Framework/src/PathsAndConsumesOfModules.cc +++ b/FWCore/Framework/src/PathsAndConsumesOfModules.cc @@ -87,7 +87,7 @@ namespace edm { } } - std::set const& PathsAndConsumesOfModules::modulesInPreviousProcessesWhoseProductsAreConsumedBy( + std::vector const& PathsAndConsumesOfModules::modulesInPreviousProcessesWhoseProductsAreConsumedBy( unsigned int moduleID) const { return modulesInPreviousProcessesWhoseProductsAreConsumedBy_.at(moduleIndex(moduleID)); } @@ -157,8 +157,8 @@ namespace { } // namespace namespace edm { - std::vector nonConsumedUnscheduledModules(edm::PathsAndConsumesOfModulesBase const& iPnC, - std::set& consumedByChildren) { + std::vector nonConsumedUnscheduledModules( + edm::PathsAndConsumesOfModulesBase const& iPnC, std::vector& consumedByChildren) { const std::string kTriggerResults("TriggerResults"); std::vector pathNames = iPnC.paths(); @@ -191,10 +191,12 @@ namespace edm { if (description->moduleLabel() == kTriggerResults or std::find(pathNames.begin(), pathNames.end(), description->moduleLabel()) != pathNames.end()) { consumerModules.push_back(description); - } else if (consumedByChildren.find(ModuleProcessName{description->moduleLabel(), description->processName()}) != - consumedByChildren.end() or - consumedByChildren.find(ModuleProcessName{description->moduleLabel(), ""}) != - consumedByChildren.end()) { + } else if (std::binary_search(consumedByChildren.begin(), + consumedByChildren.end(), + ModuleProcessName{description->moduleLabel(), description->processName()}) or + std::binary_search(consumedByChildren.begin(), + consumedByChildren.end(), + ModuleProcessName{description->moduleLabel(), ""})) { consumerModules.push_back(description); } } diff --git a/FWCore/Framework/src/Schedule.cc b/FWCore/Framework/src/Schedule.cc index 8955cbeca6ea0..459e09a79114e 100644 --- a/FWCore/Framework/src/Schedule.cc +++ b/FWCore/Framework/src/Schedule.cc @@ -1479,7 +1479,7 @@ namespace edm { std::vector& allModuleDescriptions, std::vector>& moduleIDToIndex, std::array>, NumBranchTypes>& modulesWhoseProductsAreConsumedBy, - std::vector>& modulesInPreviousProcessesWhoseProductsAreConsumedBy, + std::vector>& modulesInPreviousProcessesWhoseProductsAreConsumedBy, ProductRegistry const& preg) const { allModuleDescriptions.clear(); moduleIDToIndex.clear(); @@ -1513,7 +1513,7 @@ namespace edm { modules[iBranchType] = &modulesWhoseProductsAreConsumedBy[iBranchType].at(i); } - std::set& modulesInPreviousProcesses = + std::vector& modulesInPreviousProcesses = modulesInPreviousProcessesWhoseProductsAreConsumedBy.at(i); try { worker->modulesWhoseProductsAreConsumed(modules, modulesInPreviousProcesses, preg, labelToDesc); diff --git a/FWCore/Framework/src/SubProcess.cc b/FWCore/Framework/src/SubProcess.cc index 8f899dea71c1e..b35b2b67ec4e8 100644 --- a/FWCore/Framework/src/SubProcess.cc +++ b/FWCore/Framework/src/SubProcess.cc @@ -220,7 +220,7 @@ namespace edm { SubProcess::~SubProcess() {} - std::set SubProcess::keepOnlyConsumedUnscheduledModules() { + std::vector SubProcess::keepOnlyConsumedUnscheduledModules() { ServiceRegistry::Operate operate(serviceToken_); schedule_->convertCurrentProcessAlias(processConfiguration_->processName()); pathsAndConsumesOfModules_.initialize(schedule_.get(), preg_); @@ -229,10 +229,17 @@ namespace edm { checkForModuleDependencyCorrectness(pathsAndConsumesOfModules_, false); // Consumes information from the child SubProcesses - std::set consumedByChildren; + std::vector consumedByChildren; for_all(subProcesses_, [&consumedByChildren](auto& subProcess) { auto c = subProcess.keepOnlyConsumedUnscheduledModules(); - consumedByChildren.insert(c.begin(), c.end()); + if (consumedByChildren.empty()) { + std::swap(consumedByChildren, c); + } else if (not c.empty()) { + std::vector tmp; + tmp.reserve(consumedByChildren.size() + c.size()); + std::merge(consumedByChildren.begin(), consumedByChildren.end(), c.begin(), c.end(), std::back_inserter(tmp)); + std::swap(consumedByChildren, tmp); + } }); // Non-consumed unscheduled modules in this SubProcess, take into account of the consumes from child SubProcesses @@ -257,7 +264,10 @@ namespace edm { for (auto const& description : pathsAndConsumesOfModules_.allModules()) { for (auto const& dep : pathsAndConsumesOfModules_.modulesInPreviousProcessesWhoseProductsAreConsumedBy(description->id())) { - consumedByChildren.emplace(dep.moduleLabel(), dep.processName()); + auto it = std::lower_bound(consumedByChildren.begin(), + consumedByChildren.end(), + ModuleProcessName{dep.moduleLabel(), dep.processName()}); + consumedByChildren.emplace(it, dep.moduleLabel(), dep.processName()); } } return consumedByChildren; diff --git a/FWCore/Framework/src/SubProcess.h b/FWCore/Framework/src/SubProcess.h index 2d61e8c1c416d..2f662f11c179c 100644 --- a/FWCore/Framework/src/SubProcess.h +++ b/FWCore/Framework/src/SubProcess.h @@ -78,7 +78,7 @@ namespace edm { // Returns the set of modules whose products may be consumed by // modules in this SubProcess or its child SubProcesses - std::set keepOnlyConsumedUnscheduledModules(); + std::vector keepOnlyConsumedUnscheduledModules(); void doBeginJob(); void doEndJob(); diff --git a/FWCore/Framework/src/Worker.h b/FWCore/Framework/src/Worker.h index 8ba8879529a19..cb8217b4b7bb4 100644 --- a/FWCore/Framework/src/Worker.h +++ b/FWCore/Framework/src/Worker.h @@ -213,7 +213,7 @@ namespace edm { virtual void modulesWhoseProductsAreConsumed( std::array*, NumBranchTypes>& modules, - std::set& modulesInPreviousProcesses, + std::vector& modulesInPreviousProcesses, ProductRegistry const& preg, std::map const& labelsToDesc) const = 0; diff --git a/FWCore/Framework/src/WorkerT.h b/FWCore/Framework/src/WorkerT.h index a87f3385bed4a..e8818c2504b15 100644 --- a/FWCore/Framework/src/WorkerT.h +++ b/FWCore/Framework/src/WorkerT.h @@ -111,7 +111,7 @@ namespace edm { void modulesWhoseProductsAreConsumed( std::array*, NumBranchTypes>& modules, - std::set& modulesInPreviousProcesses, + std::vector& modulesInPreviousProcesses, ProductRegistry const& preg, std::map const& labelsToDesc) const override { module_->modulesWhoseProductsAreConsumed( diff --git a/FWCore/Framework/src/stream/EDAnalyzerAdaptorBase.cc b/FWCore/Framework/src/stream/EDAnalyzerAdaptorBase.cc index dc762510f44ac..96f0076c59730 100644 --- a/FWCore/Framework/src/stream/EDAnalyzerAdaptorBase.cc +++ b/FWCore/Framework/src/stream/EDAnalyzerAdaptorBase.cc @@ -123,7 +123,7 @@ const edm::EDConsumerBase* EDAnalyzerAdaptorBase::consumer() const { return m_st void EDAnalyzerAdaptorBase::modulesWhoseProductsAreConsumed( std::array*, NumBranchTypes>& modules, - std::set& modulesInPreviousProcesses, + std::vector& modulesInPreviousProcesses, ProductRegistry const& preg, std::map const& labelsToDesc, std::string const& processName) const { diff --git a/FWCore/Framework/src/stream/ProducingModuleAdaptorBase.cc b/FWCore/Framework/src/stream/ProducingModuleAdaptorBase.cc index 7b8ff2c777d5f..447dd2b64874e 100644 --- a/FWCore/Framework/src/stream/ProducingModuleAdaptorBase.cc +++ b/FWCore/Framework/src/stream/ProducingModuleAdaptorBase.cc @@ -119,7 +119,7 @@ namespace edm { template void ProducingModuleAdaptorBase::modulesWhoseProductsAreConsumed( std::array*, NumBranchTypes>& modules, - std::set& modulesInPreviousProcesses, + std::vector& modulesInPreviousProcesses, ProductRegistry const& preg, std::map const& labelsToDesc, std::string const& processName) const { From 8873e2b14f48a58d4a24a7bad03305e15bfa2a6b Mon Sep 17 00:00:00 2001 From: Matti Kortelainen Date: Wed, 3 Jun 2020 20:15:15 +0200 Subject: [PATCH 21/26] Rename module in non-event ordering tests to avoid accidental success From David Dagenhart: > It is not at all obvious looking at it, but this change just > coincidentally happens to give the execution order 'a' then 'c' then > 'b'. Since the purpose of the test is to demonstrate the Framework > is forcing that execution order we should probably perturb this in > some way so it isn't just doing it by accident. Renaming 'a' to 'd' > seems to do the trick. --- .../Framework/test/test_non_event_ordering_beginLumi_cfg.py | 6 +++--- .../Framework/test/test_non_event_ordering_beginRun_cfg.py | 6 +++--- .../Framework/test/test_non_event_ordering_endLumi_cfg.py | 6 +++--- FWCore/Framework/test/test_non_event_ordering_endRun_cfg.py | 6 +++--- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/FWCore/Framework/test/test_non_event_ordering_beginLumi_cfg.py b/FWCore/Framework/test/test_non_event_ordering_beginLumi_cfg.py index 8f8df84c240bd..38a31cea99f4d 100644 --- a/FWCore/Framework/test/test_non_event_ordering_beginLumi_cfg.py +++ b/FWCore/Framework/test/test_non_event_ordering_beginLumi_cfg.py @@ -6,13 +6,13 @@ process.maxEvents.input = 3 -process.a = cms.EDProducer("NonEventIntProducer", ivalue = cms.int32(1)) +process.d = cms.EDProducer("NonEventIntProducer", ivalue = cms.int32(1)) process.b = cms.EDProducer("NonEventIntProducer", ivalue = cms.int32(2), consumesBeginLuminosityBlock = cms.InputTag("c","beginLumi"), expectBeginLuminosityBlock = cms.untracked.int32(3) ) process.c = cms.EDProducer("NonEventIntProducer", ivalue = cms.int32(3), - consumesBeginLuminosityBlock = cms.InputTag("a", "beginLumi"), + consumesBeginLuminosityBlock = cms.InputTag("d", "beginLumi"), expectBeginLuminosityBlock = cms.untracked.int32(1)) -process.t = cms.Task(process.a, process.c) +process.t = cms.Task(process.d, process.c) process.p = cms.Path(process.b, process.t) diff --git a/FWCore/Framework/test/test_non_event_ordering_beginRun_cfg.py b/FWCore/Framework/test/test_non_event_ordering_beginRun_cfg.py index a0e276fd42285..ef4dc3a84a35d 100644 --- a/FWCore/Framework/test/test_non_event_ordering_beginRun_cfg.py +++ b/FWCore/Framework/test/test_non_event_ordering_beginRun_cfg.py @@ -6,13 +6,13 @@ process.maxEvents.input = 3 -process.a = cms.EDProducer("NonEventIntProducer", ivalue = cms.int32(1)) +process.d = cms.EDProducer("NonEventIntProducer", ivalue = cms.int32(1)) process.b = cms.EDProducer("NonEventIntProducer", ivalue = cms.int32(2), consumesBeginRun = cms.InputTag("c","beginRun"), expectBeginRun = cms.untracked.int32(3) ) process.c = cms.EDProducer("NonEventIntProducer", ivalue = cms.int32(3), - consumesBeginRun = cms.InputTag("a", "beginRun"), + consumesBeginRun = cms.InputTag("d", "beginRun"), expectBeginRun = cms.untracked.int32(1)) -process.t = cms.Task(process.a, process.c) +process.t = cms.Task(process.d, process.c) process.p = cms.Path(process.b, process.t) diff --git a/FWCore/Framework/test/test_non_event_ordering_endLumi_cfg.py b/FWCore/Framework/test/test_non_event_ordering_endLumi_cfg.py index a7b6b50ce61f4..0e0e0bfd5fa10 100644 --- a/FWCore/Framework/test/test_non_event_ordering_endLumi_cfg.py +++ b/FWCore/Framework/test/test_non_event_ordering_endLumi_cfg.py @@ -6,13 +6,13 @@ process.maxEvents.input = 3 -process.a = cms.EDProducer("NonEventIntProducer", ivalue = cms.int32(1)) +process.d = cms.EDProducer("NonEventIntProducer", ivalue = cms.int32(1)) process.b = cms.EDProducer("NonEventIntProducer", ivalue = cms.int32(2), consumesEndLuminosityBlock = cms.InputTag("c","endLumi"), expectEndLuminosityBlock = cms.untracked.int32(3) ) process.c = cms.EDProducer("NonEventIntProducer", ivalue = cms.int32(3), - consumesEndLuminosityBlock = cms.InputTag("a", "endLumi"), + consumesEndLuminosityBlock = cms.InputTag("d", "endLumi"), expectEndLuminosityBlock = cms.untracked.int32(1)) -process.t = cms.Task(process.a, process.c) +process.t = cms.Task(process.d, process.c) process.p = cms.Path(process.b, process.t) diff --git a/FWCore/Framework/test/test_non_event_ordering_endRun_cfg.py b/FWCore/Framework/test/test_non_event_ordering_endRun_cfg.py index 6bf298262e9e2..7e5cd37db381c 100644 --- a/FWCore/Framework/test/test_non_event_ordering_endRun_cfg.py +++ b/FWCore/Framework/test/test_non_event_ordering_endRun_cfg.py @@ -6,9 +6,9 @@ process.maxEvents.input = 3 -process.a = cms.EDProducer("NonEventIntProducer", ivalue = cms.int32(1)) +process.d = cms.EDProducer("NonEventIntProducer", ivalue = cms.int32(1)) process.b = cms.EDProducer("NonEventIntProducer", ivalue = cms.int32(2), consumesEndRun = cms.InputTag("c","endRun"), expectEndRun = cms.untracked.int32(3) ) -process.c = cms.EDProducer("NonEventIntProducer", ivalue = cms.int32(3), consumesEndRun = cms.InputTag("a", "endRun"), expectEndRun = cms.untracked.int32(1) ) +process.c = cms.EDProducer("NonEventIntProducer", ivalue = cms.int32(3), consumesEndRun = cms.InputTag("d", "endRun"), expectEndRun = cms.untracked.int32(1) ) -process.t = cms.Task(process.a, process.c) +process.t = cms.Task(process.d, process.c) process.p = cms.Path(process.b, process.t) From eaac76f1f026d5ed30c82ce7acd9fd9c314d6d02 Mon Sep 17 00:00:00 2001 From: Matti Kortelainen Date: Thu, 4 Jun 2020 02:05:44 +0200 Subject: [PATCH 22/26] Disable module deletion if a job has an EDLooper --- FWCore/Framework/src/EventProcessor.cc | 33 +++++++++++------- FWCore/Framework/src/SubProcess.cc | 34 ++++++++++--------- FWCore/Framework/src/SubProcess.h | 2 +- .../Framework/test/run_module_delete_tests.sh | 2 ++ .../test/test_module_delete_looper_cfg.py | 21 ++++++++++++ 5 files changed, 62 insertions(+), 30 deletions(-) create mode 100644 FWCore/Framework/test/test_module_delete_looper_cfg.py diff --git a/FWCore/Framework/src/EventProcessor.cc b/FWCore/Framework/src/EventProcessor.cc index dae882afb392b..9e4d2bf5842ec 100644 --- a/FWCore/Framework/src/EventProcessor.cc +++ b/FWCore/Framework/src/EventProcessor.cc @@ -547,9 +547,13 @@ namespace edm { actReg_->preallocateSignal_(bounds); schedule_->convertCurrentProcessAlias(processConfiguration_->processName()); pathsAndConsumesOfModules_.initialize(schedule_.get(), preg()); + + // in presence of looper do not delete modules + bool const deleteModules = not looper_; + std::vector consumedBySubProcesses; - for_all(subProcesses_, [&consumedBySubProcesses](auto& subProcess) { - auto c = subProcess.keepOnlyConsumedUnscheduledModules(); + for_all(subProcesses_, [&consumedBySubProcesses, deleteModules](auto& subProcess) { + auto c = subProcess.keepOnlyConsumedUnscheduledModules(deleteModules); if (consumedBySubProcesses.empty()) { consumedBySubProcesses = std::move(c); } else if (not c.empty()) { @@ -563,19 +567,22 @@ namespace edm { // Note: all these may throw checkForModuleDependencyCorrectness(pathsAndConsumesOfModules_, printDependencies_); - if (auto const unusedModules = nonConsumedUnscheduledModules(pathsAndConsumesOfModules_, consumedBySubProcesses); - not unusedModules.empty()) { - pathsAndConsumesOfModules_.removeModules(unusedModules); - - edm::LogWarning("DeleteModules").log([&unusedModules](auto& l) { - l << "Following modules are not in any Path or EndPath, nor is their output consumed by any other module, and " - "therefore they are deleted before beginJob transition."; + if (deleteModules) { + if (auto const unusedModules = nonConsumedUnscheduledModules(pathsAndConsumesOfModules_, consumedBySubProcesses); + not unusedModules.empty()) { + pathsAndConsumesOfModules_.removeModules(unusedModules); + + edm::LogWarning("DeleteModules").log([&unusedModules](auto& l) { + l << "Following modules are not in any Path or EndPath, nor is their output consumed by any other module, " + "and " + "therefore they are deleted before beginJob transition."; + for (auto const& description : unusedModules) { + l << "\n " << description->moduleLabel(); + } + }); for (auto const& description : unusedModules) { - l << "\n " << description->moduleLabel(); + schedule_->deleteModule(description->moduleLabel(), actReg_.get()); } - }); - for (auto const& description : unusedModules) { - schedule_->deleteModule(description->moduleLabel(), actReg_.get()); } } diff --git a/FWCore/Framework/src/SubProcess.cc b/FWCore/Framework/src/SubProcess.cc index b35b2b67ec4e8..22b65f29ef6c3 100644 --- a/FWCore/Framework/src/SubProcess.cc +++ b/FWCore/Framework/src/SubProcess.cc @@ -220,8 +220,7 @@ namespace edm { SubProcess::~SubProcess() {} - std::vector SubProcess::keepOnlyConsumedUnscheduledModules() { - ServiceRegistry::Operate operate(serviceToken_); + std::vector SubProcess::keepOnlyConsumedUnscheduledModules(bool deleteModules) { schedule_->convertCurrentProcessAlias(processConfiguration_->processName()); pathsAndConsumesOfModules_.initialize(schedule_.get(), preg_); @@ -230,8 +229,8 @@ namespace edm { // Consumes information from the child SubProcesses std::vector consumedByChildren; - for_all(subProcesses_, [&consumedByChildren](auto& subProcess) { - auto c = subProcess.keepOnlyConsumedUnscheduledModules(); + for_all(subProcesses_, [&consumedByChildren, deleteModules](auto& subProcess) { + auto c = subProcess.keepOnlyConsumedUnscheduledModules(deleteModules); if (consumedByChildren.empty()) { std::swap(consumedByChildren, c); } else if (not c.empty()) { @@ -243,20 +242,23 @@ namespace edm { }); // Non-consumed unscheduled modules in this SubProcess, take into account of the consumes from child SubProcesses - if (auto const unusedModules = nonConsumedUnscheduledModules(pathsAndConsumesOfModules_, consumedByChildren); - not unusedModules.empty()) { - pathsAndConsumesOfModules_.removeModules(unusedModules); - - edm::LogWarning("DeleteModules").log([&unusedModules, this](auto& l) { - l << "Following modules are not in any Path or EndPath, nor is their output consumed by any other module, and " - "therefore they are deleted from SubProcess " - << processConfiguration_->processName() << " before beginJob transition."; + if (deleteModules) { + if (auto const unusedModules = nonConsumedUnscheduledModules(pathsAndConsumesOfModules_, consumedByChildren); + not unusedModules.empty()) { + pathsAndConsumesOfModules_.removeModules(unusedModules); + + edm::LogWarning("DeleteModules").log([&unusedModules, this](auto& l) { + l << "Following modules are not in any Path or EndPath, nor is their output consumed by any other module, " + "and " + "therefore they are deleted from SubProcess " + << processConfiguration_->processName() << " before beginJob transition."; + for (auto const& description : unusedModules) { + l << "\n " << description->moduleLabel(); + } + }); for (auto const& description : unusedModules) { - l << "\n " << description->moduleLabel(); + schedule_->deleteModule(description->moduleLabel(), actReg_.get()); } - }); - for (auto const& description : unusedModules) { - schedule_->deleteModule(description->moduleLabel(), actReg_.get()); } } diff --git a/FWCore/Framework/src/SubProcess.h b/FWCore/Framework/src/SubProcess.h index 2f662f11c179c..ac82e3ecfebaa 100644 --- a/FWCore/Framework/src/SubProcess.h +++ b/FWCore/Framework/src/SubProcess.h @@ -78,7 +78,7 @@ namespace edm { // Returns the set of modules whose products may be consumed by // modules in this SubProcess or its child SubProcesses - std::vector keepOnlyConsumedUnscheduledModules(); + std::vector keepOnlyConsumedUnscheduledModules(bool deleteModules); void doBeginJob(); void doEndJob(); diff --git a/FWCore/Framework/test/run_module_delete_tests.sh b/FWCore/Framework/test/run_module_delete_tests.sh index 93a7c8dc11240..097aaa9beb0fe 100755 --- a/FWCore/Framework/test/run_module_delete_tests.sh +++ b/FWCore/Framework/test/run_module_delete_tests.sh @@ -11,3 +11,5 @@ cmsRun $TEST_DIR/test_module_delete_subprocess_cfg.py || die "module deletion te echo "module deletion test with subprocess succeeded" cmsRun $TEST_DIR/test_module_delete_improperDependencies_cfg.py && die "module deletion with improper module ordering test failed" $? echo "module deletion test with improper module ordering succeeded" +cmsRun $TEST_DIR/test_module_delete_looper_cfg.py || die "module deletetion test with looper failed" $? +echo "module deletion test with looper succeeded" diff --git a/FWCore/Framework/test/test_module_delete_looper_cfg.py b/FWCore/Framework/test/test_module_delete_looper_cfg.py new file mode 100644 index 0000000000000..86a5f16887ddc --- /dev/null +++ b/FWCore/Framework/test/test_module_delete_looper_cfg.py @@ -0,0 +1,21 @@ +import FWCore.ParameterSet.Config as cms + +process = cms.Process("TESTMODULEDELETE") + +process.maxEvents.input = 4 +process.source = cms.Source("EmptySource") + +# In presence of looper nothing is deleted +process.producerEventNotConsumed = cms.EDProducer("edmtest::MustRunIntProducer", ivalue = cms.int32(1), mustRunEvent = cms.bool(False)) + +process.pInt = cms.EDProducer("IntProducer", + ivalue = cms.int32(1) +) +#Dummy looper will loop twice +process.looper = cms.Looper("TestModuleChangeLooper", + startingValue = cms.untracked.int32(1), + tag = cms.untracked.InputTag("pInt") +) + +process.t1 = cms.Task(process.producerEventNotConsumed) +process.p1 = cms.Path(process.pInt, process.t1) From f77931e482cf743ad653a932bc1c5acd058ecd36 Mon Sep 17 00:00:00 2001 From: Matti Kortelainen Date: Thu, 4 Jun 2020 03:52:30 +0200 Subject: [PATCH 23/26] Protect DependencyGraph for module IDs larger than the number of modules --- .../interface/PathsAndConsumesOfModules.h | 2 + .../src/PathsAndConsumesOfModules.cc | 5 + .../Framework/test/run_module_delete_tests.sh | 2 + .../test_module_delete_dependencygraph_cfg.py | 184 ++++++++++++++++++ .../interface/PathsAndConsumesOfModulesBase.h | 3 + FWCore/Services/plugins/DependencyGraph.cc | 4 +- 6 files changed, 198 insertions(+), 2 deletions(-) create mode 100644 FWCore/Framework/test/test_module_delete_dependencygraph_cfg.py diff --git a/FWCore/Framework/interface/PathsAndConsumesOfModules.h b/FWCore/Framework/interface/PathsAndConsumesOfModules.h index c642a17be5f7f..c8bc758e946e0 100644 --- a/FWCore/Framework/interface/PathsAndConsumesOfModules.h +++ b/FWCore/Framework/interface/PathsAndConsumesOfModules.h @@ -55,6 +55,8 @@ namespace edm { std::vector doConsumesInfo(unsigned int moduleID) const override; + unsigned int doLargestModuleID() const override; + unsigned int moduleIndex(unsigned int moduleID) const; // data members diff --git a/FWCore/Framework/src/PathsAndConsumesOfModules.cc b/FWCore/Framework/src/PathsAndConsumesOfModules.cc index 666253850dabe..646d22834cebe 100644 --- a/FWCore/Framework/src/PathsAndConsumesOfModules.cc +++ b/FWCore/Framework/src/PathsAndConsumesOfModules.cc @@ -123,6 +123,11 @@ namespace edm { return worker->consumesInfo(); } + unsigned int PathsAndConsumesOfModules::doLargestModuleID() const { + // moduleIDToIndex_ is sorted, so last element has the largest ID + return moduleIDToIndex_.empty() ? 0 : moduleIDToIndex_.back().first; + } + unsigned int PathsAndConsumesOfModules::moduleIndex(unsigned int moduleID) const { unsigned int dummy = 0; auto target = std::make_pair(moduleID, dummy); diff --git a/FWCore/Framework/test/run_module_delete_tests.sh b/FWCore/Framework/test/run_module_delete_tests.sh index 097aaa9beb0fe..2540d379210e5 100755 --- a/FWCore/Framework/test/run_module_delete_tests.sh +++ b/FWCore/Framework/test/run_module_delete_tests.sh @@ -13,3 +13,5 @@ cmsRun $TEST_DIR/test_module_delete_improperDependencies_cfg.py && die "module d echo "module deletion test with improper module ordering succeeded" cmsRun $TEST_DIR/test_module_delete_looper_cfg.py || die "module deletetion test with looper failed" $? echo "module deletion test with looper succeeded" +cmsRun $TEST_DIR/test_module_delete_dependencygraph_cfg.py || die "module deletetion test with DependencyGraph failed" $? +echo "module deletion test with DependencyGraph succeeded" diff --git a/FWCore/Framework/test/test_module_delete_dependencygraph_cfg.py b/FWCore/Framework/test/test_module_delete_dependencygraph_cfg.py new file mode 100644 index 0000000000000..ecc809d9ce7f7 --- /dev/null +++ b/FWCore/Framework/test/test_module_delete_dependencygraph_cfg.py @@ -0,0 +1,184 @@ +import FWCore.ParameterSet.Config as cms + +process = cms.Process("A") + +process.maxEvents.input = 1 +process.source = cms.Source("EmptySource") + +process.load("FWCore.Services.DependencyGraph_cfi") +process.DependencyGraph.fileName = "test_module_delete_dependencygraph.gv" + +intEventProducer = cms.EDProducer("IntProducer", ivalue = cms.int32(1)) +intEventProducerMustRun = cms.EDProducer("edmtest::MustRunIntProducer", ivalue = cms.int32(1), mustRunEvent = cms.bool(True)) +intEventConsumer = cms.EDAnalyzer("IntTestAnalyzer", + moduleLabel = cms.untracked.InputTag("producerEventConsumed"), + valueMustMatch = cms.untracked.int32(1) +) +intGenericConsumer = cms.EDAnalyzer("edmtest::GenericIntsAnalyzer", + srcEvent = cms.untracked.VInputTag(), + inputShouldExist = cms.untracked.bool(True) +) + +process.producerAEventConsumedInB = intEventProducer.clone(ivalue = 1) +process.producerAEventConsumedInBA = intEventProducer.clone(ivalue = 10) + +process.producerEventNotConsumed = cms.EDProducer("edmtest::TestModuleDeleteProducer") +process.producerBeginLumiNotConsumed = cms.EDProducer("edmtest::TestModuleDeleteInLumiProducer") +process.producerBeginRunNotConsumed = cms.EDProducer("edmtest::TestModuleDeleteInRunProducer") + +# These producers do not get the event transitions for the events +# where the same-name producers in the SubProcesses produce a product. +# Nevertheless, these producers must not be deleted early, because +# their event transitions might get called. +process.producerEventMaybeConsumedInB = intEventProducerMustRun.clone(mustRunEvent=False) +process.producerEventMaybeConsumedInBA = intEventProducerMustRun.clone(mustRunEvent=False) + +process.producerAEventNotConsumedInB = cms.EDProducer("edmtest::TestModuleDeleteProducer") +process.producerAEventNotConsumedInBA = cms.EDProducer("edmtest::TestModuleDeleteProducer") + +process.producerEventConsumedInB1 = intEventProducerMustRun.clone() +process.producerEventConsumedInB2 = intEventProducerMustRun.clone() +process.producerEventConsumedInBA1 = intEventProducerMustRun.clone() +process.producerEventConsumedInBA2 = intEventProducerMustRun.clone() + +process.intAnalyzerDelete = cms.EDAnalyzer("edmtest::TestModuleDeleteAnalyzer") + +process.t = cms.Task( + process.producerAEventConsumedInB, + # + process.producerAEventConsumedInBA, + # + process.producerEventNotConsumed, + process.producerBeginLumiNotConsumed, + process.producerBeginRunNotConsumed, + # + process.producerEventMaybeConsumedInB, + process.producerEventMaybeConsumedInBA, + # + process.producerAEventNotConsumedInB, + process.producerAEventNotConsumedInBA, + # + process.producerEventConsumedInB1, + process.producerEventConsumedInB2, + process.producerEventConsumedInBA1, + process.producerEventConsumedInBA2, +) + +process.p = cms.Path(process.intAnalyzerDelete, process.t) + +#################### +subprocessB = cms.Process("B") +process.addSubProcess( cms.SubProcess( + process = subprocessB, + SelectEvents = cms.untracked.PSet(), + outputCommands = cms.untracked.vstring() +) ) + +subprocessB.consumerEventFromA = intEventConsumer.clone(moduleLabel = "producerAEventConsumedInB", valueMustMatch = 1) + +subprocessB.producerEventNotConsumed = cms.EDProducer("edmtest::TestModuleDeleteProducer") + +subprocessB.producerEventMaybeConsumedInB = intEventProducerMustRun.clone() +subprocessB.producerEventMaybeConsumedInBA = intEventProducerMustRun.clone(mustRunEvent=False) +subprocessB.consumerEventMaybeInB = intGenericConsumer.clone(srcEvent = ["producerEventMaybeConsumedInB"]) + +subprocessB.producerAEventNotConsumedInB = intEventProducerMustRun.clone() +subprocessB.producerAEventNotConsumedInBA = cms.EDProducer("edmtest::TestModuleDeleteProducer") +subprocessB.consumerAEventNotConsumedInB = intGenericConsumer.clone(srcEvent = ["producerAEventNotConsumedInB::B"]) + +subprocessB.producerEventConsumedInB1 = cms.EDProducer("edmtest::TestModuleDeleteProducer") +subprocessB.producerEventConsumedInB2 = cms.EDProducer("edmtest::TestModuleDeleteProducer") +subprocessB.consumerEventNotConsumedInB1 = intGenericConsumer.clone(srcEvent = ["producerEventConsumedInB1::A"]) +subprocessB.consumerEventNotConsumedInB2 = intGenericConsumer.clone(srcEvent = [cms.InputTag("producerEventConsumedInB2", "", cms.InputTag.skipCurrentProcess())]) +subprocessB.producerBEventConsumedInBA1 = intEventProducerMustRun.clone() +subprocessB.producerBEventConsumedInBA2 = intEventProducerMustRun.clone() + +subprocessB.producerBEventConsumedInB1 = intEventProducerMustRun.clone() +subprocessB.producerBEventConsumedInB2 = intEventProducerMustRun.clone() +subprocessB.producerBEventConsumedInB3 = intEventProducerMustRun.clone() +subprocessB.consumerBEventConsumedInB1 = intGenericConsumer.clone(srcEvent = ["producerBEventConsumedInB1"]) +subprocessB.consumerBEventConsumedInB2 = intGenericConsumer.clone(srcEvent = ["producerBEventConsumedInB2::B"]) +subprocessB.consumerBEventConsumedInB3 = intGenericConsumer.clone(srcEvent = [cms.InputTag("producerBEventConsumedInB3", "", cms.InputTag.currentProcess())]) + + +subprocessB.t = cms.Task( + subprocessB.producerEventNotConsumed, + # + subprocessB.producerEventMaybeConsumedInB, + subprocessB.producerEventMaybeConsumedInBA, + # + subprocessB.producerAEventNotConsumedInB, + subprocessB.producerAEventNotConsumedInBA, + # + subprocessB.producerEventConsumedInB1, + subprocessB.producerEventConsumedInB2, + subprocessB.producerBEventConsumedInBA1, + subprocessB.producerBEventConsumedInBA2, + # + subprocessB.producerBEventConsumedInB1, + subprocessB.producerBEventConsumedInB2, + subprocessB.producerBEventConsumedInB3, +) +subprocessB.p = cms.Path( + subprocessB.consumerEventFromA+ + # + subprocessB.consumerEventMaybeInB+ + # + subprocessB.consumerAEventNotConsumedInB+ + subprocessB.consumerEventNotConsumedInB1+ + subprocessB.consumerEventNotConsumedInB2+ + # + subprocessB.consumerBEventConsumedInB1+ + subprocessB.consumerBEventConsumedInB2+ + subprocessB.consumerBEventConsumedInB3 + ,subprocessB.t +) + +#################### +subprocessBA = cms.Process("BA") +subprocessB.addSubProcess( cms.SubProcess( + process = subprocessBA, + SelectEvents = cms.untracked.PSet(), + outputCommands = cms.untracked.vstring() +) ) + +subprocessBA.consumerEventFromA = intEventConsumer.clone(moduleLabel = "producerAEventConsumedInBA", valueMustMatch = 10) + +subprocessBA.producerEventMaybeConsumedInBA = intEventProducerMustRun.clone() +subprocessBA.consumerEventMaybeInBA = intGenericConsumer.clone(srcEvent = ["producerEventMaybeConsumedInBA"]) + +subprocessBA.producerAEventNotConsumedInBA = intEventProducerMustRun.clone() +subprocessBA.consumerAEventNotConsumedInBA = intGenericConsumer.clone(srcEvent = ["producerAEventNotConsumedInBA::BA"]) + +subprocessBA.producerEventConsumedInBA1 = cms.EDProducer("edmtest::TestModuleDeleteProducer") +subprocessBA.producerEventConsumedInBA2 = cms.EDProducer("edmtest::TestModuleDeleteProducer") +subprocessBA.producerBEventConsumedInBA1 = cms.EDProducer("edmtest::TestModuleDeleteProducer") +subprocessBA.producerBEventConsumedInBA2 = cms.EDProducer("edmtest::TestModuleDeleteProducer") +subprocessBA.consumerEventNotConsumedInBA1 = intGenericConsumer.clone(srcEvent = ["producerEventConsumedInBA1::A", + "producerBEventConsumedInBA1::B"]) +subprocessBA.consumerEventNotConsumedInBA2 = intGenericConsumer.clone(srcEvent = [ + cms.InputTag("producerEventConsumedInBA2", "", cms.InputTag.skipCurrentProcess()), + cms.InputTag("producerBEventConsumedInBA2", "", cms.InputTag.skipCurrentProcess()) +]) + +subprocessBA.t = cms.Task( + subprocessBA.producerEventMaybeConsumedInBA, + # + subprocessBA.producerAEventNotConsumedInBA, + # + subprocessBA.producerEventConsumedInBA1, + subprocessBA.producerEventConsumedInBA2, + subprocessBA.producerBEventConsumedInBA1, + subprocessBA.producerBEventConsumedInBA2, +) +subprocessBA.p = cms.Path( + subprocessBA.consumerEventFromA+ + # + subprocessBA.consumerEventMaybeInBA+ + # + subprocessBA.consumerAEventNotConsumedInBA+ + # + subprocessBA.consumerEventNotConsumedInBA1+ + subprocessBA.consumerEventNotConsumedInBA2 + , subprocessBA.t +) diff --git a/FWCore/ServiceRegistry/interface/PathsAndConsumesOfModulesBase.h b/FWCore/ServiceRegistry/interface/PathsAndConsumesOfModulesBase.h index 99882765ab8c8..efab365fe6ec6 100644 --- a/FWCore/ServiceRegistry/interface/PathsAndConsumesOfModulesBase.h +++ b/FWCore/ServiceRegistry/interface/PathsAndConsumesOfModulesBase.h @@ -75,6 +75,8 @@ namespace edm { // than just a pointer. std::vector consumesInfo(unsigned int moduleID) const { return doConsumesInfo(moduleID); } + unsigned int largestModuleID() const { return doLargestModuleID(); } + private: virtual std::vector const& doPaths() const = 0; virtual std::vector const& doEndPaths() const = 0; @@ -85,6 +87,7 @@ namespace edm { virtual std::vector const& doModulesWhoseProductsAreConsumedBy( unsigned int moduleID, BranchType branchType) const = 0; virtual std::vector doConsumesInfo(unsigned int moduleID) const = 0; + virtual unsigned int doLargestModuleID() const = 0; }; } // namespace edm #endif diff --git a/FWCore/Services/plugins/DependencyGraph.cc b/FWCore/Services/plugins/DependencyGraph.cc index 0e98319ac386a..8ec47c5ea751e 100644 --- a/FWCore/Services/plugins/DependencyGraph.cc +++ b/FWCore/Services/plugins/DependencyGraph.cc @@ -222,7 +222,7 @@ void DependencyGraph::preBeginJob(PathsAndConsumesOfModulesBase const &pathsAndC boost::get_property(m_graph, boost::graph_graph_attribute)["labelloc"] = "top"; // create graph vertices associated to all modules in the process - auto size = pathsAndConsumes.allModules().size(); + auto size = pathsAndConsumes.largestModuleID() - boost::num_vertices(m_graph) + 1; for (size_t i = 0; i < size; ++i) boost::add_vertex(m_graph); @@ -237,7 +237,7 @@ void DependencyGraph::preBeginJob(PathsAndConsumesOfModulesBase const &pathsAndC boost::get_property(graph, boost::graph_graph_attribute)["labelloc"] = "top"; // create graph vertices associated to all modules in the subprocess - auto size = pathsAndConsumes.allModules().size(); + auto size = pathsAndConsumes.largestModuleID() - boost::num_vertices(m_graph) + 1; for (size_t i = 0; i < size; ++i) boost::add_vertex(graph); } From 897ef777473183869ea0f6b87318289f8554f643 Mon Sep 17 00:00:00 2001 From: Matti Kortelainen Date: Mon, 13 Jul 2020 21:44:29 +0200 Subject: [PATCH 24/26] Extend module deletion tests to ProcessBlock --- .../Framework/test/stubs/TestModuleDelete.cc | 63 +++++++++++++++++++ FWCore/Framework/test/stubs/ToyAnalyzers.cc | 12 +++- .../Framework/test/test_module_delete_cfg.py | 18 +++++- .../test_module_delete_dependencygraph_cfg.py | 2 + .../test/test_module_delete_subprocess_cfg.py | 37 ++++++++--- 5 files changed, 121 insertions(+), 11 deletions(-) diff --git a/FWCore/Framework/test/stubs/TestModuleDelete.cc b/FWCore/Framework/test/stubs/TestModuleDelete.cc index 4a335058d25fc..524a75b24e9e6 100644 --- a/FWCore/Framework/test/stubs/TestModuleDelete.cc +++ b/FWCore/Framework/test/stubs/TestModuleDelete.cc @@ -21,6 +21,9 @@ namespace { std::vector g_run_constructed; std::vector g_run_destructed; + + std::vector g_process_constructed; + std::vector g_process_destructed; } // namespace namespace edmtest { @@ -29,6 +32,9 @@ namespace edmtest { public: explicit TestModuleDeleteProducer(edm::ParameterSet const& p) : moduleLabel_{p.getParameter("@module_label")} { + for (const auto& tag : p.getUntrackedParameter>("srcBeginProcess")) { + consumes(tag); + } for (const auto& tag : p.getUntrackedParameter>("srcBeginRun")) { consumes(tag); } @@ -44,6 +50,7 @@ namespace edmtest { ~TestModuleDeleteProducer() override { g_destructed.push_back(moduleLabel_); } static void fillDescriptions(edm::ConfigurationDescriptions& descriptions) { edm::ParameterSetDescription desc; + desc.addUntracked>("srcBeginProcess", std::vector{}); desc.addUntracked>("srcBeginRun", std::vector{}); desc.addUntracked>("srcBeginLumi", std::vector{}); desc.addUntracked>("srcEvent", std::vector{}); @@ -61,6 +68,9 @@ namespace edmtest { public: explicit TestModuleDeleteInLumiProducer(edm::ParameterSet const& p) : moduleLabel_{p.getParameter("@module_label")} { + for (const auto& tag : p.getUntrackedParameter>("srcBeginProcess")) { + consumes(tag); + } for (const auto& tag : p.getUntrackedParameter>("srcBeginRun")) { consumes(tag); } @@ -76,6 +86,7 @@ namespace edmtest { ~TestModuleDeleteInLumiProducer() override { g_lumi_destructed.push_back(moduleLabel_); } static void fillDescriptions(edm::ConfigurationDescriptions& descriptions) { edm::ParameterSetDescription desc; + desc.addUntracked>("srcBeginProcess", std::vector{}); desc.addUntracked>("srcBeginRun", std::vector{}); desc.addUntracked>("srcBeginLumi", std::vector{}); desc.addUntracked>("srcEvent", std::vector{}); @@ -96,6 +107,9 @@ namespace edmtest { public: explicit TestModuleDeleteInRunProducer(edm::ParameterSet const& p) : moduleLabel_{p.getParameter("@module_label")} { + for (const auto& tag : p.getUntrackedParameter>("srcBeginProcess")) { + consumes(tag); + } for (const auto& tag : p.getUntrackedParameter>("srcBeginRun")) { consumes(tag); } @@ -111,6 +125,7 @@ namespace edmtest { ~TestModuleDeleteInRunProducer() override { g_run_destructed.push_back(moduleLabel_); } static void fillDescriptions(edm::ConfigurationDescriptions& descriptions) { edm::ParameterSetDescription desc; + desc.addUntracked>("srcBeginProcess", std::vector{}); desc.addUntracked>("srcBeginRun", std::vector{}); desc.addUntracked>("srcBeginLumi", std::vector{}); desc.addUntracked>("srcEvent", std::vector{}); @@ -127,6 +142,41 @@ namespace edmtest { std::string moduleLabel_; }; + class TestModuleDeleteInProcessProducer : public edm::global::EDProducer { + public: + explicit TestModuleDeleteInProcessProducer(edm::ParameterSet const& p) + : moduleLabel_{p.getParameter("@module_label")} { + for (const auto& tag : p.getUntrackedParameter>("srcBeginRun")) { + consumes(tag); + } + for (const auto& tag : p.getUntrackedParameter>("srcBeginLumi")) { + consumes(tag); + } + for (const auto& tag : p.getUntrackedParameter>("srcEvent")) { + consumes(tag); + } + produces(); + g_process_constructed.push_back(moduleLabel_); + } + ~TestModuleDeleteInProcessProducer() override { g_process_destructed.push_back(moduleLabel_); } + static void fillDescriptions(edm::ConfigurationDescriptions& descriptions) { + edm::ParameterSetDescription desc; + desc.addUntracked>("srcBeginRun", std::vector{}); + desc.addUntracked>("srcBeginLumi", std::vector{}); + desc.addUntracked>("srcEvent", std::vector{}); + descriptions.addDefault(desc); + } + void beginProcessBlockProduce(edm::ProcessBlock&) const override { + throw edm::Exception(edm::errors::NotFound) << "Intentional 'NotFound' exception for testing purposes\n"; + } + void produce(edm::StreamID, edm::Event& e, edm::EventSetup const& c) const override { + throw edm::Exception(edm::errors::NotFound) << "Intentional 'NotFound' exception for testing purposes\n"; + } + + private: + std::string moduleLabel_; + }; + class TestModuleDeleteAnalyzer : public edm::global::EDAnalyzer<> { public: explicit TestModuleDeleteAnalyzer(edm::ParameterSet const& /*p*/){}; @@ -185,6 +235,18 @@ namespace edmtest { formatException(g_run_constructed, g_run_destructed, ex); throw ex; } + + if (g_process_constructed.size() == 0) { + throw cms::Exception("LogicError") + << "No TestModuleDeleteInProcessProducer modules constructed in this job, the test is meaningless"; + } + if (g_process_constructed.size() != g_process_destructed.size()) { + cms::Exception ex("Assert"); + ex << "Number of TestModuleDeleteInProcessProducer constructors " << g_process_constructed.size() + << " differs from the number of destructors " << g_process_destructed.size() << "."; + formatException(g_process_constructed, g_process_destructed, ex); + throw ex; + } } void analyze(edm::StreamID, edm::Event const& e, edm::EventSetup const& c) const override {} @@ -194,4 +256,5 @@ namespace edmtest { DEFINE_FWK_MODULE(edmtest::TestModuleDeleteProducer); DEFINE_FWK_MODULE(edmtest::TestModuleDeleteInLumiProducer); DEFINE_FWK_MODULE(edmtest::TestModuleDeleteInRunProducer); +DEFINE_FWK_MODULE(edmtest::TestModuleDeleteInProcessProducer); DEFINE_FWK_MODULE(edmtest::TestModuleDeleteAnalyzer); diff --git a/FWCore/Framework/test/stubs/ToyAnalyzers.cc b/FWCore/Framework/test/stubs/ToyAnalyzers.cc index 1c76f1ae22494..a82e0c4b0ac56 100644 --- a/FWCore/Framework/test/stubs/ToyAnalyzers.cc +++ b/FWCore/Framework/test/stubs/ToyAnalyzers.cc @@ -109,7 +109,10 @@ namespace edmtest { class GenericAnalyzerT : public edm::global::EDAnalyzer<> { public: GenericAnalyzerT(edm::ParameterSet const& iPSet) - : tokensBeginRun_( + : tokensBeginProcess_(edm::vector_transform( + iPSet.getUntrackedParameter>("srcBeginProcess"), + [this](edm::InputTag const& tag) { return this->consumes(tag); })), + tokensBeginRun_( edm::vector_transform(iPSet.getUntrackedParameter>("srcBeginRun"), [this](edm::InputTag const& tag) { return this->consumes(tag); })), tokensBeginLumi_( @@ -123,6 +126,9 @@ namespace edmtest { tokensEndRun_( edm::vector_transform(iPSet.getUntrackedParameter>("srcEndRun"), [this](edm::InputTag const& tag) { return this->consumes(tag); })), + tokensEndProcess_(edm::vector_transform( + iPSet.getUntrackedParameter>("srcEndProcess"), + [this](edm::InputTag const& tag) { return this->consumes(tag); })), shouldExist_(iPSet.getUntrackedParameter("inputShouldExist")), shouldBeMissing_(iPSet.getUntrackedParameter("inputShouldBeMissing")) { if (shouldExist_ and shouldBeMissing_) { @@ -150,22 +156,26 @@ namespace edmtest { static void fillDescriptions(edm::ConfigurationDescriptions& descriptions) { edm::ParameterSetDescription desc; + desc.addUntracked>("srcBeginProcess", std::vector{}); desc.addUntracked>("srcBeginRun", std::vector{}); desc.addUntracked>("srcBeginLumi", std::vector{}); desc.addUntracked>("srcEvent", std::vector{}); desc.addUntracked>("srcEndLumi", std::vector{}); desc.addUntracked>("srcEndRun", std::vector{}); + desc.addUntracked>("srcEndProcess", std::vector{}); desc.addUntracked("inputShouldExist", false); desc.addUntracked("inputShouldBeMissing", false); descriptions.addDefault(desc); } private: + const std::vector> tokensBeginProcess_; const std::vector> tokensBeginRun_; const std::vector> tokensBeginLumi_; const std::vector> tokensEvent_; const std::vector> tokensEndLumi_; const std::vector> tokensEndRun_; + const std::vector> tokensEndProcess_; const bool shouldExist_; const bool shouldBeMissing_; }; diff --git a/FWCore/Framework/test/test_module_delete_cfg.py b/FWCore/Framework/test/test_module_delete_cfg.py index ffc1875460eb3..c8304da748b40 100644 --- a/FWCore/Framework/test/test_module_delete_cfg.py +++ b/FWCore/Framework/test/test_module_delete_cfg.py @@ -52,6 +52,7 @@ def nonEventConsumer(transition, sourcePattern, expected): process.producerEventNotConsumed = cms.EDProducer("edmtest::TestModuleDeleteProducer") process.producerBeginLumiNotConsumed = cms.EDProducer("edmtest::TestModuleDeleteInLumiProducer") process.producerBeginRunNotConsumed = cms.EDProducer("edmtest::TestModuleDeleteInRunProducer") +process.producerBeginProcessNotConsumed = cms.EDProducer("edmtest::TestModuleDeleteInProcessProducer") process.producerEventNotConsumedChain1 = cms.EDProducer("edmtest::TestModuleDeleteProducer") process.producerEventNotConsumedChain2 = cms.EDProducer("edmtest::TestModuleDeleteProducer", @@ -69,12 +70,18 @@ def nonEventConsumer(transition, sourcePattern, expected): process.producerEventNotConsumedChain6 = cms.EDProducer("edmtest::TestModuleDeleteInRunProducer", srcBeginLumi = cms.untracked.VInputTag("producerEventNotConsumedChain5") ) -process.producerEventNotConsumedChain7 = cms.EDProducer("edmtest::TestModuleDeleteInLumiProducer", +process.producerEventNotConsumedChain7 = cms.EDProducer("edmtest::TestModuleDeleteInProcessProducer", srcBeginRun = cms.untracked.VInputTag("producerEventNotConsumedChain6") ) -process.producerEventNotConsumedChain8 = cms.EDProducer("edmtest::TestModuleDeleteProducer", +process.producerEventNotConsumedChain8 = cms.EDProducer("edmtest::TestModuleDeleteInRunProducer", srcBeginLumi = cms.untracked.VInputTag("producerEventNotConsumedChain7") ) +process.producerEventNotConsumedChain9 = cms.EDProducer("edmtest::TestModuleDeleteInLumiProducer", + srcBeginRun = cms.untracked.VInputTag("producerEventNotConsumedChain8") +) +process.producerEventNotConsumedChain10 = cms.EDProducer("edmtest::TestModuleDeleteProducer", + srcBeginLumi = cms.untracked.VInputTag("producerEventNotConsumedChain9") +) process.producerEventPartiallyConsumedChain1 = intEventProducerMustRun.clone() process.producerEventPartiallyConsumedChain2 = cms.EDProducer("AddIntsProducer", labels = cms.VInputTag("producerEventPartiallyConsumedChain1")) @@ -123,6 +130,10 @@ def __init__(self, **kargs): process.consumerNotExist = cms.EDAnalyzer("edmtest::GenericIntsAnalyzer", inputShouldBeMissing = cms.untracked.bool(True), + srcBeginProcess = cms.untracked.VInputTag( + "producerBeginProcessNotConsumed:doesNotExist", + cms.InputTag("producerBeginProcessNotConsumed", "", cms.InputTag.skipCurrentProcess()) + ), srcBeginRun = cms.untracked.VInputTag( "producerBeginRunNotConsumed:doesNotExist", cms.InputTag("producerBeginRunNotConsumed", "", cms.InputTag.skipCurrentProcess()) @@ -149,6 +160,7 @@ def __init__(self, **kargs): process.producerEventNotConsumed, process.producerBeginLumiNotConsumed, process.producerBeginRunNotConsumed, + process.producerBeginProcessNotConsumed, # process.producerEventNotConsumedChain1, process.producerEventNotConsumedChain2, @@ -158,6 +170,8 @@ def __init__(self, **kargs): process.producerEventNotConsumedChain6, process.producerEventNotConsumedChain7, process.producerEventNotConsumedChain8, + process.producerEventNotConsumedChain9, + process.producerEventNotConsumedChain10, # process.producerEventPartiallyConsumedChain1, process.producerEventPartiallyConsumedChain3, diff --git a/FWCore/Framework/test/test_module_delete_dependencygraph_cfg.py b/FWCore/Framework/test/test_module_delete_dependencygraph_cfg.py index ecc809d9ce7f7..0974efa4db6d4 100644 --- a/FWCore/Framework/test/test_module_delete_dependencygraph_cfg.py +++ b/FWCore/Framework/test/test_module_delete_dependencygraph_cfg.py @@ -25,6 +25,7 @@ process.producerEventNotConsumed = cms.EDProducer("edmtest::TestModuleDeleteProducer") process.producerBeginLumiNotConsumed = cms.EDProducer("edmtest::TestModuleDeleteInLumiProducer") process.producerBeginRunNotConsumed = cms.EDProducer("edmtest::TestModuleDeleteInRunProducer") +process.producerBeginProcessNotConsumed = cms.EDProducer("edmtest::TestModuleDeleteInProcessProducer") # These producers do not get the event transitions for the events # where the same-name producers in the SubProcesses produce a product. @@ -51,6 +52,7 @@ process.producerEventNotConsumed, process.producerBeginLumiNotConsumed, process.producerBeginRunNotConsumed, + process.producerBeginProcessNotConsumed, # process.producerEventMaybeConsumedInB, process.producerEventMaybeConsumedInBA, diff --git a/FWCore/Framework/test/test_module_delete_subprocess_cfg.py b/FWCore/Framework/test/test_module_delete_subprocess_cfg.py index 78486d89ee8de..25be66a44bc97 100644 --- a/FWCore/Framework/test/test_module_delete_subprocess_cfg.py +++ b/FWCore/Framework/test/test_module_delete_subprocess_cfg.py @@ -21,9 +21,9 @@ # \- DA # Cases to test -# - event/lumi/run product consumed in a B or C, module kept -# - event/lumi/run product consumed in BA, module kept -# - event/lumi/run product not consumed anywhere, module deleted +# - event/lumi/run/process product consumed in B or C, module kept +# - event/lumi/run/process product consumed in BA, module kept +# - event/lumi/run/process product not consumed anywhere, module deleted # - event(/lumi/run) product produced in A and any SubProcess, consumed with empty process name, module kept # - event(/lumi/run) product produced in A and any SubProcess, consumed with SubProcess name, A module deleted # - event(/lumi/run) product produced in A and any SubProcess, consumed with A name or skipProcess, SubProcess module deleted @@ -49,29 +49,34 @@ def nonEventConsumer(transition, sourcePattern, expected): transCap = transition[0].upper() + transition[1:] - runOrLumiBlock = transCap - if "Lumi" in runOrLumiBlock: - runOrLumiBlock = runOrLumiBlock+"nosityBlock" + blockName = transCap + if "Lumi" in blockName: + blockName = blockName+"nosityBlock" ret = intNonEventProducer.clone() - setattr(ret, "consumes%s"%runOrLumiBlock, cms.InputTag(sourcePattern%transCap, transition)) - setattr(ret, "expect%s"%runOrLumiBlock, cms.untracked.int32(expected)) + setattr(ret, "consumes%s"%blockName, cms.InputTag(sourcePattern%transCap, transition)) + setattr(ret, "expect%s"%blockName, cms.untracked.int32(expected)) return ret process.producerAEventConsumedInB = intEventProducer.clone(ivalue = 1) process.producerABeginLumiConsumedInB = intNonEventProducer.clone(ivalue = 2) process.producerAEndRunConsumedInB = intNonEventProducer.clone(ivalue = 5) +process.producerABeginProcessBlockConsumedInB = intNonEventProducer.clone(ivalue = 6) process.producerAEndLumiConsumedInC = intNonEventProducer.clone(ivalue = 3) process.producerABeginRunConsumedInC = intNonEventProducer.clone(ivalue = 4) +process.producerAEndProcessBlockConsumedInC = intNonEventProducer.clone(ivalue = 7) process.producerAEventConsumedInBA = intEventProducer.clone(ivalue = 10) process.producerABeginLumiConsumedInBA = intNonEventProducer.clone(ivalue = 20) process.producerAEndLumiConsumedInBA = intNonEventProducer.clone(ivalue = 30) process.producerABeginRunConsumedInBA = intNonEventProducer.clone(ivalue = 40) process.producerAEndRunConsumedInBA = intNonEventProducer.clone(ivalue = 50) +process.producerABeginProcessBlockConsumedInBA = intNonEventProducer.clone(ivalue = 60) +process.producerAEndProcessBlockConsumedInBA = intNonEventProducer.clone(ivalue = 70) process.producerEventNotConsumed = cms.EDProducer("edmtest::TestModuleDeleteProducer") process.producerBeginLumiNotConsumed = cms.EDProducer("edmtest::TestModuleDeleteInLumiProducer") process.producerBeginRunNotConsumed = cms.EDProducer("edmtest::TestModuleDeleteInRunProducer") +process.producerBeginProcessBlockNotConsumed = cms.EDProducer("edmtest::TestModuleDeleteInProcessProducer") # These producers do not get the event transitions for the events # where the same-name producers in the SubProcesses produce a product. @@ -107,18 +112,23 @@ def nonEventConsumer(transition, sourcePattern, expected): process.producerAEventConsumedInB, process.producerABeginLumiConsumedInB, process.producerAEndRunConsumedInB, + process.producerABeginProcessBlockConsumedInB, process.producerAEndLumiConsumedInC, process.producerABeginRunConsumedInC, + process.producerAEndProcessBlockConsumedInC, # process.producerAEventConsumedInBA, process.producerABeginLumiConsumedInBA, process.producerAEndLumiConsumedInBA, process.producerABeginRunConsumedInBA, process.producerAEndRunConsumedInBA, + process.producerABeginProcessBlockConsumedInBA, + process.producerAEndProcessBlockConsumedInBA, # process.producerEventNotConsumed, process.producerBeginLumiNotConsumed, process.producerBeginRunNotConsumed, + process.producerBeginProcessBlockNotConsumed, # process.producerEventMaybeConsumedInB, process.producerEventMaybeConsumedInBA, @@ -140,6 +150,7 @@ def nonEventConsumer(transition, sourcePattern, expected): process.producerANotConsumedChainRun, ) + process.p = cms.Path( process.intAnalyzerDelete , @@ -157,6 +168,7 @@ def nonEventConsumer(transition, sourcePattern, expected): subprocessB.consumerEventFromA = intEventConsumer.clone(moduleLabel = "producerAEventConsumedInB", valueMustMatch = 1) subprocessB.consumerBeginLumiFromA = nonEventConsumer("beginLumi", "producerA%sConsumedInB", 2) subprocessB.consumerEndRunFromA = nonEventConsumer("endRun", "producerA%sConsumedInB", 5) +subprocessB.consumerBeginProcessBlockFromA = nonEventConsumer("beginProcessBlock", "producerA%sConsumedInB", 6) subprocessB.consumerAEventNotConsumed = intGenericConsumer.clone( srcEvent = [ @@ -177,6 +189,7 @@ def nonEventConsumer(transition, sourcePattern, expected): subprocessB.producerEventNotConsumed = cms.EDProducer("edmtest::TestModuleDeleteProducer") subprocessB.producerBeginLumiNotConsumed = cms.EDProducer("edmtest::TestModuleDeleteInLumiProducer") subprocessB.producerBeginRunNotConsumed = cms.EDProducer("edmtest::TestModuleDeleteInRunProducer") +subprocessB.producerBeginProcessBlockNotConsumed = cms.EDProducer("edmtest::TestModuleDeleteInProcessProducer") subprocessB.producerEventMaybeConsumedInB = intEventProducerMustRun.clone() subprocessB.producerEventMaybeConsumedInBA = intEventProducerMustRun.clone(mustRunEvent=False) @@ -217,6 +230,7 @@ def nonEventConsumer(transition, sourcePattern, expected): subprocessB.producerEventNotConsumed, subprocessB.producerBeginLumiNotConsumed, subprocessB.producerBeginRunNotConsumed, + subprocessB.producerBeginProcessBlockNotConsumed, # subprocessB.producerEventMaybeConsumedInB, subprocessB.producerEventMaybeConsumedInBA, @@ -244,6 +258,7 @@ def nonEventConsumer(transition, sourcePattern, expected): subprocessB.consumerEventFromA+ subprocessB.consumerBeginLumiFromA+ subprocessB.consumerEndRunFromA+ + subprocessB.consumerBeginProcessBlockFromA+ # subprocessB.consumerAEventNotConsumed+ subprocessB.consumerAEventNotConsumed2+ @@ -273,6 +288,8 @@ def nonEventConsumer(transition, sourcePattern, expected): subprocessBA.consumerEndLumiFromA = nonEventConsumer("endLumi", "producerA%sConsumedInBA", 30) subprocessBA.consumerBeginRunFromA = nonEventConsumer("beginRun", "producerA%sConsumedInBA", 40) subprocessBA.consumerEndRunFromA = nonEventConsumer("endRun", "producerA%sConsumedInBA", 50) +subprocessBA.consumerBeginProcessBlockFromA = nonEventConsumer("beginProcessBlock", "producerA%sConsumedInBA", 60) +subprocessBA.consumerEndProcessBlockFromA = nonEventConsumer("endProcessBlock", "producerA%sConsumedInBA", 70) subprocessBA.consumerABEventNotConsumed = intGenericConsumer.clone( srcEvent = [ @@ -347,6 +364,8 @@ def nonEventConsumer(transition, sourcePattern, expected): subprocessBA.consumerEndLumiFromA+ subprocessBA.consumerBeginRunFromA+ subprocessBA.consumerEndRunFromA+ + subprocessBA.consumerBeginProcessBlockFromA+ + subprocessBA.consumerEndProcessBlockFromA+ # subprocessBA.consumerABEventNotConsumed+ subprocessBA.consumerABEventNotConsumed2+ @@ -370,6 +389,7 @@ def nonEventConsumer(transition, sourcePattern, expected): subprocessC.consumerEndLumiFromA = nonEventConsumer("endLumi", "producerA%sConsumedInC", 3) subprocessC.consumerBeginRunFromA = nonEventConsumer("beginRun", "producerA%sConsumedInC", 4) +subprocessC.consumerEndProcessBlockFromA = nonEventConsumer("endProcessBlock", "producerA%sConsumedInC", 7) subprocessC.consumerAEventNotConsumed = intGenericConsumer.clone( srcEvent = [ @@ -427,6 +447,7 @@ def nonEventConsumer(transition, sourcePattern, expected): subprocessC.p = cms.Path( subprocessC.consumerEndLumiFromA+ subprocessC.consumerBeginRunFromA+ + subprocessC.consumerEndProcessBlockFromA+ # subprocessC.consumerAEventNotConsumed+ subprocessC.consumerAEventNotConsumed2+ From 9c053f91ae754eb0a0362d3bd612432b2dd4a265 Mon Sep 17 00:00:00 2001 From: Matti Kortelainen Date: Thu, 1 Oct 2020 20:33:03 +0200 Subject: [PATCH 25/26] code checks --- FWCore/Services/plugins/ConcurrentModuleTimer.cc | 4 +++- IOMC/RandomEngine/src/RandomNumberGeneratorService.cc | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/FWCore/Services/plugins/ConcurrentModuleTimer.cc b/FWCore/Services/plugins/ConcurrentModuleTimer.cc index aa4e168b9bcb4..37f30c0c403d7 100644 --- a/FWCore/Services/plugins/ConcurrentModuleTimer.cc +++ b/FWCore/Services/plugins/ConcurrentModuleTimer.cc @@ -9,6 +9,8 @@ // Original Author: Chris Jones // Created: Tue, 10 Dec 2013 21:16:00 GMT // +#include + #include #include #include @@ -148,7 +150,7 @@ ConcurrentModuleTimer::ConcurrentModuleTimer(edm::ParameterSet const& iConfig, e iReg.watchPreallocate([this](edm::service::SystemBounds const& iBounds) { m_nTimeSums = iBounds.maxNumberOfThreads() + 1 + m_padding; - m_timeSums.reset(new std::atomic[m_nTimeSums]); + m_timeSums = std::make_unique[]>(m_nTimeSums); for (unsigned int i = 0; i < m_nTimeSums; ++i) { m_timeSums[i] = 0; } diff --git a/IOMC/RandomEngine/src/RandomNumberGeneratorService.cc b/IOMC/RandomEngine/src/RandomNumberGeneratorService.cc index 1ce292684ceb4..49017f5bcead9 100644 --- a/IOMC/RandomEngine/src/RandomNumberGeneratorService.cc +++ b/IOMC/RandomEngine/src/RandomNumberGeneratorService.cc @@ -87,7 +87,7 @@ namespace edm { // The saveFileName must correspond to a file name without any path specification. // Throw if that is not true. - if (!saveFileName_.empty() && (saveFileName_.find("/") != std::string::npos)) { + if (!saveFileName_.empty() && (saveFileName_.find('/') != std::string::npos)) { throw Exception(errors::Configuration) << "The saveFileName parameter must be a simple file name with no path\n" << "specification. In the configuration, it was given the value \"" << saveFileName_ << "\"\n"; From c38022db35429a81db27c9b63bf09266b4613af3 Mon Sep 17 00:00:00 2001 From: Matti Kortelainen Date: Thu, 1 Oct 2020 22:01:37 +0200 Subject: [PATCH 26/26] Add configuration parameter to allow disabling the module deletion --- FWCore/Framework/interface/EventProcessor.h | 1 + FWCore/Framework/src/EventProcessor.cc | 37 +++++++++++-------- .../Framework/test/run_module_delete_tests.sh | 2 + .../test/test_module_delete_disable_cfg.py | 26 +++++++++++++ FWCore/ParameterSet/python/Config.py | 2 + .../src/validateTopLevelParameterSets.cc | 4 ++ 6 files changed, 56 insertions(+), 16 deletions(-) create mode 100644 FWCore/Framework/test/test_module_delete_disable_cfg.py diff --git a/FWCore/Framework/interface/EventProcessor.h b/FWCore/Framework/interface/EventProcessor.h index d7ef9d735c4fc..82fd029b31ee7 100644 --- a/FWCore/Framework/interface/EventProcessor.h +++ b/FWCore/Framework/interface/EventProcessor.h @@ -362,6 +362,7 @@ namespace edm { ExcludedDataMap eventSetupDataToExcludeFromPrefetching_; bool printDependencies_ = false; + bool deleteNonConsumedUnscheduledModules_ = true; }; // class EventProcessor //-------------------------------------------------------------------- diff --git a/FWCore/Framework/src/EventProcessor.cc b/FWCore/Framework/src/EventProcessor.cc index 9e4d2bf5842ec..8c575ebd9182c 100644 --- a/FWCore/Framework/src/EventProcessor.cc +++ b/FWCore/Framework/src/EventProcessor.cc @@ -399,6 +399,8 @@ namespace edm { IllegalParameters::setThrowAnException(optionsPset.getUntrackedParameter("throwIfIllegalParameter")); printDependencies_ = optionsPset.getUntrackedParameter("printDependencies"); + deleteNonConsumedUnscheduledModules_ = + optionsPset.getUntrackedParameter("deleteNonConsumedUnscheduledModules"); // Now do general initialization ScheduleItems items; @@ -433,6 +435,8 @@ namespace edm { nStreams = 1; nConcurrentLumis = 1; nConcurrentRuns = 1; + // in presence of looper do not delete modules + deleteNonConsumedUnscheduledModules_ = false; } espController_->setMaxConcurrentIOVs(nStreams, nConcurrentLumis); @@ -548,26 +552,27 @@ namespace edm { schedule_->convertCurrentProcessAlias(processConfiguration_->processName()); pathsAndConsumesOfModules_.initialize(schedule_.get(), preg()); - // in presence of looper do not delete modules - bool const deleteModules = not looper_; - std::vector consumedBySubProcesses; - for_all(subProcesses_, [&consumedBySubProcesses, deleteModules](auto& subProcess) { - auto c = subProcess.keepOnlyConsumedUnscheduledModules(deleteModules); - if (consumedBySubProcesses.empty()) { - consumedBySubProcesses = std::move(c); - } else if (not c.empty()) { - std::vector tmp; - tmp.reserve(consumedBySubProcesses.size() + c.size()); - std::merge( - consumedBySubProcesses.begin(), consumedBySubProcesses.end(), c.begin(), c.end(), std::back_inserter(tmp)); - std::swap(consumedBySubProcesses, tmp); - } - }); + for_all(subProcesses_, + [&consumedBySubProcesses, deleteModules = deleteNonConsumedUnscheduledModules_](auto& subProcess) { + auto c = subProcess.keepOnlyConsumedUnscheduledModules(deleteModules); + if (consumedBySubProcesses.empty()) { + consumedBySubProcesses = std::move(c); + } else if (not c.empty()) { + std::vector tmp; + tmp.reserve(consumedBySubProcesses.size() + c.size()); + std::merge(consumedBySubProcesses.begin(), + consumedBySubProcesses.end(), + c.begin(), + c.end(), + std::back_inserter(tmp)); + std::swap(consumedBySubProcesses, tmp); + } + }); // Note: all these may throw checkForModuleDependencyCorrectness(pathsAndConsumesOfModules_, printDependencies_); - if (deleteModules) { + if (deleteNonConsumedUnscheduledModules_) { if (auto const unusedModules = nonConsumedUnscheduledModules(pathsAndConsumesOfModules_, consumedBySubProcesses); not unusedModules.empty()) { pathsAndConsumesOfModules_.removeModules(unusedModules); diff --git a/FWCore/Framework/test/run_module_delete_tests.sh b/FWCore/Framework/test/run_module_delete_tests.sh index 2540d379210e5..3738258fd6611 100755 --- a/FWCore/Framework/test/run_module_delete_tests.sh +++ b/FWCore/Framework/test/run_module_delete_tests.sh @@ -15,3 +15,5 @@ cmsRun $TEST_DIR/test_module_delete_looper_cfg.py || die "module deletetion test echo "module deletion test with looper succeeded" cmsRun $TEST_DIR/test_module_delete_dependencygraph_cfg.py || die "module deletetion test with DependencyGraph failed" $? echo "module deletion test with DependencyGraph succeeded" +cmsRun $TEST_DIR/test_module_delete_disable_cfg.py || die "module deletetion test with disabling the deletion failed" $? +echo "module deletion test with disabling the deletion succeeded" diff --git a/FWCore/Framework/test/test_module_delete_disable_cfg.py b/FWCore/Framework/test/test_module_delete_disable_cfg.py new file mode 100644 index 0000000000000..6c53b453a9b1e --- /dev/null +++ b/FWCore/Framework/test/test_module_delete_disable_cfg.py @@ -0,0 +1,26 @@ +import FWCore.ParameterSet.Config as cms + +process = cms.Process("TESTMODULEDELETE") + +process.maxEvents.input = 1 +process.options.deleteNonConsumedUnscheduledModules = False +process.source = cms.Source("EmptySource") + +process.intEventProducer = cms.EDProducer("IntProducer", ivalue = cms.int32(1)) +process.intEventConsumer = cms.EDAnalyzer("IntTestAnalyzer", + moduleLabel = cms.untracked.InputTag("intEventProducer"), + valueMustMatch = cms.untracked.int32(1) +) +process.intProducerNotConsumed = cms.EDProducer("edmtest::MustRunIntProducer", + ivalue = cms.int32(1), + mustRunEvent = cms.bool(False) +) + +process.t = cms.Task( + process.intEventProducer, + process.intProducerNotConsumed +) +process.p = cms.Path( + process.intEventConsumer, + process.t +) diff --git a/FWCore/ParameterSet/python/Config.py b/FWCore/ParameterSet/python/Config.py index 218dc66ec151d..712b87eb94a53 100644 --- a/FWCore/ParameterSet/python/Config.py +++ b/FWCore/ParameterSet/python/Config.py @@ -227,6 +227,7 @@ def defaultOptions_(): forceEventSetupCacheClearOnNewRun = untracked.bool(False), throwIfIllegalParameter = untracked.bool(True), printDependencies = untracked.bool(False), + deleteNonConsumedUnscheduledModules = untracked.bool(True), sizeOfStackForThreadsInKB = optional.untracked.uint32, Rethrow = untracked.vstring(), SkipEvent = untracked.vstring(), @@ -2008,6 +2009,7 @@ def testProcessDumpPython(self): SkipEvent = cms.untracked.vstring(), allowUnscheduled = cms.obsolete.untracked.bool, canDeleteEarly = cms.untracked.vstring(), + deleteNonConsumedUnscheduledModules = cms.untracked.bool(True), emptyRunLumiMode = cms.obsolete.untracked.string, eventSetup = cms.untracked.PSet( forceNumberOfConcurrentIOVs = cms.untracked.PSet( diff --git a/FWCore/ParameterSet/src/validateTopLevelParameterSets.cc b/FWCore/ParameterSet/src/validateTopLevelParameterSets.cc index e512232fbb042..f59e4d6a55680 100644 --- a/FWCore/ParameterSet/src/validateTopLevelParameterSets.cc +++ b/FWCore/ParameterSet/src/validateTopLevelParameterSets.cc @@ -41,6 +41,10 @@ namespace edm { description.addUntracked("throwIfIllegalParameter", true) ->setComment("Set false to disable exception throws when configuration validation detects illegal parameters"); description.addUntracked("printDependencies", false)->setComment("Print data dependencies between modules"); + description.addUntracked("deleteNonConsumedUnscheduledModules", true) + ->setComment( + "Delete modules that are unscheduled, i.e. only in Tasks, whose products are not consumed by any other " + "otherwise-running module"); // No default for this one because the parameter value is // actually used in the main function in cmsRun.cpp before