From 9f7b73b35b830d01db9ec99b0878957c3919a855 Mon Sep 17 00:00:00 2001
From: Rafal Bielski <rafal.bielski@cern.ch>
Date: Mon, 3 Sep 2018 19:36:06 +0200
Subject: [PATCH] Rework how EventSelector and InputSvc deal with EventContext
 and ByteStreamAddress for EventInfo

Now it works like this:
1. Event loop mgr creates an `EventContext` and records it in the event store.
2. Event loop mgr calls `IEvtSelector::createAddress`
3. `TrigEventSelectorByteStream::createAddress` creates a `ByteStreamAddress` for `EventInfo`, which is in line with its name and with the workflow described in the [documentation](/Event/ByteStreamCnvSvc/doc/ByteStream.txt). It also attaches an `EventContext` obtained from the event store to the `ByteStreamAddress`.
4. Event loop mgr calls `IEvtSelector::next` which calls `ByteStreamInputSvc::nextEvent`.
5. `TrigByteStreamInputSvc::nextEvent` gets a new event from `DataCollector::getNext` and passes it on to `ROBDataProviderSvc` using an `EventContext` obtained from the event store.
6. Event loop mgr asks the event store to `loadEventProxies` and then to retrieve an `EventInfo` object. This triggers an automatic conversion from BS (`RawEvent`) to `EventInfo`.
---
 .../src/TrigByteStreamInputSvc.cxx            | 36 +++++++--------
 .../src/TrigByteStreamInputSvc.h              |  1 -
 .../src/TrigEventSelectorByteStream.cxx       | 42 +++++++++--------
 .../TrigServices/HltEventLoopMgr.h            |  2 +-
 .../TrigServices/src/HltEventLoopMgr.cxx      | 45 ++++++++++---------
 5 files changed, 66 insertions(+), 60 deletions(-)

diff --git a/HLT/Event/TrigByteStreamCnvSvc/src/TrigByteStreamInputSvc.cxx b/HLT/Event/TrigByteStreamCnvSvc/src/TrigByteStreamInputSvc.cxx
index e6fe37f4005..1ed6ee2794e 100644
--- a/HLT/Event/TrigByteStreamCnvSvc/src/TrigByteStreamInputSvc.cxx
+++ b/HLT/Event/TrigByteStreamCnvSvc/src/TrigByteStreamInputSvc.cxx
@@ -2,12 +2,16 @@
   Copyright (C) 2002-2018 CERN for the benefit of the ATLAS collaboration
 */
 
+// Trigger includes
 #include "TrigByteStreamInputSvc.h"
-#include "ByteStreamCnvSvcBase/ByteStreamAddress.h"
+#include "TrigKernel/HltExceptions.h"
+
+// Athena includes
+#include "AthenaKernel/EventContextClid.h"
 #include "EventInfo/EventInfo.h"
 #include "StoreGate/StoreGateSvc.h"
-#include "TrigKernel/HltExceptions.h"
 
+// TDAQ includes
 #include "hltinterface/DataCollector.h"
 #include "eformat/write/FullEventFragment.h"
 
