Skip to content

Commit

Permalink
Remove code related to legacy modules and SharedResources
Browse files Browse the repository at this point in the history
  • Loading branch information
wddgit committed Feb 29, 2024
1 parent ec33848 commit 06efcc6
Show file tree
Hide file tree
Showing 9 changed files with 18 additions and 142 deletions.
12 changes: 0 additions & 12 deletions FWCore/Framework/interface/SharedResourcesRegistry.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
// Created: Sun, 06 Oct 2013 15:48:44 GMT
//

// system include files
#include <vector>
#include <string>
#include <map>
Expand All @@ -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 {
Expand All @@ -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<std::string> const&) const;

std::pair<SharedResourcesAcquirer, std::shared_ptr<std::recursive_mutex>> 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&);

Expand All @@ -76,8 +66,6 @@ namespace edm {
edm::propagate_const<std::shared_ptr<std::recursive_mutex>> resourceForDelayedReader_;

edm::propagate_const<std::shared_ptr<SerialTaskQueue>> queueForDelayedReader_;

unsigned int nLegacy_;
};
} // namespace edm

Expand Down
1 change: 0 additions & 1 deletion FWCore/Framework/interface/one/implementors.h
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,6 @@ namespace edm {

protected:
void usesResource(std::string const& iName);
void usesResource();

private:
SharedResourcesAcquirer createAcquirer() override;
Expand Down
76 changes: 14 additions & 62 deletions FWCore/Framework/src/SharedResourcesRegistry.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10,53 +10,31 @@
// Created: Sun, 06 Oct 2013 15:48:50 GMT
//

// system include files
#include <algorithm>
#include <cassert>

// 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<SerialTaskQueue>();
}
++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<SerialTaskQueue>();
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<SerialTaskQueue>();
}
// 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<SerialTaskQueue>();
}
}

Expand All @@ -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::pair<unsigned int, std::string>, std::shared_ptr<SerialTaskQueue>> 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));
}
}

Expand Down
4 changes: 0 additions & 4 deletions FWCore/Framework/src/one/implementorsMethods.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,6 @@ namespace edm {
resourceNames_.insert(iName);
SharedResourcesRegistry::instance()->registerSharedResource(iName);
}
template <typename T>
void SharedResourcesUser<T>::usesResource() {
this->usesResource(SharedResourcesRegistry::kLegacyModuleResourceName);
}

template <typename T>
SharedResourcesAcquirer SharedResourcesUser<T>::createAcquirer() {
Expand Down
2 changes: 1 addition & 1 deletion FWCore/Framework/test/one_outputmodule_t.cppunit.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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<edm::one::SharedResources>(iPSet) {
usesResource();
usesResource("foo");
}
unsigned int m_count = 0;

Expand Down
59 changes: 0 additions & 59 deletions FWCore/Framework/test/sharedresourcesregistry_t.cppunit.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -28,7 +27,6 @@ class testSharedResourcesRegistry : public CppUnit::TestFixture {
void tearDown() {}

void oneTest();
void legacyTest();
void multipleTest();
};

Expand Down Expand Up @@ -58,63 +56,6 @@ void testSharedResourcesRegistry::oneTest() {
}
}

void testSharedResourcesRegistry::legacyTest() {
std::vector<std::string> 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();
Expand Down
2 changes: 1 addition & 1 deletion FWCore/Framework/test/stubs/TestOneAnalyzers.cc
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ namespace edmtest {
class SharedResourcesAnalyzer : public edm::one::EDAnalyzer<edm::one::SharedResources> {
public:
explicit SharedResourcesAnalyzer(edm::ParameterSet const& p) : trans_(p.getParameter<int>("transitions")) {
usesResource();
usesResource("foo");
callWhenNewProductsRegistered([](edm::BranchDescription const& desc) {
std::cout << "one::SharedResourcesAnalyzer " << desc.moduleLabel() << std::endl;
});
Expand Down
2 changes: 1 addition & 1 deletion FWCore/Framework/test/stubs/TestOneFilters.cc
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ namespace edmtest {
public:
explicit SharedResourcesFilter(edm::ParameterSet const& p) : trans_(p.getParameter<int>("transitions")) {
produces<int>();
usesResource();
usesResource("foo");
}
const unsigned int trans_;
mutable std::atomic<unsigned int> m_count{0};
Expand Down
2 changes: 1 addition & 1 deletion FWCore/Framework/test/stubs/TestOneProducers.cc
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ namespace edmtest {
public:
explicit SharedResourcesProducer(edm::ParameterSet const& p) : trans_(p.getParameter<int>("transitions")) {
produces<int>();
usesResource();
usesResource("foo");
}
const unsigned int trans_;
unsigned int m_count = 0;
Expand Down

0 comments on commit 06efcc6

Please sign in to comment.