Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Delete unscheduled modules that are not consumed by any scheduled module #29553

Merged
merged 26 commits into from
Dec 2, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
902039b
Make Worker::description() to return a pointer to ModuleDescription
makortel Apr 1, 2020
d16fc6a
Delete unscheduled modules whose products are not consumed by anything
makortel Mar 30, 2020
0c888f6
Delete non-consumed unscheduled modules in SubProcessses
makortel Apr 2, 2020
3795ad1
Keep Accumulator modules running by adding data dependencies in globa…
makortel Apr 17, 2020
29d29dc
Add pre and post signals for early module deletion
makortel Apr 19, 2020
e1c17df
Remove deleted modules from SystemTimeKeeper
makortel Apr 19, 2020
61dd9db
Keep modules in non-event ordering unit tests running by moving the l…
makortel Apr 19, 2020
5a41a17
Add comment about modules being deleted being ok in empty Paths tests
makortel Apr 19, 2020
1ab4449
Add data dependencies to modules that were running before in SubProce…
makortel Apr 19, 2020
1c1a8a5
Find entire sub-DAGs of non-consumed modules and delete them
makortel Apr 20, 2020
bf39115
Watch {pre,post}ModuleDestruction signals in MessageLogger
makortel Apr 22, 2020
52799a9
Invalidate deleted modules in RandomNumberGeneratorService
makortel Apr 22, 2020
6ac7ac9
Remove deleted modules from the excluded modules list in ConcurrentMo…
makortel Apr 22, 2020
381480e
Remove deleted modules in StallMonitor
makortel Apr 22, 2020
add0c31
Watch {pre,post}ModuleDestruction signals in Timing
makortel Apr 22, 2020
a756679
Watch {pre,post}ModuleDestruction signals in Tracer
makortel Apr 22, 2020
f26bf97
Watch {pre,post}ModuleDestruction signals in NVProfilerService
makortel Apr 22, 2020
cb79bdb
Include EDAlias and SwitchProducer in the tests
makortel Apr 23, 2020
1c41364
Move to std::array<T, NumBranchTypes>
makortel Jun 3, 2020
bb852dd
Replace set<ModuleProcessName> with sorted vector<ModuleProcessName>
makortel Jun 3, 2020
8873e2b
Rename module in non-event ordering tests to avoid accidental success
makortel Jun 3, 2020
eaac76f
Disable module deletion if a job has an EDLooper
makortel Jun 4, 2020
f77931e
Protect DependencyGraph for module IDs larger than the number of modules
makortel Jun 4, 2020
897ef77
Extend module deletion tests to ProcessBlock
makortel Jul 13, 2020
9c053f9
code checks
makortel Oct 1, 2020
c38022d
Add configuration parameter to allow disabling the module deletion
makortel Oct 1, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion FWCore/Framework/interface/EDConsumerBase.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@

