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

Don't return pointer to principal #641

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
15 changes: 6 additions & 9 deletions DQMServices/FwkIO/plugins/DQMRootSource.cc
Original file line number Diff line number Diff line change
Expand Up @@ -348,9 +348,9 @@ class DQMRootSource : public edm::InputSource
//NOTE: the following is really read next run auxiliary
virtual boost::shared_ptr<edm::RunAuxiliary> readRunAuxiliary_() override ;
virtual boost::shared_ptr<edm::LuminosityBlockAuxiliary> readLuminosityBlockAuxiliary_() override ;
virtual boost::shared_ptr<edm::RunPrincipal> readRun_(boost::shared_ptr<edm::RunPrincipal> rpCache) override;
virtual boost::shared_ptr<edm::LuminosityBlockPrincipal> readLuminosityBlock_( boost::shared_ptr<edm::LuminosityBlockPrincipal> lbCache) override;
virtual edm::EventPrincipal* readEvent_(edm::EventPrincipal&) override ;
virtual void readRun_(boost::shared_ptr<edm::RunPrincipal> rpCache) override;
virtual void readLuminosityBlock_( boost::shared_ptr<edm::LuminosityBlockPrincipal> lbCache) override;
virtual void readEvent_(edm::EventPrincipal&) override ;

virtual std::unique_ptr<edm::FileBlock> readFile_() override;
virtual void closeFile_() override;
Expand Down Expand Up @@ -482,9 +482,8 @@ DQMRootSource::~DQMRootSource()
//
// member functions
//
edm::EventPrincipal* DQMRootSource::readEvent_(edm::EventPrincipal&)
void DQMRootSource::readEvent_(edm::EventPrincipal&)
{
return 0;
}

edm::InputSource::ItemType DQMRootSource::getNextItemType()
Expand Down Expand Up @@ -524,7 +523,7 @@ DQMRootSource::readLuminosityBlockAuxiliary_()
return boost::shared_ptr<edm::LuminosityBlockAuxiliary>(new edm::LuminosityBlockAuxiliary(m_lumiAux));
}

boost::shared_ptr<edm::RunPrincipal>
void
DQMRootSource::readRun_(boost::shared_ptr<edm::RunPrincipal> rpCache)
{
assert(m_presentIndexItr != m_orderedIndices.end());
Expand Down Expand Up @@ -578,10 +577,9 @@ DQMRootSource::readRun_(boost::shared_ptr<edm::RunPrincipal> rpCache)
jr->reportInputRunNumber(rpCache->id().run());

rpCache->fillRunPrincipal();
return rpCache;
}

boost::shared_ptr<edm::LuminosityBlockPrincipal>
void
DQMRootSource::readLuminosityBlock_( boost::shared_ptr<edm::LuminosityBlockPrincipal> lbCache)
{
assert(m_presentIndexItr != m_orderedIndices.end());
Expand Down Expand Up @@ -616,7 +614,6 @@ DQMRootSource::readLuminosityBlock_( boost::shared_ptr<edm::LuminosityBlockPrinc
jr->reportInputLumiSection(lbCache->id().run(),lbCache->id().luminosityBlock());

lbCache->fillLuminosityBlockPrincipal();
return lbCache;
}

std::unique_ptr<edm::FileBlock>
Expand Down
6 changes: 2 additions & 4 deletions EventFilter/StorageManager/src/DQMHttpSource.cc
Original file line number Diff line number Diff line change
Expand Up @@ -61,16 +61,14 @@ namespace edm
return true;
}

