-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
Test how to do blocking hand-off between TBB and module thread #34084
Changes from 11 commits
2bbc26b
d8ff91c
56d7540
7c2082a
7740e96
9e3c0d8
291ce4e
0a17d4b
0a965ce
eb972f5
64e6fde
beadab1
e6135ca
54aa65d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,119 @@ | ||
#include "FWCore/Framework/interface/stream/EDAnalyzer.h" | ||
#include "FWCore/Framework/interface/MakerMacros.h" | ||
#include "FWCore/Framework/interface/Event.h" | ||
#include "FWCore/Framework/interface/ModuleContextSentry.h" | ||
#include "FWCore/ServiceRegistry/interface/ServiceRegistry.h" | ||
#include "FWCore/ServiceRegistry/interface/Service.h" | ||
|
||
#include "FWCore/Utilities/interface/RandomNumberGenerator.h" | ||
#include "FWCore/MessageLogger/interface/MessageLogger.h" | ||
#include "FWCore/MessageLogger/interface/edm_MessageLogger.h" | ||
#include <thread> | ||
#include <mutex> | ||
#include <condition_variable> | ||
#include <memory> | ||
#include <iostream> | ||
#include <exception> | ||
|
||
#include "CLHEP/Random/RandFlat.h" | ||
|
||
namespace edmtest { | ||
class TestServicesOnNonFrameworkThreadsAnalyzer : public edm::stream::EDAnalyzer<> { | ||
public: | ||
TestServicesOnNonFrameworkThreadsAnalyzer(edm::ParameterSet const&); | ||
~TestServicesOnNonFrameworkThreadsAnalyzer() override; | ||
|
||
void analyze(edm::Event const&, edm::EventSetup const&) final; | ||
|
||
private: | ||
void runOnOtherThread(); | ||
void shutdownThread(); | ||
std::unique_ptr<std::thread> m_thread; | ||
std::mutex m_mutex; | ||
std::condition_variable m_condVar; | ||
|
||
bool m_managerThreadReady = false; | ||
bool m_continueProcessing = false; | ||
bool m_eventWorkDone = false; | ||
|
||
//context info | ||
edm::ModuleCallingContext const* m_moduleCallingContext = nullptr; | ||
edm::ServiceToken* m_serviceToken = nullptr; | ||
edm::StreamID m_streamID; | ||
std::exception_ptr m_except; | ||
}; | ||
|
||
TestServicesOnNonFrameworkThreadsAnalyzer::TestServicesOnNonFrameworkThreadsAnalyzer(edm::ParameterSet const&) | ||
: m_streamID(edm::StreamID::invalidStreamID()) { | ||
m_thread = std::make_unique<std::thread>([this]() { this->runOnOtherThread(); }); | ||
|
||
m_mutex.lock(); | ||
m_managerThreadReady = true; | ||
m_continueProcessing = true; | ||
} | ||
|
||
TestServicesOnNonFrameworkThreadsAnalyzer::~TestServicesOnNonFrameworkThreadsAnalyzer() { | ||
if (m_thread) { | ||
shutdownThread(); | ||
} | ||
} | ||
|
||
void TestServicesOnNonFrameworkThreadsAnalyzer::analyze(edm::Event const& iEvent, edm::EventSetup const&) { | ||
m_eventWorkDone = false; | ||
m_moduleCallingContext = iEvent.moduleCallingContext(); | ||
edm::ServiceToken token = edm::ServiceRegistry::instance().presentToken(); | ||
m_serviceToken = &token; | ||
m_streamID = iEvent.streamID(); | ||
{ edm::LogSystem("FrameworkThread") << "new Event"; } | ||
m_mutex.unlock(); | ||
{ | ||
std::unique_lock<std::mutex> lk(m_mutex); | ||
m_condVar.notify_one(); | ||
m_condVar.wait(lk, [this] { return this->m_eventWorkDone; }); | ||
lk.release(); | ||
} | ||
edm::LogSystem("FrameworkThread") << " done"; | ||
m_managerThreadReady = true; | ||
if (m_except) { | ||
std::rethrow_exception(m_except); | ||
} | ||
} | ||
|
||
void TestServicesOnNonFrameworkThreadsAnalyzer::runOnOtherThread() { | ||
std::unique_lock<std::mutex> lk(m_mutex); | ||
|
||
do { | ||
m_condVar.wait(lk, [this] { return m_managerThreadReady; }); | ||
if (m_continueProcessing) { | ||
edm::ModuleCallingContext newContext(*m_moduleCallingContext); | ||
edm::ModuleContextSentry sentry(&newContext, m_moduleCallingContext->parent()); | ||
|
||
edm::ServiceRegistry::Operate srSentry(*m_serviceToken); | ||
try { | ||
edm::Service<edm::RandomNumberGenerator> rng; | ||
edm::Service<edm::MessageLogger> ml; | ||
ml->setThreadContext(*m_moduleCallingContext); | ||
edm::LogSystem("ModuleThread") << " ++running with rng " | ||
<< CLHEP::RandFlat::shootInt(&rng->getEngine(m_streamID), 10); | ||
} catch (...) { | ||
m_except = std::current_exception(); | ||
} | ||
} | ||
m_eventWorkDone = true; | ||
m_managerThreadReady = false; | ||
lk.unlock(); | ||
m_condVar.notify_one(); | ||
lk.lock(); | ||
} while (m_continueProcessing); | ||
} | ||
|
||
void TestServicesOnNonFrameworkThreadsAnalyzer::shutdownThread() { | ||
m_continueProcessing = false; | ||
m_mutex.unlock(); | ||
m_condVar.notify_one(); | ||
m_thread->join(); | ||
} | ||
|
||
} // namespace edmtest | ||
|
||
DEFINE_FWK_MODULE(edmtest::TestServicesOnNonFrameworkThreadsAnalyzer); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
import FWCore.ParameterSet.Config as cms | ||
|
||
process = cms.Process("Test") | ||
|
||
process.source = cms.Source("EmptySource") | ||
|
||
process.maxEvents.input = 10 | ||
|
||
process.test = cms.EDAnalyzer("edmtest::TestServicesOnNonFrameworkThreadsAnalyzer") | ||
|
||
process.p = cms.EndPath(process.test) | ||
|
||
process.add_(cms.Service("RandomNumberGeneratorService", | ||
test = cms.PSet(initialSeed = cms.untracked.uint32(12345)) | ||
)) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,41 @@ | ||
#ifndef FWCore_MessageService_MessageLogger_h | ||
#define FWCore_MessageService_MessageLogger_h | ||
|
||
// -*- C++ -*- | ||
// | ||
// Package: MessageService | ||
// Class : MessageLogger | ||
// | ||
/**\class edm::MessageLogger MessageLogger.h FWCore/MessageService/plugins/MessageLogger.h | ||
|
||
Description: Abstract interface for MessageLogger Service | ||
|
||
Usage: | ||
<usage> | ||
|
||
*/ | ||
// | ||
|
||
// system include files | ||
|
||
// user include files | ||
|
||
// forward declarations | ||
|
||
namespace edm { | ||
class ModuleCallingContext; | ||
|
||
class MessageLogger { | ||
public: | ||
virtual ~MessageLogger(); | ||
|
||
virtual void setThreadContext(ModuleCallingContext const&) = 0; | ||
|
||
protected: | ||
MessageLogger() = default; | ||
|
||
}; // MessageLogger | ||
|
||
} // namespace edm | ||
|
||
#endif // FWCore_MessageService_MessageLogger_h |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
// | ||
// MessageLogger.cc | ||
// CMSSW | ||
// | ||
// Created by Chris Jones on 6/10/21. | ||
// | ||
|
||
#include "FWCore/MessageLogger/interface/edm_MessageLogger.h" | ||
|
||
edm::MessageLogger::~MessageLogger() = default; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,6 @@ | ||
<library file="*.cc" name="FWCoreMessageServicePlugins"> | ||
<flags EDM_PLUGIN="1"/> | ||
<use name="FWCore/MessageService"/> | ||
<use name="FWCore/ServiceRegistry"/> | ||
<use name="DataFormats/Provenance"/> | ||
</library> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,11 +1,14 @@ | ||
#include "FWCore/PluginManager/interface/PresenceMacros.h" | ||
#include "FWCore/MessageService/interface/MessageLogger.h" | ||
#include "FWCore/MessageService/plugins/MessageLogger.h" | ||
Dr15Jones marked this conversation as resolved.
Show resolved
Hide resolved
|
||
#include "FWCore/MessageService/interface/SingleThreadMSPresence.h" | ||
#include "FWCore/ServiceRegistry/interface/ServiceMaker.h" | ||
|
||
#pragma GCC visibility push(hidden) | ||
using edm::service::MessageLogger; | ||
using edm::service::SingleThreadMSPresence; | ||
DEFINE_FWK_SERVICE(MessageLogger); | ||
|
||
using MessageLoggerMaker = edm::serviceregistry::AllArgsMaker<edm::MessageLogger, MessageLogger>; | ||
DEFINE_FWK_SERVICE_MAKER(MessageLogger, MessageLoggerMaker); | ||
|
||
DEFINE_FWK_PRESENCE(SingleThreadMSPresence); | ||
#pragma GCC visibility pop |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,8 +17,6 @@ | |
|
||
#include "FWCore/ParameterSet/interface/ParameterSet.h" | ||
|
||
#include "FWCore/MessageService/interface/MessageLogger.h" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why is this no longer needed for e.g. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Things like There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah OK, thanks. I guess those files are getting |
||
|
||
#include "HLTrigger/HLTcore/interface/HLTFilter.h" | ||
#include "DataFormats/HLTReco/interface/TriggerFilterObjectWithRefs.h" | ||
#include "DataFormats/TrackReco/interface/Track.h" | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would this need any specific treatment for ProcessBlock?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know. I didn't look to see if David extended the message logger for that case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@wddgit Could you comment?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK. I will take a look at this one after lunch. There were a few MessageLogger changes I vaguely remember making in the PR, but not many. It might have actually been in the first PR...
I've been trying to finish up this documentation and some slides for next week's Core meeting, before I start addressing the PR comments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like ProcessBlock doesn't work with the interface. That is because, in my opinion, the ProcessBlock transitions in the MessageLogger service are calling the wrong
establishModule
andunestablishModule
calls of the messaging system. They should be calling the ones using theModuleCallingContext
but they are using theModuleDescription
instead.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One comment. For ProcessBlock it always a global context. There are no stream transitions. I am not sure if that is relevant here yet.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe it was incorrect a year ago. I would expect the ProcessBlock transitions to work like the Run or LuminosityBlock transitions, not like the constructor or beginJob transitions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am still looking and trying to remember why I did it this way. There is a unit test and at least for beginProcessBlock and endProcessBlock it seems to be working:
https://cmssdt.cern.ch/lxr/source/FWCore/MessageService/test/unit_test_outputs/u33_all.log
lines 7 and 93
There is no accessInputProcessBlock transition test.
At some level, I intuitively see why you would it expect it to be like Run and LuminosityBlock. Although there are a lot of similarities to beginJob and endJob also. The ProcessBlock transitions are always global, no stream transitions. The modules can run concurrently but otherwise no concurrency, I am not sure how that plays into it. Still looking...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see three differences in the establishModule/unestablishModule functions.
Everything else in the function overloads seems the same. Unless I am missing something, the ModuleDescription overloads are correct and all is OK.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
establishModule
gets aModuleCallingContext
which is needed if a module can get data from another module therby cause the other module to be run. The context allows one to trace that.