Skip to content

Commit 6d8243b

Browse files
mmusichmmusich
committed
follow-up: reduction of memory allocation in string manipulation in SiStrip DQM
1 parent 341edfe commit 6d8243b

File tree

5 files changed

+52
-62
lines changed

5 files changed

+52
-62
lines changed

‎DQM/SiStripCommon/interface/SiStripFolderOrganizer.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,12 +89,15 @@ class SiStripFolderOrganizer {
8989
// SubDetector Folder
9090
void getSubDetFolder(const uint32_t& detid, const TrackerTopology* tTopo, std::string& folder_name);
9191
std::pair<std::string, std::string_view> getSubDetFolderAndTag(const uint32_t& detid, const TrackerTopology* tTopo);
92+
const std::string_view getSubDetTag(const uint32_t& detid, const TrackerTopology* tTopo);
9293

9394
SiStripFolderOrganizer(const SiStripFolderOrganizer&) = delete; // stop default
9495
const SiStripFolderOrganizer& operator=(const SiStripFolderOrganizer&) = delete; // stop default
9596

9697
private:
9798
std::string TopFolderName;
9899
DQMStore* dbe_;
100+
101+
std::pair<std::string_view, std::string_view> getSubdetStrings(const uint32_t& detid, const TrackerTopology* tTopo);
99102
};
100103
#endif

‎DQM/SiStripCommon/src/SiStripFolderOrganizer.cc‎

