From 6fcf9d0a375c65db1dc06c317e2d60f4d26ed289 Mon Sep 17 00:00:00 2001
From: scott snyder <sss@karma>
Date: Fri, 21 Aug 2020 23:41:36 -0400
Subject: [PATCH] TRT_RawDataByteStreamCnv: threading fixes
Rework to avoid calling naughtyRetrieve.
Remove ATLAS_NOT_THREAD_SAFE annotations --- just need to avoid a (needless!)
AttributeList copy.
Make sure to protect TRTRawDataProviderTool::m_cache with a lock.
Not safe to omit it.
Note that the original code was sometimes unpacking the same channel
multiple times into the same collection. This behavior is fixed
by the change, but it results in RootComp differences for a couple
TriggerTest ART tests.
---
.../src/TRTRawDataProviderTool.cxx | 4 +-
.../src/TRT_RodDecoder.cxx | 79 +++++++++++--------
2 files changed, 46 insertions(+), 37 deletions(-)
diff --git a/InnerDetector/InDetEventCnv/TRT_RawDataByteStreamCnv/src/TRTRawDataProviderTool.cxx b/InnerDetector/InDetEventCnv/TRT_RawDataByteStreamCnv/src/TRTRawDataProviderTool.cxx
index dbf8dcf4b699..a8b820d2dace 100644
--- a/InnerDetector/InDetEventCnv/TRT_RawDataByteStreamCnv/src/TRTRawDataProviderTool.cxx
+++ b/InnerDetector/InDetEventCnv/TRT_RawDataByteStreamCnv/src/TRTRawDataProviderTool.cxx
@@ -99,9 +99,7 @@ StatusCode TRTRawDataProviderTool::convert(const std::vector<const ROBFragment*>
std::vector<const ROBFragment*>::const_iterator rob_it = vecRobs.begin();
const EventContext& ctx{Gaudi::Hive::currentContext()};
- // std::lock_guard<std::mutex> lock{m_mutex};
- // Do not lock mutex because this convert method is non-const.
- // But we should make this method const.
+ std::lock_guard<std::mutex> lock{m_mutex};
CacheEntry* ent{m_cache.get(ctx)};
if (ent->m_evt!=ctx.evt()) { // New event in this slot
ent->m_LastLvl1ID = 0xffffffff;
diff --git a/InnerDetector/InDetEventCnv/TRT_RawDataByteStreamCnv/src/TRT_RodDecoder.cxx b/InnerDetector/InDetEventCnv/TRT_RawDataByteStreamCnv/src/TRT_RodDecoder.cxx
index e4ef27f56482..09bf2cd292cd 100644
--- a/InnerDetector/InDetEventCnv/TRT_RawDataByteStreamCnv/src/TRT_RodDecoder.cxx
+++ b/InnerDetector/InDetEventCnv/TRT_RawDataByteStreamCnv/src/TRT_RodDecoder.cxx
@@ -557,11 +557,12 @@ TRT_RodDecoder::int_fillExpanded( const ROBFragment* robFrag,
const Identifier NULLId(0);
TRT_RDORawData* rdo = 0;
- TRT_RDO_Collection* theColl = 0;
// get the data of the fragment
OFFLINE_FRAGMENTS_NAMESPACE::PointerType vint;
robFrag->rod_data( vint );
+
+ std::unordered_map<IdentifierHash, std::unique_ptr<TRT_RDO_Collection> > colls;
// loop over the data in the fragment
unsigned int i;
@@ -633,30 +634,28 @@ TRT_RodDecoder::int_fillExpanded( const ROBFragment* robFrag,
}
}
+ // Skip if this collection has already been done.
+ if (rdoIdc->indexFindPtr (idHash)) {
+ continue;
+ }
+
// get the collection
- ATH_CHECK(rdoIdc->naughtyRetrieve( idHash, theColl ));
+ std::unique_ptr<TRT_RDO_Collection>& theColl = colls[idHash];
// Check if the Collection is already created.
- if ( theColl == nullptr )
+ if ( !theColl )
{
#ifdef TRT_BSC_DEBUG
ATH_MSG_DEBUG( " Collection ID = " << idHash \
<< " does not exist, create it " );
#endif
// create new collection
- theColl = new TRT_RDO_Collection ( idHash );
+ theColl = std::make_unique<TRT_RDO_Collection> ( idHash );
// get identifier from the hash, this is not nice
Identifier ident;
m_trt_id->get_id(idHash,ident,&m_straw_layer_context);
// get the Identifier to be nice to downstream clients
theColl->setIdentifier(ident);
- // add collection into IDC
- StatusCode sc = rdoIdc->addCollection(theColl, idHash);
- if ( sc.isFailure() )
- {
- ATH_MSG_WARNING ( "Failed to add TRT RDO collection to container" );
- return sc;
- }
}
// Now the Collection is there for sure. Create RDO and push it
@@ -675,6 +674,11 @@ TRT_RodDecoder::int_fillExpanded( const ROBFragment* robFrag,
theColl->push_back( rdo );
} // End of loop over all words in ROD
+
+ // add collections into IDC
+ for (auto& p : colls) {
+ ATH_CHECK( rdoIdc->addCollection (p.second.release(), p.first) );
+ }
return StatusCode::SUCCESS;
}
@@ -731,13 +735,14 @@ TRT_RodDecoder::int_fillMinimalCompress( const ROBFragment *robFrag,
const Identifier NULLId(0);
TRT_RDORawData* rdo = 0;
- TRT_RDO_Collection* theColl = 0;
// get the data of the fragment
OFFLINE_FRAGMENTS_NAMESPACE::PointerType vint;
robFrag->rod_data( vint );
+ std::unordered_map<IdentifierHash, std::unique_ptr<TRT_RDO_Collection> > colls;
+
int bit=0;
int v;
@@ -810,7 +815,7 @@ TRT_RodDecoder::int_fillMinimalCompress( const ROBFragment *robFrag,
// Make an Identifier for the RDO and get the IdHash
idStraw = m_CablingSvc->getIdentifier( (eformat::SubDetector) 0 /*unused*/, robid,
bufferOffset, idHash );
-
+
if ( NULLId == idStraw )
{
#ifdef TRT_BSC_DEBUG
@@ -854,30 +859,28 @@ TRT_RodDecoder::int_fillMinimalCompress( const ROBFragment *robFrag,
}
}
+ // Skip if this collection has already been done.
+ if (rdoIdc->indexFindPtr (idHash)) {
+ continue;
+ }
+
// get the collection
- ATH_CHECK(rdoIdc->naughtyRetrieve( idHash, theColl ));
+ std::unique_ptr<TRT_RDO_Collection>& theColl = colls[idHash];
// Check if the Collection is already created.
- if ( theColl==nullptr )
+ if ( !theColl )
{
#ifdef TRT_BSC_DEBUG
ATH_MSG_DEBUG( " Collection ID = " << idHash \
<< " does not exist, create it " );
#endif
// create new collection
- theColl = new TRT_RDO_Collection ( idHash );
+ theColl = std::make_unique<TRT_RDO_Collection> ( idHash );
// get identifier from the hash, this is not nice
Identifier ident;
m_trt_id->get_id( idHash, ident, &m_straw_layer_context );
// get the Identifier to be nice to downstream clients
theColl->setIdentifier(ident);
- // add collection into IDC
- StatusCode sc = rdoIdc->addCollection(theColl, idHash);
- if ( sc.isFailure() )
- {
- ATH_MSG_WARNING ( "Failed to add TRT RDO collection to container" );
- return sc;
- }
}
// Now the Collection is there for sure. Create RDO and push it
@@ -908,6 +911,11 @@ TRT_RodDecoder::int_fillMinimalCompress( const ROBFragment *robFrag,
} // End of loop over all words in ROD
+ // add collections into IDC
+ for (auto& p : colls) {
+ ATH_CHECK( rdoIdc->addCollection (p.second.release(), p.first) );
+ }
+
return StatusCode::SUCCESS;
}
@@ -964,12 +972,13 @@ TRT_RodDecoder::int_fillFullCompress( const ROBFragment *robFrag,
const Identifier NULLId(0);
TRT_RDORawData* rdo = 0;
- TRT_RDO_Collection* theColl = 0;
// get the data of the fragment
OFFLINE_FRAGMENTS_NAMESPACE::PointerType vint;
robFrag->rod_data( vint );
+ std::unordered_map<IdentifierHash, std::unique_ptr<TRT_RDO_Collection> > colls;
+
int bit=0;
int v,l;
int i;
@@ -1130,30 +1139,28 @@ TRT_RodDecoder::int_fillFullCompress( const ROBFragment *robFrag,
}
}
+ // Skip if this collection has already been done.
+ if (rdoIdc->indexFindPtr (idHash)) {
+ continue;
+ }
+
// get the collection
- ATH_CHECK(rdoIdc->naughtyRetrieve( idHash, theColl ));
+ std::unique_ptr<TRT_RDO_Collection>& theColl = colls[idHash];
// Check if the Collection is already created.
- if ( theColl == nullptr )
+ if ( !theColl )
{
#ifdef TRT_BSC_DEBUG
ATH_MSG_DEBUG( " Collection ID = " << idHash \
<< " does not exist, create it " );
#endif
// create new collection
- theColl = new TRT_RDO_Collection ( idHash );
+ theColl = std::make_unique<TRT_RDO_Collection> ( idHash );
// get identifier from the hash, this is not nice
Identifier ident;
m_trt_id->get_id( idHash, ident, &m_straw_layer_context );
// get the Identifier to be nice to downstream clients
theColl->setIdentifier(ident);
- // add collection into IDC
- StatusCode sc = rdoIdc->addCollection(theColl, idHash);
- if ( sc.isFailure() )
- {
- ATH_MSG_WARNING ( "Failed to add TRT RDO collection to container" );
- return sc;
- }
}
// Now the Collection is there for sure. Create RDO and push it
@@ -1185,6 +1192,10 @@ TRT_RodDecoder::int_fillFullCompress( const ROBFragment *robFrag,
} // if phase == 1
} // End of loop over all words in ROD
+ // add collections into IDC
+ for (auto& p : colls) {
+ ATH_CHECK( rdoIdc->addCollection (p.second.release(), p.first) );
+ }
// ATH_MSG_INFO( "Input: " << in_ptr << " / " << v_size << " Output: " << out_ptr << " / 1920" );
--
GitLab