Skip to content

Commit

Permalink
Merge pull request #22163 from wddgit/eventSetupTimingMonitoring
Browse files Browse the repository at this point in the history
Add ability to measure time in the EventSetup
  • Loading branch information
cmsbuild committed Feb 9, 2018
2 parents 71eb66c + 230481c commit 2039650
Show file tree
Hide file tree
Showing 26 changed files with 429 additions and 73 deletions.
6 changes: 4 additions & 2 deletions FWCore/Framework/interface/DataProxy.h
Expand Up @@ -27,6 +27,8 @@

// forward declarations
namespace edm {
class ActivityRegistry;

namespace eventsetup {
struct ComponentDescription;
class DataKey;
Expand All @@ -41,8 +43,8 @@ namespace edm {
// ---------- const member functions ---------------------
bool cacheIsValid() const { return cacheIsValid_.load(std::memory_order_acquire); }

void doGet(EventSetupRecord const& iRecord, DataKey const& iKey, bool iTransiently) const;
void const* get(EventSetupRecord const&, DataKey const& iKey, bool iTransiently) const;
void doGet(EventSetupRecord const& iRecord, DataKey const& iKey, bool iTransiently, ActivityRegistry*) const;
void const* get(EventSetupRecord const&, DataKey const& iKey, bool iTransiently, ActivityRegistry*) const;

///returns the description of the DataProxyProvider which owns this Proxy
ComponentDescription const* providerDescription() const {
Expand Down
9 changes: 8 additions & 1 deletion FWCore/Framework/interface/EventSetup.h
Expand Up @@ -35,6 +35,7 @@
// forward declarations

namespace edm {
class ActivityRegistry;
class ESInputTag;

namespace eventsetup {
Expand Down Expand Up @@ -116,6 +117,9 @@ namespace edm {
getAvoidCompilerBug(const T*& iValue) const {
iValue = &(get<T>());
}

friend class eventsetup::EventSetupRecord;

protected:
//Only called by EventSetupProvider
void setKnownRecordsSupplier(eventsetup::EventSetupKnownRecordsSupplier const* iSupplier) {
Expand All @@ -127,12 +131,14 @@ namespace edm {
void clear();

private:
EventSetup();
EventSetup(ActivityRegistry*);

EventSetup(EventSetup const&) = delete; // stop default

EventSetup const& operator=(EventSetup const&) = delete; // stop default

ActivityRegistry* activityRegistry() const { return activityRegistry_; }

void insert(const eventsetup::EventSetupRecordKey&,
const eventsetup::EventSetupRecord*);

Expand All @@ -141,6 +147,7 @@ namespace edm {
//NOTE: the records are not owned
std::map<eventsetup::EventSetupRecordKey, eventsetup::EventSetupRecord const *> recordMap_;
eventsetup::EventSetupKnownRecordsSupplier const* knownRecords_;
ActivityRegistry* activityRegistry_;
};

// Free functions to retrieve an object from the EventSetup.
Expand Down
3 changes: 2 additions & 1 deletion FWCore/Framework/interface/EventSetupProvider.h
Expand Up @@ -33,6 +33,7 @@

// forward declarations
namespace edm {
class ActivityRegistry;
class EventSetupRecordIntervalFinder;
class IOVSyncValue;
class ParameterSet;
Expand All @@ -57,7 +58,7 @@ class EventSetupProvider {
typedef std::multimap<RecordName, DataKeyInfo> RecordToDataMap;
typedef std::map<ComponentDescription, RecordToDataMap> PreferredProviderInfo;

EventSetupProvider(unsigned subProcessIndex = 0U, PreferredProviderInfo const* iInfo = nullptr);
EventSetupProvider(ActivityRegistry*, unsigned subProcessIndex = 0U, PreferredProviderInfo const* iInfo = nullptr);
virtual ~EventSetupProvider();

// ---------- const member functions ---------------------
Expand Down
3 changes: 2 additions & 1 deletion FWCore/Framework/interface/EventSetupProviderMaker.h
Expand Up @@ -6,13 +6,14 @@

// forward declarations
namespace edm {
class ActivityRegistry;
class ParameterSet;
namespace eventsetup {
class EventSetupProvider;
class EventSetupsController;

std::unique_ptr<EventSetupProvider>
makeEventSetupProvider(ParameterSet const& params, unsigned subProcessIndex);
makeEventSetupProvider(ParameterSet const& params, unsigned subProcessIndex, ActivityRegistry*);

void
fillEventSetupProvider(EventSetupsController& esController,
Expand Down
45 changes: 39 additions & 6 deletions FWCore/Framework/src/DataProxy.cc
Expand Up @@ -18,7 +18,7 @@
#include "FWCore/Framework/interface/ComponentDescription.h"
#include "FWCore/Framework/interface/MakeDataException.h"
#include "FWCore/Framework/interface/EventSetupRecord.h"

#include "FWCore/ServiceRegistry/interface/ActivityRegistry.h"

//
// constants, enums and typedefs
Expand Down Expand Up @@ -97,14 +97,47 @@ namespace {
const DataKey& iKey) {
throw MakeDataException(iRecord.key(),iKey);
}

class ESSignalSentry {
public:
ESSignalSentry(const EventSetupRecord& iRecord,
const DataKey& iKey,
ComponentDescription const* componentDescription,
ActivityRegistry* activityRegistry) :
eventSetupRecord_(iRecord),
dataKey_(iKey),
componentDescription_(componentDescription),
calledPostLock_(false),
activityRegistry_(activityRegistry) {

activityRegistry->preLockEventSetupGetSignal_(componentDescription_, eventSetupRecord_.key(), dataKey_);
}
void sendPostLockSignal() {
calledPostLock_ = true;
activityRegistry_->postLockEventSetupGetSignal_(componentDescription_, eventSetupRecord_.key(), dataKey_);
}
~ESSignalSentry() noexcept(false) {
if (!calledPostLock_) {
activityRegistry_->postLockEventSetupGetSignal_(componentDescription_, eventSetupRecord_.key(), dataKey_);
}
activityRegistry_->postEventSetupGetSignal_(componentDescription_, eventSetupRecord_.key(), dataKey_);
}
private:
EventSetupRecord const& eventSetupRecord_;
DataKey const& dataKey_;
ComponentDescription const* componentDescription_;
bool calledPostLock_;
ActivityRegistry* activityRegistry_;
};
}



const void*
DataProxy::get(const EventSetupRecord& iRecord, const DataKey& iKey, bool iTransiently) const
DataProxy::get(const EventSetupRecord& iRecord, const DataKey& iKey, bool iTransiently, ActivityRegistry* activityRegistry) const
{
if(!cacheIsValid()) {
ESSignalSentry signalSentry(iRecord, iKey, providerDescription(), activityRegistry);
std::lock_guard<std::recursive_mutex> guard(s_esGlobalMutex);
signalSentry.sendPostLockSignal();
if(!cacheIsValid()) {
cache_ = const_cast<DataProxy*>(this)->getImpl(iRecord, iKey);
cacheIsValid_.store(true,std::memory_order_release);
Expand All @@ -123,8 +156,8 @@ DataProxy::get(const EventSetupRecord& iRecord, const DataKey& iKey, bool iTrans
return cache_;
}

void DataProxy::doGet(const EventSetupRecord& iRecord, const DataKey& iKey, bool iTransiently) const {
get(iRecord, iKey, iTransiently);
void DataProxy::doGet(const EventSetupRecord& iRecord, const DataKey& iKey, bool iTransiently, ActivityRegistry* activityRegistry) const {
get(iRecord, iKey, iTransiently, activityRegistry);
}


Expand Down
2 changes: 1 addition & 1 deletion FWCore/Framework/src/EventProcessor.cc
Expand Up @@ -475,7 +475,7 @@ namespace edm {
std::shared_ptr<CommonParams> common(items.initMisc(*parameterSet));

// intialize the event setup provider
esp_ = espController_->makeProvider(*parameterSet);
esp_ = espController_->makeProvider(*parameterSet, items.actReg_.get());

// initialize the looper, if any
looper_ = fillLooper(*espController_, *esp_, *parameterSet);
Expand Down
5 changes: 4 additions & 1 deletion FWCore/Framework/src/EventSetup.cc
Expand Up @@ -31,7 +31,10 @@ namespace edm {
//
// constructors and destructor
//
EventSetup::EventSetup() : recordMap_()
EventSetup::EventSetup(ActivityRegistry* activityRegistry) :
recordMap_(),
activityRegistry_(activityRegistry)

{
}

Expand Down
6 changes: 4 additions & 2 deletions FWCore/Framework/src/EventSetupProvider.cc
Expand Up @@ -61,8 +61,10 @@ namespace edm {
//
// constructors and destructor
//
EventSetupProvider::EventSetupProvider(unsigned subProcessIndex, const PreferredProviderInfo* iInfo) :
eventSetup_(),
EventSetupProvider::EventSetupProvider(ActivityRegistry* activityRegistry,
unsigned subProcessIndex,
const PreferredProviderInfo* iInfo) :
eventSetup_(activityRegistry),
providers_(),
knownRecordsSupplier_( std::make_unique<KnownRecordsSupplierImpl>(providers_)),
mustFinishConfiguration_(true),
Expand Down
6 changes: 3 additions & 3 deletions FWCore/Framework/src/EventSetupProviderMaker.cc
Expand Up @@ -23,12 +23,12 @@ namespace edm {
namespace eventsetup {
// ---------------------------------------------------------------
std::unique_ptr<EventSetupProvider>
makeEventSetupProvider(ParameterSet const& params, unsigned subProcessIndex) {
makeEventSetupProvider(ParameterSet const& params, unsigned subProcessIndex, ActivityRegistry* activityRegistry) {
std::vector<std::string> prefers =
params.getParameter<std::vector<std::string> >("@all_esprefers");

if(prefers.empty()) {
return std::make_unique<EventSetupProvider>(subProcessIndex);
return std::make_unique<EventSetupProvider>(activityRegistry, subProcessIndex);
}

EventSetupProvider::PreferredProviderInfo preferInfo;
Expand Down Expand Up @@ -96,7 +96,7 @@ namespace edm {
preferPSet.getParameter<std::string>("@module_label"),
false)] = recordToData;
}
return std::make_unique<EventSetupProvider>(subProcessIndex, &preferInfo);
return std::make_unique<EventSetupProvider>(activityRegistry, subProcessIndex, &preferInfo);
}

// ---------------------------------------------------------------
Expand Down
5 changes: 3 additions & 2 deletions FWCore/Framework/src/EventSetupRecord.cc
Expand Up @@ -17,6 +17,7 @@

// user include files
#include "FWCore/Framework/interface/EventSetupRecord.h"
#include "FWCore/Framework/interface/EventSetup.h"
#include "FWCore/Framework/interface/EventSetupRecordKey.h"
#include "FWCore/Framework/interface/DataProxy.h"
#include "FWCore/Framework/interface/ComponentDescription.h"
Expand Down Expand Up @@ -181,7 +182,7 @@ EventSetupRecord::getFromProxy(DataKey const & iKey ,
if(nullptr!=proxy) {
try {
convertException::wrap([&]() {
hold = proxy->get(*this, iKey,iTransientAccessOnly);
hold = proxy->get(*this, iKey,iTransientAccessOnly, eventSetup_->activityRegistry());
iDesc = proxy->providerDescription();
});
}
Expand Down Expand Up @@ -211,7 +212,7 @@ EventSetupRecord::doGet(const DataKey& aKey, bool aGetTransiently) const {
if(nullptr != proxy) {
try {
convertException::wrap([&]() {
proxy->doGet(*this, aKey, aGetTransiently);
proxy->doGet(*this, aKey, aGetTransiently, eventSetup_->activityRegistry());
});
}
catch( cms::Exception& e) {
Expand Down
4 changes: 2 additions & 2 deletions FWCore/Framework/src/EventSetupsController.cc
Expand Up @@ -29,12 +29,12 @@ namespace edm {
}

std::shared_ptr<EventSetupProvider>
EventSetupsController::makeProvider(ParameterSet& iPSet) {
EventSetupsController::makeProvider(ParameterSet& iPSet, ActivityRegistry* activityRegistry) {

// Makes an EventSetupProvider
// Also parses the prefer information from ParameterSets and puts
// it in a map that is stored in the EventSetupProvider
std::shared_ptr<EventSetupProvider> returnValue(makeEventSetupProvider(iPSet, providers_.size()) );
std::shared_ptr<EventSetupProvider> returnValue(makeEventSetupProvider(iPSet, providers_.size(), activityRegistry) );

// Construct the ESProducers and ESSources
// shared_ptrs to them are temporarily stored in this
Expand Down
3 changes: 2 additions & 1 deletion FWCore/Framework/src/EventSetupsController.h
Expand Up @@ -26,6 +26,7 @@

namespace edm {

class ActivityRegistry;
class EventSetupRecordIntervalFinder;
class ParameterSet;
class IOVSyncValue;
Expand Down Expand Up @@ -74,7 +75,7 @@ namespace edm {
public:
EventSetupsController();

std::shared_ptr<EventSetupProvider> makeProvider(ParameterSet&);
std::shared_ptr<EventSetupProvider> makeProvider(ParameterSet&, ActivityRegistry*);

void eventSetupForInstance(IOVSyncValue const& syncValue);

Expand Down
2 changes: 1 addition & 1 deletion FWCore/Framework/src/SubProcess.cc
Expand Up @@ -145,7 +145,7 @@ namespace edm {
items.initMisc(*processParameterSet_);

// intialize the event setup provider
esp_ = esController.makeProvider(*processParameterSet_);
esp_ = esController.makeProvider(*processParameterSet_, actReg_.get());

branchIDListHelper_ = items.branchIDListHelper();
updateBranchIDListHelper(parentBranchIDListHelper->branchIDLists());
Expand Down
16 changes: 10 additions & 6 deletions FWCore/Framework/test/dependentrecord_t.cppunit.cc
Expand Up @@ -21,6 +21,7 @@
#include "FWCore/Framework/interface/EventSetupRecordProvider.h"
#include "FWCore/Framework/interface/NoRecordException.h"
#include "FWCore/Framework/interface/print_eventsetup_record_dependencies.h"
#include "FWCore/ServiceRegistry/interface/ActivityRegistry.h"

#include "cppunit/extensions/HelperMacros.h"
#include <cstring>
Expand Down Expand Up @@ -76,6 +77,9 @@ CPPUNIT_TEST_SUITE_REGISTRATION(testdependentrecord);
*/

namespace {

edm::ActivityRegistry activityRegistry;

class DummyProxyProvider : public edm::eventsetup::DataProxyProvider {
public:
DummyProxyProvider() {
Expand Down Expand Up @@ -508,7 +512,7 @@ void testdependentrecord::timeAndRunTest()

{
//check that going all the way through EventSetup works properly
edm::eventsetup::EventSetupProvider provider;
edm::eventsetup::EventSetupProvider provider(&activityRegistry);
std::shared_ptr<edm::eventsetup::DataProxyProvider> dummyProv = std::make_shared<DummyProxyProvider>();
provider.add(dummyProv);

Expand Down Expand Up @@ -556,7 +560,7 @@ void testdependentrecord::timeAndRunTest()
{
//check that going all the way through EventSetup works properly
// using two records with open ended IOVs
edm::eventsetup::EventSetupProvider provider;
edm::eventsetup::EventSetupProvider provider(&activityRegistry);
std::shared_ptr<edm::eventsetup::DataProxyProvider> dummyProv = std::make_shared<DummyProxyProvider>();
provider.add(dummyProv);

Expand Down Expand Up @@ -626,7 +630,7 @@ void testdependentrecord::dependentSetproviderTest()

void testdependentrecord::getTest()
{
edm::eventsetup::EventSetupProvider provider;
edm::eventsetup::EventSetupProvider provider(&activityRegistry);
std::shared_ptr<edm::eventsetup::DataProxyProvider> dummyProv = std::make_shared<DummyProxyProvider>();
provider.add(dummyProv);

Expand All @@ -652,7 +656,7 @@ void testdependentrecord::getTest()

void testdependentrecord::oneOfTwoRecordTest()
{
edm::eventsetup::EventSetupProvider provider;
edm::eventsetup::EventSetupProvider provider(&activityRegistry);
std::shared_ptr<edm::eventsetup::DataProxyProvider> dummyProv = std::make_shared<DummyProxyProvider>();
provider.add(dummyProv);

Expand Down Expand Up @@ -682,7 +686,7 @@ void testdependentrecord::oneOfTwoRecordTest()
}
void testdependentrecord::resetTest()
{
edm::eventsetup::EventSetupProvider provider;
edm::eventsetup::EventSetupProvider provider(&activityRegistry);
std::shared_ptr<edm::eventsetup::DataProxyProvider> dummyProv = std::make_shared<DummyProxyProvider>();
provider.add(dummyProv);

Expand Down Expand Up @@ -830,7 +834,7 @@ void testdependentrecord::invalidRecordTest()

void testdependentrecord::extendIOVTest()
{
edm::eventsetup::EventSetupProvider provider;
edm::eventsetup::EventSetupProvider provider(&activityRegistry);
std::shared_ptr<edm::eventsetup::DataProxyProvider> dummyProv = std::make_shared<DummyProxyProvider>();
provider.add(dummyProv);

Expand Down

0 comments on commit 2039650

Please sign in to comment.