namespace edm {
class ModuleDescription;
class ModuleProcessName;
class ProductResolverIndexHelper;
class ProductRegistry;
class ConsumesCollector;
Expand Down Expand Up @@ -104,7 +105,8 @@ namespace edm {
typedef ProductLabels Labels;
void labelsForToken(EDGetToken iToken, Labels& oLabels) const;

void modulesWhoseProductsAreConsumed(std::vector<ModuleDescription const*>& modules,
void modulesWhoseProductsAreConsumed(std::array<std::vector<ModuleDescription const*>*, NumBranchTypes>& modulesAll,
std::vector<ModuleProcessName>& modulesInPreviousProcesses,
ProductRegistry const& preg,
std::map<std::string, ModuleDescription const*> const& labelsToDesc,
std::string const& processName) const;
Expand Down
1 change: 1 addition & 0 deletions FWCore/Framework/interface/EventProcessor.h
Original file line number Diff line number Diff line change
Expand Up @@ -362,6 +362,7 @@ namespace edm {
ExcludedDataMap eventSetupDataToExcludeFromPrefetching_;

bool printDependencies_ = false;
bool deleteNonConsumedUnscheduledModules_ = true;
}; // class EventProcessor

//--------------------------------------------------------------------
Expand Down
30 changes: 30 additions & 0 deletions FWCore/Framework/interface/ModuleProcessName.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
#ifndef FWCore_Framework_ModuleProcessName_h
#define FWCore_Framework_ModuleProcessName_h

#include <string_view>

namespace edm {
/**
* Helper class to hold a module label and a process name
*
* Note: does NOT own the string storage, be careful to use.
*/
class ModuleProcessName {
makortel marked this conversation as resolved.
Show resolved Hide resolved
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
19 changes: 17 additions & 2 deletions FWCore/Framework/interface/PathsAndConsumesOfModules.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@
#include "FWCore/ServiceRegistry/interface/ConsumesInfo.h"
#include "FWCore/ServiceRegistry/interface/PathsAndConsumesOfModulesBase.h"

#include "FWCore/Framework/interface/ModuleProcessName.h"
#include "FWCore/Utilities/interface/BranchType.h"

#include <memory>
#include <string>
#include <utility>
Expand All @@ -28,10 +31,16 @@ namespace edm {

class PathsAndConsumesOfModules : public PathsAndConsumesOfModulesBase {
public:
PathsAndConsumesOfModules();
~PathsAndConsumesOfModules() override;

void initialize(Schedule const*, std::shared_ptr<ProductRegistry const>);

void removeModules(std::vector<ModuleDescription const*> const& modules);

std::vector<ModuleProcessName> const& modulesInPreviousProcessesWhoseProductsAreConsumedBy(
unsigned int moduleID) const;

private:
std::vector<std::string> const& doPaths() const override { return paths_; }
std::vector<std::string> const& doEndPaths() const override { return endPaths_; }
Expand All @@ -42,10 +51,12 @@ namespace edm {
std::vector<ModuleDescription const*> const& doModulesOnPath(unsigned int pathIndex) const override;
std::vector<ModuleDescription const*> const& doModulesOnEndPath(unsigned int endPathIndex) const override;
std::vector<ModuleDescription const*> const& doModulesWhoseProductsAreConsumedBy(
unsigned int moduleID) const override;
unsigned int moduleID, BranchType branchType = InEvent) const override;

std::vector<ConsumesInfo> doConsumesInfo(unsigned int moduleID) const override;

unsigned int doLargestModuleID() const override;

unsigned int moduleIndex(unsigned int moduleID) const;

// data members
Expand All @@ -62,12 +73,16 @@ namespace edm {
// following data member
std::vector<std::pair<unsigned int, unsigned int> > moduleIDToIndex_;

std::vector<std::vector<ModuleDescription const*> > modulesWhoseProductsAreConsumedBy_;
std::array<std::vector<std::vector<ModuleDescription const*> >, NumBranchTypes> modulesWhoseProductsAreConsumedBy_;
std::vector<std::vector<ModuleProcessName> > modulesInPreviousProcessesWhoseProductsAreConsumedBy_;

Schedule const* schedule_;
std::shared_ptr<ProductRegistry const> preg_;
};

std::vector<ModuleDescription const*> nonConsumedUnscheduledModules(
edm::PathsAndConsumesOfModulesBase const& iPnC, std::vector<ModuleProcessName>& consumedByChildren);

void checkForModuleDependencyCorrectness(edm::PathsAndConsumesOfModulesBase const& iPnC, bool iPrintDependencies);
} // namespace edm
#endif
14 changes: 10 additions & 4 deletions FWCore/Framework/interface/Schedule.h
Original file line number Diff line number Diff line change
Expand Up @@ -230,10 +230,13 @@ namespace edm {
std::vector<ModuleDescription const*>& descriptions,
unsigned int hint) const;

void fillModuleAndConsumesInfo(std::vector<ModuleDescription const*>& allModuleDescriptions,
std::vector<std::pair<unsigned int, unsigned int>>& moduleIDToIndex,
std::vector<std::vector<ModuleDescription const*>>& modulesWhoseProductsAreConsumedBy,
ProductRegistry const& preg) const;
void fillModuleAndConsumesInfo(
std::vector<ModuleDescription const*>& allModuleDescriptions,
std::vector<std::pair<unsigned int, unsigned int>>& moduleIDToIndex,
std::array<std::vector<std::vector<ModuleDescription const*>>, NumBranchTypes>&
modulesWhoseProductsAreConsumedBy,
std::vector<std::vector<ModuleProcessName>>& modulesInPreviousProcessesWhoseProductsAreConsumedBy,
ProductRegistry const& preg) const;

/// Return the number of events this Schedule has tried to process
/// (inclues both successes and failures, including failures due
Expand Down Expand Up @@ -277,6 +280,9 @@ namespace edm {
const ProductRegistry& iRegistry,
eventsetup::ESRecordsToProxyIndices const&);

/// Deletes module with label iLabel
void deleteModule(std::string const& iLabel, ActivityRegistry* areg);

/// returns the collection of pointers to workers
AllWorkers const& allWorkers() const;

Expand Down
7 changes: 7 additions & 0 deletions FWCore/Framework/interface/UnscheduledCallProducer.h
Original file line number Diff line number Diff line change
Expand Up @@ -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_; }
Expand Down
3 changes: 3 additions & 0 deletions FWCore/Framework/interface/WorkerManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,9 @@ namespace edm {
WorkerManager(std::shared_ptr<ModuleRegistry> modReg,
std::shared_ptr<ActivityRegistry> actReg,
ExceptionToActionTable const& actions);

void deleteModuleIfExists(std::string const& moduleLabel);

void addToUnscheduledWorkers(ParameterSet& pset,
ProductRegistry& preg,
PreallocationConfiguration const* prealloc,
Expand Down
4 changes: 3 additions & 1 deletion FWCore/Framework/interface/stream/EDAnalyzerAdaptorBase.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@

namespace edm {
class ModuleCallingContext;
class ModuleProcessName;
class ProductResolverIndexHelper;
class EDConsumerBase;
class PreallocationConfiguration;
Expand Down Expand Up @@ -109,7 +110,8 @@ namespace edm {

const EDConsumerBase* consumer() const;

void modulesWhoseProductsAreConsumed(std::vector<ModuleDescription const*>& modules,
void modulesWhoseProductsAreConsumed(std::array<std::vector<ModuleDescription const*>*, NumBranchTypes>& modules,
std::vector<ModuleProcessName>& modulesInPreviousProcesses,
ProductRegistry const& preg,
std::map<std::string, ModuleDescription const*> const& labelsToDesc,
std::string const& processName) const;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
namespace edm {
class Event;
class ModuleCallingContext;
class ModuleProcessName;
class ProductResolverIndexHelper;
class EDConsumerBase;
class PreallocationConfiguration;
Expand Down Expand Up @@ -100,7 +101,8 @@ namespace edm {
void updateLookup(BranchType iBranchType, ProductResolverIndexHelper const&, bool iPrefetchMayGet);
void updateLookup(eventsetup::ESRecordsToProxyIndices const&);

void modulesWhoseProductsAreConsumed(std::vector<ModuleDescription const*>& modules,
void modulesWhoseProductsAreConsumed(std::array<std::vector<ModuleDescription const*>*, NumBranchTypes>& modules,
std::vector<ModuleProcessName>& modulesInPreviousProcesses,
ProductRegistry const& preg,
std::map<std::string, ModuleDescription const*> const& labelsToDesc,
std::string const& processName) const;
Expand Down
80 changes: 56 additions & 24 deletions FWCore/Framework/src/EDConsumerBase.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -473,41 +474,55 @@ namespace {
}
} // namespace

void EDConsumerBase::modulesWhoseProductsAreConsumed(std::vector<ModuleDescription const*>& modules,
ProductRegistry const& preg,
std::map<std::string, ModuleDescription const*> const& labelsToDesc,
std::string const& processName) const {
ProductResolverIndexHelper const& iHelper = *preg.productLookup(InEvent);

void EDConsumerBase::modulesWhoseProductsAreConsumed(
std::array<std::vector<ModuleDescription const*>*, NumBranchTypes>& modulesAll,
std::vector<ModuleProcessName>& modulesInPreviousProcesses,
ProductRegistry const& preg,
std::map<std::string, ModuleDescription const*> const& labelsToDesc,
std::string const& processName) const {
std::set<std::string> alreadyFound;

auto modulesInPreviousProcessesEmplace = [&modulesInPreviousProcesses](std::string_view module,
std::string_view process) {
auto it = std::lower_bound(
modulesInPreviousProcesses.begin(), modulesInPreviousProcesses.end(), ModuleProcessName(module, process));
modulesInPreviousProcesses.emplace(it, module, process);
};

auto itKind = m_tokenInfo.begin<kKind>();
auto itLabels = m_tokenInfo.begin<kLabels>();
for (auto itInfo = m_tokenInfo.begin<kLookupInfo>(), itEnd = m_tokenInfo.end<kLookupInfo>(); itInfo != itEnd;
++itInfo, ++itKind, ++itLabels) {
if (itInfo->m_branchType == InEvent and (not itInfo->m_index.skipCurrentProcess())) {
const unsigned int labelStart = itLabels->m_startOfModuleLabel;
const char* const consumedModuleLabel = &(m_tokenLabels[labelStart]);
const char* const consumedProductInstance = consumedModuleLabel + itLabels->m_deltaToProductInstance;
const char* const consumedProcessName = consumedModuleLabel + itLabels->m_deltaToProcessName;
ProductResolverIndexHelper const& helper = *preg.productLookup(itInfo->m_branchType);
std::vector<ModuleDescription const*>& modules = *modulesAll[itInfo->m_branchType];

const unsigned int labelStart = itLabels->m_startOfModuleLabel;
const char* const consumedModuleLabel = &(m_tokenLabels[labelStart]);
const char* const consumedProductInstance = consumedModuleLabel + itLabels->m_deltaToProductInstance;
const char* const consumedProcessName = consumedModuleLabel + itLabels->m_deltaToProcessName;

if (not itInfo->m_index.skipCurrentProcess()) {
if (*consumedModuleLabel != '\0') { // not a consumesMany
if (*consumedProcessName != '\0') { // process name is specified in consumes call
if (processName == consumedProcessName &&
iHelper.index(
if (helper.index(
*itKind, itInfo->m_type, consumedModuleLabel, consumedProductInstance, consumedProcessName) !=
ProductResolverIndexInvalid) {
insertFoundModuleLabel(*itKind,
itInfo->m_type,
consumedModuleLabel,
consumedProductInstance,
modules,
alreadyFound,
labelsToDesc,
preg);
ProductResolverIndexInvalid) {
if (processName == consumedProcessName) {
insertFoundModuleLabel(*itKind,
itInfo->m_type,
consumedModuleLabel,
consumedProductInstance,
modules,
alreadyFound,
labelsToDesc,
preg);
} else {
// Product explicitly from different process than the current process, so must refer to an earlier process (unless it ends up "not found")
modulesInPreviousProcessesEmplace(consumedModuleLabel, consumedProcessName);
}
}
} else { // process name was empty
auto matches = iHelper.relatedIndexes(*itKind, itInfo->m_type, consumedModuleLabel, consumedProductInstance);
auto matches = helper.relatedIndexes(*itKind, itInfo->m_type, consumedModuleLabel, consumedProductInstance);
for (unsigned int j = 0; j < matches.numberOfMatches(); ++j) {
if (processName == matches.processName(j)) {
insertFoundModuleLabel(*itKind,
Expand All @@ -518,12 +533,16 @@ void EDConsumerBase::modulesWhoseProductsAreConsumed(std::vector<ModuleDescripti
alreadyFound,
labelsToDesc,
preg);
} else {
// Product did not match to current process, so must refer to an earlier process (unless it ends up "not found")
// Recall that empty process name means "in the latest process" that can change event-by-event
modulesInPreviousProcessesEmplace(consumedModuleLabel, matches.processName(j));
}
}
}
// consumesMany case
} else if (itInfo->m_index.productResolverIndex() == ProductResolverIndexInvalid) {
auto matches = iHelper.relatedIndexes(*itKind, itInfo->m_type);
auto matches = helper.relatedIndexes(*itKind, itInfo->m_type);
for (unsigned int j = 0; j < matches.numberOfMatches(); ++j) {
if (processName == matches.processName(j)) {
insertFoundModuleLabel(*itKind,
Expand All @@ -534,9 +553,22 @@ void EDConsumerBase::modulesWhoseProductsAreConsumed(std::vector<ModuleDescripti
alreadyFound,
labelsToDesc,
preg);
} else {
modulesInPreviousProcessesEmplace(matches.moduleLabel(j), matches.processName(j));
}
}
}
} else {
// The skipCurrentProcess means the same as empty process name,
// except the current process is skipped. Therefore need to do
// the same matching as above. There is no consumesMany branch
// in this case.
auto matches = helper.relatedIndexes(*itKind, itInfo->m_type, consumedModuleLabel, consumedProductInstance);
for (unsigned int j = 0; j < matches.numberOfMatches(); ++j) {
if (processName != matches.processName(j)) {
modulesInPreviousProcessesEmplace(matches.moduleLabel(j), matches.processName(j));
}
}
}
}
}
Expand Down
Loading