Lines changed: 22 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -406,7 +406,7 @@ void SiStripFolderOrganizer::getSubDetFolder(const uint32_t& detid,
406406
const TrackerTopology* tTopo,
407407
std::string& folder_name) {
408408
auto subdet_and_tag = getSubDetFolderAndTag(detid, tTopo);
409-
folder_name = std::string(subdet_and_tag.first);
409+
folder_name = subdet_and_tag.first;
410410
}
411411
//
412412
// -- Get the name of Subdetector Layer folder
@@ -468,49 +468,36 @@ void SiStripFolderOrganizer::getLayerFolderName(std::stringstream& ss,
468468
return;
469469
}
470470
}
471-
//
472-
// -- Get Subdetector Folder name and the Tag
473-
//
474-
std::pair<std::string, std::string_view> SiStripFolderOrganizer::getSubDetFolderAndTag(const uint32_t& detid,
475-
const TrackerTopology* tTopo) {
476-
std::string_view subdet_folder;
477-
std::string_view tag;
478471

472+
using namespace std::literals::string_view_literals;
473+
474+
std::pair<std::string_view, std::string_view> SiStripFolderOrganizer::getSubdetStrings(const uint32_t& detid,
475+
const TrackerTopology* tTopo) {
476+
using std::string_view;
479477
switch (StripSubdetector::SubDetector(StripSubdetector(detid).subdetId())) {
480478
case StripSubdetector::TIB:
481-
subdet_folder = "TIB";
482-
tag = subdet_folder;
483-
break;
479+
return {"TIB", "TIB"};
484480
case StripSubdetector::TOB:
485-
subdet_folder = "TOB";
486-
tag = subdet_folder;
487-
break;
481+
return {"TOB", "TOB"};
488482
case StripSubdetector::TID:
489-
if (tTopo->tidSide(detid) == 2) {
490-
subdet_folder = "TID/PLUS";
491-
tag = "TID__PLUS";
492-
} else if (tTopo->tidSide(detid) == 1) {
493-
subdet_folder = "TID/MINUS";
494-
tag = "TID__MINUS";
495-
}
496-
break;
483+
return (tTopo->tidSide(detid) == 2) ? std::make_pair("TID/PLUS"sv, "TID__PLUS"sv)
484+
: std::make_pair("TID/MINUS"sv, "TID__MINUS"sv);
497485
case StripSubdetector::TEC:
498-
if (tTopo->tecSide(detid) == 2) {
499-
subdet_folder = "TEC/PLUS";
500-
tag = "TEC__PLUS";
501-
} else if (tTopo->tecSide(detid) == 1) {
502-
subdet_folder = "TEC/MINUS";
503-
tag = "TEC__MINUS";
504-
}
505-
break;
486+
return (tTopo->tecSide(detid) == 2) ? std::make_pair("TEC/PLUS"sv, "TEC__PLUS"sv)
487+
: std::make_pair("TEC/MINUS"sv, "TEC__MINUS"sv);
506488
default:
507489
edm::LogWarning("SiStripCommon") << "WARNING!!! this detid does not belong to tracker" << std::endl;
508-
subdet_folder = "";
509-
tag = "";
490+
return {"", ""};
510491
}
492+
}
511493

512-
// Concatenate the folder path
513-
std::string folder = std::string(TopFolderName) + SEP + MECHANICAL_FOLDER_NAME + SEP + std::string(subdet_folder);
494+
const std::string_view SiStripFolderOrganizer::getSubDetTag(const uint32_t& detid, const TrackerTopology* tTopo) {
495+
return getSubdetStrings(detid, tTopo).second;
496+
}
514497

515-
return {folder, tag};
498+
std::pair<std::string, std::string_view> SiStripFolderOrganizer::getSubDetFolderAndTag(const uint32_t& detid,
499+
const TrackerTopology* tTopo) {
500+
auto [folder_component, tag] = getSubdetStrings(detid, tTopo);
501+
std::string folder = TopFolderName + SEP + MECHANICAL_FOLDER_NAME + SEP + std::string(folder_component);
502+
return {std::move(folder), tag};
516503
}

‎DQM/SiStripCommon/src/SiStripHistoId.cc‎

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
#include <iostream>
1414
#include <sstream>
1515
#include <cstdio>
16+
#include <fmt/format.h>
1617

1718
#include "FWCore/MessageLogger/interface/MessageLogger.h"
1819
#include "DQM/SiStripCommon/interface/SiStripHistoId.h"
@@ -79,38 +80,38 @@ std::string SiStripHistoId::createHistoLayer(std::string description,
7980
}
8081

8182
// std::string SiStripHistoId::getSubdetid(uint32_t id, const TrackerTopology* tTopo, bool flag_ring, bool flag_thickness) {
82-
std::string SiStripHistoId::getSubdetid(uint32_t id, const TrackerTopology *tTopo, bool flag_ring) {
83+
std::string SiStripHistoId::getSubdetid(uint32_t id, const TrackerTopology* tTopo, bool flag_ring) {
8384
StripSubdetector subdet(id);
8485

8586
if (subdet.subdetId() == StripSubdetector::TIB) {
8687
// --------------------------- TIB --------------------------- //
8788
return "TIB__layer__" + std::to_string(tTopo->tibLayer(id));
8889
} else if (subdet.subdetId() == StripSubdetector::TID) {
8990
// --------------------------- TID --------------------------- //
90-
std::string side = (tTopo->tidSide(id) == 1) ? "MINUS" : "PLUS";
91+
const char* side = (tTopo->tidSide(id) == 1) ? "MINUS" : "PLUS";
9192

9293
if (flag_ring)
93-
return "TID__" + side + "__ring__" + std::to_string(tTopo->tidRing(id));
94+
return fmt::format("TID__{}__ring__{}", side, tTopo->tidRing(id));
9495
else
95-
return "TID__" + side + "__wheel__" + std::to_string(tTopo->tidWheel(id));
96+
return fmt::format("TID__{}__wheel__{}", side, tTopo->tidWheel(id));
9697
} else if (subdet.subdetId() == StripSubdetector::TOB) {
9798
// --------------------------- TOB --------------------------- //
9899
return "TOB__layer__" + std::to_string(tTopo->tobLayer(id));
99100
} else if (subdet.subdetId() == StripSubdetector::TEC) {
100101
// --------------------------- TEC --------------------------- //
101-
std::string side = (tTopo->tecSide(id) == 1) ? "MINUS" : "PLUS";
102+
const char* side = (tTopo->tecSide(id) == 1) ? "MINUS" : "PLUS";
102103

103104
if (flag_ring) {
104-
return "TEC__" + side + "__ring__" + std::to_string(tTopo->tecRing(id));
105+
return fmt::format("TEC__{}__ring__{}", side, tTopo->tecRing(id));
105106
} else {
106107
/*
107108
if (flag_thickness) {
108109
uint32_t ring = tTopo->tecRing(id);
109110
std::string thickness = (ring >= 1 && ring <= 4) ? "__THIN" : "__THICK";
110-
return "TEC__" + side + "__wheel__" + std::to_string(tTopo->tecWheel(id)) + thickness;
111+
return fmt::format("TEC__{}__wheel__{}{}", side, tTopo->tecWheel(id), thickness);
111112
} else
112113
*/
113-
return "TEC__" + side + "__wheel__" + std::to_string(tTopo->tecWheel(id));
114+
return fmt::format("TEC__{}__wheel__{}", side, tTopo->tecWheel(id));
114115
}
115116
} else {
116117
// --------------------------- ??? --------------------------- //

‎DQM/SiStripMonitorDigi/src/SiStripMonitorDigi.cc‎

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -830,33 +830,33 @@ void SiStripMonitorDigi::analyze(const edm::Event& iEvent, const edm::EventSetup
830830

831831
for (auto& it : SubDetMEsMap) {
832832
if (subdetswitchtotdigifailureon) {
833-
if (strcmp(it.first.data(), "TEC__MINUS") == 0) {
833+
if (it.first == "TEC__MINUS") {
834834
digiFailureMEs.SubDetTotDigiProfLS->Fill(1, it.second.totNDigis);
835-
} else if (strcmp(it.first.data(), "TEC__PLUS") == 0) {
835+
} else if (it.first == "TEC__PLUS") {
836836
digiFailureMEs.SubDetTotDigiProfLS->Fill(2, it.second.totNDigis);
837-
} else if (strcmp(it.first.data(), "TIB") == 0) {
837+
} else if (it.first == "TIB") {
838838
digiFailureMEs.SubDetTotDigiProfLS->Fill(3, it.second.totNDigis);
839-
} else if (strcmp(it.first.data(), "TID__MINUS") == 0) {
839+
} else if (it.first == "TID__MINUS") {
840840
digiFailureMEs.SubDetTotDigiProfLS->Fill(4, it.second.totNDigis);
841-
} else if (strcmp(it.first.data(), "TID__PLUS") == 0) {
841+
} else if (it.first == "TID__PLUS") {
842842
digiFailureMEs.SubDetTotDigiProfLS->Fill(5, it.second.totNDigis);
843-
} else if (strcmp(it.first.data(), "TOB") == 0) {
843+
} else if (it.first == "TOB") {
844844
digiFailureMEs.SubDetTotDigiProfLS->Fill(6, it.second.totNDigis);
845845
}
846846
}
847847

848848
if (globalsummaryapvshotson) {
849-
if (strcmp(it.first.data(), "TEC__MINUS") == 0) {
849+
if (it.first == "TEC__MINUS") {
850850
NApvShotsGlobalProf->Fill(1, it.second.SubDetApvShots.size());
851-
} else if (strcmp(it.first.data(), "TEC__PLUS") == 0) {
851+
} else if (it.first == "TEC__PLUS") {
852852
NApvShotsGlobalProf->Fill(2, it.second.SubDetApvShots.size());
853-
} else if (strcmp(it.first.data(), "TIB") == 0) {
853+
} else if (it.first == "TIB") {
854854
NApvShotsGlobalProf->Fill(3, it.second.SubDetApvShots.size());
855-
} else if (strcmp(it.first.data(), "TID__MINUS") == 0) {
855+
} else if (it.first == "TID__MINUS") {
856856
NApvShotsGlobalProf->Fill(4, it.second.SubDetApvShots.size());
857-
} else if (strcmp(it.first.data(), "TID__PLUS") == 0) {
857+
} else if (it.first == "TID__PLUS") {
858858
NApvShotsGlobalProf->Fill(5, it.second.SubDetApvShots.size());
859-
} else if (strcmp(it.first.data(), "TOB") == 0) {
859+
} else if (it.first == "TOB") {
860860
NApvShotsGlobalProf->Fill(6, it.second.SubDetApvShots.size());
861861
}
862862
}
@@ -922,9 +922,8 @@ void SiStripMonitorDigi::analyze(const edm::Event& iEvent, const edm::EventSetup
922922
long long tbx = event_history->absoluteBX();
923923

924924
for (auto& it : SubDetMEsMap) {
925-
SubDetMEs subdetmes;
926925
const auto& subdet = std::string(it.first);
927-
subdetmes = it.second;
926+
SubDetMEs& subdetmes = it.second;
928927

929928
int the_phase = APVCyclePhaseCollection::invalid;
930929
long long tbx_corr = tbx;

‎DQM/SiStripMonitorTrack/src/SiStripMonitorTrack.cc‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -731,19 +731,19 @@ void SiStripMonitorTrack::bookSubDetMEs(DQMStore::IBooker& ibooker, std::string_
731731
theSubDetMEs.nClustersOffTrack->setAxisTitle(axisName);
732732

733733
double xmaximum = 0;
734-
if (sname.find("TIB") != std::string::npos) {
734+
if (sname.find("TIB") != std::string_view::npos) {
735735
xmaximum = 40000.0;
736736
theSubDetMEs.nClustersOffTrack->setAxisRange(0.0, xmaximum, 1);
737737
}
738-
if (sname.find("TOB") != std::string::npos) {
738+
if (sname.find("TOB") != std::string_view::npos) {
739739
xmaximum = 40000.0;
740740
theSubDetMEs.nClustersOffTrack->setAxisRange(0.0, xmaximum, 1);
741741
}
742-
if (sname.find("TID") != std::string::npos) {
742+
if (sname.find("TID") != std::string_view::npos) {
743743
xmaximum = 10000.0;
744744
theSubDetMEs.nClustersOffTrack->setAxisRange(0.0, xmaximum, 1);
745745
}
746-
if (sname.find("TEC") != std::string::npos) {
746+
if (sname.find("TEC") != std::string_view::npos) {
747747
xmaximum = 40000.0;
748748
theSubDetMEs.nClustersOffTrack->setAxisRange(0.0, xmaximum, 1);
749749
}
@@ -1334,7 +1334,7 @@ SiStripMonitorTrack::Det2MEs SiStripMonitorTrack::findMEs(const TrackerTopology*
13341334

13351335
std::string layer_id = hidmanager1.getSubdetid(detid, tTopo, false);
13361336
std::string ring_id = hidmanager1.getSubdetid(detid, tTopo, true);
1337-
std::string_view sdet_tag = folderOrganizer_.getSubDetFolderAndTag(detid, tTopo).second;
1337+
const std::string_view sdet_tag = folderOrganizer_.getSubDetTag(detid, tTopo);
13381338

13391339
Det2MEs me;
13401340
me.iLayer = nullptr;

0 commit comments

Comments
 (0)