Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Code refactor #408

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Code refactor #408

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
651a80d
Using '= default', remove unused vars, code simplify
GermanAizek Jun 3, 2022
f7de464
Vars renamed like others, using std::move() and str to char optimize
GermanAizek Jun 3, 2022
b0662ef
Added const reference params
GermanAizek Jun 3, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/cpucounters.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
2 changes: 1 addition & 1 deletion src/cpucounters.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
2 changes: 1 addition & 1 deletion src/pcm-core.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down
28 changes: 14 additions & 14 deletions src/pcm-iio.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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;
Expand All @@ -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;
Expand All @@ -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?
Expand Down Expand Up @@ -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;
});
Expand Down Expand Up @@ -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);
}
}
}
Expand Down Expand Up @@ -920,19 +920,19 @@ 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;
while (std::getline(iss, item, ',')) {
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;

Expand Down Expand Up @@ -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);
}
Expand Down
2 changes: 1 addition & 1 deletion src/pcm-memory.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor

@ogbrugge-work ogbrugge-work Jun 3, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you merge this change into the previous change? I would like each change to compile on its own and it is clear that this cannot compile on its own and needs both changes

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the change of the parameter name is not necessary and requires changing the name in other code lines. Could you please keep the old name?

<< " --help | [delay] [options] [-- external_program [external_program_options]]\n";
Expand Down
6 changes: 3 additions & 3 deletions src/pcm.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor

@ogbrugge-work ogbrugge-work Jun 3, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you merge this change into the previous change? I would like each change to compile on its own and it is clear that this cannot compile on its own and needs both changes

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here. Please keep the old name.

<< " --help | [delay] [options] [-- external_program [external_program_options]]\n";
Expand Down Expand Up @@ -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())
Expand Down