From c33777d5a1d0fc82bb2447a3718bba5a3c0a2e46 Mon Sep 17 00:00:00 2001
From: Alaettin Serhan Mete <alaettin.serhan.mete@cern.ch>
Date: Tue, 30 Mar 2021 06:05:27 +0000
Subject: [PATCH] MdtRawDataMonitoring: Memory-leak fix and clean-up
---
.../MdtRawDataMonitoring/MdtRawDataMonAlg.h | 8 +-
.../MdtRawDataMonitoring/MdtRawDataValAlg.h | 5 +-
.../src/MDTRawDataUtils.cxx | 4 +-
.../src/MDTRawDataUtilsRun3.cxx | 4 +-
.../src/MdtRawDataMonAlg.cxx | 93 +++++++++++--------
5 files changed, 64 insertions(+), 50 deletions(-)
diff --git a/MuonSpectrometer/MuonValidation/MuonDQA/MuonRawDataMonitoring/MdtRawDataMonitoring/MdtRawDataMonitoring/MdtRawDataMonAlg.h b/MuonSpectrometer/MuonValidation/MuonDQA/MuonRawDataMonitoring/MdtRawDataMonitoring/MdtRawDataMonitoring/MdtRawDataMonAlg.h
index feeca790eeda..cba4d684877b 100755
--- a/MuonSpectrometer/MuonValidation/MuonDQA/MuonRawDataMonitoring/MdtRawDataMonitoring/MdtRawDataMonitoring/MdtRawDataMonAlg.h
+++ b/MuonSpectrometer/MuonValidation/MuonDQA/MuonRawDataMonitoring/MdtRawDataMonitoring/MdtRawDataMonitoring/MdtRawDataMonAlg.h
@@ -110,10 +110,6 @@ class MdtRawDataMonAlg: public AthMonitorAlgorithm {
private:
- TH2* m_mdthitspermultilayerLumi[4][4];
- TH2* m_mdthitsperchamber_InnerMiddleOuterLumi[2];
- TH2* m_mdthitsperML_byLayer[3];//These are alternative Global hit coverage plots
-
MDTNoisyTubes* m_masked_tubes;
ServiceHandle<Muon::IMuonIdHelperSvc> m_idHelperSvc {this, "MuonIdHelperSvc", "Muon::MuonIdHelperSvc/MuonIdHelperSvc"};
@@ -136,8 +132,8 @@ class MdtRawDataMonAlg: public AthMonitorAlgorithm {
//MDTRawDataUtils_cxx
bool AinB( int A, std::vector<int> & B ) const;
- virtual StatusCode binMdtGlobal( TH2* &, char ecap );
- virtual StatusCode binMdtRegional( TH2* &, std::string &xAxis);
+ virtual StatusCode binMdtGlobal( TH2*, char ecap );
+ virtual StatusCode binMdtRegional( TH2*, std::string &xAxis );
virtual StatusCode binMdtGlobal_byLayer( TH2*, TH2*, TH2*);
virtual StatusCode binMdtOccVsLB(TH2* &h, int region, int layer);
virtual StatusCode binMdtOccVsLB_Crate(TH2* &h, int region, int crate);
diff --git a/MuonSpectrometer/MuonValidation/MuonDQA/MuonRawDataMonitoring/MdtRawDataMonitoring/MdtRawDataMonitoring/MdtRawDataValAlg.h b/MuonSpectrometer/MuonValidation/MuonDQA/MuonRawDataMonitoring/MdtRawDataMonitoring/MdtRawDataMonitoring/MdtRawDataValAlg.h
index 6df5cbf4b05f..e7fa452cfbf5 100755
--- a/MuonSpectrometer/MuonValidation/MuonDQA/MuonRawDataMonitoring/MdtRawDataMonitoring/MdtRawDataMonitoring/MdtRawDataValAlg.h
+++ b/MuonSpectrometer/MuonValidation/MuonDQA/MuonRawDataMonitoring/MdtRawDataMonitoring/MdtRawDataMonitoring/MdtRawDataValAlg.h
@@ -39,6 +39,7 @@
#include <fstream>
#include <cstdlib>
#include <iostream>
+#include <memory>
class MuonDQAHistList;
@@ -131,8 +132,8 @@ class MdtRawDataValAlg: public ManagedMonitorToolBase {
StatusCode handleEvent_effCalc(const Trk::SegmentCollection* segms);//, const Muon::MdtPrepDataContainer* mdt_container );
bool AinB( int A, std::vector<int> & B );
- virtual StatusCode binMdtGlobal( TH2* &, char ecap );
- virtual StatusCode binMdtRegional( TH2* &, std::string &xAxis);
+ virtual StatusCode binMdtGlobal( TH2*, char ecap );
+ virtual StatusCode binMdtRegional( TH2*, std::string &xAxis );
virtual StatusCode binMdtGlobal_byLayer( TH2*, TH2*, TH2*);
virtual StatusCode binMdtOccVsLB(TH2* &h, int region, int layer);
virtual StatusCode binMdtOccVsLB_Crate(TH2* &h, int region, int crate);
diff --git a/MuonSpectrometer/MuonValidation/MuonDQA/MuonRawDataMonitoring/MdtRawDataMonitoring/src/MDTRawDataUtils.cxx b/MuonSpectrometer/MuonValidation/MuonDQA/MuonRawDataMonitoring/MdtRawDataMonitoring/src/MDTRawDataUtils.cxx
index 370f37cec4b5..3faa6e3c0e0f 100644
--- a/MuonSpectrometer/MuonValidation/MuonDQA/MuonRawDataMonitoring/MdtRawDataMonitoring/src/MDTRawDataUtils.cxx
+++ b/MuonSpectrometer/MuonValidation/MuonDQA/MuonRawDataMonitoring/MdtRawDataMonitoring/src/MDTRawDataUtils.cxx
@@ -24,7 +24,7 @@
using std::string;
-StatusCode MdtRawDataValAlg::binMdtGlobal( TH2* &h, char ecap ) {
+StatusCode MdtRawDataValAlg::binMdtGlobal( TH2* h, char ecap ) {
//Set x-axis labels
int LowerEta=0;
@@ -220,7 +220,7 @@ StatusCode MdtRawDataValAlg::binMdtGlobal( TH2* &h, char ecap ) {
return StatusCode::SUCCESS;
}
-StatusCode MdtRawDataValAlg::binMdtRegional( TH2* &h, string &xAxis){
+StatusCode MdtRawDataValAlg::binMdtRegional( TH2* h, string &xAxis ) {
///SET Ghost Entries
int LowerEta=-0;
diff --git a/MuonSpectrometer/MuonValidation/MuonDQA/MuonRawDataMonitoring/MdtRawDataMonitoring/src/MDTRawDataUtilsRun3.cxx b/MuonSpectrometer/MuonValidation/MuonDQA/MuonRawDataMonitoring/MdtRawDataMonitoring/src/MDTRawDataUtilsRun3.cxx
index 27b797159e88..85e4288e3a42 100644
--- a/MuonSpectrometer/MuonValidation/MuonDQA/MuonRawDataMonitoring/MdtRawDataMonitoring/src/MDTRawDataUtilsRun3.cxx
+++ b/MuonSpectrometer/MuonValidation/MuonDQA/MuonRawDataMonitoring/MdtRawDataMonitoring/src/MDTRawDataUtilsRun3.cxx
@@ -23,7 +23,7 @@
using std::string;
-StatusCode MdtRawDataMonAlg::binMdtGlobal( TH2* &h, char ecap ) {
+StatusCode MdtRawDataMonAlg::binMdtGlobal( TH2* h, char ecap ) {
//Set x-axis labels
int LowerEta=0;
@@ -74,7 +74,7 @@ StatusCode MdtRawDataMonAlg::binMdtGlobal( TH2* &h, char ecap ) {
return StatusCode::SUCCESS;
}
-StatusCode MdtRawDataMonAlg::binMdtRegional( TH2* &h, string &xAxis){
+StatusCode MdtRawDataMonAlg::binMdtRegional( TH2* h, string &xAxis ) {
///SET Ghost Entries
int LowerEta=-0;
diff --git a/MuonSpectrometer/MuonValidation/MuonDQA/MuonRawDataMonitoring/MdtRawDataMonitoring/src/MdtRawDataMonAlg.cxx b/MuonSpectrometer/MuonValidation/MuonDQA/MuonRawDataMonitoring/MdtRawDataMonitoring/src/MdtRawDataMonAlg.cxx
index 75b6660e772c..d789ecc93297 100755
--- a/MuonSpectrometer/MuonValidation/MuonDQA/MuonRawDataMonitoring/MdtRawDataMonitoring/src/MdtRawDataMonAlg.cxx
+++ b/MuonSpectrometer/MuonValidation/MuonDQA/MuonRawDataMonitoring/MdtRawDataMonitoring/src/MdtRawDataMonAlg.cxx
@@ -57,8 +57,6 @@ namespace {
static constexpr unsigned int const maxNTubesPerLayer = 120;
}
-enum {enumInner, enumMiddle, enumOuter, enumExtra};
-
struct MDTOverviewHistogramStruct {
std::vector<float> mdt_tube_x_barrel;
std::vector<float> mdt_tube_y_barrel;
@@ -240,41 +238,60 @@ StatusCode MdtRawDataMonAlg::initialize()
}
}
- std::string MDTHits_BE[2] = {"Barrel", "EndCap"};
- std::string ecap[4]={"BA","BC","EA","EC"};
- std::string layer[4]={"Inner","Middle","Outer","Extra"};
-
- for (int iecap=0;iecap<4;iecap++) {//BA,BC,EA,EC
- for (int ilayer=0;ilayer<4;ilayer++) { //inner, middle, outer, extra
- std::string s = "NumberOfHitsIn"+ecap[iecap]+layer[ilayer]+"PerMultiLayer_ADCCut";
- m_mdthitspermultilayerLumi[iecap][ilayer] = new TH2F(s.c_str(), s.c_str(), 1, 0, 1, 1, 0, 1);
- m_mdthitspermultilayerLumi[iecap][ilayer]->SetDirectory(0);
- std::string xAxis = ecap[iecap].substr(0,1) + layer[ilayer].substr(0,1) + ecap[iecap].substr(1,1);
- ATH_CHECK(binMdtRegional(m_mdthitspermultilayerLumi[iecap][ilayer], xAxis));
-
- if( ilayer==0 && ((iecap==0||iecap==2)) ) {
- s = "NumberOfHits"+MDTHits_BE[iecap/2];
- m_mdthitsperchamber_InnerMiddleOuterLumi[iecap/2] = new TH2F(s.c_str(), s.c_str(), 1, 0, 1, 1, 0, 1);
- m_mdthitsperchamber_InnerMiddleOuterLumi[iecap/2]->SetDirectory(0);
- ATH_CHECK(binMdtGlobal(m_mdthitsperchamber_InnerMiddleOuterLumi[iecap/2], MDTHits_BE[iecap/2].at(0)));
- }
- }
- }
-
- std::string s = "NumberOfHitsInMDTInner_ADCCut";
- m_mdthitsperML_byLayer[enumInner] = new TH2F(s.c_str(), s.c_str(), 1, 0, 1, 1, 0, 1);
- m_mdthitsperML_byLayer[enumInner]->SetDirectory(0);
- s = "NumberOfHitsInMDTMIddle_ADCCut";
- m_mdthitsperML_byLayer[enumMiddle] = new TH2F(s.c_str(), s.c_str(), 1, 0, 1, 1, 0, 1);
- m_mdthitsperML_byLayer[enumMiddle]->SetDirectory(0);
- s = "NumberOfHitsInMDTOuter_ADCCut";
- m_mdthitsperML_byLayer[enumOuter] = new TH2F(s.c_str(), s.c_str(), 1, 0, 1, 1, 0, 1);
- m_mdthitsperML_byLayer[enumOuter]->SetDirectory(0);
- ATH_CHECK(binMdtGlobal_byLayer(m_mdthitsperML_byLayer[enumInner], m_mdthitsperML_byLayer[enumMiddle], m_mdthitsperML_byLayer[enumOuter]));
+ /* It seems as if a bunch of histograms are created below on the heap
+ * Then these are filled in the binMdtFooBar methods
+ * Later these are passed onto relevant methods of MDTChamber where internals are filled
+ * The original histograms are transient and not needed past initialize here
+ * The logic is a little too convoluted but left as is
+ */
+ unsigned int counter { 0 };
+ std::string s { "" }, xAxis { "" };
+
+ // Create Inner/Middle/Outer/Extra histograms per BA/BC/EA/EC
+ std::vector< std::string > ecap { "BA", "BC", "EA", "EC" };
+ std::vector< std::string > layer { "Inner", "Middle", "Outer", "Extra" };
+ std::vector< std::unique_ptr<TH2F> > mdtHitsPerMultiLayerLumi;
+ mdtHitsPerMultiLayerLumi.reserve( ecap.size() * layer.size() );
+
+ for ( const auto& iecap : ecap ) {
+ for ( const auto& ilayer : layer ) {
+ s = "NumberOfHitsIn" + iecap + ilayer + "PerMultiLayer_ADCCut";
+ mdtHitsPerMultiLayerLumi.push_back( std::make_unique<TH2F>( s.c_str(), s.c_str(), 1, 0, 1, 1, 0, 1 ) );
+ xAxis = iecap.substr(0,1) + ilayer.substr(0,1) + iecap.substr(1,1);
+ ATH_CHECK( binMdtRegional( mdtHitsPerMultiLayerLumi[ counter ].get(), xAxis ) );
+ counter++;
+ } // end of iecap
+ } // end of ilayer
+ counter = 0;
+
+ // Create Barrel/EndCap histrograms here
+ std::vector< std::string > mdtHitsBE { "Barrel", "EndCap" };
+ std::vector< std::unique_ptr<TH2F> > mdtHitsPerChamberIMOLumi;
+ mdtHitsPerChamberIMOLumi.reserve( mdtHitsBE.size() );
+
+ for ( const auto& imdt : mdtHitsBE ) {
+ s = "NumberOfHits" + imdt;
+ mdtHitsPerChamberIMOLumi.push_back( std::make_unique<TH2F>( s.c_str(), s.c_str(), 1, 0, 1, 1, 0, 1 ) );
+ ATH_CHECK( binMdtGlobal( mdtHitsPerChamberIMOLumi[ counter ].get(), imdt.at(0) ) );
+ counter++;
+ } // end of imdt
+ counter = 0;
+
+ // Create Inner/Middle/Outer histograms here
+ std::vector< std::unique_ptr<TH2F> > mdtHitsPerMLByLayer;
+ mdtHitsPerMLByLayer.reserve( layer.size() - 1 );
+
+ for ( const auto& ilayer : layer ) {
+ if ( ilayer == "Extra" ) continue;
+ s = "NumberOfHitsInMDT" + ilayer + "_ADCCut";
+ mdtHitsPerMLByLayer.push_back( std::make_unique<TH2F> ( s.c_str(), s.c_str(), 1, 0, 1, 1, 0, 1 ) );
+ } // end of ilayer
+ ATH_CHECK( binMdtGlobal_byLayer( mdtHitsPerMLByLayer[ 0 ].get(),
+ mdtHitsPerMLByLayer[ 1 ].get(),
+ mdtHitsPerMLByLayer[ 2 ].get() ) );
//DEV this before was in bookHistogramsRecurrent-- problably still needed since we need to fill m_hist_hash_list??
clear_hist_map();
- int counter = 0;
for(std::vector<Identifier>::const_iterator itr = m_chambersId.begin(); itr != m_chambersId.end(); ++itr, ++counter){
std::string hardware_name = convertChamberName(m_idHelperSvc->mdtIdHelper().stationName(*itr),m_idHelperSvc->mdtIdHelper().stationEta(*itr),
@@ -284,10 +301,10 @@ StatusCode MdtRawDataMonAlg::initialize()
MDTChamber* chamber = new MDTChamber(hardware_name);
(*m_hist_hash_list)[m_chambersIdHash.at(counter)] = chamber;
-
- chamber->SetMDTHitsPerChamber_IMO_Bin(dynamic_cast<TH2F*> (m_mdthitsperchamber_InnerMiddleOuterLumi[chamber->GetBarrelEndcapEnum()]));
- chamber->SetMDTHitsPerML_byLayer_Bins(dynamic_cast<TH2F*> (m_mdthitspermultilayerLumi[chamber->GetRegionEnum()][chamber->GetLayerEnum()])
- ,dynamic_cast<TH2F*> (m_mdthitsperML_byLayer[ (chamber->GetLayerEnum() < 3 ? chamber->GetLayerEnum() : 0) ]));
+
+ chamber->SetMDTHitsPerChamber_IMO_Bin( mdtHitsPerChamberIMOLumi[ chamber->GetBarrelEndcapEnum() ].get() );
+ chamber->SetMDTHitsPerML_byLayer_Bins( mdtHitsPerMultiLayerLumi[ chamber->GetRegionEnum() * ecap.size() + chamber->GetLayerEnum() ].get(),
+ mdtHitsPerMLByLayer[ (chamber->GetLayerEnum() < 3 ? chamber->GetLayerEnum() : 0) ].get() );
m_tubesperchamber_map[hardware_name] = GetTubeMax(*itr, hardware_name); // total number of tubes in chamber
--
GitLab