@@ -72,8 +76,14 @@ const RawEvent* TrigByteStreamInputSvc::nextEvent() {
   ATH_MSG_VERBOSE("start of " << __FUNCTION__);
   std::lock_guard<std::mutex> lock( m_readerMutex ); // probably don't need a lock for InputSvc
 
-  // not a nice solution - depends on the Gaudi::currentContext being set correctly upstream
-  const EventContext context{ Gaudi::Hive::currentContext() };
+  // get event context from event store
+  EventContext* eventContext = nullptr;
+  if (m_evtStore->retrieve(eventContext).isFailure()) {
+    ATH_MSG_ERROR("Failed to retrieve EventContext from the event store, new event cannot be read");
+    return nullptr;
+  }
+
+  ATH_MSG_DEBUG("Reading new event for event context " << *eventContext);
 
   eformat::write::FullEventFragment l1r;
   hltinterface::DataCollector::Status status = hltinterface::DataCollector::instance()->getNext(l1r);
@@ -100,7 +110,7 @@ const RawEvent* TrigByteStreamInputSvc::nextEvent() {
   RawEvent* newRawEvent = new RawEvent(buf);
 
   // find the cache corresponding to the current slot
-  EventCache* cache = m_eventsCache.get(context);
+  EventCache* cache = m_eventsCache.get(*eventContext);
 
   // free the memory allocated to the previous event processed in the current slot
   // -- if we make sure ROBDataProviderSvc does this, then TrigByteStreamInputSvc won't need a cache
@@ -109,7 +119,7 @@ const RawEvent* TrigByteStreamInputSvc::nextEvent() {
   // put the new raw event into the cache
   cache->rawEvent = newRawEvent;
 
-  m_robDataProviderSvc->setNextEvent(context,newRawEvent);
+  m_robDataProviderSvc->setNextEvent(*eventContext,newRawEvent);
   ATH_MSG_VERBOSE("end of " << __FUNCTION__);
   return newRawEvent;
 }
@@ -130,20 +140,6 @@ const RawEvent* TrigByteStreamInputSvc::currentEvent() const {
   return nullptr;
 }
 
-// =============================================================================
-// Implementation of ByteStreamInputSvc::generateDataHeader
-// Unlike offline, we do not actually generate any DataHeader here. We only create and record an EventInfo address.
-// =============================================================================
-StatusCode TrigByteStreamInputSvc::generateDataHeader() {
-  ATH_MSG_VERBOSE("start of " << __FUNCTION__);
-  
-  IOpaqueAddress* iop = new ByteStreamAddress(ClassID_traits<EventInfo>::ID(), "ByteStreamEventInfo", "");
-  CHECK(m_evtStore->recordAddress("ByteStreamEventInfo",iop));
-
-  ATH_MSG_VERBOSE("end of " << __FUNCTION__);
-  return StatusCode::SUCCESS;
-}
-
 // =============================================================================
 void TrigByteStreamInputSvc::releaseEvent(EventCache* cache)
 {
diff --git a/HLT/Event/TrigByteStreamCnvSvc/src/TrigByteStreamInputSvc.h b/HLT/Event/TrigByteStreamCnvSvc/src/TrigByteStreamInputSvc.h
index 60b8245d1fe..50ce78bb53e 100644
--- a/HLT/Event/TrigByteStreamCnvSvc/src/TrigByteStreamInputSvc.h
+++ b/HLT/Event/TrigByteStreamCnvSvc/src/TrigByteStreamInputSvc.h
@@ -38,7 +38,6 @@ public:
   virtual const RawEvent* nextEvent() override;
   virtual const RawEvent* previousEvent() override;
   virtual const RawEvent* currentEvent() const override;
-  virtual StatusCode generateDataHeader() override;
 
 private:
   // ------------------------- service handles ---- ----------------------------
diff --git a/HLT/Event/TrigByteStreamCnvSvc/src/TrigEventSelectorByteStream.cxx b/HLT/Event/TrigByteStreamCnvSvc/src/TrigEventSelectorByteStream.cxx
index 128d3d14e09..727e3c665a5 100644
--- a/HLT/Event/TrigByteStreamCnvSvc/src/TrigEventSelectorByteStream.cxx
+++ b/HLT/Event/TrigByteStreamCnvSvc/src/TrigEventSelectorByteStream.cxx
@@ -2,14 +2,19 @@
   Copyright (C) 2002-2018 CERN for the benefit of the ATLAS collaboration
 */
 
+// Trigger includes
 #include "TrigEventSelectorByteStream.h"
+#include "TrigKernel/HltExceptions.h"
 
-#include "ByteStreamData/RawEvent.h"
+// Athena includes
+#include "AthenaKernel/EventContextClid.h"
 #include "ByteStreamCnvSvc/ByteStreamInputSvc.h"
+#include "ByteStreamCnvSvcBase/ByteStreamAddress.h"
+#include "ByteStreamData/RawEvent.h"
 #include "EventInfo/EventInfo.h"
 #include "StoreGate/StoreGateSvc.h"
-#include "TrigKernel/HltExceptions.h"
 
+// TDAQ includes
 #include "hltinterface/DataCollector.h"
 
 // =============================================================================
@@ -51,8 +56,8 @@ StatusCode TrigEventSelectorByteStream::initialize()
 {
   ATH_MSG_VERBOSE("start of " << __FUNCTION__);
 
-  CHECK(m_eventSource.retrieve());
-  CHECK(m_evtStore.retrieve());
+  ATH_CHECK(m_eventSource.retrieve());
+  ATH_CHECK(m_evtStore.retrieve());
 
   ATH_MSG_VERBOSE("end of " << __FUNCTION__);
   return StatusCode::SUCCESS;
@@ -101,9 +106,6 @@ StatusCode TrigEventSelectorByteStream::next(IEvtSelector::Context& /*c*/) const
     return StatusCode::FAILURE;
   }
 
-  // In online implementation, this creates an EventInfo address for use in event processing
-  CHECK(m_eventSource->generateDataHeader());
-
   ATH_MSG_VERBOSE("end of " << __FUNCTION__);
   return StatusCode::SUCCESS;
 }
@@ -128,21 +130,25 @@ StatusCode TrigEventSelectorByteStream::releaseContext(IEvtSelector::Context*& /
 
 // =============================================================================
 // Implementation of IEvtSelector::createAddress(Context&,IOpaqueAddress*&)
-// Unlike offline, we do not provide a DataHeader proxy here, but an EventInfo proxy
 // =============================================================================
 StatusCode TrigEventSelectorByteStream::createAddress(const IEvtSelector::Context& /*c*/, IOpaqueAddress*& iop) const
 {
   ATH_MSG_VERBOSE("start of " << __FUNCTION__);
-  SG::DataProxy* proxy = m_evtStore->proxy(ClassID_traits<EventInfo>::ID(),"ByteStreamEventInfo");
-  if (proxy !=0) {
-    iop = proxy->address();
-    ATH_MSG_VERBOSE("end of " << __FUNCTION__);
-    return StatusCode::SUCCESS;
-  } else {
-    iop = 0;
-    ATH_MSG_VERBOSE("end of " << __FUNCTION__);
-    return StatusCode::FAILURE;
-  }
+
+  // Get event context from event store
+  EventContext* eventContext = nullptr;
+  ATH_CHECK(m_evtStore->retrieve(eventContext));
+
+  // Perhaps the name shouldn't be hard-coded
+  ByteStreamAddress* addr = new ByteStreamAddress(ClassID_traits<EventInfo>::ID(), "ByteStreamEventInfo", "");
+  addr->setEventContext(*eventContext);
+  iop = static_cast<IOpaqueAddress*>(addr);
+  ATH_CHECK(m_evtStore->recordAddress("ByteStreamEventInfo",iop));
+
+  ATH_MSG_DEBUG("Recorded new ByteStreamAddress for EventInfo with event context " << *eventContext);
+
+  ATH_MSG_VERBOSE("end of " << __FUNCTION__);
+  return StatusCode::SUCCESS;
 }
 
 // =============================================================================
diff --git a/HLT/Trigger/TrigControl/TrigServices/TrigServices/HltEventLoopMgr.h b/HLT/Trigger/TrigControl/TrigServices/TrigServices/HltEventLoopMgr.h
index 12ba925cf18..2a8dcf7dc92 100644
--- a/HLT/Trigger/TrigControl/TrigServices/TrigServices/HltEventLoopMgr.h
+++ b/HLT/Trigger/TrigControl/TrigServices/TrigServices/HltEventLoopMgr.h
@@ -34,10 +34,10 @@
 
 // Forward declarations
 class CondAttrListCollection;
+class EventContext;
 class IAlgExecStateSvc;
 class IAlgorithm;
 class IAlgResourcePool;
-// class IEventInfoCnvTool;
 class IHiveWhiteBoard;
 class IIncidentSvc;
 class IROBDataProviderSvc;
diff --git a/HLT/Trigger/TrigControl/TrigServices/src/HltEventLoopMgr.cxx b/HLT/Trigger/TrigControl/TrigServices/src/HltEventLoopMgr.cxx
index 61ffadc47e4..a3cfe89d7ed 100644
--- a/HLT/Trigger/TrigControl/TrigServices/src/HltEventLoopMgr.cxx
+++ b/HLT/Trigger/TrigControl/TrigServices/src/HltEventLoopMgr.cxx
@@ -643,6 +643,7 @@ StatusCode HltEventLoopMgr::nextEvent(int /*maxevt*/)
         ATH_MSG_ERROR("Failed to create event context");
         // what do we do now? we haven't requested the next event from DataCollector yet,
         // we have a failure while not processing an event
+        failedEvent(evtContext);
         continue;
       }
 
@@ -651,13 +652,34 @@ StatusCode HltEventLoopMgr::nextEvent(int /*maxevt*/)
         ATH_MSG_ERROR("Slot " << evtContext->slot() << " could not be selected for the WhiteBoard");
         // what do we do now? we haven't requested the next event from DataCollector yet,
         // we have a failure while not processing an event
+        failedEvent(evtContext);
         continue;
       }
 
-      // Not so nice behind-the-scenes way to inform the InputSvc / ROBDataProvider about the slot for the new event
-      // to be read. This is likely also used by other services.
+      // Record EventContext in current whiteboard
+      if (m_evtStore->record(std::make_unique<EventContext> (*evtContext), "EventContext").isFailure()) {
+        ATH_MSG_ERROR("Error recording event context object");
+        // what do we do now? we haven't requested the next event from DataCollector yet,
+        // we have a failure while not processing an event
+        failedEvent(evtContext);
+        continue;
+      }
+
+      // Not so nice behind-the-scenes way to inform some services about the current context.
+      // If possible, services should use EventContext from the event store as recorded above.
       Gaudi::Hive::setCurrentContext(*evtContext);
 
+      //------------------------------------------------------------------------
+      // Create a new address for EventInfo to facilitate automatic conversion from input data
+      //------------------------------------------------------------------------
+      IOpaqueAddress* addr = nullptr;
+      if (m_evtSelector->createAddress(*m_evtSelContext, addr).isFailure()) {
+        ATH_MSG_ERROR("Could not create an IOpaqueAddress");
+        // we have not read a new event yet, should we try to create any output?
+        failedEvent(evtContext);
+        continue;
+      }
+
       //------------------------------------------------------------------------
       // Get the next event
       //------------------------------------------------------------------------
@@ -699,16 +721,8 @@ StatusCode HltEventLoopMgr::nextEvent(int /*maxevt*/)
       m_timeoutCond.notify_all();
 
       //------------------------------------------------------------------------
-      // Set up proxy and get the event info
+      // Load event proxies and get event info
       //------------------------------------------------------------------------
-      IOpaqueAddress* addr = nullptr;
-      if (m_evtSelector->createAddress(*m_evtSelContext, addr).isFailure()) {
-        ATH_MSG_ERROR("Could not create an IOpaqueAddress");
-        // we cannot get the EventInfo, so we don't know the event number
-        // should we return anything to the DataCollector?
-        failedEvent(evtContext);
-        continue;
-      }
 
       if (addr != nullptr) {
         /* do we need this???
@@ -773,15 +787,6 @@ StatusCode HltEventLoopMgr::nextEvent(int /*maxevt*/)
       // update thread-local EventContext after setting EventID and conditions runNumber
       Gaudi::Hive::setCurrentContext(*evtContext);
 
-      //------------------------------------------------------------------------
-      // Record EventContext in current whiteboard
-      //------------------------------------------------------------------------
-      if (m_evtStore->record(std::make_unique<EventContext> (*evtContext), "EventContext").isFailure()) {
-        ATH_MSG_ERROR("Error recording event context object");
-        failedEvent(evtContext,eventInfo);
-        continue;
-      }
-
       //------------------------------------------------------------------------
       // Record an empty HLT Result
       //------------------------------------------------------------------------
-- 
GitLab