From fbf266bcc649f252937d7f69b9e792d585ad8273 Mon Sep 17 00:00:00 2001
From: Tadej Novak <tadej.novak@cern.ch>
Date: Tue, 22 Dec 2020 11:59:37 +0100
Subject: [PATCH] Better memory handling in PseudoJetContainer
---
.../Jet/JetRec/JetRec/PseudoJetContainer.h | 20 +++++-------
.../Jet/JetRec/Root/JetByVertexFinder.cxx | 4 +--
.../Jet/JetRec/Root/PseudoJetContainer.cxx | 31 +++++++------------
.../src/MuonSegmentPseudoJetAlgorithm.cxx | 2 +-
.../Jet/JetRec/src/PseudoJetAlgorithm.cxx | 2 +-
.../test/PseudoJetContainerOfflineTest.cxx | 12 +++----
.../test/PseudoJetContainerTriggerTest.cxx | 12 +++----
Reconstruction/Jet/JetRec/test/testHelpers.h | 12 +++----
.../TrigHLTJetRec/src/TriggerJetBuildTool.cxx | 20 ++++++------
.../src/TriggerPseudoJetGetter2.cxx | 8 ++---
10 files changed, 53 insertions(+), 70 deletions(-)
diff --git a/Reconstruction/Jet/JetRec/JetRec/PseudoJetContainer.h b/Reconstruction/Jet/JetRec/JetRec/PseudoJetContainer.h
index b4b47d1ef4f..ff826d001bd 100644
--- a/Reconstruction/Jet/JetRec/JetRec/PseudoJetContainer.h
+++ b/Reconstruction/Jet/JetRec/JetRec/PseudoJetContainer.h
@@ -40,6 +40,7 @@
#include <vector>
#include <string>
#include <set>
+#include <memory>
typedef std::vector<fastjet::PseudoJet> PseudoJetVector;
@@ -51,12 +52,10 @@ public:
// Constructor from a ConstituentExtractor and vector<PseudoJet>
// PseudoJet OWNS their ConstituentExtractors
- PseudoJetContainer(const IConstituentExtractor* c,
+ PseudoJetContainer(std::unique_ptr<const IConstituentExtractor> c,
const std::vector<PseudoJet> & vecPJ,
bool debug=false);
- ~PseudoJetContainer();
-
// fill xAOD jet with constit&ghosts extracted from final PSeudoJet
bool extractConstituents(xAOD::Jet&, const std::vector<PseudoJet>&) const;
bool extractConstituents(xAOD::Jet& jet, const PseudoJet &finalPJ) const;
@@ -103,8 +102,8 @@ private:
struct ExtractorRange {
ExtractorRange(unsigned int lo,
unsigned int hi,
- const IConstituentExtractor* e):
- m_lo(lo), m_hi(hi), m_e(e){
+ std::unique_ptr<const IConstituentExtractor> e):
+ m_lo(lo), m_hi(hi), m_e(std::move(e)){
}
ExtractorRange(const ExtractorRange& other)
@@ -125,19 +124,14 @@ private:
swap(*this, other);
return *this;
}
-
-
- ~ExtractorRange(){
- delete m_e;
- }
ExtractorRange bump(int step) const {
- return ExtractorRange(m_lo + step, m_hi + step, m_e->clone());
+ return ExtractorRange(m_lo + step, m_hi + step, std::unique_ptr<const IConstituentExtractor>(m_e->clone()));
}
int m_lo;
int m_hi;
- const IConstituentExtractor* m_e;
+ std::unique_ptr<const IConstituentExtractor> m_e{};
};
std::vector<ExtractorRange> m_extractorRanges;
@@ -146,7 +140,7 @@ private:
// created by the creating PseudoJetGetter. We need to keep track
// of the empty extractors to fill zero information (such as 0 counts)
// into the jets.
- std::set<const IConstituentExtractor*> m_emptyExtractors;
+ std::set<std::unique_ptr<const IConstituentExtractor>> m_emptyExtractors;
mutable bool m_debug{false};
};
diff --git a/Reconstruction/Jet/JetRec/Root/JetByVertexFinder.cxx b/Reconstruction/Jet/JetRec/Root/JetByVertexFinder.cxx
index 37803539ab0..0c4cabcd51b 100644
--- a/Reconstruction/Jet/JetRec/Root/JetByVertexFinder.cxx
+++ b/Reconstruction/Jet/JetRec/Root/JetByVertexFinder.cxx
@@ -84,9 +84,7 @@ int JetByVertexFinder::find_(const PseudoJetContainer& cont,
string sinp0 = xAOD::JetInput::typeName(inputtype);
ATH_MSG_DEBUG(" Input type: " << sinp0);
- // FIXME - bypass const of cont parameter
- PseudoJetContainer content = cont;
- const std::vector<PseudoJet>* vpj = content.casVectorPseudoJet();
+ const std::vector<PseudoJet>* vpj = cont.casVectorPseudoJet();
std::vector<PseudoJetVector> psjvectors;
std::vector<const Vertex*> vertices;
diff --git a/Reconstruction/Jet/JetRec/Root/PseudoJetContainer.cxx b/Reconstruction/Jet/JetRec/Root/PseudoJetContainer.cxx
index 104ede8af88..7536c404f36 100644
--- a/Reconstruction/Jet/JetRec/Root/PseudoJetContainer.cxx
+++ b/Reconstruction/Jet/JetRec/Root/PseudoJetContainer.cxx
@@ -20,15 +20,15 @@ using fastjet::PseudoJet;
PseudoJetContainer::PseudoJetContainer() {
checkInvariants("PseudoJetContainer()");
}
-
-PseudoJetContainer::PseudoJetContainer(const IConstituentExtractor* c,
+
+PseudoJetContainer::PseudoJetContainer(std::unique_ptr<const IConstituentExtractor> c,
const std::vector<PseudoJet> & vecPJ,
bool debug){
m_debug = debug;
if (vecPJ.empty()){
- m_emptyExtractors.insert(c);
+ m_emptyExtractors.insert(std::move(c));
return;
}
@@ -37,21 +37,12 @@ PseudoJetContainer::PseudoJetContainer(const IConstituentExtractor* c,
// the limits of the Extractor index range correposnd to the
// numbering of the EDM objects in th extractors.
- m_extractorRanges.push_back(ExtractorRange(0,
- vecPJ.back().user_index(),
- c));
+ m_extractorRanges.emplace_back(0,
+ vecPJ.back().user_index(),
+ std::move(c));
if (m_debug){checkInvariants("PseudoJetContainer(vcPJ, c)");}
}
-
-PseudoJetContainer::~PseudoJetContainer()
-{
- for (const IConstituentExtractor* e : m_emptyExtractors) {
- delete e;
- }
-}
-
-
bool
PseudoJetContainer::extractConstituents(xAOD::Jet& jet,
const std::vector<PseudoJet>& inConstits) const
@@ -69,12 +60,12 @@ PseudoJetContainer::extractConstituents(xAOD::Jet& jet,
// to the extractor is received. But an empty list is used
// by the extractors to fill zeros into the jet.
for(const auto& er : m_extractorRanges){
- sorter.emplace(er.m_e, std::vector<int>{});
+ sorter.emplace(er.m_e.get(), std::vector<int>{});
}
// see header file for explanation of empty extractors.
for(const auto& e : m_emptyExtractors){
- sorter.emplace(e, std::vector<int>{});
+ sorter.emplace(e.get(), std::vector<int>{});
}
// check whether inputs are lgal if m_debug == true
@@ -100,7 +91,7 @@ PseudoJetContainer::extractConstituents(xAOD::Jet& jet,
if(e == m_extractorRanges.end()){return false;}
- sorter[(*e).m_e].push_back(pj_ind - (*e).m_lo);
+ sorter[(*e).m_e.get()].push_back(pj_ind - (*e).m_lo);
}
// send the jet to the each extractor with a vector of EDM
@@ -145,7 +136,7 @@ void PseudoJetContainer::append(const PseudoJetContainer* other) {
pj.set_user_index(pj.user_index() + offset);return pj;}
);
- for(auto e : other->m_emptyExtractors){m_emptyExtractors.insert(e->clone());}
+ for(const auto &e : other->m_emptyExtractors){m_emptyExtractors.emplace(e->clone());}
if (m_debug){checkInvariants("append()");}
}
@@ -166,7 +157,7 @@ std::string PseudoJetContainer::toString(int level, int extLevel) const {
oss << "\n Extractor dump: \n";
for(const auto& er : m_extractorRanges){
oss << "Extractor at [" ;
- oss << er.m_e;
+ oss << er.m_e.get();
oss << "]\n";
oss << er.m_e->toString(extLevel) << '\n';
}
diff --git a/Reconstruction/Jet/JetRec/src/MuonSegmentPseudoJetAlgorithm.cxx b/Reconstruction/Jet/JetRec/src/MuonSegmentPseudoJetAlgorithm.cxx
index 16eee29f733..3fa800047b8 100644
--- a/Reconstruction/Jet/JetRec/src/MuonSegmentPseudoJetAlgorithm.cxx
+++ b/Reconstruction/Jet/JetRec/src/MuonSegmentPseudoJetAlgorithm.cxx
@@ -43,7 +43,7 @@ StatusCode MuonSegmentPseudoJetAlgorithm::execute(const EventContext& ctx) const
ATH_MSG_DEBUG("Created extractor: " << extractor->toString(0));
// Put the PseudoJetContainer together
- auto pjcont = std::make_unique<PseudoJetContainer>(extractor.release(), vpj);
+ auto pjcont = std::make_unique<PseudoJetContainer>(std::move(extractor), vpj);
auto outcoll = SG::makeHandle<PseudoJetContainer>(m_outcoll,ctx);
ATH_MSG_DEBUG("New PseudoJetContainer size " << pjcont->size());
diff --git a/Reconstruction/Jet/JetRec/src/PseudoJetAlgorithm.cxx b/Reconstruction/Jet/JetRec/src/PseudoJetAlgorithm.cxx
index 25f09cf804f..92ece334c49 100644
--- a/Reconstruction/Jet/JetRec/src/PseudoJetAlgorithm.cxx
+++ b/Reconstruction/Jet/JetRec/src/PseudoJetAlgorithm.cxx
@@ -81,7 +81,7 @@ std::unique_ptr<PseudoJetContainer> PseudoJetAlgorithm::createPJContainer(const
}
// Put the PseudoJetContainer together
- auto pjcont = std::make_unique<PseudoJetContainer>(extractor.release(), vpj);
+ auto pjcont = std::make_unique<PseudoJetContainer>(std::move(extractor), vpj);
ATH_MSG_DEBUG("New PseudoJetContainer size " << pjcont->size());
return pjcont;
diff --git a/Reconstruction/Jet/JetRec/test/PseudoJetContainerOfflineTest.cxx b/Reconstruction/Jet/JetRec/test/PseudoJetContainerOfflineTest.cxx
index 1015c929096..fc5c97f6feb 100644
--- a/Reconstruction/Jet/JetRec/test/PseudoJetContainerOfflineTest.cxx
+++ b/Reconstruction/Jet/JetRec/test/PseudoJetContainerOfflineTest.cxx
@@ -77,8 +77,8 @@ protected:
std::vector<PseudoJet> m_pjVec0;
std::vector<PseudoJet> m_pjVec1;
- IParticleExtractor* m_pExtractor_noghost;
- IParticleExtractor* m_pExtractor_ghost;
+ std::unique_ptr<const IParticleExtractor> m_pExtractor_noghost{};
+ std::unique_ptr<const IParticleExtractor> m_pExtractor_ghost{};
Jet* m_testjet0;
Jet* m_testjet1;
@@ -103,7 +103,7 @@ TEST_F(PseudoJetContainerOfflineTest, test_noghost) {
// create the PseudoContainer
- PseudoJetContainer psc(m_pExtractor_noghost, m_pjVec0);
+ PseudoJetContainer psc(std::move(m_pExtractor_noghost), m_pjVec0);
// check the pseudojet accessors
// EXPECT_TRUE(psc.asVectorPseudoJet() == m_pjVec0);
@@ -134,7 +134,7 @@ TEST_F(PseudoJetContainerOfflineTest, test_ghost) {
// The jet containers have a JetAuxContainer, and are stored in the test store
// create the PseudoContainer
- PseudoJetContainer psc(m_pExtractor_ghost, m_pjVec0);
+ PseudoJetContainer psc(std::move(m_pExtractor_ghost), m_pjVec0);
// check the pseudojet accessors
// EXPECT_TRUE(psc.asVectorPseudoJet() == m_pjVec0);
@@ -171,8 +171,8 @@ TEST_F(PseudoJetContainerOfflineTest, test_append) {
bool debug{false};
// create the PseudoContainers
- PseudoJetContainer psc0(m_pExtractor_noghost, m_pjVec0);
- PseudoJetContainer psc1(m_pExtractor_ghost, m_pjVec1);
+ PseudoJetContainer psc0(std::move(m_pExtractor_noghost), m_pjVec0);
+ PseudoJetContainer psc1(std::move(m_pExtractor_ghost), m_pjVec1);
psc0.append(&psc1);
diff --git a/Reconstruction/Jet/JetRec/test/PseudoJetContainerTriggerTest.cxx b/Reconstruction/Jet/JetRec/test/PseudoJetContainerTriggerTest.cxx
index 17119ffe748..df243563d2b 100644
--- a/Reconstruction/Jet/JetRec/test/PseudoJetContainerTriggerTest.cxx
+++ b/Reconstruction/Jet/JetRec/test/PseudoJetContainerTriggerTest.cxx
@@ -82,8 +82,8 @@ protected:
std::vector<PseudoJet> m_pjVec0;
std::vector<PseudoJet> m_pjVec1;
- IParticleExtractor* m_pExtractor_noghost;
- IParticleExtractor* m_pExtractor_ghost;
+ std::unique_ptr<const IParticleExtractor> m_pExtractor_noghost{};
+ std::unique_ptr<const IParticleExtractor> m_pExtractor_ghost{};
Jet* m_testjet0;
Jet* m_testjet1;
@@ -108,7 +108,7 @@ TEST_F(PseudoJetContainerTriggerTest, test_noghost) {
// create the PseudoContainer
- PseudoJetContainer psc(m_pExtractor_noghost, m_pjVec0);
+ PseudoJetContainer psc(std::move(m_pExtractor_noghost), m_pjVec0);
// check the pseudojet accessors
EXPECT_TRUE(*psc.casVectorPseudoJet() == m_pjVec0);
@@ -138,7 +138,7 @@ TEST_F(PseudoJetContainerTriggerTest, test_ghost) {
// The jet containers have a JetAuxContainer, and are stored in the test store
// create the PseudoContainer
- PseudoJetContainer psc(m_pExtractor_ghost, m_pjVec0);
+ PseudoJetContainer psc(std::move(m_pExtractor_ghost), m_pjVec0);
// check the pseudojet accessors
EXPECT_TRUE(*psc.casVectorPseudoJet() == m_pjVec0);
@@ -174,8 +174,8 @@ TEST_F(PseudoJetContainerTriggerTest, test_append) {
bool debug{true};
// create the PseudoContainers
- PseudoJetContainer psc0(m_pExtractor_noghost, m_pjVec0);
- PseudoJetContainer psc1(m_pExtractor_ghost, m_pjVec1);
+ PseudoJetContainer psc0(std::move(m_pExtractor_noghost), m_pjVec0);
+ PseudoJetContainer psc1(std::move(m_pExtractor_ghost), m_pjVec1);
psc0.append(&psc1);
diff --git a/Reconstruction/Jet/JetRec/test/testHelpers.h b/Reconstruction/Jet/JetRec/test/testHelpers.h
index 80164291001..ad264b0c51f 100644
--- a/Reconstruction/Jet/JetRec/test/testHelpers.h
+++ b/Reconstruction/Jet/JetRec/test/testHelpers.h
@@ -19,6 +19,7 @@
#include <iostream>
#include <sstream>
+#include <memory>
int JetContainerIndex (0);
@@ -80,15 +81,14 @@ struct IPtoPSConverter{
}
};
-IParticleExtractor* makeExtractor(const xAOD::IParticleContainer* iparticles,
+std::unique_ptr<const IParticleExtractor> makeExtractor(const xAOD::IParticleContainer* iparticles,
bool isGhost, bool isTrigger=false){
// Create an Extractor
std::string label("EMTopo");
- IParticleExtractor* extractor = new IParticleExtractor(iparticles,
- label,
- isGhost,
- isTrigger);
- return extractor;
+ return std::make_unique<const IParticleExtractor>(iparticles,
+ label,
+ isGhost,
+ isTrigger);
}
diff --git a/Trigger/TrigAlgorithms/TrigHLTJetRec/src/TriggerJetBuildTool.cxx b/Trigger/TrigAlgorithms/TrigHLTJetRec/src/TriggerJetBuildTool.cxx
index 14e591edb49..b17a79d563e 100644
--- a/Trigger/TrigAlgorithms/TrigHLTJetRec/src/TriggerJetBuildTool.cxx
+++ b/Trigger/TrigAlgorithms/TrigHLTJetRec/src/TriggerJetBuildTool.cxx
@@ -144,10 +144,10 @@ void TriggerJetBuildTool::prime(const xAOD::IParticleContainer* inputs){
ATH_MSG_DEBUG("Entering prime(), call " << ++m_nprime);
constexpr bool isGhost = false;
- IParticleExtractor* extractor = new IParticleExtractor(inputs,
- m_concreteTypeStr,
- isGhost,
- m_isTrigger);
+ auto extractor = std::make_unique<const IParticleExtractor>(inputs,
+ m_concreteTypeStr,
+ isGhost,
+ m_isTrigger);
ATH_MSG_DEBUG("No of IParticle inputs: " << inputs->size());
@@ -180,7 +180,7 @@ void TriggerJetBuildTool::prime(const xAOD::IParticleContainer* inputs){
- PseudoJetContainer pjc(extractor, vpj);
+ PseudoJetContainer pjc(std::move(extractor), vpj);
m_inContainer.append(&pjc);
}
@@ -194,10 +194,10 @@ void TriggerJetBuildTool::primeGhost(const xAOD::IParticleContainer* inputs, std
ATH_MSG_DEBUG("Entering primeGhost(), call " << ++m_nprime);
constexpr bool isGhost = true;
- IParticleExtractor* extractor = new IParticleExtractor(inputs,
- ghostlabel,
- isGhost,
- m_isTrigger);
+ auto extractor = std::make_unique<const IParticleExtractor>(inputs,
+ ghostlabel,
+ isGhost,
+ m_isTrigger);
ATH_MSG_DEBUG("No of ghost IParticle inputs: " << inputs->size());
@@ -231,7 +231,7 @@ void TriggerJetBuildTool::primeGhost(const xAOD::IParticleContainer* inputs, std
- PseudoJetContainer pjc(extractor, vpj);
+ PseudoJetContainer pjc(std::move(extractor), vpj);
m_inContainer.append(&pjc);
}
diff --git a/Trigger/TrigAlgorithms/TrigHLTJetRec/src/TriggerPseudoJetGetter2.cxx b/Trigger/TrigAlgorithms/TrigHLTJetRec/src/TriggerPseudoJetGetter2.cxx
index 0e4f80cdc59..f3b9126a4d9 100644
--- a/Trigger/TrigAlgorithms/TrigHLTJetRec/src/TriggerPseudoJetGetter2.cxx
+++ b/Trigger/TrigAlgorithms/TrigHLTJetRec/src/TriggerPseudoJetGetter2.cxx
@@ -37,9 +37,9 @@ void TriggerPseudoJetGetter2::prime(const xAOD::CaloClusterContainer* inputs) {
// to determine the function used to select the incomming IParticles.
constexpr bool isGhost = false;
- IConstituentExtractor* extractor = new IParticleExtractor(inputs,
- m_label,
- isGhost);
+ auto extractor = std::make_unique<const IParticleExtractor>(inputs,
+ m_label,
+ isGhost);
constexpr bool noRejection = true;
std::vector<fastjet::PseudoJet> vpj =
@@ -48,7 +48,7 @@ void TriggerPseudoJetGetter2::prime(const xAOD::CaloClusterContainer* inputs) {
m_noNegE,
noRejection);
- auto ppjc(std::make_unique<const PseudoJetContainer>(extractor, vpj));
+ auto ppjc(std::make_unique<const PseudoJetContainer>(std::move(extractor), vpj));
m_pseudoJetContainer.swap(ppjc);
}
--
GitLab