From c85ea3032e25831b2adfd46d347c9f75e5aa853f Mon Sep 17 00:00:00 2001
From: Tadej Novak <tadej.novak@cern.ch>
Date: Sat, 14 Nov 2020 15:40:17 +0100
Subject: [PATCH] Cleanup memory handling of MM_DigitizationTool
---
.../MM_Digitization/MM_DigitizationTool.h | 31 +++--
.../src/MM_DigitizationTool.cxx | 126 +++++++-----------
2 files changed, 66 insertions(+), 91 deletions(-)
diff --git a/MuonSpectrometer/MuonDigitization/MM_Digitization/MM_Digitization/MM_DigitizationTool.h b/MuonSpectrometer/MuonDigitization/MM_Digitization/MM_Digitization/MM_DigitizationTool.h
index 37c82a45a5d..8365ce9dcbe 100644
--- a/MuonSpectrometer/MuonDigitization/MM_Digitization/MM_Digitization/MM_DigitizationTool.h
+++ b/MuonSpectrometer/MuonDigitization/MM_Digitization/MM_Digitization/MM_DigitizationTool.h
@@ -34,6 +34,7 @@
In the execute() method...
*/
+#include "PileUpTools/PileUpMergeSvc.h"
#include "PileUpTools/PileUpToolBase.h"
#include "GaudiKernel/AlgTool.h"
#include "GaudiKernel/ServiceHandle.h"
@@ -60,6 +61,7 @@
#include "MagFieldElements/AtlasFieldCache.h"
#include "MM_Digitization/IMM_DigitizationTool.h"
+#include <memory>
#include <string>
#include <sstream>
#include <vector>
@@ -71,9 +73,7 @@ namespace MuonGM{
class MuonDetectorManager;
}
-class PileUpMergeSvc;
class MicromegasHitIdHelper;
-class StoreGateSvc;
class TTree;
class TFile;
@@ -126,7 +126,10 @@ class MM_DigitizationTool : public PileUpToolBase {
SG::ReadCondHandleKey<AtlasFieldCacheCondObj> m_fieldCondObjInputKey {this, "AtlasFieldCacheCondObj", "fieldCondObj"};
- Gaudi::Property<std::string> m_inputObjectName{this,"InputObjectName","MicromegasSensitiveDetector","name of the input objects"};
+ Gaudi::Property<bool> m_onlyUseContainerName{this, "OnlyUseContainerName", true, "Don't use the ReadHandleKey directly. Just extract the container name from it."};
+ SG::ReadHandleKey<MMSimHitCollection> m_hitsContainerKey{this, "InputObjectName", "MicromegasSensitiveDetector", "name of the input objects"};
+ std::string m_inputObjectName{""};
+
Gaudi::Property<std::string> m_vmmReadoutMode{this,"vmmReadoutMode","peak","For readout (DAQ) path. Can be peak or threshold"};
Gaudi::Property<std::string> m_vmmARTMode{this,"vmmARTMode","threshold","For ART (trigger) path. Can be peak or threshold"};
@@ -162,20 +165,20 @@ class MM_DigitizationTool : public PileUpToolBase {
Gaudi::Property<float> m_stripdeadtime{this,"StripDeadTime",200,"dead-time for strip, default value 200 ns = 8 BCs"};
Gaudi::Property<float> m_ARTdeadtime{this,"ARTDeadTime",200,"dead-time for ART, default value 200 ns = 8 BCs"};
- SG::WriteHandleKey<MmDigitContainer> m_outputDigitCollectionKey{this,"OutputObjectName","MM_DIGITS","WriteHandleKey for Output MmigitContainer"}; // name of the output digits
- SG::WriteHandleKey<MuonSimDataCollection> m_outputSDO_CollectionKey{this,"OutputSDOName","MM_SDO","WriteHandleKey for Output MuonSimDataCollection"}; // name of the output SDOs
+ SG::WriteHandleKey<MmDigitContainer> m_outputDigitCollectionKey{this,"OutputObjectName","MM_DIGITS","WriteHandleKey for Output MmigitContainer"}; // name of the output digits
+ SG::WriteHandleKey<MuonSimDataCollection> m_outputSDO_CollectionKey{this,"OutputSDOName","MM_SDO","WriteHandleKey for Output MuonSimDataCollection"}; // name of the output SDOs
- PileUpMergeSvc *m_mergeSvc; // Pile up service
+ ServiceHandle<PileUpMergeSvc> m_mergeSvc{this, "MergeSvc", "PileUpMergeSvc", "Merge service used in digitization"};
- TFile *m_file;
- TTree *m_ntuple;
+ TFile *m_file{};
+ TTree *m_ntuple{};
- MicromegasHitIdHelper* m_muonHelper;
- const MuonGM::MuonDetectorManager* m_MuonGeoMgr;
- std::list<MMSimHitCollection*> m_MMHitCollList;
- TimedHitCollection<MMSimHit>* m_timedHitCollection_MM; // the pileup hits
- MM_StripsResponseSimulation* m_StripsResponseSimulation;
- MM_ElectronicsResponseSimulation* m_ElectronicsResponseSimulation;
+ const MicromegasHitIdHelper* m_muonHelper{}; // not owned
+ const MuonGM::MuonDetectorManager* m_MuonGeoMgr{}; // not owned
+ std::list<std::unique_ptr<MMSimHitCollection>> m_MMHitCollList{};
+ std::unique_ptr<TimedHitCollection<MMSimHit>> m_timedHitCollection_MM{}; // the pileup hits
+ std::unique_ptr<MM_StripsResponseSimulation> m_StripsResponseSimulation{};
+ std::unique_ptr<MM_ElectronicsResponseSimulation> m_ElectronicsResponseSimulation{};
float m_driftVelocity;
int m_n_Station_side;
diff --git a/MuonSpectrometer/MuonDigitization/MM_Digitization/src/MM_DigitizationTool.cxx b/MuonSpectrometer/MuonDigitization/MM_Digitization/src/MM_DigitizationTool.cxx
index cbd698615cd..b5d47b1f6d9 100644
--- a/MuonSpectrometer/MuonDigitization/MM_Digitization/src/MM_DigitizationTool.cxx
+++ b/MuonSpectrometer/MuonDigitization/MM_Digitization/src/MM_DigitizationTool.cxx
@@ -47,9 +47,6 @@
#include "TrkEventPrimitives/LocalDirection.h"
#include "TrkSurfaces/Surface.h"
-//Pile-up
-#include "PileUpTools/PileUpMergeSvc.h"
-
//Truth
#include "CLHEP/Units/PhysicalConstants.h"
#include "GeneratorObjects/HepMcParticleLink.h"
@@ -79,15 +76,6 @@ using namespace MuonGM;
/*******************************************************************************/
MM_DigitizationTool::MM_DigitizationTool(const std::string& type, const std::string& name, const IInterface* parent):
PileUpToolBase(type, name, parent),
- m_mergeSvc(nullptr),
- m_file(nullptr),
- m_ntuple(nullptr),
- m_muonHelper(nullptr),
- m_MuonGeoMgr(nullptr),
- m_MMHitCollList(),
- m_timedHitCollection_MM(nullptr),
- m_StripsResponseSimulation(nullptr),
- m_ElectronicsResponseSimulation(nullptr),
m_driftVelocity(0),
// Tree Branches...
m_n_Station_side(-INT_MAX),
@@ -126,10 +114,8 @@ StatusCode MM_DigitizationTool::initialize() {
ATH_MSG_DEBUG ("MM_DigitizationTool:: in initialize()") ;
// Initialize transient detector store and MuonGeoModel OR MuonDetDescrManager
- StoreGateSvc* detStore=nullptr;
- ATH_CHECK( serviceLocator()->service("DetectorStore", detStore) );
- if(detStore->contains<MuonGM::MuonDetectorManager>( "Muon" )){
- ATH_CHECK( detStore->retrieve(m_MuonGeoMgr) );
+ if(detStore()->contains<MuonGM::MuonDetectorManager>( "Muon" )){
+ ATH_CHECK( detStore()->retrieve(m_MuonGeoMgr) );
ATH_MSG_DEBUG ( "Retrieved MuonGeoModelDetectorManager from StoreGate" );
}
@@ -138,24 +124,33 @@ StatusCode MM_DigitizationTool::initialize() {
// Digit tools
ATH_CHECK(m_digitTool.retrieve());
- //initialize the output WriteHandleKeys
- if(m_outputDigitCollectionKey.key()=="") {
- ATH_MSG_FATAL("Property OutputObjectName not set !");
- return StatusCode::FAILURE;
- }
- ATH_CHECK(m_outputDigitCollectionKey.initialize());
- ATH_CHECK(m_outputSDO_CollectionKey.initialize());
- ATH_MSG_DEBUG("Output Digits: '"<<m_outputDigitCollectionKey.key()<<"'");
+ if (m_hitsContainerKey.key().empty()) {
+ ATH_MSG_FATAL("Property InputObjectName not set !");
+ return StatusCode::FAILURE;
+ }
+
+ if (m_onlyUseContainerName) m_inputObjectName = m_hitsContainerKey.key();
+ ATH_MSG_DEBUG("Input objects in container: '" << m_inputObjectName << "'");
+
+ // Pile-up merge service
+ if (m_onlyUseContainerName) {
+ ATH_CHECK(m_mergeSvc.retrieve());
+ }
+
+ // Initialize ReadHandleKey
+ ATH_CHECK(m_hitsContainerKey.initialize(!m_onlyUseContainerName));
- ATH_CHECK(m_fieldCondObjInputKey.initialize());
- ATH_CHECK(m_calibrationTool.retrieve());
+ // Initialize the output WriteHandleKeys
+ ATH_CHECK(m_outputDigitCollectionKey.initialize());
+ ATH_CHECK(m_outputSDO_CollectionKey.initialize());
+ ATH_MSG_DEBUG("Output Digits: '"<<m_outputDigitCollectionKey.key()<<"'");
+
+ ATH_CHECK(m_fieldCondObjInputKey.initialize());
+ ATH_CHECK(m_calibrationTool.retrieve());
//simulation identifier helper
m_muonHelper = MicromegasHitIdHelper::GetHelper();
- // Locate the PileUpMergeSvc and initialize our local ptr
- ATH_CHECK(service("PileUpMergeSvc", m_mergeSvc, true));
-
// Validation File Output
if (m_writeOutputFile){
m_file = new TFile("MM_Digitization_plots.root","RECREATE");
@@ -192,7 +187,7 @@ StatusCode MM_DigitizationTool::initialize() {
}
// StripsResponseSimulation Creation
- m_StripsResponseSimulation = new MM_StripsResponseSimulation();
+ m_StripsResponseSimulation = std::make_unique<MM_StripsResponseSimulation>();
m_StripsResponseSimulation->setQThreshold(m_qThreshold);
m_StripsResponseSimulation->setDriftGapWidth(m_driftGapWidth);
m_StripsResponseSimulation->setCrossTalk1(m_crossTalk1);
@@ -216,7 +211,7 @@ StatusCode MM_DigitizationTool::initialize() {
m_StripsResponseSimulation->initialize();
// ElectronicsResponseSimulation Creation
- m_ElectronicsResponseSimulation = new MM_ElectronicsResponseSimulation();
+ m_ElectronicsResponseSimulation = std::make_unique<MM_ElectronicsResponseSimulation>();
m_ElectronicsResponseSimulation->setPeakTime(m_peakTime); // VMM peak time parameter
m_ElectronicsResponseSimulation->setTimeWindowLowerOffset(m_timeWindowLowerOffset);
m_ElectronicsResponseSimulation->setTimeWindowUpperOffset(m_timeWindowUpperOffset);
@@ -281,8 +276,8 @@ StatusCode MM_DigitizationTool::prepareEvent(const EventContext& /*ctx*/, unsign
m_MMHitCollList.clear();
- if(!m_timedHitCollection_MM) {
- m_timedHitCollection_MM = new TimedHitCollection<MMSimHit>();
+ if (m_timedHitCollection_MM == nullptr) {
+ m_timedHitCollection_MM = std::make_unique<TimedHitCollection<MMSimHit>>();
} else {
ATH_MSG_ERROR ( "m_timedHitCollection_MM is not null" );
return StatusCode::FAILURE;
@@ -315,19 +310,19 @@ StatusCode MM_DigitizationTool::processBunchXing(int bunchXing,
// Iterating over the list of collections
for( ; iColl != endColl; iColl++){
-
- MMSimHitCollection *hitCollPtr = new MMSimHitCollection(*iColl->second);
+
+ auto hitCollPtr = std::make_unique<MMSimHitCollection>(*iColl->second);
PileUpTimeEventIndex timeIndex(iColl->first);
-
+
ATH_MSG_DEBUG("MMSimHitCollection found with " << hitCollPtr->size() <<
" hits");
ATH_MSG_VERBOSE("time index info. time: " << timeIndex.time()
<< " index: " << timeIndex.index()
<< " type: " << timeIndex.type());
-
- m_timedHitCollection_MM->insert(timeIndex, hitCollPtr);
- m_MMHitCollList.push_back(hitCollPtr);
-
+
+ m_timedHitCollection_MM->insert(timeIndex, hitCollPtr.get());
+ m_MMHitCollList.push_back(std::move(hitCollPtr));
+
}
return StatusCode::SUCCESS;
}
@@ -340,10 +335,6 @@ StatusCode MM_DigitizationTool::getNextEvent() {
ATH_MSG_DEBUG ( "MM_DigitizationTool::getNextEvent()" );
- if (!m_mergeSvc) {
- ATH_CHECK(service("PileUpMergeSvc", m_mergeSvc, true));
- }
-
// get the container(s)
typedef PileUpMergeSvc::TimedList<MMSimHitCollection>::type TimedHitCollList;
@@ -360,8 +351,8 @@ StatusCode MM_DigitizationTool::getNextEvent() {
}
// create a new hits collection - Define Hit Collection
- if(!m_timedHitCollection_MM) {
- m_timedHitCollection_MM = new TimedHitCollection<MMSimHit>();
+ if (m_timedHitCollection_MM == nullptr) {
+ m_timedHitCollection_MM = std::make_unique<TimedHitCollection<MMSimHit>>();
}else{
ATH_MSG_ERROR ( "m_timedHitCollection_MM is not null" );
return StatusCode::FAILURE;
@@ -388,19 +379,10 @@ StatusCode MM_DigitizationTool::mergeEvent(const EventContext& ctx) {
ATH_CHECK( doDigitization(ctx) );
- // reset the pointer (delete null pointer should be safe)
- if (m_timedHitCollection_MM){
- delete m_timedHitCollection_MM;
- m_timedHitCollection_MM = nullptr;
- }
+ // reset the pointer
+ m_timedHitCollection_MM.reset();
- // remove cloned one in processBunchXing......
- std::list<MMSimHitCollection*>::iterator MMHitColl = m_MMHitCollList.begin();
- std::list<MMSimHitCollection*>::iterator MMHitCollEnd = m_MMHitCollList.end();
- while(MMHitColl!=MMHitCollEnd) {
- delete (*MMHitColl);
- ++MMHitColl;
- }
+ // clear cloned list
m_MMHitCollList.clear();
return StatusCode::SUCCESS;
}
@@ -419,11 +401,8 @@ StatusCode MM_DigitizationTool::processAllSubEvents(const EventContext& ctx) {
ATH_CHECK( doDigitization(ctx) );
- // reset the pointer (delete null pointer should be safe)
- if (m_timedHitCollection_MM){
- delete m_timedHitCollection_MM;
- m_timedHitCollection_MM = nullptr;
- }
+ // reset the pointer
+ m_timedHitCollection_MM.reset();
return StatusCode::SUCCESS;
}
/*******************************************************************************/
@@ -437,10 +416,6 @@ StatusCode MM_DigitizationTool::finalize() {
m_file->Close();
}
-
- delete m_StripsResponseSimulation;
- delete m_ElectronicsResponseSimulation;
-
return StatusCode::SUCCESS;
}
/*******************************************************************************/
@@ -469,7 +444,7 @@ StatusCode MM_DigitizationTool::doDigitization(const EventContext& ctx) {
}
// Perform null check on m_thpcCSC
- if(!m_timedHitCollection_MM) {
+ if (m_timedHitCollection_MM == nullptr) {
ATH_MSG_ERROR ( "m_timedHitCollection_MM is null" );
return StatusCode::FAILURE;
}
@@ -1049,7 +1024,7 @@ StatusCode MM_DigitizationTool::doDigitization(const EventContext& ctx) {
std::unique_ptr<MmDigit> newDigit = nullptr;
if ( !m_doSmearing ) {
- newDigit = std::unique_ptr<MmDigit>(new MmDigit( stripDigitOutputAllHits.digitID(),
+ newDigit = std::make_unique<MmDigit>(stripDigitOutputAllHits.digitID(),
electronicsOutputForReadout->stripTime(),
electronicsOutputForReadout->stripPos(),
electronicsOutputForReadout->stripCharge(),
@@ -1061,7 +1036,7 @@ StatusCode MM_DigitizationTool::doDigitization(const EventContext& ctx) {
finalElectronicsTriggerOutput.chipCharge(),
finalElectronicsTriggerOutput.MMFEVMMid(),
finalElectronicsTriggerOutput.VMMid()
- ));
+ );
}
else {
@@ -1091,7 +1066,7 @@ StatusCode MM_DigitizationTool::doDigitization(const EventContext& ctx) {
}
if ( stripPosSmeared.size() > 0 ) {
- newDigit = std::unique_ptr<MmDigit>(new MmDigit( digitId,
+ newDigit = std::make_unique<MmDigit>(digitId,
stripTimeSmeared,
stripPosSmeared,
stripChargeSmeared,
@@ -1103,7 +1078,7 @@ StatusCode MM_DigitizationTool::doDigitization(const EventContext& ctx) {
finalElectronicsTriggerOutput.chipCharge(),
finalElectronicsTriggerOutput.MMFEVMMid(),
finalElectronicsTriggerOutput.VMMid()
- ));
+ );
}
else {
continue;
@@ -1126,7 +1101,7 @@ StatusCode MM_DigitizationTool::doDigitization(const EventContext& ctx) {
// put new collection in storegate
// Get the messaging service, print where you are
const MmDigitCollection* coll = digitContainer->indexFindPtr(moduleHash );
- if (nullptr == coll) {
+ if (coll == nullptr) {
digitCollection = new MmDigitCollection( elemId, moduleHash );
digitCollection->push_back(std::move(newDigit));
ATH_CHECK(digitContainer->addCollection(digitCollection, moduleHash ) );
@@ -1149,10 +1124,7 @@ StatusCode MM_DigitizationTool::doDigitization(const EventContext& ctx) {
ATH_MSG_DEBUG ( "MM_Digitization Done!" );
- if (m_timedHitCollection_MM){
- delete m_timedHitCollection_MM;
- m_timedHitCollection_MM = nullptr;
- }
+ m_timedHitCollection_MM.reset();
return StatusCode::SUCCESS;
}
--
GitLab