Merge pull request cms-sw#46903 from srimanob/140X_BP_46451_46543

[14_0_X] Backports of 46543 and 46451 to fix memory leak

DQM/BeamMonitor/plugins/AlcaBeamMonitor.cc

@@ -245,8 +245,8 @@ std::shared_ptr<alcabeammonitor::BeamSpotInfo> AlcaBeamMonitor::globalBeginLumin
     BeamSpot::Point apoint(spotDB->x(), spotDB->y(), spotDB->z());
 
     BeamSpot::CovarianceMatrix matrix;
-    for (int i = 0; i < 7; ++i) {
-      for (int j = 0; j < 7; ++j) {
+    for (int i = 0; i < reco::BeamSpot::dimension; ++i) {
+      for (int j = 0; j < reco::BeamSpot::dimension; ++j) {
         matrix(i, j) = spotDB->covariance(i, j);
       }
     }
@@ -296,7 +296,15 @@ void AlcaBeamMonitor::analyze(const Event& iEvent, const EventSetup& iSetup) {
   //------ Primary Vertices
   Handle<VertexCollection> PVCollection;
   if (iEvent.getByToken(primaryVertexLabel_, PVCollection)) {
-    beamSpotInfo->vertices_.push_back(*PVCollection.product());
+    std::vector<alcabeammonitor::pvPosAndErr> vertices;
+    vertices.reserve(PVCollection->size());
+    for (const auto& pv : *PVCollection.product()) {
+      if (pv.isFake() || pv.tracksSize() < 10)
+        continue;
+      vertices.emplace_back(pv);
+    }
+    vertices.shrink_to_fit();
+    beamSpotInfo->vertices_.emplace_back(std::move(vertices));
   }
 
   if (beamSpotInfo->beamSpotMap_.find("SC") == beamSpotInfo->beamSpotMap_.end()) {
@@ -363,24 +371,19 @@ void AlcaBeamMonitor::globalEndLuminosityBlock(const LuminosityBlock& iLumi, con
       }
     }
     vertexResults.clear();
-    for (vector<VertexCollection>::iterator itPV = beamSpotInfo->vertices_.begin();
-         itPV != beamSpotInfo->vertices_.end();
-         itPV++) {
-      if (!itPV->empty()) {
-        for (VertexCollection::const_iterator pv = itPV->begin(); pv != itPV->end(); pv++) {
-          if (pv->isFake() || pv->tracksSize() < 10)
-            continue;
-          if (*itV == "x") {
-            vertexResults.push_back(pair<double, double>(pv->x(), pv->xError()));
-          } else if (*itV == "y") {
-            vertexResults.push_back(pair<double, double>(pv->y(), pv->yError()));
-          } else if (*itV == "z") {
-            vertexResults.push_back(pair<double, double>(pv->z(), pv->zError()));
-          } else if (*itV != "sigmaX" && *itV != "sigmaY" && *itV != "sigmaZ") {
-            LogInfo("AlcaBeamMonitor") << "The histosMap_ has been built with the name " << *itV
-                                       << " that I can't recognize!";
-            //assert(0);
-          }
+
+    for (const auto& itPV : beamSpotInfo->vertices_) {
+      for (const auto& pv : itPV) {
+        if (*itV == "x") {
+          vertexResults.push_back(pv.xWithError());
+        } else if (*itV == "y") {
+          vertexResults.push_back(pv.yWithError());
+        } else if (*itV == "z") {
+          vertexResults.push_back(pv.zWithError());
+        } else if (*itV != "sigmaX" && *itV != "sigmaY" && *itV != "sigmaZ") {
+          LogInfo("AlcaBeamMonitor") << "The histosMap_ has been built with the name " << *itV
+                                     << " that I can't recognize!";
+          //assert(0);
         }
       }
     }

DQM/BeamMonitor/plugins/AlcaBeamMonitor.h

@@ -8,8 +8,11 @@
  */
 // C++
 #include <map>
+#include <array>
 #include <vector>
 #include <string>
+#include <utility>
+
 // CMS
 #include "FWCore/Framework/interface/Frameworkfwd.h"
 #include "FWCore/Framework/interface/Event.h"
@@ -28,8 +31,23 @@ class BeamFitter;
 class PVFitter;
 
 namespace alcabeammonitor {
+
+  struct pvPosAndErr {
+    // Array of pairs: (value, error) for x, y, z
+    std::array<std::pair<double, double>, 3> data;
+
+    // Constructor initializes the array with values and errors from a reco::Vertex
+    pvPosAndErr(const reco::Vertex& vertex)
+        : data{{{vertex.x(), vertex.xError()}, {vertex.y(), vertex.yError()}, {vertex.z(), vertex.zError()}}} {}
+
+    // Accessor functions that return pairs (value, error) directly
+    std::pair<double, double> xWithError() const { return data[0]; }
+    std::pair<double, double> yWithError() const { return data[1]; }
+    std::pair<double, double> zWithError() const { return data[2]; }
+  };
+
   struct BeamSpotInfo {
-    std::vector<reco::VertexCollection> vertices_;
+    std::vector<std::vector<pvPosAndErr>> vertices_;
     typedef std::map<std::string, reco::BeamSpot> BeamSpotContainer;
     BeamSpotContainer beamSpotMap_;
   };

L1Trigger/L1TCaloLayer1/plugins/L1TCaloLayer1.cc

@@ -74,16 +74,16 @@ class L1TCaloLayer1 : public edm::stream::EDProducer<> {
   edm::EDPutTokenT<L1CaloRegionCollection> regionPutToken;
   const L1TCaloLayer1FetchLUTsTokens lutsTokens;
 
-  std::vector<std::array<std::array<std::array<uint32_t, nEtBins>, nCalSideBins>, nCalEtaBins> > ecalLUT;
-  std::vector<std::array<std::array<std::array<uint32_t, nEtBins>, nCalSideBins>, nCalEtaBins> > hcalLUT;
-  std::vector<std::array<std::array<uint32_t, nEtBins>, nHfEtaBins> > hfLUT;
+  std::vector<std::array<std::array<std::array<uint32_t, nEtBins>, nCalSideBins>, nCalEtaBins>> ecalLUT;
+  std::vector<std::array<std::array<std::array<uint32_t, nEtBins>, nCalSideBins>, nCalEtaBins>> hcalLUT;
+  std::vector<std::array<std::array<uint32_t, nEtBins>, nHfEtaBins>> hfLUT;
   std::vector<unsigned long long int> hcalFBLUT;
 
   std::vector<unsigned int> ePhiMap;
   std::vector<unsigned int> hPhiMap;
   std::vector<unsigned int> hfPhiMap;
 
-  std::vector<UCTTower*> twrList;
+  std::vector<std::shared_ptr<UCTTower>> twrList;
 
   bool useLSB;
   bool useCalib;
@@ -140,7 +140,7 @@ L1TCaloLayer1::L1TCaloLayer1(const edm::ParameterSet& iConfig)
     for (uint32_t crd = 0; crd < cards.size(); crd++) {
       vector<UCTRegion*> regions = cards[crd]->getRegions();
       for (uint32_t rgn = 0; rgn < regions.size(); rgn++) {
-        vector<UCTTower*> towers = regions[rgn]->getTowers();
+        vector<std::shared_ptr<UCTTower>> towers = regions[rgn]->getTowers();
         for (uint32_t twr = 0; twr < towers.size(); twr++) {
           twrList.push_back(towers[twr]);
         }
@@ -150,7 +150,7 @@ L1TCaloLayer1::L1TCaloLayer1(const edm::ParameterSet& iConfig)
 
   // This sort corresponds to the sort condition on
   // the output CaloTowerBxCollection
-  std::sort(twrList.begin(), twrList.end(), [](UCTTower* a, UCTTower* b) {
+  std::sort(twrList.begin(), twrList.end(), [](std::shared_ptr<UCTTower> a, std::shared_ptr<UCTTower> b) {
     return CaloTools::caloTowerHash(a->caloEta(), a->caloPhi()) < CaloTools::caloTowerHash(b->caloEta(), b->caloPhi());
   });
 }

L1Trigger/L1TCaloLayer1/plugins/L1TCaloSummary.cc

@@ -195,8 +195,8 @@ void L1TCaloSummary<INPUT, OUTPUT>::produce(edm::Event& iEvent, const edm::Event
   // of size 7*2. Indices are mapped in UCTSummaryCard accordingly.
   UCTSummaryCard summaryCard =
       UCTSummaryCard(&pumLUT, jetSeed, tauSeed, tauIsolationFactor, eGammaSeed, eGammaIsolationFactor);
-  std::vector<UCTRegion*> inputRegions;
-  inputRegions.clear();
+  std::vector<UCTRegion> inputRegions;
+  inputRegions.reserve(252);  // 252 calorimeter regions. 18 phi * 14 eta
   edm::Handle<std::vector<L1CaloRegion>> regionCollection;
   if (!iEvent.getByToken(regionToken, regionCollection))
     edm::LogError("L1TCaloSummary") << "UCT: Failed to get regions from region collection!";
@@ -222,8 +222,8 @@ void L1TCaloSummary<INPUT, OUTPUT>::produce(edm::Event& iEvent, const edm::Event
     uint32_t crate = g.getCrate(t.first, t.second);
     uint32_t card = g.getCard(t.first, t.second);
     uint32_t region = g.getRegion(absCaloEta, absCaloPhi);
-    UCTRegion* test = new UCTRegion(crate, card, negativeEta, region, fwVersion);
-    test->setRegionSummary(i.raw());
+    UCTRegion test = UCTRegion(crate, card, negativeEta, region, fwVersion);
+    test.setRegionSummary(i.raw());
     inputRegions.push_back(test);
     //This *should* fill the tensor in the proper order to be fed to the anomaly model
     //We take 4 off of the GCT eta/iEta.
@@ -282,9 +282,10 @@ void L1TCaloSummary<INPUT, OUTPUT>::produce(edm::Event& iEvent, const edm::Event
   double phi = -999.;
   double mass = 0;
 
-  std::list<UCTObject*> boostedJetObjs = summaryCard.getBoostedJetObjs();
-  for (std::list<UCTObject*>::const_iterator i = boostedJetObjs.begin(); i != boostedJetObjs.end(); i++) {
-    const UCTObject* object = *i;
+  std::list<std::shared_ptr<UCTObject>> boostedJetObjs = summaryCard.getBoostedJetObjs();
+  for (std::list<std::shared_ptr<UCTObject>>::const_iterator i = boostedJetObjs.begin(); i != boostedJetObjs.end();
+       i++) {
+    const std::shared_ptr<UCTObject> object = *i;
     pt = ((double)object->et()) * caloScaleFactor * boostedJetPtFactor;
     eta = g.getUCTTowerEta(object->iEta());
     phi = g.getUCTTowerPhi(object->iPhi());

L1Trigger/L1TCaloLayer1/src/UCTLayer1.cc

@@ -62,7 +62,7 @@ const UCTRegion* UCTLayer1::getRegion(int regionEtaIndex, uint32_t regionPhiInde
   return region;
 }
 
-const UCTTower* UCTLayer1::getTower(int caloEta, int caloPhi) const {
+const std::shared_ptr<UCTTower> UCTLayer1::getTower(int caloEta, int caloPhi) const {
   if (caloPhi < 0) {
     LOG_ERROR << "UCT::getTower - Negative caloPhi is unacceptable -- bailing" << std::endl;
     exit(1);
@@ -71,7 +71,7 @@ const UCTTower* UCTLayer1::getTower(int caloEta, int caloPhi) const {
   UCTTowerIndex twr = UCTTowerIndex(caloEta, caloPhi);
   const UCTRegionIndex rgn = g.getUCTRegionIndex(twr);
   const UCTRegion* region = getRegion(rgn);
-  const UCTTower* tower = region->getTower(twr);
+  const std::shared_ptr<UCTTower> tower = region->getTower(twr);
   return tower;
 }
 

L1Trigger/L1TCaloLayer1/src/UCTLayer1.hh

@@ -2,6 +2,7 @@
 #define UCTLayer1_hh
 
 #include <vector>
+#include <memory>
 
 class UCTCrate;
 class UCTRegion;
@@ -36,7 +37,7 @@ public:
 
   std::vector<UCTCrate*>& getCrates() { return crates; }
   const UCTRegion* getRegion(UCTRegionIndex r) const { return getRegion(r.first, r.second); }
-  const UCTTower* getTower(UCTTowerIndex t) const { return getTower(t.first, t.second); }
+  const std::shared_ptr<UCTTower> getTower(UCTTowerIndex t) const { return getTower(t.first, t.second); }
 
   // To zero out event in case of selective tower filling
   bool clearEvent();
@@ -58,7 +59,7 @@ private:
   // Helper functions
 
   const UCTRegion* getRegion(int regionEtaIndex, uint32_t regionPhiIndex) const;
-  const UCTTower* getTower(int caloEtaIndex, int caloPhiIndex) const;
+  const std::shared_ptr<UCTTower> getTower(int caloEtaIndex, int caloPhiIndex) const;
 
   //Private data
 

L1Trigger/L1TCaloLayer1/src/UCTRegion.cc

@@ -68,6 +68,14 @@ uint32_t getHitTowerLocation(uint32_t* et) {
   return iAve;
 }
 
+UCTRegion::UCTRegion(const UCTRegion& otherRegion)
+    : crate(otherRegion.crate),
+      card(otherRegion.card),
+      region(otherRegion.region),
+      towers(otherRegion.towers),
+      regionSummary(otherRegion.regionSummary),
+      fwVersion(otherRegion.fwVersion) {}
+
 UCTRegion::UCTRegion(uint32_t crt, uint32_t crd, bool ne, uint32_t rgn, int fwv)
     : crate(crt), card(crd), region(rgn), negativeEta(ne), regionSummary(0), fwVersion(fwv) {
   UCTGeometry g;
@@ -76,24 +84,17 @@ UCTRegion::UCTRegion(uint32_t crt, uint32_t crd, bool ne, uint32_t rgn, int fwv)
   towers.clear();
   for (uint32_t iEta = 0; iEta < nEta; iEta++) {
     for (uint32_t iPhi = 0; iPhi < nPhi; iPhi++) {
-      towers.push_back(new UCTTower(crate, card, ne, region, iEta, iPhi, fwVersion));
+      towers.push_back(std::make_shared<UCTTower>(crate, card, ne, region, iEta, iPhi, fwVersion));
     }
   }
 }
 
-UCTRegion::~UCTRegion() {
-  for (uint32_t i = 0; i < towers.size(); i++) {
-    if (towers[i] != nullptr)
-      delete towers[i];
-  }
-}
-
-const UCTTower* UCTRegion::getTower(uint32_t caloEta, uint32_t caloPhi) const {
+const std::shared_ptr<UCTTower> UCTRegion::getTower(uint32_t caloEta, uint32_t caloPhi) const {
   UCTGeometry g;
   uint32_t nPhi = g.getNPhi(region);
   uint32_t iEta = g.getiEta(caloEta);
   uint32_t iPhi = g.getiPhi(caloPhi);
-  UCTTower* tower = towers[iEta * nPhi + iPhi];
+  std::shared_ptr<UCTTower> tower = towers[iEta * nPhi + iPhi];
   return tower;
 }
 
@@ -229,7 +230,7 @@ bool UCTRegion::setECALData(UCTTowerIndex t, bool ecalFG, uint32_t ecalET) {
   uint32_t absCaloPhi = abs(t.second);
   uint32_t iEta = g.getiEta(absCaloEta);
   uint32_t iPhi = g.getiPhi(absCaloPhi);
-  UCTTower* tower = towers[iEta * nPhi + iPhi];
+  std::shared_ptr<UCTTower> tower = towers[iEta * nPhi + iPhi];
   return tower->setECALData(ecalFG, ecalET);
 }
 
@@ -244,7 +245,7 @@ bool UCTRegion::setHCALData(UCTTowerIndex t, uint32_t hcalFB, uint32_t hcalET) {
     // Valid data are:
     //    absCaloEta = 30-39, 1 < absCaloPhi <= 72 (every second value)
     for (uint32_t iPhi = iPhiStart; iPhi < iPhiStart + 2; iPhi++) {  // For artificial splitting in half
-      UCTTower* tower = towers[iEta * nPhi + iPhi];
+      std::shared_ptr<UCTTower> tower = towers[iEta * nPhi + iPhi];
       // We divide by 2 in output section, after LUT
       if (!tower->setHFData(hcalFB, hcalET))
         return false;
@@ -253,14 +254,14 @@ bool UCTRegion::setHCALData(UCTTowerIndex t, uint32_t hcalFB, uint32_t hcalET) {
     // Valid data are:
     //    absCaloEta = 40,41, 1 < absCaloPhi <= 72 (every fourth value)
     for (uint32_t iPhi = 0; iPhi < 4; iPhi++) {  // For artificial splitting in quarter
-      UCTTower* tower = towers[iEta * nPhi + iPhi];
+      std::shared_ptr<UCTTower> tower = towers[iEta * nPhi + iPhi];
       // We divide by 4 in output section, after LUT
       if (!tower->setHFData(hcalFB, hcalET))
         return false;
     }
   } else {
     uint32_t iPhi = g.getiPhi(absCaloPhi);
-    UCTTower* tower = towers[iEta * nPhi + iPhi];
+    std::shared_ptr<UCTTower> tower = towers[iEta * nPhi + iPhi];
     return tower->setHCALData(hcalFB, hcalET);
   }
   return true;

L1Trigger/L1TCaloLayer1/src/UCTRegion.hh

@@ -3,6 +3,7 @@
 
 #include <vector>
 #include <iostream>
+#include <memory>
 
 #include "UCTTower.hh"
 
@@ -30,19 +31,19 @@ public:
 
   UCTRegion() = delete;
 
-  // No copy constructor is needed
-
-  UCTRegion(const UCTRegion&) = delete;
+  //copy constructor helps gettting around some memory leakage
+  // and ownership problems in the summary card emulation
+  UCTRegion(const UCTRegion&);
 
   // No equality operator is needed
 
   const UCTRegion& operator=(const UCTRegion&) = delete;
 
-  virtual ~UCTRegion();
+  virtual ~UCTRegion() = default;
 
   // To setData for towers before processing
 
-  const std::vector<UCTTower*>& getTowers() { return towers; }
+  const std::vector<std::shared_ptr<UCTTower>>& getTowers() { return towers; }
 
   // To process event
 
@@ -92,14 +93,14 @@ public:
 
   const bool isNegativeEta() const { return negativeEta; }
 
-  const UCTTower* getTower(UCTTowerIndex t) const { return getTower(t.first, t.second); }
+  const std::shared_ptr<UCTTower> getTower(UCTTowerIndex t) const { return getTower(t.first, t.second); }
 
   friend std::ostream& operator<<(std::ostream&, const UCTRegion&);
 
 protected:
   // Helper functions
 
-  const UCTTower* getTower(uint32_t caloEta, uint32_t caloPhi) const;
+  const std::shared_ptr<UCTTower> getTower(uint32_t caloEta, uint32_t caloPhi) const;
 
   // Region location definition
 
@@ -110,7 +111,7 @@ protected:
 
   // Owned region level data
 
-  std::vector<UCTTower*> towers;
+  std::vector<std::shared_ptr<UCTTower>> towers;
 
   uint32_t regionSummary;