From 651a80dec7a210e6b8f7a965dc63d81c0cc5f8e0 Mon Sep 17 00:00:00 2001
From: germanaizek <GermanAizek@yandex.ru>
Date: Fri, 3 Jun 2022 18:00:34 +0300
Subject: [PATCH 1/3] Using '= default', remove unused vars, code simplify
---
src/cpucounters.cpp | 5 +----
src/cpucounters.h | 1 -
src/lspci.h | 2 +-
src/pcm-iio.cpp | 1 -
src/pcm.cpp | 29 ++++++++++++++++++-----------
src/utils.h | 4 ++--
6 files changed, 22 insertions(+), 20 deletions(-)
diff --git a/src/cpucounters.cpp b/src/cpucounters.cpp
index 4bc24772..bec00989 100644
--- a/src/cpucounters.cpp
+++ b/src/cpucounters.cpp
@@ -6693,10 +6693,7 @@ size_t ServerPCICFGUncore::getNumMCChannels(const uint32 controller) const
return 0;
}
-ServerPCICFGUncore::~ServerPCICFGUncore()
-{
-}
-
+ServerPCICFGUncore::~ServerPCICFGUncore() = default;
void ServerPCICFGUncore::programServerUncoreMemoryMetrics(const ServerUncoreMemoryMetrics & metrics, const int rankA, const int rankB)
{
diff --git a/src/cpucounters.h b/src/cpucounters.h
index 11ce5f94..1ea3694b 100644
--- a/src/cpucounters.h
+++ b/src/cpucounters.h
@@ -977,7 +977,6 @@ class PCM_API PCM
auto ctrl = pmu.counterControl[c];
if (ctrl.get() != nullptr)
{
- *ctrl = MC_CH_PCI_PMON_CTL_EN;
*ctrl = MC_CH_PCI_PMON_CTL_EN | *curEvent;
}
}
diff --git a/src/lspci.h b/src/lspci.h
index 31a0cd40..7a218322 100644
--- a/src/lspci.h
+++ b/src/lspci.h
@@ -418,7 +418,7 @@ void print_pci(struct pci p, const PCIDB & pciDB)
void load_PCIDB(PCIDB & pciDB)
{
std::ifstream in(PCI_IDS_PATH);
- std::string line, item;
+ std::string line;
if (!in.is_open())
{
diff --git a/src/pcm-iio.cpp b/src/pcm-iio.cpp
index 7fa14b11..3fff66e3 100644
--- a/src/pcm-iio.cpp
+++ b/src/pcm-iio.cpp
@@ -386,7 +386,6 @@ vector<string> build_csv(vector<struct iio_stacks_on_socket>& iios, vector<struc
for (std::map<uint32_t,map<uint32_t,struct counter*>>::const_iterator vunit = v_sort.cbegin(); vunit != v_sort.cend(); ++vunit) {
map<uint32_t, struct counter*> h_array = vunit->second;
uint32_t vv_id = vunit->first;
- vector<uint64_t> h_data;
string v_name = h_array[0]->v_event_name;
if (human_readable) {
v_name += string(max_name_width - (v_name.size()), ' ');
diff --git a/src/pcm.cpp b/src/pcm.cpp
index 40b69f24..bcbd819a 100644
--- a/src/pcm.cpp
+++ b/src/pcm.cpp
@@ -185,14 +185,22 @@ void print_output(PCM * m,
cout << " L3MPI : number of L3 (read) cache misses per instruction\n";
if (m->isL2CacheMissesAvailable())
cout << " L2MPI : number of L2 (read) cache misses per instruction\n";
- if (m->memoryTrafficMetricsAvailable()) cout << " READ : bytes read from main memory controller (in GBytes)\n";
- if (m->memoryTrafficMetricsAvailable()) cout << " WRITE : bytes written to main memory controller (in GBytes)\n";
- if (m->localMemoryRequestRatioMetricAvailable()) cout << " LOCAL : ratio of local memory requests to memory controller in %\n";
- if (m->LLCReadMissLatencyMetricsAvailable()) cout << "LLCRDMISSLAT: average latency of last level cache miss for reads and prefetches (in ns)\n";
- if (m->PMMTrafficMetricsAvailable()) cout << " PMM RD : bytes read from PMM memory (in GBytes)\n";
- if (m->PMMTrafficMetricsAvailable()) cout << " PMM WR : bytes written to PMM memory (in GBytes)\n";
- if (m->MCDRAMmemoryTrafficMetricsAvailable()) cout << " MCDRAM READ : bytes read from MCDRAM controller (in GBytes)\n";
- if (m->MCDRAMmemoryTrafficMetricsAvailable()) cout << " MCDRAM WRITE : bytes written to MCDRAM controller (in GBytes)\n";
+ if (m->memoryTrafficMetricsAvailable()) {
+ cout << " READ : bytes read from main memory controller (in GBytes)\n";
+ cout << " WRITE : bytes written to main memory controller (in GBytes)\n";
+ }
+ if (m->localMemoryRequestRatioMetricAvailable()) {
+ cout << " LOCAL : ratio of local memory requests to memory controller in %\n";
+ cout << "LLCRDMISSLAT: average latency of last level cache miss for reads and prefetches (in ns)\n";
+ }
+ if (m->PMMTrafficMetricsAvailable()) {
+ cout << " PMM RD : bytes read from PMM memory (in GBytes)\n";
+ cout << " PMM WR : bytes written to PMM memory (in GBytes)\n";
+ }
+ if (m->MCDRAMmemoryTrafficMetricsAvailable()) {
+ cout << " MCDRAM READ : bytes read from MCDRAM controller (in GBytes)\n";
+ cout << " MCDRAM WRITE : bytes written to MCDRAM controller (in GBytes)\n";
+ }
if (m->memoryIOTrafficMetricAvailable()) {
cout << " IO : bytes read/written due to IO requests to memory controller (in GBytes); this may be an over estimate due to same-cache-line partial requests\n";
cout << " IA : bytes read/written due to IA requests to memory controller (in GBytes); this may be an over estimate due to same-cache-line partial requests\n";
@@ -437,12 +445,11 @@ void print_output(PCM * m,
cout << " PMM RD | PMM WR |";
if (m->MCDRAMmemoryTrafficMetricsAvailable())
cout << " MCDRAM READ | MCDRAM WRITE |";
- if (m->memoryIOTrafficMetricAvailable())
+ if (m->memoryIOTrafficMetricAvailable()) {
cout << " IO |";
- if (m->memoryIOTrafficMetricAvailable())
cout << " IA |";
- if (m->memoryIOTrafficMetricAvailable())
cout << " GT |";
+ }
if (m->packageEnergyMetricsAvailable())
cout << " CPU energy |";
if (m->dramEnergyMetricsAvailable())
diff --git a/src/utils.h b/src/utils.h
index bb2a9ec6..6355bdb7 100644
--- a/src/utils.h
+++ b/src/utils.h
@@ -322,7 +322,7 @@ class MainLoop
{
unsigned numberOfIterations = 0;
public:
- MainLoop() {}
+ MainLoop() = default;
bool parseArg(const char * arg)
{
if (strncmp(arg, "-i", 2) == 0 ||
@@ -372,7 +372,7 @@ struct StackedBarItem {
double fraction{0.0};
std::string label{""}; // not used currently
char fill{'0'};
- StackedBarItem() {}
+ StackedBarItem() = default;
StackedBarItem(double fraction_,
const std::string & label_,
char fill_) : fraction(fraction_), label(label_), fill(fill_) {}
From f7de464a574968fba59cffede2320a49bfba2f2d Mon Sep 17 00:00:00 2001
From: germanaizek <GermanAizek@yandex.ru>
Date: Fri, 3 Jun 2022 18:04:07 +0300
Subject: [PATCH 2/3] Vars renamed like others, using std::move() and str to
char optimize
---
src/cpucounters.cpp | 11 ++++++-----
src/pcm-memory.cpp | 8 ++++----
src/pcm-power.cpp | 2 +-
src/pcm.cpp | 10 +++++-----
src/topology.h | 2 +-
src/utils.cpp | 2 +-
6 files changed, 18 insertions(+), 17 deletions(-)
diff --git a/src/cpucounters.cpp b/src/cpucounters.cpp
index bec00989..81f7fd1b 100644
--- a/src/cpucounters.cpp
+++ b/src/cpucounters.cpp
@@ -4700,10 +4700,11 @@ PCM::ErrorCode PCM::program(const RawPMUConfigs& curPMUConfigs_, const bool sile
}
if (globalRegPos < corePMUConfig.programmable.size())
{
- conf.OffcoreResponseMsrValue[0] = corePMUConfig.programmable[globalRegPos].first[OCR0Pos];
- conf.OffcoreResponseMsrValue[1] = corePMUConfig.programmable[globalRegPos].first[OCR1Pos];
- conf.LoadLatencyMsrValue = corePMUConfig.programmable[globalRegPos].first[LoadLatencyPos];
- conf.FrontendMsrValue = corePMUConfig.programmable[globalRegPos].first[FrontendPos];
+ const auto& cfgProg = corePMUConfig.programmable[globalRegPos];
+ conf.OffcoreResponseMsrValue[0] = cfgProg.first[OCR0Pos];
+ conf.OffcoreResponseMsrValue[1] = cfgProg.first[OCR1Pos];
+ conf.LoadLatencyMsrValue = cfgProg.first[LoadLatencyPos];
+ conf.FrontendMsrValue = cfgProg.first[FrontendPos];
}
conf.nGPCounters = (uint32)c;
conf.gpCounterCfg = regs;
@@ -5696,7 +5697,7 @@ void initSocket2Bus(std::vector<std::pair<uint32, uint32> > & socket2bus, uint32
if(DEV_IDS[i] == device_id)
{
// std::cout << "DEBUG: found bus " << std::hex << bus << " with device ID " << device_id << std::dec << "\n";
- socket2bus.push_back(std::make_pair(mcfg[s].PCISegmentGroupNumber,bus));
+ socket2bus.emplace_back(std::make_pair(mcfg[s].PCISegmentGroupNumber,bus));
break;
}
}
diff --git a/src/pcm-memory.cpp b/src/pcm-memory.cpp
index a1d48cd9..e6e9de05 100644
--- a/src/pcm-memory.cpp
+++ b/src/pcm-memory.cpp
@@ -83,7 +83,7 @@ bool skipInactiveChannels = true;
void print_help(const string prog_name)
{
- cerr << "\n Usage: \n " << prog_name
+ cerr << "\n Usage: \n " << progname
<< " --help | [delay] [options] [-- external_program [external_program_options]]\n";
cerr << " <delay> => time interval to sample performance counters.\n";
cerr << " If not specified, or 0, with external program given\n";
@@ -107,9 +107,9 @@ void print_help(const string prog_name)
cerr << " --uninstallDriver | --installDriver=> (un)install driver\n";
#endif
cerr << " Examples:\n";
- cerr << " " << prog_name << " 1 => print counters every second without core and socket output\n";
- cerr << " " << prog_name << " 0.5 -csv=test.log => twice a second save counter values to test.log in CSV format\n";
- cerr << " " << prog_name << " /csv 5 2>/dev/null => one sampe every 5 seconds, and discard all diagnostic output\n";
+ cerr << " " << progname << " 1 => print counters every second without core and socket output\n";
+ cerr << " " << progname << " 0.5 -csv=test.log => twice a second save counter values to test.log in CSV format\n";
+ cerr << " " << progname << " /csv 5 2>/dev/null => one sampe every 5 seconds, and discard all diagnostic output\n";
cerr << "\n";
}
diff --git a/src/pcm-power.cpp b/src/pcm-power.cpp
index 72f8d2fd..6fb4df06 100644
--- a/src/pcm-power.cpp
+++ b/src/pcm-power.cpp
@@ -274,7 +274,7 @@ int main(int argc, char * argv[])
std::vector<std::string> skxLicenseStr = { "Core cycles where the core was running with power-delivery for baseline license level 0. This includes non-AVX codes, SSE, AVX 128-bit, and low-current AVX 256-bit codes.",
"Core cycles where the core was running with power-delivery for license level 1. This includes high current AVX 256-bit instructions as well as low current AVX 512-bit instructions.",
"Core cycles where the core was running with power-delivery for license level 2 (introduced in Skylake Server michroarchtecture). This includes high current AVX 512-bit instructions." };
- licenseStr = skxLicenseStr;
+ licenseStr = std::move(skxLicenseStr);
regs[0].fields.event_select = 0x28; // CORE_POWER.LVL0_TURBO_LICENSE
regs[0].fields.umask = 0x07; // CORE_POWER.LVL0_TURBO_LICENSE
regs[1].fields.event_select = 0x28; // CORE_POWER.LVL1_TURBO_LICENSE
diff --git a/src/pcm.cpp b/src/pcm.cpp
index bcbd819a..0b1b44df 100644
--- a/src/pcm.cpp
+++ b/src/pcm.cpp
@@ -81,7 +81,7 @@ double getAverageUncoreFrequencyGhz(const UncoreStateType& before, const UncoreS
void print_help(const string prog_name)
{
- cerr << "\n Usage: \n " << prog_name
+ cerr << "\n Usage: \n " << progname
<< " --help | [delay] [options] [-- external_program [external_program_options]]\n";
cerr << " <delay> => time interval to sample performance counters.\n";
cerr << " If not specified, or 0, with external program given\n";
@@ -103,10 +103,10 @@ void print_help(const string prog_name)
cerr << " -i[=number] | /i[=number] => allow to determine number of iterations\n";
print_help_force_rtm_abort_mode(37);
cerr << " Examples:\n";
- cerr << " " << prog_name << " 1 -nc -ns => print counters every second without core and socket output\n";
- cerr << " " << prog_name << " 1 -i=10 => print counters every second 10 times and exit\n";
- cerr << " " << prog_name << " 0.5 -csv=test.log => twice a second save counter values to test.log in CSV format\n";
- cerr << " " << prog_name << " /csv 5 2>/dev/null => one sampe every 5 seconds, and discard all diagnostic output\n";
+ cerr << " " << progname << " 1 -nc -ns => print counters every second without core and socket output\n";
+ cerr << " " << progname << " 1 -i=10 => print counters every second 10 times and exit\n";
+ cerr << " " << progname << " 0.5 -csv=test.log => twice a second save counter values to test.log in CSV format\n";
+ cerr << " " << progname << " /csv 5 2>/dev/null => one sampe every 5 seconds, and discard all diagnostic output\n";
cerr << "\n";
}
diff --git a/src/topology.h b/src/topology.h
index 336323bc..96819a4c 100644
--- a/src/topology.h
+++ b/src/topology.h
@@ -107,7 +107,7 @@ class HyperThread : public SystemObject
}
void addMSRHandle( std::shared_ptr<SafeMsrHandle> handle ) {
- msrHandle_ = handle;
+ msrHandle_ = std::move(handle);
}
int32 threadID() const {
diff --git a/src/utils.cpp b/src/utils.cpp
index c48272c6..c7aac31d 100644
--- a/src/utils.cpp
+++ b/src/utils.cpp
@@ -596,7 +596,7 @@ std::vector<std::string> split(const std::string & str, const char delim)
uint64 read_number(const char* str)
{
std::istringstream stream(str);
- if (strstr(str, "x")) stream >> std::hex;
+ if (strchr(str, 'x')) stream >> std::hex;
uint64 result = 0;
stream >> result;
return result;
From b0662eff48db66c38dbb77bc96f75cdf886b8756 Mon Sep 17 00:00:00 2001
From: germanaizek <GermanAizek@yandex.ru>
Date: Fri, 3 Jun 2022 18:04:35 +0300
Subject: [PATCH 3/3] Added const reference params
---
src/cpucounters.cpp | 2 +-
src/cpucounters.h | 2 +-
src/pcm-core.cpp | 2 +-
src/pcm-iio.cpp | 28 ++++++++++++++--------------
src/pcm-memory.cpp | 2 +-
src/pcm.cpp | 6 +++---
6 files changed, 21 insertions(+), 21 deletions(-)
diff --git a/src/cpucounters.cpp b/src/cpucounters.cpp
index 81f7fd1b..cb322c53 100644
--- a/src/cpucounters.cpp
+++ b/src/cpucounters.cpp
@@ -4046,7 +4046,7 @@ void PCM::cleanupRDT(const bool silent)
if (!silent) std::cerr << " Freeing up all RMIDs\n";
}
-void PCM::setOutput(const std::string filename, const bool cerrToo)
+void PCM::setOutput(const std::string & filename, const bool cerrToo)
{
outfile = new std::ofstream(filename.c_str());
backup_ofile = std::cout.rdbuf();
diff --git a/src/cpucounters.h b/src/cpucounters.h
index 1ea3694b..94f79da1 100644
--- a/src/cpucounters.h
+++ b/src/cpucounters.h
@@ -675,7 +675,7 @@ class PCM_API PCM
}
//! \brief Redirects output destination to provided file, instead of std::cout and std::cerr (optional)
- static void setOutput(const std::string filename, const bool cerrToo = false);
+ static void setOutput(const std::string & filename, const bool cerrToo = false);
//! \brief Restores output, closes output file if opened
void restoreOutput();
diff --git a/src/pcm-core.cpp b/src/pcm-core.cpp
index a219208d..aa46331b 100644
--- a/src/pcm-core.cpp
+++ b/src/pcm-core.cpp
@@ -120,7 +120,7 @@ extern "C" {
}
}
-void print_usage(const string progname)
+void print_usage(const string & progname)
{
cerr << "\n Usage: \n " << progname
<< " --help | [delay] [options] [-- external_program [external_program_options]]\n";
diff --git a/src/pcm-iio.cpp b/src/pcm-iio.cpp
index 3fff66e3..992f7e64 100644
--- a/src/pcm-iio.cpp
+++ b/src/pcm-iio.cpp
@@ -180,14 +180,14 @@ void print_nameMap() {
}
-string a_title (const string &init, const string &name) {
+string a_title(const string &init, const string &name) {
char begin = init[0];
string row = init;
row += name;
return row + begin;
}
-string a_data (string init, struct data d) {
+string a_data(const string &init, struct data d) {
char begin = init[0];
string row = init;
string str_d = unit_format(d.value);
@@ -198,7 +198,7 @@ string a_data (string init, struct data d) {
return row + begin;
}
-string build_line(string init, string name, bool last_char = true, char this_char = '_')
+string build_line(const string &init, const string &name, bool last_char = true, char this_char = '_')
{
char begin = init[0];
string row = init;
@@ -209,12 +209,12 @@ string build_line(string init, string name, bool last_char = true, char this_cha
}
-string a_header_footer (string init, string name)
+string a_header_footer(const string &init, const string &name)
{
return build_line(init, name);
}
-vector<string> combine_stack_name_and_counter_names(string stack_name)
+vector<string> combine_stack_name_and_counter_names(const string &stack_name)
{
vector<string> v;
@@ -223,7 +223,7 @@ vector<string> combine_stack_name_and_counter_names(string stack_name)
for (std::map<string,std::pair<h_id,std::map<string,v_id>>>::const_iterator iunit = nameMap.begin(); iunit != nameMap.end(); ++iunit) {
string h_name = iunit->first;
int h_id = (iunit->second).first;
- tmp[h_id] = h_name;
+ tmp[h_id] = std::move(h_name);
//cout << "h_id:" << h_id << " name:" << h_name << "\n";
}
//XXX: How to simplify and just combine tmp & v?
@@ -350,7 +350,7 @@ vector<string> build_display(vector<struct iio_stacks_on_socket>& iios, vector<s
std::string build_csv_row(const std::vector<std::string>& chunks, const std::string& delimiter)
{
- return std::accumulate(chunks.begin(), chunks.end(), std::string(""),
+ return std::accumulate(chunks.begin(), chunks.end(), std::string(),
[delimiter](const string &left, const string &right){
return left.empty() ? right : left + delimiter + right;
});
@@ -544,7 +544,7 @@ bool IPlatformMapping10Nm::getSadIdRootBusMap(uint32_t socket_id, std::map<uint8
if ((sad_ctrl_cfg & 0xf) == socket_id) {
uint8_t sid = (sad_ctrl_cfg >> 4) & 0x7;
- sad_id_bus_map.insert(std::pair<uint8_t, uint8_t>(sid, (uint8_t)bus));
+ sad_id_bus_map.emplace(sid, (uint8_t)bus);
}
}
}
@@ -920,10 +920,10 @@ vector<struct counter> load_events(PCM * m, const char* fn)
/* Ignore anyline with # */
//TODO: substring until #, if len == 0, skip, else parse normally
pccr->set_ccr_value(0);
- if (line.find("#") != std::string::npos)
+ if (line.find('#') != std::string::npos)
continue;
/* If line does not have any deliminator, we ignore it as well */
- if (line.find("=") == std::string::npos)
+ if (line.find('=') == std::string::npos)
continue;
std::istringstream iss(line);
string h_name, v_name;
@@ -931,8 +931,8 @@ vector<struct counter> load_events(PCM * m, const char* fn)
std::string key, value;
uint64 numValue;
/* assume the token has the format <key>=<value> */
- key = item.substr(0,item.find("="));
- value = item.substr(item.find("=")+1);
+ key = item.substr(0,item.find('='));
+ value = item.substr(item.find('=')+1);
istringstream iss2(value);
iss2 >> setbase(0) >> numValue;
@@ -1119,11 +1119,11 @@ bool extract_argument_value(const char* arg, std::initializer_list<const char*>
const auto arg_name_len = strlen(arg_name);
if (arg_len > arg_name_len && strncmp(arg, arg_name, arg_name_len) == 0 && arg[arg_name_len] == '=') {
value = arg + arg_name_len + 1;
- const auto last_pos = value.find_last_not_of("\"");
+ const auto last_pos = value.find_last_not_of('\"');
if (last_pos != string::npos) {
value.erase(last_pos + 1);
}
- const auto first_pos = value.find_first_not_of("\"");
+ const auto first_pos = value.find_first_not_of('\"');
if (first_pos != string::npos) {
value.erase(0, first_pos);
}
diff --git a/src/pcm-memory.cpp b/src/pcm-memory.cpp
index e6e9de05..53dc81d1 100644
--- a/src/pcm-memory.cpp
+++ b/src/pcm-memory.cpp
@@ -81,7 +81,7 @@ bool anyPmem(const ServerUncoreMemoryMetrics & metrics)
bool skipInactiveChannels = true;
-void print_help(const string prog_name)
+void print_help(const string & progname)
{
cerr << "\n Usage: \n " << progname
<< " --help | [delay] [options] [-- external_program [external_program_options]]\n";
diff --git a/src/pcm.cpp b/src/pcm.cpp
index 0b1b44df..97bac423 100644
--- a/src/pcm.cpp
+++ b/src/pcm.cpp
@@ -79,7 +79,7 @@ double getAverageUncoreFrequencyGhz(const UncoreStateType& before, const UncoreS
return getAverageUncoreFrequency(before, after) / 1e9;
}
-void print_help(const string prog_name)
+void print_help(const string & progname)
{
cerr << "\n Usage: \n " << progname
<< " --help | [delay] [options] [-- external_program [external_program_options]]\n";
@@ -554,13 +554,13 @@ void print_basic_metrics_csv_header(const PCM * m)
cout << "Frontend_bound(%),Bad_Speculation(%),Backend_Bound(%),Retiring(%),";
}
-void print_csv_header_helper(string header, int count=1){
+void print_csv_header_helper(const string & header, int count=1){
for(int i = 0; i < count; i++){
cout << header << ",";
}
}
-void print_basic_metrics_csv_semicolons(const PCM * m, string header)
+void print_basic_metrics_csv_semicolons(const PCM * m, const string & header)
{
print_csv_header_helper(header, 3); // EXEC;IPC;FREQ;
if (m->isActiveRelativeFrequencyAvailable())