From 06efcc60c61036fed1c472b517d7f6de4d1bcb7a Mon Sep 17 00:00:00 2001 From: "W. David Dagenhart" Date: Thu, 29 Feb 2024 16:15:21 +0100 Subject: [PATCH] Remove code related to legacy modules and SharedResources --- .../interface/SharedResourcesRegistry.h | 12 --- FWCore/Framework/interface/one/implementors.h | 1 - .../Framework/src/SharedResourcesRegistry.cc | 76 ++++--------------- .../Framework/src/one/implementorsMethods.h | 4 - .../test/one_outputmodule_t.cppunit.cc | 2 +- .../test/sharedresourcesregistry_t.cppunit.cc | 59 -------------- .../Framework/test/stubs/TestOneAnalyzers.cc | 2 +- FWCore/Framework/test/stubs/TestOneFilters.cc | 2 +- .../Framework/test/stubs/TestOneProducers.cc | 2 +- 9 files changed, 18 insertions(+), 142 deletions(-) diff --git a/FWCore/Framework/interface/SharedResourcesRegistry.h b/FWCore/Framework/interface/SharedResourcesRegistry.h index 79d3222055472..bfb4d606df635 100644 --- a/FWCore/Framework/interface/SharedResourcesRegistry.h +++ b/FWCore/Framework/interface/SharedResourcesRegistry.h @@ -18,7 +18,6 @@ // Created: Sun, 06 Oct 2013 15:48:44 GMT // -// system include files #include #include #include @@ -28,9 +27,6 @@ #include "FWCore/Concurrency/interface/SerialTaskQueue.h" #include "FWCore/Utilities/interface/propagate_const.h" -// user include files - -// forward declarations class testSharedResourcesRegistry; namespace edm { @@ -44,18 +40,12 @@ namespace edm { SharedResourcesRegistry(const SharedResourcesRegistry&) = delete; // stop default const SharedResourcesRegistry& operator=(const SharedResourcesRegistry&) = delete; // stop default - // ---------- const member functions --------------------- SharedResourcesAcquirer createAcquirer(std::vector const&) const; std::pair> createAcquirerForSourceDelayedReader(); - // ---------- static member functions -------------------- static SharedResourcesRegistry* instance(); - ///All legacy modules share this resource - static const std::string kLegacyModuleResourceName; - - // ---------- member functions --------------------------- ///A resource name must be registered before it can be used in the createAcquirer call void registerSharedResource(const std::string&); @@ -76,8 +66,6 @@ namespace edm { edm::propagate_const> resourceForDelayedReader_; edm::propagate_const> queueForDelayedReader_; - - unsigned int nLegacy_; }; } // namespace edm diff --git a/FWCore/Framework/interface/one/implementors.h b/FWCore/Framework/interface/one/implementors.h index d2aaa0219015b..7747d75e56612 100644 --- a/FWCore/Framework/interface/one/implementors.h +++ b/FWCore/Framework/interface/one/implementors.h @@ -80,7 +80,6 @@ namespace edm { protected: void usesResource(std::string const& iName); - void usesResource(); private: SharedResourcesAcquirer createAcquirer() override; diff --git a/FWCore/Framework/src/SharedResourcesRegistry.cc b/FWCore/Framework/src/SharedResourcesRegistry.cc index 752f079f52f6d..c1d86e269ca7d 100644 --- a/FWCore/Framework/src/SharedResourcesRegistry.cc +++ b/FWCore/Framework/src/SharedResourcesRegistry.cc @@ -10,53 +10,31 @@ // Created: Sun, 06 Oct 2013 15:48:50 GMT // -// system include files #include #include -// user include files #include "FWCore/Framework/interface/SharedResourcesRegistry.h" #include "FWCore/Framework/interface/SharedResourcesAcquirer.h" namespace edm { - const std::string SharedResourcesRegistry::kLegacyModuleResourceName{"__legacy__"}; - SharedResourcesRegistry* SharedResourcesRegistry::instance() { static SharedResourcesRegistry s_instance; return &s_instance; } - SharedResourcesRegistry::SharedResourcesRegistry() : nLegacy_(0) {} + SharedResourcesRegistry::SharedResourcesRegistry() {} void SharedResourcesRegistry::registerSharedResource(const std::string& resourceName) { auto& queueAndCounter = resourceMap_[resourceName]; - if (resourceName == kLegacyModuleResourceName) { - ++nLegacy_; - for (auto& resource : resourceMap_) { - if (!resource.second.first) { - resource.second.first = std::make_shared(); - } - ++resource.second.second; - } - } else { - // count the number of times the resource was registered - ++queueAndCounter.second; + // count the number of times the resource was registered + ++queueAndCounter.second; - // When first registering a nonlegacy resource, we have to - // account for any legacy resource registrations already made. - if (queueAndCounter.second == 1) { - if (nLegacy_ > 0U) { - queueAndCounter.first = std::make_shared(); - queueAndCounter.second += nLegacy_; - } - // If registering a nonlegacy resource the second time and - // the legacy resource has not been registered yet, - // we know we will need the queue so go ahead and create it. - } else if (queueAndCounter.second == 2) { - queueAndCounter.first = std::make_shared(); - } + // If registering a resource the second time + // we know we will need the queue so go ahead and create it. + if (queueAndCounter.second == 2) { + queueAndCounter.first = std::make_shared(); } } @@ -78,43 +56,17 @@ namespace edm { // modules using the same resource will not be run until the module // that acquired the resources completes its task. - // The legacy shared resource is special. - // Legacy modules cannot run concurrently with each other or - // any other module that has declared any shared resource. Treat - // one modules that call usesResource with no argument in the - // same way. - // Sort by how often used and then by name // Consistent sorting avoids deadlocks and this particular order optimizes performance std::map, std::shared_ptr> sortedResources; - // Is this acquirer for a module that depends on the legacy shared resource? - if (std::find(resourceNames.begin(), resourceNames.end(), kLegacyModuleResourceName) != resourceNames.end()) { - for (auto const& resource : resourceMap_) { - // It's redundant to declare legacy if the legacy modules - // all declare all the other resources, so just skip it. - // But if the only shared resource is the legacy resource don't skip it. - if (resource.first == kLegacyModuleResourceName && resourceMap_.size() > 1) - continue; - //legacy modules are not allowed to depend on ES shared resources - if (resource.first.substr(0, 3) == "es_") - continue; - //If only one module wants it, it really isn't shared - if (resource.second.second > 1) { - sortedResources.insert( - std::make_pair(std::make_pair(resource.second.second, resource.first), resource.second.first)); - } - } - // Handle cases where the module does not declare the legacy resource - } else { - for (auto const& name : resourceNames) { - auto resource = resourceMap_.find(name); - assert(resource != resourceMap_.end()); - //If only one module wants it, it really isn't shared - if (resource->second.second > 1) { - sortedResources.insert( - std::make_pair(std::make_pair(resource->second.second, resource->first), resource->second.first)); - } + for (auto const& name : resourceNames) { + auto resource = resourceMap_.find(name); + assert(resource != resourceMap_.end()); + // If only one module wants it, it really isn't shared + if (resource->second.second > 1) { + sortedResources.insert( + std::make_pair(std::make_pair(resource->second.second, resource->first), resource->second.first)); } } diff --git a/FWCore/Framework/src/one/implementorsMethods.h b/FWCore/Framework/src/one/implementorsMethods.h index 726b73d06abe0..589310216f275 100644 --- a/FWCore/Framework/src/one/implementorsMethods.h +++ b/FWCore/Framework/src/one/implementorsMethods.h @@ -35,10 +35,6 @@ namespace edm { resourceNames_.insert(iName); SharedResourcesRegistry::instance()->registerSharedResource(iName); } - template - void SharedResourcesUser::usesResource() { - this->usesResource(SharedResourcesRegistry::kLegacyModuleResourceName); - } template SharedResourcesAcquirer SharedResourcesUser::createAcquirer() { diff --git a/FWCore/Framework/test/one_outputmodule_t.cppunit.cc b/FWCore/Framework/test/one_outputmodule_t.cppunit.cc index 75bff62affcc4..a195292e661f0 100644 --- a/FWCore/Framework/test/one_outputmodule_t.cppunit.cc +++ b/FWCore/Framework/test/one_outputmodule_t.cppunit.cc @@ -216,7 +216,7 @@ class testOneOutputModule : public CppUnit::TestFixture { using edm::one::OutputModuleBase::doPreallocate; ResourceOutputModule(edm::ParameterSet const& iPSet) : edm::one::OutputModuleBase(iPSet), edm::one::OutputModule(iPSet) { - usesResource(); + usesResource("foo"); } unsigned int m_count = 0; diff --git a/FWCore/Framework/test/sharedresourcesregistry_t.cppunit.cc b/FWCore/Framework/test/sharedresourcesregistry_t.cppunit.cc index 4b89854b3d10a..9a13b86e4595f 100644 --- a/FWCore/Framework/test/sharedresourcesregistry_t.cppunit.cc +++ b/FWCore/Framework/test/sharedresourcesregistry_t.cppunit.cc @@ -18,7 +18,6 @@ class testSharedResourcesRegistry : public CppUnit::TestFixture { CPPUNIT_TEST_SUITE(testSharedResourcesRegistry); CPPUNIT_TEST(oneTest); - CPPUNIT_TEST(legacyTest); CPPUNIT_TEST(multipleTest); CPPUNIT_TEST_SUITE_END(); @@ -28,7 +27,6 @@ class testSharedResourcesRegistry : public CppUnit::TestFixture { void tearDown() {} void oneTest(); - void legacyTest(); void multipleTest(); }; @@ -58,63 +56,6 @@ void testSharedResourcesRegistry::oneTest() { } } -void testSharedResourcesRegistry::legacyTest() { - std::vector res{edm::SharedResourcesRegistry::kLegacyModuleResourceName}; - { - edm::SharedResourcesRegistry reg; - - reg.registerSharedResource(edm::SharedResourcesRegistry::kLegacyModuleResourceName); - auto tester = reg.createAcquirer(res); - - CPPUNIT_ASSERT(1 == tester.numberOfResources()); - } - { - edm::SharedResourcesRegistry reg; - - reg.registerSharedResource(edm::SharedResourcesRegistry::kLegacyModuleResourceName); - reg.registerSharedResource(edm::SharedResourcesRegistry::kLegacyModuleResourceName); - - auto tester = reg.createAcquirer(res); - - CPPUNIT_ASSERT(1 == tester.numberOfResources()); - } - { - edm::SharedResourcesRegistry reg; - - reg.registerSharedResource("foo"); - reg.registerSharedResource("zoo"); - - auto const& resourceMap = reg.resourceMap(); - CPPUNIT_ASSERT(resourceMap.at(std::string("foo")).first.get() == nullptr); - CPPUNIT_ASSERT(resourceMap.at(std::string("zoo")).first.get() == nullptr); - CPPUNIT_ASSERT(resourceMap.size() == 2); - CPPUNIT_ASSERT(resourceMap.at(std::string("foo")).second == 1); - CPPUNIT_ASSERT(resourceMap.at(std::string("zoo")).second == 1); - - reg.registerSharedResource(edm::SharedResourcesRegistry::kLegacyModuleResourceName); - CPPUNIT_ASSERT(resourceMap.at(std::string("foo")).first.get() != nullptr); - CPPUNIT_ASSERT(resourceMap.at(std::string("zoo")).first.get() != nullptr); - CPPUNIT_ASSERT(resourceMap.at(edm::SharedResourcesRegistry::kLegacyModuleResourceName).first.get() != nullptr); - CPPUNIT_ASSERT(resourceMap.at(std::string("foo")).second == 2); - CPPUNIT_ASSERT(resourceMap.at(std::string("zoo")).second == 2); - CPPUNIT_ASSERT(resourceMap.at(edm::SharedResourcesRegistry::kLegacyModuleResourceName).second == 1); - CPPUNIT_ASSERT(resourceMap.size() == 3); - - reg.registerSharedResource(edm::SharedResourcesRegistry::kLegacyModuleResourceName); - reg.registerSharedResource("bar"); - reg.registerSharedResource("zoo"); - CPPUNIT_ASSERT(resourceMap.at(std::string("bar")).first.get() != nullptr); - CPPUNIT_ASSERT(resourceMap.at(std::string("bar")).second == 3); - CPPUNIT_ASSERT(resourceMap.at(std::string("foo")).second == 3); - CPPUNIT_ASSERT(resourceMap.at(std::string("zoo")).second == 4); - CPPUNIT_ASSERT(resourceMap.at(edm::SharedResourcesRegistry::kLegacyModuleResourceName).second == 2); - - auto tester = reg.createAcquirer(res); - - CPPUNIT_ASSERT(3 == tester.numberOfResources()); - } -} - void testSharedResourcesRegistry::multipleTest() { edm::SharedResourcesRegistry reg; auto const& resourceMap = reg.resourceMap(); diff --git a/FWCore/Framework/test/stubs/TestOneAnalyzers.cc b/FWCore/Framework/test/stubs/TestOneAnalyzers.cc index 89642cfaa1895..689af44cdfddc 100644 --- a/FWCore/Framework/test/stubs/TestOneAnalyzers.cc +++ b/FWCore/Framework/test/stubs/TestOneAnalyzers.cc @@ -36,7 +36,7 @@ namespace edmtest { class SharedResourcesAnalyzer : public edm::one::EDAnalyzer { public: explicit SharedResourcesAnalyzer(edm::ParameterSet const& p) : trans_(p.getParameter("transitions")) { - usesResource(); + usesResource("foo"); callWhenNewProductsRegistered([](edm::BranchDescription const& desc) { std::cout << "one::SharedResourcesAnalyzer " << desc.moduleLabel() << std::endl; }); diff --git a/FWCore/Framework/test/stubs/TestOneFilters.cc b/FWCore/Framework/test/stubs/TestOneFilters.cc index 0424d494c7b7c..2d1e26b2194ca 100644 --- a/FWCore/Framework/test/stubs/TestOneFilters.cc +++ b/FWCore/Framework/test/stubs/TestOneFilters.cc @@ -34,7 +34,7 @@ namespace edmtest { public: explicit SharedResourcesFilter(edm::ParameterSet const& p) : trans_(p.getParameter("transitions")) { produces(); - usesResource(); + usesResource("foo"); } const unsigned int trans_; mutable std::atomic m_count{0}; diff --git a/FWCore/Framework/test/stubs/TestOneProducers.cc b/FWCore/Framework/test/stubs/TestOneProducers.cc index 25e2a4b0769f3..5bc620fae1871 100644 --- a/FWCore/Framework/test/stubs/TestOneProducers.cc +++ b/FWCore/Framework/test/stubs/TestOneProducers.cc @@ -34,7 +34,7 @@ namespace edmtest { public: explicit SharedResourcesProducer(edm::ParameterSet const& p) : trans_(p.getParameter("transitions")) { produces(); - usesResource(); + usesResource("foo"); } const unsigned int trans_; unsigned int m_count = 0;