EventPrincipal* DQMHttpSource::read(EventPrincipal& eventPrincipal)
void DQMHttpSource::read(EventPrincipal& eventPrincipal)
{
// make a fake event principal containing no data but the evId and runId from DQMEvent
// and the time stamp from the event at update
EventPrincipal* e = makeEvent(
makeEvent(
eventPrincipal,
eventAuxiliary()
);

return e;
}


Expand Down
2 changes: 1 addition & 1 deletion EventFilter/StorageManager/src/DQMHttpSource.h
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ namespace edm
void setEventAuxiliary(std::unique_ptr<EventAuxiliary> aux) {
eventAuxiliary_ = std::move(aux);
}
virtual EventPrincipal* read(EventPrincipal& eventPrincipal);
virtual void read(EventPrincipal& eventPrincipal);
virtual bool checkNextEvent();
void initializeDQMStore();

Expand Down
4 changes: 2 additions & 2 deletions FWCore/Framework/interface/EventProcessor.h
Original file line number Diff line number Diff line change
Expand Up @@ -255,9 +255,9 @@ namespace edm {
virtual void beginLumi(ProcessHistoryID const& phid, RunNumber_t run, LuminosityBlockNumber_t lumi);
virtual void endLumi(ProcessHistoryID const& phid, RunNumber_t run, LuminosityBlockNumber_t lumi, bool cleaningUpAfterException);

virtual statemachine::Run readAndCacheRun();
virtual statemachine::Run readRun();
virtual statemachine::Run readAndMergeRun();
virtual int readAndCacheLumi();
virtual int readLuminosityBlock();
virtual int readAndMergeLumi();
virtual void writeRun(statemachine::Run const& run);
virtual void deleteRunFromCache(statemachine::Run const& run);
Expand Down
4 changes: 2 additions & 2 deletions FWCore/Framework/interface/IEventProcessor.h
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,9 @@ namespace edm {
virtual void beginLumi(ProcessHistoryID const& phid, RunNumber_t run, LuminosityBlockNumber_t lumi) = 0;
virtual void endLumi(ProcessHistoryID const& phid, RunNumber_t run, LuminosityBlockNumber_t lumi, bool cleaningUpAfterException) = 0;

virtual statemachine::Run readAndCacheRun() = 0;
virtual statemachine::Run readRun() = 0;
virtual statemachine::Run readAndMergeRun() = 0;
virtual int readAndCacheLumi() = 0;
virtual int readLuminosityBlock() = 0;
virtual int readAndMergeLumi() = 0;
virtual void writeRun(statemachine::Run const& run) = 0;
virtual void deleteRunFromCache(statemachine::Run const& run) = 0;
Expand Down
17 changes: 8 additions & 9 deletions FWCore/Framework/interface/InputSource.h
Original file line number Diff line number Diff line change
Expand Up @@ -106,10 +106,10 @@ namespace edm {

/// Read next event
/// Indicate inability to get a new event by returning a null ptr.
EventPrincipal* readEvent(EventPrincipal& ep, StreamContext *);
void readEvent(EventPrincipal& ep, StreamContext *);

/// Read a specific event
EventPrincipal* readEvent(EventPrincipal& ep, EventID const&, StreamContext *);
bool readEvent(EventPrincipal& ep, EventID const&, StreamContext *);

/// Read next luminosity block Auxilary
boost::shared_ptr<LuminosityBlockAuxiliary> readLuminosityBlockAuxiliary();
Expand All @@ -118,13 +118,13 @@ namespace edm {
boost::shared_ptr<RunAuxiliary> readRunAuxiliary();

/// Read next run (new run)
boost::shared_ptr<RunPrincipal> readAndCacheRun(HistoryAppender& historyAppender);
void readRun(boost::shared_ptr<RunPrincipal> runPrincipal, HistoryAppender& historyAppender);

/// Read next run (same as a prior run)
void readAndMergeRun(boost::shared_ptr<RunPrincipal> rp);

/// Read next luminosity block (new lumi)
boost::shared_ptr<LuminosityBlockPrincipal> readAndCacheLumi(HistoryAppender& historyAppender);
void readLuminosityBlock(boost::shared_ptr<LuminosityBlockPrincipal> lumiPrincipal, HistoryAppender& historyAppender);

/// Read next luminosity block (same as a prior lumi)
void readAndMergeLumi(boost::shared_ptr<LuminosityBlockPrincipal> lbp);
Expand Down Expand Up @@ -373,11 +373,10 @@ namespace edm {
ItemType nextItemType_();
virtual boost::shared_ptr<RunAuxiliary> readRunAuxiliary_() = 0;
virtual boost::shared_ptr<LuminosityBlockAuxiliary> readLuminosityBlockAuxiliary_() = 0;
virtual boost::shared_ptr<RunPrincipal> readRun_(boost::shared_ptr<RunPrincipal> runPrincipal);
virtual boost::shared_ptr<LuminosityBlockPrincipal> readLuminosityBlock_(
boost::shared_ptr<LuminosityBlockPrincipal> lumiPrincipal);
virtual EventPrincipal* readEvent_(EventPrincipal& eventPrincipal) = 0;
virtual EventPrincipal* readIt(EventID const&, EventPrincipal& eventPrincipal);
virtual void readRun_(boost::shared_ptr<RunPrincipal> runPrincipal);
virtual void readLuminosityBlock_(boost::shared_ptr<LuminosityBlockPrincipal> lumiPrincipal);
virtual void readEvent_(EventPrincipal& eventPrincipal) = 0;
virtual bool readIt(EventID const&, EventPrincipal& eventPrincipal);
virtual std::unique_ptr<FileBlock> readFile_();
virtual void closeFile_() {}
virtual bool goToEvent_(EventID const& eventID);
Expand Down
4 changes: 2 additions & 2 deletions FWCore/Framework/src/EPStates.cc
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ namespace statemachine {
void HandleRuns::setupCurrentRun() {

runException_ = true;
currentRun_ = ep_.readAndCacheRun();
currentRun_ = ep_.readRun();
runException_ = false;

if(context<Machine>().emptyRunLumiMode() != doNotHandleEmptyRunsAndLumis) {
Expand Down Expand Up @@ -410,7 +410,7 @@ namespace statemachine {
Run const& run = context<HandleRuns>().currentRun();
assert(run != INVALID_RUN);
lumiException_ = true;
currentLumi_ = HandleLumis::LumiID(run.processHistoryID(), run.runNumber(), ep_.readAndCacheLumi());
currentLumi_ = HandleLumis::LumiID(run.processHistoryID(), run.runNumber(), ep_.readLuminosityBlock());

if(context<Machine>().emptyRunLumiMode() == handleEmptyRunsAndLumis) {
assert(context<HandleRuns>().beginRunCalled());
Expand Down
24 changes: 14 additions & 10 deletions FWCore/Framework/src/EventProcessor.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2127,14 +2127,16 @@ namespace edm {
}
}

statemachine::Run EventProcessor::readAndCacheRun() {
statemachine::Run EventProcessor::readRun() {
if (principalCache_.hasRunPrincipal()) {
throw edm::Exception(edm::errors::LogicError)
<< "EventProcessor::readAndCacheRun\n"
<< "EventProcessor::readRun\n"
<< "Illegal attempt to insert run into cache\n"
<< "Contact a Framework Developer\n";
}
principalCache_.insert(input_->readAndCacheRun(*historyAppender_));
boost::shared_ptr<RunPrincipal> rp(new RunPrincipal(input_->runAuxiliary(), preg_, *processConfiguration_, historyAppender_.get(), 0));
input_->readRun(rp, *historyAppender_);
principalCache_.insert(rp);
return statemachine::Run(input_->reducedProcessHistoryID(), input_->run());
}

Expand All @@ -2144,22 +2146,24 @@ namespace edm {
return statemachine::Run(input_->reducedProcessHistoryID(), input_->run());
}

int EventProcessor::readAndCacheLumi() {
int EventProcessor::readLuminosityBlock() {
if (principalCache_.hasLumiPrincipal()) {
throw edm::Exception(edm::errors::LogicError)
<< "EventProcessor::readAndCacheRun\n"
<< "EventProcessor::readRun\n"
<< "Illegal attempt to insert lumi into cache\n"
<< "Contact a Framework Developer\n";
}
if (!principalCache_.hasRunPrincipal()) {
throw edm::Exception(edm::errors::LogicError)
<< "EventProcessor::readAndCacheRun\n"
<< "EventProcessor::readRun\n"
<< "Illegal attempt to insert lumi into cache\n"
<< "Run is invalid\n"
<< "Contact a Framework Developer\n";
}
principalCache_.insert(input_->readAndCacheLumi(*historyAppender_));
principalCache_.lumiPrincipalPtr()->setRunPrincipal(principalCache_.runPrincipalPtr());
boost::shared_ptr<LuminosityBlockPrincipal> lbp(new LuminosityBlockPrincipal(input_->luminosityBlockAuxiliary(), preg_, *processConfiguration_, historyAppender_.get(), 0));
input_->readLuminosityBlock(lbp, *historyAppender_);
lbp->setRunPrincipal(principalCache_.runPrincipalPtr());
principalCache_.insert(lbp);
return input_->luminosityBlock();
}

Expand Down Expand Up @@ -2196,9 +2200,9 @@ namespace edm {
void EventProcessor::readAndProcessEvent() {
//TODO this will have to become per stream
StreamContext streamContext(StreamID{0}, &processContext_);
EventPrincipal *pep = input_->readEvent(principalCache_.eventPrincipal(), &streamContext);
input_->readEvent(principalCache_.eventPrincipal(), &streamContext);
FDEBUG(1) << "\treadEvent\n";
assert(pep != 0);
EventPrincipal* pep = &principalCache_.eventPrincipal();
pep->setLuminosityBlockPrincipal(principalCache_.lumiPrincipalPtr());
assert(pep->luminosityBlockPrincipalPtrValid());
assert(principalCache_.lumiPrincipalPtr()->run() == pep->run());
Expand Down
67 changes: 25 additions & 42 deletions FWCore/Framework/src/InputSource.cc
Original file line number Diff line number Diff line change
Expand Up @@ -283,98 +283,81 @@ namespace edm {
return std::unique_ptr<FileBlock>(new FileBlock);
}

boost::shared_ptr<RunPrincipal>
InputSource::readAndCacheRun(HistoryAppender& historyAppender) {
void
InputSource::readRun(boost::shared_ptr<RunPrincipal> runPrincipal, HistoryAppender& historyAppender) {
RunSourceSentry sentry(*this);
boost::shared_ptr<RunPrincipal> rp(new RunPrincipal(runAuxiliary(), productRegistry_, processConfiguration(), &historyAppender,0));
callWithTryCatchAndPrint<boost::shared_ptr<RunPrincipal> >( [this,&rp](){ return readRun_(rp); }, "Calling InputSource::readRun_" );
return rp;
callWithTryCatchAndPrint<void>( [this,&runPrincipal](){ readRun_(runPrincipal); }, "Calling InputSource::readRun_" );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI technically, passing runPrincipal by value is fine here since it can't be modified.

}

void
InputSource::readAndMergeRun(boost::shared_ptr<RunPrincipal> rp) {
RunSourceSentry sentry(*this);
callWithTryCatchAndPrint<boost::shared_ptr<RunPrincipal> >( [this,&rp](){ return readRun_(rp); }, "Calling InputSource::readRun_" );
callWithTryCatchAndPrint<void>( [this,&rp](){ readRun_(rp); }, "Calling InputSource::readRun_" );
}

boost::shared_ptr<LuminosityBlockPrincipal>
InputSource::readAndCacheLumi(HistoryAppender& historyAppender) {
void
InputSource::readLuminosityBlock(boost::shared_ptr<LuminosityBlockPrincipal> lumiPrincipal, HistoryAppender& historyAppender) {
LumiSourceSentry sentry(*this);
boost::shared_ptr<LuminosityBlockPrincipal> lbp(
new LuminosityBlockPrincipal(luminosityBlockAuxiliary(),
productRegistry_,
processConfiguration(),
&historyAppender,0));
callWithTryCatchAndPrint<boost::shared_ptr<LuminosityBlockPrincipal> >( [this,&lbp](){ return readLuminosityBlock_(lbp); },
"Calling InputSource::readLuminosityBlock_" );
callWithTryCatchAndPrint<void>( [this,&lumiPrincipal](){ readLuminosityBlock_(lumiPrincipal); }, "Calling InputSource::readLuminosityBlock_" );
if(remainingLumis_ > 0) {
--remainingLumis_;
}
return lbp;
}

void
InputSource::readAndMergeLumi(boost::shared_ptr<LuminosityBlockPrincipal> lbp) {
LumiSourceSentry sentry(*this);
callWithTryCatchAndPrint<boost::shared_ptr<LuminosityBlockPrincipal> >( [this,&lbp](){ return readLuminosityBlock_(lbp); },
"Calling InputSource::readLuminosityBlock_" );
callWithTryCatchAndPrint<void>( [this,&lbp](){ readLuminosityBlock_(lbp); }, "Calling InputSource::readLuminosityBlock_" );
if(remainingLumis_ > 0) {
--remainingLumis_;
}
}

boost::shared_ptr<RunPrincipal>
void
InputSource::readRun_(boost::shared_ptr<RunPrincipal> runPrincipal) {
// Note: For the moment, we do not support saving and restoring the state of the
// random number generator if random numbers are generated during processing of runs
// (e.g. beginRun(), endRun())
runPrincipal->fillRunPrincipal();
return runPrincipal;
}

boost::shared_ptr<LuminosityBlockPrincipal>
void
InputSource::readLuminosityBlock_(boost::shared_ptr<LuminosityBlockPrincipal> lumiPrincipal) {
lumiPrincipal->fillLuminosityBlockPrincipal();
return lumiPrincipal;
}

EventPrincipal*
void
InputSource::readEvent(EventPrincipal& ep, StreamContext* streamContext) {
assert(state_ == IsEvent);
assert(!eventLimitReached());

EventPrincipal* result = callWithTryCatchAndPrint<EventPrincipal*>( [this,&ep](){ return readEvent_(ep); }, "Calling InputSource::readEvent_" );
callWithTryCatchAndPrint<void>( [this,&ep](){ readEvent_(ep); }, "Calling InputSource::readEvent_" );
if(receiver_) {
--numberOfEventsBeforeBigSkip_;
}

if(result != 0) {

Event event(*result, moduleDescription(), nullptr);
postRead(event);
if(remainingEvents_ > 0) --remainingEvents_;
++readCount_;
setTimestamp(result->time());
issueReports(result->id());
}
return result;
Event event(ep, moduleDescription(), nullptr);
postRead(event);
if(remainingEvents_ > 0) --remainingEvents_;
++readCount_;
setTimestamp(ep.time());
issueReports(ep.id());
}

EventPrincipal*
bool
InputSource::readEvent(EventPrincipal& ep, EventID const& eventID, StreamContext* streamContext) {
EventPrincipal* result = 0;
bool result = false;

if(!limitReached()) {
//result = callWithTryCatchAndPrint<EventPrincipal*>( [this,ep,&eventID](){ return readIt(eventID, ep); }, "Calling InputSource::readIt" );
//result = callWithTryCatchAndPrint<bool>( [this,ep,&eventID](){ return readIt(eventID, ep); }, "Calling InputSource::readIt" );
result = readIt(eventID, ep);
if(result) {

if(result != 0) {

Event event(*result, moduleDescription(), nullptr);
Event event(ep, moduleDescription(), nullptr);
postRead(event);
if(remainingEvents_ > 0) --remainingEvents_;
++readCount_;
issueReports(result->id());
issueReports(ep.id());
}
}
return result;
Expand Down Expand Up @@ -423,7 +406,7 @@ namespace edm {
// At some point we may want to initiate checkpointing here
}

EventPrincipal*
bool
InputSource::readIt(EventID const&, EventPrincipal&) {
throw Exception(errors::LogicError)
<< "InputSource::readIt()\n"
Expand Down
8 changes: 4 additions & 4 deletions FWCore/Framework/test/MockEventProcessor.cc
Original file line number Diff line number Diff line change
Expand Up @@ -158,8 +158,8 @@ namespace edm {
output_ << "\tendLumi " << run << "/" << lumi << "\n";
}

statemachine::Run MockEventProcessor::readAndCacheRun() {
output_ << "\treadAndCacheRun " << run_ << "\n";
statemachine::Run MockEventProcessor::readRun() {
output_ << "\treadRun " << run_ << "\n";
return statemachine::Run(ProcessHistoryID(), run_);
}

Expand All @@ -168,8 +168,8 @@ namespace edm {
return statemachine::Run(ProcessHistoryID(), run_);
}

int MockEventProcessor::readAndCacheLumi() {
output_ << "\treadAndCacheLumi " << lumi_ << "\n";
int MockEventProcessor::readLuminosityBlock() {
output_ << "\treadLuminosityBlock " << lumi_ << "\n";
return lumi_;
}

Expand Down
4 changes: 2 additions & 2 deletions FWCore/Framework/test/MockEventProcessor.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,9 @@ namespace edm {
virtual void beginLumi(ProcessHistoryID const& phid, RunNumber_t run, LuminosityBlockNumber_t lumi);
virtual void endLumi(ProcessHistoryID const& phid, RunNumber_t run, LuminosityBlockNumber_t lumi, bool cleaningUpAfterException);

virtual statemachine::Run readAndCacheRun();
virtual statemachine::Run readRun();
virtual statemachine::Run readAndMergeRun();
virtual int readAndCacheLumi();
virtual int readLuminosityBlock();
virtual int readAndMergeLumi();
virtual void writeRun(statemachine::Run const& run);
virtual void deleteRunFromCache(statemachine::Run const& run);
Expand Down
Loading