From e77e887f8a1dd538fb1af3ef9e47efc168711d36 Mon Sep 17 00:00:00 2001 From: Matti Kortelainen Date: Thu, 26 Dec 2024 23:35:34 +0100 Subject: [PATCH] Use WaitingTaskHolder to signal doneWaiting() instead of WaitingTaskWithArenalHolder in framework In the framework the doneWaiting() is always called from within the main arena in the TBB thread pool, and therefore using WaitingTaskHolder is safe (WaitingTaskWithArenaHolder is needed only when doneWaiting() is called outside of the TBB arena). Avoiding WaitingTaskWithArenaHolder allows to avoid enqueue() operation when the doneWaiting() calls in the framework are the ones that decrease the task reference count to 0. --- .../interface/WaitingTaskWithArenaHolder.h | 5 +- .../src/WaitingTaskWithArenaHolder.cc | 2 +- .../interface/CallbackExternalWork.h | 48 +++++++++---------- FWCore/Framework/interface/StreamSchedule.h | 2 + .../Framework/interface/global/EDFilterBase.h | 8 +--- .../interface/global/EDProducerBase.h | 8 +--- .../interface/global/OutputModuleBase.h | 7 +-- .../Framework/interface/global/implementors.h | 4 +- .../global/outputmoduleAbilityToImplementor.h | 4 +- FWCore/Framework/interface/maker/Worker.h | 27 +++++------ FWCore/Framework/interface/maker/WorkerT.h | 3 +- FWCore/Framework/interface/stream/EDFilter.h | 6 +-- .../interface/stream/EDFilterAdaptorBase.h | 6 +-- .../Framework/interface/stream/EDFilterBase.h | 3 +- .../Framework/interface/stream/EDProducer.h | 9 +--- .../interface/stream/EDProducerAdaptorBase.h | 6 +-- .../interface/stream/EDProducerBase.h | 3 +- .../interface/stream/ProducingModuleHelper.h | 6 +-- FWCore/Framework/src/EventProcessor.cc | 1 + FWCore/Framework/src/TransformerBase.cc | 11 +++-- FWCore/Framework/src/Worker.cc | 10 ++-- FWCore/Framework/src/WorkerT.cc | 24 +++++----- FWCore/Framework/src/global/EDFilterBase.cc | 9 ++-- FWCore/Framework/src/global/EDProducerBase.cc | 9 ++-- .../Framework/src/global/OutputModuleBase.cc | 4 +- .../src/global/implementorsMethods.h | 4 +- .../src/stream/EDFilterAdaptorBase.cc | 4 +- .../src/stream/EDProducerAdaptorBase.cc | 4 +- .../src/stream/ProducingModuleHelper.cc | 10 ++-- 29 files changed, 106 insertions(+), 141 deletions(-) diff --git a/FWCore/Concurrency/interface/WaitingTaskWithArenaHolder.h b/FWCore/Concurrency/interface/WaitingTaskWithArenaHolder.h index 255376d9bdeb8..746181d3eb96c 100644 --- a/FWCore/Concurrency/interface/WaitingTaskWithArenaHolder.h +++ b/FWCore/Concurrency/interface/WaitingTaskWithArenaHolder.h @@ -40,9 +40,8 @@ namespace edm { // eventually intend for the task to be spawned. explicit WaitingTaskWithArenaHolder(oneapi::tbb::task_group&, WaitingTask* iTask); - // Takes ownership of the underlying task and uses the current - // arena. - explicit WaitingTaskWithArenaHolder(WaitingTaskHolder&& iTask); + // Captures the current arena. + explicit WaitingTaskWithArenaHolder(WaitingTaskHolder iTask); ~WaitingTaskWithArenaHolder(); diff --git a/FWCore/Concurrency/src/WaitingTaskWithArenaHolder.cc b/FWCore/Concurrency/src/WaitingTaskWithArenaHolder.cc index 5eeb0e75e5e46..306a49d807baa 100644 --- a/FWCore/Concurrency/src/WaitingTaskWithArenaHolder.cc +++ b/FWCore/Concurrency/src/WaitingTaskWithArenaHolder.cc @@ -25,7 +25,7 @@ namespace edm { m_task->increment_ref_count(); } - WaitingTaskWithArenaHolder::WaitingTaskWithArenaHolder(WaitingTaskHolder&& iTask) + WaitingTaskWithArenaHolder::WaitingTaskWithArenaHolder(WaitingTaskHolder iTask) : m_task(iTask.release_no_decrement()), m_group(iTask.group()), m_arena(std::make_shared(oneapi::tbb::task_arena::attach())) {} diff --git a/FWCore/Framework/interface/CallbackExternalWork.h b/FWCore/Framework/interface/CallbackExternalWork.h index a4acc2889c919..3ffa975292fe2 100644 --- a/FWCore/Framework/interface/CallbackExternalWork.h +++ b/FWCore/Framework/interface/CallbackExternalWork.h @@ -114,10 +114,9 @@ namespace edm { WaitingTaskHolder produceTask = Base::makeProduceTask(group, token, record, es, emitPostPrefetchingSignal, std::move(produceFunctor)); - WaitingTaskWithArenaHolder waitingTaskWithArenaHolder = - makeExceptionHandlerTask(std::move(produceTask), group); + WaitingTaskHolder waitingTaskHolder = makeExceptionHandlerTask(std::move(produceTask), group); - return makeAcquireTask(std::move(waitingTaskWithArenaHolder), group, token, record, es); + return makeAcquireTask(std::move(waitingTaskHolder), group, token, record, es); }, std::move(iTask), iRecord, @@ -134,7 +133,7 @@ namespace edm { const TDecorator& iDec = TDecorator()) : Base(iProd, std::move(iProduceFunc), iID, iDec), acquireFunction_(std::move(iAcquireFunc)) {} - WaitingTaskHolder makeAcquireTask(WaitingTaskWithArenaHolder waitingTaskWithArenaHolder, + WaitingTaskHolder makeAcquireTask(WaitingTaskHolder waitingTaskHolder, oneapi::tbb::task_group* group, ServiceWeakToken const& serviceToken, EventSetupRecordImpl const* record, @@ -142,7 +141,7 @@ namespace edm { return WaitingTaskHolder( *group, make_waiting_task( - [this, holder = std::move(waitingTaskWithArenaHolder), group, serviceToken, record, eventSetupImpl]( + [this, holder = std::move(waitingTaskHolder), group, serviceToken, record, eventSetupImpl]( std::exception_ptr const* iException) mutable { std::exception_ptr excptr; if (iException) { @@ -191,7 +190,7 @@ namespace edm { ESModuleCallingContext const& context_; }; EndGuard guard(record, context); - acquireCache_ = (*acquireFunction_)(rec, holder); + acquireCache_ = (*acquireFunction_)(rec, WaitingTaskWithArenaHolder(holder)); }); } catch (cms::Exception& iException) { iException.addContext("Running acquire"); @@ -202,25 +201,24 @@ namespace edm { })); } - WaitingTaskWithArenaHolder makeExceptionHandlerTask(WaitingTaskHolder produceTask, - oneapi::tbb::task_group* group) { - return WaitingTaskWithArenaHolder(*group, - make_waiting_task([this, produceTask = std::move(produceTask)]( - std::exception_ptr const* iException) mutable { - std::exception_ptr excptr; - if (iException) { - excptr = *iException; - } - if (excptr) { - try { - convertException::wrap([excptr]() { std::rethrow_exception(excptr); }); - } catch (cms::Exception& exception) { - exception.addContext("Running acquire and external work"); - edm::exceptionContext(exception, Base::callingContext()); - produceTask.doneWaiting(std::current_exception()); - } - } - })); + WaitingTaskHolder makeExceptionHandlerTask(WaitingTaskHolder produceTask, oneapi::tbb::task_group* group) { + return WaitingTaskHolder(*group, + make_waiting_task([this, produceTask = std::move(produceTask)]( + std::exception_ptr const* iException) mutable { + std::exception_ptr excptr; + if (iException) { + excptr = *iException; + } + if (excptr) { + try { + convertException::wrap([excptr]() { std::rethrow_exception(excptr); }); + } catch (cms::Exception& exception) { + exception.addContext("Running acquire and external work"); + edm::exceptionContext(exception, Base::callingContext()); + produceTask.doneWaiting(std::current_exception()); + } + } + })); } std::shared_ptr acquireFunction_; diff --git a/FWCore/Framework/interface/StreamSchedule.h b/FWCore/Framework/interface/StreamSchedule.h index 09614920c13c9..503c57972d994 100644 --- a/FWCore/Framework/interface/StreamSchedule.h +++ b/FWCore/Framework/interface/StreamSchedule.h @@ -90,6 +90,8 @@ #include "FWCore/Utilities/interface/propagate_const.h" #include "FWCore/Utilities/interface/thread_safety_macros.h" +#include "oneapi/tbb/task_arena.h" + #include #include #include diff --git a/FWCore/Framework/interface/global/EDFilterBase.h b/FWCore/Framework/interface/global/EDFilterBase.h index c1b2cae96cbf5..a688e07befb0f 100644 --- a/FWCore/Framework/interface/global/EDFilterBase.h +++ b/FWCore/Framework/interface/global/EDFilterBase.h @@ -37,7 +37,6 @@ namespace edm { class StreamID; class ActivityRegistry; class ThinnedAssociationsHelper; - class WaitingTaskWithArenaHolder; class EventForTransformer; class ServiceWeakToken; @@ -75,10 +74,7 @@ namespace edm { private: bool doEvent(EventTransitionInfo const&, ActivityRegistry*, ModuleCallingContext const*); - void doAcquire(EventTransitionInfo const&, - ActivityRegistry*, - ModuleCallingContext const*, - WaitingTaskWithArenaHolder&); + void doAcquire(EventTransitionInfo const&, ActivityRegistry*, ModuleCallingContext const*, WaitingTaskHolder&&); void doTransformAsync(WaitingTaskHolder iTask, size_t iTransformIndex, EventPrincipal const& iEvent, @@ -169,7 +165,7 @@ namespace edm { virtual bool hasAcquire() const noexcept { return false; } bool hasAccumulator() const noexcept { return false; } - virtual void doAcquire_(StreamID, Event const&, edm::EventSetup const&, WaitingTaskWithArenaHolder&); + virtual void doAcquire_(StreamID, Event const&, edm::EventSetup const&, WaitingTaskHolder&&); void setModuleDescription(ModuleDescription const& md) { moduleDescription_ = md; } ModuleDescription moduleDescription_; diff --git a/FWCore/Framework/interface/global/EDProducerBase.h b/FWCore/Framework/interface/global/EDProducerBase.h index e41ad995bbdf2..47c1386ac90b0 100644 --- a/FWCore/Framework/interface/global/EDProducerBase.h +++ b/FWCore/Framework/interface/global/EDProducerBase.h @@ -38,7 +38,6 @@ namespace edm { class GlobalSchedule; class ActivityRegistry; class ThinnedAssociationsHelper; - class WaitingTaskWithArenaHolder; class EventForTransformer; class ServiceWeakToken; @@ -78,10 +77,7 @@ namespace edm { private: bool doEvent(EventTransitionInfo const&, ActivityRegistry*, ModuleCallingContext const*); - void doAcquire(EventTransitionInfo const&, - ActivityRegistry*, - ModuleCallingContext const*, - WaitingTaskWithArenaHolder&); + void doAcquire(EventTransitionInfo const&, ActivityRegistry*, ModuleCallingContext const*, WaitingTaskHolder&&); void doTransformAsync(WaitingTaskHolder iTask, size_t iTransformIndex, EventPrincipal const& iEvent, @@ -173,7 +169,7 @@ namespace edm { virtual bool hasAcquire() const noexcept { return false; } - virtual void doAcquire_(StreamID, Event const&, edm::EventSetup const&, WaitingTaskWithArenaHolder&); + virtual void doAcquire_(StreamID, Event const&, edm::EventSetup const&, WaitingTaskHolder&&); void setModuleDescription(ModuleDescription const& md) { moduleDescription_ = md; } ModuleDescription moduleDescription_; diff --git a/FWCore/Framework/interface/global/OutputModuleBase.h b/FWCore/Framework/interface/global/OutputModuleBase.h index 38ebb5ec88962..f2b8bc21d202f 100644 --- a/FWCore/Framework/interface/global/OutputModuleBase.h +++ b/FWCore/Framework/interface/global/OutputModuleBase.h @@ -59,10 +59,7 @@ namespace edm { void doEndStream(StreamID id) { doEndStream_(id); } bool doEvent(EventTransitionInfo const&, ActivityRegistry*, ModuleCallingContext const*); - void doAcquire(EventTransitionInfo const&, - ActivityRegistry*, - ModuleCallingContext const*, - WaitingTaskWithArenaHolder&); + void doAcquire(EventTransitionInfo const&, ActivityRegistry*, ModuleCallingContext const*, WaitingTaskHolder&&); //For now this is a placeholder /*virtual*/ void preActionBeforeRunEventAsync(WaitingTaskHolder iTask, ModuleCallingContext const& iModuleCallingContext, @@ -86,7 +83,7 @@ namespace edm { virtual void doEndRunSummary_(RunForOutput const&, EventSetup const&) {} virtual void doBeginLuminosityBlockSummary_(LuminosityBlockForOutput const&, EventSetup const&) {} virtual void doEndLuminosityBlockSummary_(LuminosityBlockForOutput const&, EventSetup const&) {} - virtual void doAcquire_(StreamID, EventForOutput const&, WaitingTaskWithArenaHolder&) {} + virtual void doAcquire_(StreamID, EventForOutput const&, WaitingTaskHolder&&) {} virtual bool hasAcquire() const noexcept { return false; } }; diff --git a/FWCore/Framework/interface/global/implementors.h b/FWCore/Framework/interface/global/implementors.h index 2eda1fab048f6..8c42085b76a5a 100644 --- a/FWCore/Framework/interface/global/implementors.h +++ b/FWCore/Framework/interface/global/implementors.h @@ -48,9 +48,9 @@ // forward declarations namespace edm { - class WaitingTaskWithArenaHolder; class ServiceWeakToken; class ActivityRegistry; + class WaitingTaskWithArenaHolder; namespace global { namespace impl { @@ -436,7 +436,7 @@ namespace edm { private: bool hasAcquire() const noexcept override { return true; } - void doAcquire_(StreamID, Event const&, edm::EventSetup const&, WaitingTaskWithArenaHolder&) final; + void doAcquire_(StreamID, Event const&, edm::EventSetup const&, WaitingTaskHolder&&) final; virtual void acquire(StreamID, Event const&, edm::EventSetup const&, WaitingTaskWithArenaHolder) const = 0; }; diff --git a/FWCore/Framework/interface/global/outputmoduleAbilityToImplementor.h b/FWCore/Framework/interface/global/outputmoduleAbilityToImplementor.h index be70c3e5c317e..6770e78d5db87 100644 --- a/FWCore/Framework/interface/global/outputmoduleAbilityToImplementor.h +++ b/FWCore/Framework/interface/global/outputmoduleAbilityToImplementor.h @@ -141,8 +141,8 @@ namespace edm { private: bool hasAcquire() const noexcept override { return true; } - void doAcquire_(StreamID id, EventForOutput const& event, WaitingTaskWithArenaHolder& holder) final { - acquire(id, event, holder); + void doAcquire_(StreamID id, EventForOutput const& event, WaitingTaskHolder&& holder) final { + acquire(id, event, WaitingTaskWithArenaHolder(std::move(holder))); } virtual void acquire(StreamID, EventForOutput const&, WaitingTaskWithArenaHolder) const = 0; diff --git a/FWCore/Framework/interface/maker/Worker.h b/FWCore/Framework/interface/maker/Worker.h index 06de21a714a68..51bbe1d387231 100644 --- a/FWCore/Framework/interface/maker/Worker.h +++ b/FWCore/Framework/interface/maker/Worker.h @@ -32,7 +32,6 @@ the worker is reset(). #include "FWCore/Framework/interface/ProductResolverIndexAndSkipBit.h" #include "FWCore/Concurrency/interface/WaitingTask.h" #include "FWCore/Concurrency/interface/WaitingTaskHolder.h" -#include "FWCore/Concurrency/interface/WaitingTaskWithArenaHolder.h" #include "FWCore/Concurrency/interface/WaitingTaskList.h" #include "FWCore/MessageLogger/interface/MessageLogger.h" #include "FWCore/ServiceRegistry/interface/ActivityRegistry.h" @@ -267,9 +266,7 @@ namespace edm { virtual void itemsToGetForSelection(std::vector&) const = 0; virtual bool implNeedToRunSelection() const noexcept = 0; - virtual void implDoAcquire(EventTransitionInfo const&, - ModuleCallingContext const*, - WaitingTaskWithArenaHolder&) = 0; + virtual void implDoAcquire(EventTransitionInfo const&, ModuleCallingContext const*, WaitingTaskHolder&&) = 0; virtual void implDoTransformAsync(WaitingTaskHolder, size_t iTransformIndex, @@ -394,12 +391,14 @@ namespace edm { ParentContext const&, typename T::Context const*) noexcept; - void runAcquire(EventTransitionInfo const&, ParentContext const&, WaitingTaskWithArenaHolder&); + // runAcquire() must take a copy of WaitingTaskHolder + // see comment in runAcquireAfterAsyncPrefetch() definition + void runAcquire(EventTransitionInfo const&, ParentContext const&, WaitingTaskHolder); void runAcquireAfterAsyncPrefetch(std::exception_ptr, EventTransitionInfo const&, ParentContext const&, - WaitingTaskWithArenaHolder) noexcept; + WaitingTaskHolder) noexcept; std::exception_ptr handleExternalWorkException(std::exception_ptr iEPtr, ParentContext const& parentContext) noexcept; @@ -519,7 +518,7 @@ namespace edm { typename T::TransitionInfoType const&, ServiceToken const&, ParentContext const&, - WaitingTaskWithArenaHolder) noexcept {} + WaitingTaskHolder) noexcept {} void execute() final {} }; @@ -530,7 +529,7 @@ namespace edm { EventTransitionInfo const& eventTransitionInfo, ServiceToken const& token, ParentContext const& parentContext, - WaitingTaskWithArenaHolder holder) noexcept + WaitingTaskHolder holder) noexcept : m_worker(worker), m_eventTransitionInfo(eventTransitionInfo), m_parentContext(parentContext), @@ -545,7 +544,7 @@ namespace edm { // to hold the exception_ptr std::exception_ptr temp_excptr; auto excptr = exceptionPtr(); - // Caught exception is passed to Worker::runModuleAfterAsyncPrefetch(), which propagates it via WaitingTaskWithArenaHolder + // Caught exception is passed to Worker::runModuleAfterAsyncPrefetch(), which propagates it via WaitingTaskHolder CMS_SA_ALLOW try { //pre was called in prefetchAsync m_worker->emitPostModuleEventPrefetchingSignal(); @@ -563,12 +562,12 @@ namespace edm { info = m_eventTransitionInfo, parentContext = m_parentContext, serviceToken = m_serviceToken, - holder = m_holder]() { + holder = std::move(m_holder)]() { //Need to make the services available ServiceRegistry::Operate operateRunAcquire(serviceToken.lock()); std::exception_ptr ptr; - worker->runAcquireAfterAsyncPrefetch(ptr, info, parentContext, holder); + worker->runAcquireAfterAsyncPrefetch(ptr, info, parentContext, std::move(holder)); }); return; } @@ -581,7 +580,7 @@ namespace edm { Worker* m_worker; EventTransitionInfo m_eventTransitionInfo; ParentContext const m_parentContext; - WaitingTaskWithArenaHolder m_holder; + WaitingTaskHolder m_holder; ServiceWeakToken m_serviceToken; }; @@ -1127,7 +1126,7 @@ namespace edm { auto* group = task.group(); moduleTask = make_waiting_task( [this, weakToken, transitionInfo, parentContext, ownRunTask, group](std::exception_ptr const* iExcept) { - WaitingTaskWithArenaHolder runTaskHolder( + WaitingTaskHolder runTaskHolder( *group, new HandleExternalWorkExceptionTask(this, group, ownRunTask->release(), parentContext)); AcquireTask t(this, transitionInfo, weakToken.lock(), parentContext, runTaskHolder); t.execute(); @@ -1154,7 +1153,7 @@ namespace edm { auto group = task.group(); if constexpr (T::isEvent_) { if (hasAcquire()) { - WaitingTaskWithArenaHolder runTaskHolder( + WaitingTaskHolder runTaskHolder( *group, new HandleExternalWorkExceptionTask(this, group, moduleTask, parentContext)); moduleTask = new AcquireTask(this, transitionInfo, token, parentContext, std::move(runTaskHolder)); } diff --git a/FWCore/Framework/interface/maker/WorkerT.h b/FWCore/Framework/interface/maker/WorkerT.h index 6878a753e63af..84befa4ddee64 100644 --- a/FWCore/Framework/interface/maker/WorkerT.h +++ b/FWCore/Framework/interface/maker/WorkerT.h @@ -27,7 +27,6 @@ namespace edm { class ModuleProcessName; class ProductResolverIndexAndSkipBit; class ThinnedAssociationsHelper; - class WaitingTaskWithArenaHolder; template class WorkerT : public Worker { @@ -90,7 +89,7 @@ namespace edm { void itemsToGetForSelection(std::vector&) const final; bool implNeedToRunSelection() const noexcept final; - void implDoAcquire(EventTransitionInfo const&, ModuleCallingContext const*, WaitingTaskWithArenaHolder&) final; + void implDoAcquire(EventTransitionInfo const&, ModuleCallingContext const*, WaitingTaskHolder&&) final; size_t transformIndex(edm::BranchDescription const&) const noexcept final; void implDoTransformAsync(WaitingTaskHolder, diff --git a/FWCore/Framework/interface/stream/EDFilter.h b/FWCore/Framework/interface/stream/EDFilter.h index 2b6be0cce9b3f..3109e599e807f 100644 --- a/FWCore/Framework/interface/stream/EDFilter.h +++ b/FWCore/Framework/interface/stream/EDFilter.h @@ -28,8 +28,6 @@ namespace edm { - class WaitingTaskWithArenaHolder; - namespace stream { template @@ -66,8 +64,8 @@ namespace edm { bool hasAbilityToProduceInEndLumis() const final { return HasAbilityToProduceInEndLumis::value; } private: - void doAcquire_(Event const& ev, EventSetup const& es, WaitingTaskWithArenaHolder& holder) final { - doAcquireIfNeeded(this, ev, es, holder); + void doAcquire_(Event const& ev, EventSetup const& es, WaitingTaskHolder&& holder) final { + doAcquireIfNeeded(this, ev, es, std::move(holder)); } }; diff --git a/FWCore/Framework/interface/stream/EDFilterAdaptorBase.h b/FWCore/Framework/interface/stream/EDFilterAdaptorBase.h index e0fdf609ab041..0c3d3080827f1 100644 --- a/FWCore/Framework/interface/stream/EDFilterAdaptorBase.h +++ b/FWCore/Framework/interface/stream/EDFilterAdaptorBase.h @@ -36,7 +36,6 @@ namespace edm { class ModuleCallingContext; class ActivityRegistry; - class WaitingTaskWithArenaHolder; namespace maker { template @@ -70,10 +69,7 @@ namespace edm { private: bool doEvent(EventTransitionInfo const&, ActivityRegistry*, ModuleCallingContext const*); - void doAcquire(EventTransitionInfo const&, - ActivityRegistry*, - ModuleCallingContext const*, - WaitingTaskWithArenaHolder&); + void doAcquire(EventTransitionInfo const&, ActivityRegistry*, ModuleCallingContext const*, WaitingTaskHolder&&); //For now this is a placeholder /*virtual*/ void preActionBeforeRunEventAsync(WaitingTaskHolder, diff --git a/FWCore/Framework/interface/stream/EDFilterBase.h b/FWCore/Framework/interface/stream/EDFilterBase.h index 325676c83ef3e..aebf037a31a65 100644 --- a/FWCore/Framework/interface/stream/EDFilterBase.h +++ b/FWCore/Framework/interface/stream/EDFilterBase.h @@ -33,7 +33,6 @@ namespace edm { class ProductRegistry; class ThinnedAssociationsHelper; - class WaitingTaskWithArenaHolder; namespace stream { class EDFilterAdaptorBase; @@ -71,7 +70,7 @@ namespace edm { virtual void registerThinnedAssociations(ProductRegistry const&, ThinnedAssociationsHelper&) {} - virtual void doAcquire_(Event const&, EventSetup const&, WaitingTaskWithArenaHolder&) = 0; + virtual void doAcquire_(Event const&, EventSetup const&, WaitingTaskHolder&&) = 0; void setModuleDescriptionPtr(ModuleDescription const* iDesc) { moduleDescriptionPtr_ = iDesc; } // ---------- member data -------------------------------- diff --git a/FWCore/Framework/interface/stream/EDProducer.h b/FWCore/Framework/interface/stream/EDProducer.h index 71a7dbd2e3a88..100201f5b2dc0 100644 --- a/FWCore/Framework/interface/stream/EDProducer.h +++ b/FWCore/Framework/interface/stream/EDProducer.h @@ -27,11 +27,7 @@ #include "FWCore/Framework/interface/stream/ProducingModuleHelper.h" namespace edm { - - class WaitingTaskWithArenaHolder; - namespace stream { - template class EDProducer : public AbilityToImplementor::Type..., public std::conditional::kHasIt or @@ -70,11 +66,10 @@ namespace edm { bool hasAbilityToProduceInEndLumis() const final { return HasAbilityToProduceInEndLumis::value; } private: - void doAcquire_(Event const& ev, EventSetup const& es, WaitingTaskWithArenaHolder& holder) final { - doAcquireIfNeeded(this, ev, es, holder); + void doAcquire_(Event const& ev, EventSetup const& es, WaitingTaskHolder&& holder) final { + doAcquireIfNeeded(this, ev, es, std::move(holder)); } }; - } // namespace stream } // namespace edm diff --git a/FWCore/Framework/interface/stream/EDProducerAdaptorBase.h b/FWCore/Framework/interface/stream/EDProducerAdaptorBase.h index 3653c7929cd0a..ce216d2e3f1d0 100644 --- a/FWCore/Framework/interface/stream/EDProducerAdaptorBase.h +++ b/FWCore/Framework/interface/stream/EDProducerAdaptorBase.h @@ -36,7 +36,6 @@ namespace edm { class ModuleCallingContext; class ActivityRegistry; - class WaitingTaskWithArenaHolder; namespace maker { template @@ -70,10 +69,7 @@ namespace edm { private: bool doEvent(EventTransitionInfo const&, ActivityRegistry*, ModuleCallingContext const*); - void doAcquire(EventTransitionInfo const&, - ActivityRegistry*, - ModuleCallingContext const*, - WaitingTaskWithArenaHolder&); + void doAcquire(EventTransitionInfo const&, ActivityRegistry*, ModuleCallingContext const*, WaitingTaskHolder&&); //For now this is a placeholder /*virtual*/ void preActionBeforeRunEventAsync(WaitingTaskHolder, diff --git a/FWCore/Framework/interface/stream/EDProducerBase.h b/FWCore/Framework/interface/stream/EDProducerBase.h index a272ec1ecc036..6fc4b5d023590 100644 --- a/FWCore/Framework/interface/stream/EDProducerBase.h +++ b/FWCore/Framework/interface/stream/EDProducerBase.h @@ -35,7 +35,6 @@ namespace edm { class WorkerT; class ProductRegistry; class ThinnedAssociationsHelper; - class WaitingTaskWithArenaHolder; class EventForTransformer; namespace stream { @@ -74,7 +73,7 @@ namespace edm { virtual void registerThinnedAssociations(ProductRegistry const&, ThinnedAssociationsHelper&) {} - virtual void doAcquire_(Event const&, EventSetup const&, WaitingTaskWithArenaHolder&) = 0; + virtual void doAcquire_(Event const&, EventSetup const&, WaitingTaskHolder&&) = 0; virtual size_t transformIndex_(edm::BranchDescription const& iBranch) const noexcept; virtual ProductResolverIndex transformPrefetch_(std::size_t iIndex) const noexcept; virtual void transformAsync_(WaitingTaskHolder iTask, diff --git a/FWCore/Framework/interface/stream/ProducingModuleHelper.h b/FWCore/Framework/interface/stream/ProducingModuleHelper.h index 54074507ae1fe..a1c168ec6649e 100644 --- a/FWCore/Framework/interface/stream/ProducingModuleHelper.h +++ b/FWCore/Framework/interface/stream/ProducingModuleHelper.h @@ -11,7 +11,7 @@ namespace edm { class Event; class EventSetup; - class WaitingTaskWithArenaHolder; + class WaitingTaskHolder; namespace stream { @@ -22,9 +22,9 @@ namespace edm { // Two overloaded functions, the first is called by doAcquire_ // when the module inherits from ExternalWork. The first function // calls acquire, while the second function does nothing. - void doAcquireIfNeeded(impl::ExternalWork*, Event const&, EventSetup const&, WaitingTaskWithArenaHolder&); + void doAcquireIfNeeded(impl::ExternalWork*, Event const&, EventSetup const&, WaitingTaskHolder&&); - void doAcquireIfNeeded(void*, Event const&, EventSetup const&, WaitingTaskWithArenaHolder&); + void doAcquireIfNeeded(void*, Event const&, EventSetup const&, WaitingTaskHolder&&); } // namespace stream } // namespace edm #endif diff --git a/FWCore/Framework/src/EventProcessor.cc b/FWCore/Framework/src/EventProcessor.cc index a3c4999c7341a..56708d37e1a7d 100644 --- a/FWCore/Framework/src/EventProcessor.cc +++ b/FWCore/Framework/src/EventProcessor.cc @@ -94,6 +94,7 @@ #include #include "oneapi/tbb/task.h" +#include "oneapi/tbb/task_arena.h" //Used for CPU affinity #ifndef __APPLE__ diff --git a/FWCore/Framework/src/TransformerBase.cc b/FWCore/Framework/src/TransformerBase.cc index c5cfe9244bbe8..098f1e28f4711 100644 --- a/FWCore/Framework/src/TransformerBase.cc +++ b/FWCore/Framework/src/TransformerBase.cc @@ -143,11 +143,16 @@ namespace edm { handle); } }); - WaitingTaskWithArenaHolder wta(*iHolder.group(), nextTask); + WaitingTaskHolder wth(*iHolder.group(), nextTask); CMS_SA_ALLOW try { - *cache = transformInfo_.get(iIndex)(streamContext.streamID(), *(handle->wrapper()), wta); + // wth must be copied into wta below so that the + // wth.doneWaiting() is called after the pre-transform + // function has finished + WaitingTaskWithArenaHolder wta(wth); + *cache = + transformInfo_.get(iIndex)(streamContext.streamID(), *(handle->wrapper()), std::move(wta)); } catch (...) { - wta.doneWaiting(std::current_exception()); + wth.doneWaiting(std::current_exception()); } } } else { diff --git a/FWCore/Framework/src/Worker.cc b/FWCore/Framework/src/Worker.cc index 6a4a912b1ef34..f07cd5c20c17c 100644 --- a/FWCore/Framework/src/Worker.cc +++ b/FWCore/Framework/src/Worker.cc @@ -395,10 +395,10 @@ namespace edm { void Worker::runAcquire(EventTransitionInfo const& info, ParentContext const& parentContext, - WaitingTaskWithArenaHolder& holder) { + WaitingTaskHolder holder) { ModuleContextSentry moduleContextSentry(&moduleCallingContext_, parentContext); try { - convertException::wrap([&]() { this->implDoAcquire(info, &moduleCallingContext_, holder); }); + convertException::wrap([&]() { this->implDoAcquire(info, &moduleCallingContext_, std::move(holder)); }); } catch (cms::Exception& ex) { edm::exceptionContext(ex, moduleCallingContext_); if (shouldRethrowException(std::current_exception(), parentContext, true, shouldTryToContinue_)) { @@ -411,7 +411,7 @@ namespace edm { void Worker::runAcquireAfterAsyncPrefetch(std::exception_ptr iEPtr, EventTransitionInfo const& eventTransitionInfo, ParentContext const& parentContext, - WaitingTaskWithArenaHolder holder) noexcept { + WaitingTaskHolder holder) noexcept { ranAcquireWithoutException_ = false; std::exception_ptr exceptionPtr; if (iEPtr) { @@ -420,8 +420,10 @@ namespace edm { } moduleCallingContext_.setContext(ModuleCallingContext::State::kInvalid, ParentContext(), nullptr); } else { - // Caught exception is propagated via WaitingTaskWithArenaHolder + // Caught exception is propagated via WaitingTaskHolder CMS_SA_ALLOW try { + // holder is copied to runAcquire in order to be independent + // of the lifetime of the WaitingTaskHolder inside runAcquire runAcquire(eventTransitionInfo, parentContext, holder); ranAcquireWithoutException_ = true; } catch (...) { diff --git a/FWCore/Framework/src/WorkerT.cc b/FWCore/Framework/src/WorkerT.cc index f18335f54268d..238e3ea8b446f 100644 --- a/FWCore/Framework/src/WorkerT.cc +++ b/FWCore/Framework/src/WorkerT.cc @@ -203,43 +203,41 @@ namespace edm { } template - inline void WorkerT::implDoAcquire(EventTransitionInfo const&, - ModuleCallingContext const*, - WaitingTaskWithArenaHolder&) {} + inline void WorkerT::implDoAcquire(EventTransitionInfo const&, ModuleCallingContext const*, WaitingTaskHolder&&) {} template <> inline void WorkerT::implDoAcquire(EventTransitionInfo const& info, ModuleCallingContext const* mcc, - WaitingTaskWithArenaHolder& holder) { - module_->doAcquire(info, activityRegistry(), mcc, holder); + WaitingTaskHolder&& holder) { + module_->doAcquire(info, activityRegistry(), mcc, std::move(holder)); } template <> inline void WorkerT::implDoAcquire(EventTransitionInfo const& info, ModuleCallingContext const* mcc, - WaitingTaskWithArenaHolder& holder) { - module_->doAcquire(info, activityRegistry(), mcc, holder); + WaitingTaskHolder&& holder) { + module_->doAcquire(info, activityRegistry(), mcc, std::move(holder)); } template <> inline void WorkerT::implDoAcquire(EventTransitionInfo const& info, ModuleCallingContext const* mcc, - WaitingTaskWithArenaHolder& holder) { - module_->doAcquire(info, activityRegistry(), mcc, holder); + WaitingTaskHolder&& holder) { + module_->doAcquire(info, activityRegistry(), mcc, std::move(holder)); } template <> inline void WorkerT::implDoAcquire(EventTransitionInfo const& info, ModuleCallingContext const* mcc, - WaitingTaskWithArenaHolder& holder) { - module_->doAcquire(info, activityRegistry(), mcc, holder); + WaitingTaskHolder&& holder) { + module_->doAcquire(info, activityRegistry(), mcc, std::move(holder)); } template <> inline void WorkerT::implDoAcquire(EventTransitionInfo const& info, ModuleCallingContext const* mcc, - WaitingTaskWithArenaHolder& holder) { - module_->doAcquire(info, activityRegistry(), mcc, holder); + WaitingTaskHolder&& holder) { + module_->doAcquire(info, activityRegistry(), mcc, std::move(holder)); } template diff --git a/FWCore/Framework/src/global/EDFilterBase.cc b/FWCore/Framework/src/global/EDFilterBase.cc index 44d9f7a005073..46220c89dd546 100644 --- a/FWCore/Framework/src/global/EDFilterBase.cc +++ b/FWCore/Framework/src/global/EDFilterBase.cc @@ -36,9 +36,6 @@ // constants, enums and typedefs // namespace edm { - - class WaitingTaskWithArenaHolder; - namespace global { // // static data member definitions @@ -72,7 +69,7 @@ namespace edm { void EDFilterBase::doAcquire(EventTransitionInfo const& info, ActivityRegistry* act, ModuleCallingContext const* mcc, - WaitingTaskWithArenaHolder& holder) { + WaitingTaskHolder&& holder) { EventAcquireSignalsSentry sentry(act, mcc); Event e(info, moduleDescription_, mcc); e.setConsumer(this); @@ -81,7 +78,7 @@ namespace edm { ESParentContext parentC(mcc); const EventSetup c{ info, static_cast(Transition::Event), esGetTokenIndices(Transition::Event), parentC}; - this->doAcquire_(e.streamID(), e, c, holder); + this->doAcquire_(e.streamID(), e, c, std::move(holder)); } void EDFilterBase::doTransformAsync(WaitingTaskHolder iTask, @@ -293,7 +290,7 @@ namespace edm { void EDFilterBase::clearInputProcessBlockCaches() {} - void EDFilterBase::doAcquire_(StreamID, Event const&, EventSetup const&, WaitingTaskWithArenaHolder&) {} + void EDFilterBase::doAcquire_(StreamID, Event const&, EventSetup const&, WaitingTaskHolder&&) {} void EDFilterBase::fillDescriptions(ConfigurationDescriptions& descriptions) { ParameterSetDescription desc; diff --git a/FWCore/Framework/src/global/EDProducerBase.cc b/FWCore/Framework/src/global/EDProducerBase.cc index 4377f3683e09e..d645a69d5180b 100644 --- a/FWCore/Framework/src/global/EDProducerBase.cc +++ b/FWCore/Framework/src/global/EDProducerBase.cc @@ -36,9 +36,6 @@ // constants, enums and typedefs // namespace edm { - - class WaitingTaskWithArenaHolder; - namespace global { // // static data member definitions @@ -78,7 +75,7 @@ namespace edm { void EDProducerBase::doAcquire(EventTransitionInfo const& info, ActivityRegistry* act, ModuleCallingContext const* mcc, - WaitingTaskWithArenaHolder& holder) { + WaitingTaskHolder&& holder) { EventAcquireSignalsSentry sentry(act, mcc); Event e(info, moduleDescription_, mcc); e.setConsumer(this); @@ -87,7 +84,7 @@ namespace edm { ESParentContext parentC(mcc); const EventSetup c{ info, static_cast(Transition::Event), esGetTokenIndices(Transition::Event), parentC}; - this->doAcquire_(e.streamID(), e, c, holder); + this->doAcquire_(e.streamID(), e, c, std::move(holder)); } void EDProducerBase::doTransformAsync(WaitingTaskHolder iTask, @@ -303,7 +300,7 @@ namespace edm { void EDProducerBase::clearInputProcessBlockCaches() {} - void EDProducerBase::doAcquire_(StreamID, Event const&, EventSetup const&, WaitingTaskWithArenaHolder&) {} + void EDProducerBase::doAcquire_(StreamID, Event const&, EventSetup const&, WaitingTaskHolder&&) {} void EDProducerBase::fillDescriptions(ConfigurationDescriptions& descriptions) { ParameterSetDescription desc; diff --git a/FWCore/Framework/src/global/OutputModuleBase.cc b/FWCore/Framework/src/global/OutputModuleBase.cc index 2989364e93df9..01e6ebba6f1ba 100644 --- a/FWCore/Framework/src/global/OutputModuleBase.cc +++ b/FWCore/Framework/src/global/OutputModuleBase.cc @@ -55,11 +55,11 @@ namespace edm { void OutputModuleBase::doAcquire(EventTransitionInfo const& info, ActivityRegistry* act, ModuleCallingContext const* mcc, - WaitingTaskWithArenaHolder& holder) { + WaitingTaskHolder&& holder) { EventForOutput e(info, moduleDescription(), mcc); e.setConsumer(this); EventAcquireSignalsSentry sentry(act, mcc); - this->doAcquire_(e.streamID(), e, holder); + this->doAcquire_(e.streamID(), e, std::move(holder)); } } // namespace global } // namespace edm diff --git a/FWCore/Framework/src/global/implementorsMethods.h b/FWCore/Framework/src/global/implementorsMethods.h index 6b31ce835f018..5775be9f6a41c 100644 --- a/FWCore/Framework/src/global/implementorsMethods.h +++ b/FWCore/Framework/src/global/implementorsMethods.h @@ -74,8 +74,8 @@ namespace edm { void ExternalWork::doAcquire_(StreamID s, Event const& ev, edm::EventSetup const& es, - WaitingTaskWithArenaHolder& holder) { - this->acquire(s, ev, es, holder); + WaitingTaskHolder&& holder) { + this->acquire(s, ev, es, WaitingTaskWithArenaHolder(std::move(holder))); } } // namespace impl } // namespace global diff --git a/FWCore/Framework/src/stream/EDFilterAdaptorBase.cc b/FWCore/Framework/src/stream/EDFilterAdaptorBase.cc index 9c59fa423a58d..491e96b32873e 100644 --- a/FWCore/Framework/src/stream/EDFilterAdaptorBase.cc +++ b/FWCore/Framework/src/stream/EDFilterAdaptorBase.cc @@ -66,7 +66,7 @@ namespace edm { void EDFilterAdaptorBase::doAcquire(EventTransitionInfo const& info, ActivityRegistry* act, ModuleCallingContext const* mcc, - WaitingTaskWithArenaHolder& holder) { + WaitingTaskHolder&& holder) { EventPrincipal const& ep = info.principal(); assert(ep.streamID() < m_streamModules.size()); auto mod = m_streamModules[ep.streamID()]; @@ -77,7 +77,7 @@ namespace edm { ESParentContext parentC(mcc); const EventSetup c{ info, static_cast(Transition::Event), mod->esGetTokenIndices(Transition::Event), parentC}; - mod->doAcquire_(e, c, holder); + mod->doAcquire_(e, c, std::move(holder)); } template class edm::stream::ProducingModuleAdaptorBase; diff --git a/FWCore/Framework/src/stream/EDProducerAdaptorBase.cc b/FWCore/Framework/src/stream/EDProducerAdaptorBase.cc index e2debfc6f64f5..f34bdc300cc83 100644 --- a/FWCore/Framework/src/stream/EDProducerAdaptorBase.cc +++ b/FWCore/Framework/src/stream/EDProducerAdaptorBase.cc @@ -88,7 +88,7 @@ namespace edm { void EDProducerAdaptorBase::doAcquire(EventTransitionInfo const& info, ActivityRegistry* act, ModuleCallingContext const* mcc, - WaitingTaskWithArenaHolder& holder) { + WaitingTaskHolder&& holder) { EventPrincipal const& ep = info.principal(); assert(ep.streamID() < m_streamModules.size()); auto mod = m_streamModules[ep.streamID()]; @@ -99,7 +99,7 @@ namespace edm { ESParentContext parentC(mcc); const EventSetup c{ info, static_cast(Transition::Event), mod->esGetTokenIndices(Transition::Event), parentC}; - mod->doAcquire_(e, c, holder); + mod->doAcquire_(e, c, std::move(holder)); } template class edm::stream::ProducingModuleAdaptorBase; diff --git a/FWCore/Framework/src/stream/ProducingModuleHelper.cc b/FWCore/Framework/src/stream/ProducingModuleHelper.cc index ddff4d0cc2c34..00de8832c0799 100644 --- a/FWCore/Framework/src/stream/ProducingModuleHelper.cc +++ b/FWCore/Framework/src/stream/ProducingModuleHelper.cc @@ -6,19 +6,15 @@ // Created: 1 December 2017 #include "FWCore/Framework/interface/stream/ProducingModuleHelper.h" -#include "FWCore/Concurrency/interface/WaitingTaskWithArenaHolder.h" #include "FWCore/Framework/interface/stream/implementors.h" namespace edm { namespace stream { - void doAcquireIfNeeded(impl::ExternalWork* base, - Event const& ev, - EventSetup const& es, - WaitingTaskWithArenaHolder& holder) { - base->acquire(ev, es, holder); + void doAcquireIfNeeded(impl::ExternalWork* base, Event const& ev, EventSetup const& es, WaitingTaskHolder&& holder) { + base->acquire(ev, es, WaitingTaskWithArenaHolder(std::move(holder))); } - void doAcquireIfNeeded(void*, Event const&, EventSetup const&, WaitingTaskWithArenaHolder&) {} + void doAcquireIfNeeded(void*, Event const&, EventSetup const&, WaitingTaskHolder&&) {} } // namespace stream } // namespace edm