Skip to content

Commit

Permalink
Add ability to measure time in the EventSetup
Browse files Browse the repository at this point in the history
Uses the Timing Service to measure to two time totals.
First, the time while threads are blocked by the mutex in
DataProxy::get. Second, the time to run the getImpl function
to fill the data cache after the mutex lock has been obtained.
  • Loading branch information
wddgit committed Feb 8, 2018
1 parent 5168751 commit 230481c
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 230481c

Please sign in to comment.