Skip to content

Commit

Permalink
resource: avoid copy constructor in for loops
Browse files Browse the repository at this point in the history
Problem: many range-based loops in Fluxion result in calling an object
copy constructor.

Avoid the copy constructor cost and add const type qualifier where
appropriate.
  • Loading branch information
milroy committed Jul 9, 2024
1 parent 51adff3 commit 86dc3b7
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 29 deletions.
12 changes: 6 additions & 6 deletions resource/planner/c++/planner_multi.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ planner_multi::planner_multi (int64_t base_time, uint64_t duration,

planner_multi::planner_multi (const planner_multi &o)
{
for (auto iter : o.m_types_totals_planners) {
for (auto &iter : o.m_types_totals_planners) {
planner_t *np = nullptr;
if (iter.planner) {
try {
Expand Down Expand Up @@ -92,7 +92,7 @@ planner_multi &planner_multi::operator= (const planner_multi &o)
// Erase *this so the vectors are empty
erase ();

for (auto iter : o.m_types_totals_planners) {
for (const auto &iter : o.m_types_totals_planners) {
planner_t *np = nullptr;
if (iter.planner) {
try {
Expand Down Expand Up @@ -141,10 +141,10 @@ bool planner_multi::operator== (const planner_multi &o) const

if (m_types_totals_planners.size () != o.m_types_totals_planners.size ())
return false;
auto &o_by_type = o.m_types_totals_planners.get<res_type> ();
auto &by_type = m_types_totals_planners.get<res_type> ();
for (auto data : by_type) {
auto o_data = o_by_type.find (data.resource_type);
const auto &o_by_type = o.m_types_totals_planners.get<res_type> ();
const auto &by_type = m_types_totals_planners.get<res_type> ();
for (const auto &data : by_type) {
const auto o_data = o_by_type.find (data.resource_type);
if (o_data == o_by_type.end ())
return false;
if (data.resource_type != o_data->resource_type)
Expand Down
45 changes: 24 additions & 21 deletions resource/readers/resource_reader_jgf.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -197,19 +197,19 @@ std::string diff (const resource_pool_t &r, const fetch_helper_t &f)
sstream << " name=(" << r.name << ", " << f.name << ")";
if (r.properties != f.properties) {
sstream << " properties=(";
for (auto &kv : r.properties)
for (const auto &kv : r.properties)
sstream << kv.first << "=" << kv.second << " ";
sstream << ", ";
for (auto &kv : f.properties)
for (const auto &kv : f.properties)
sstream << kv.first << "=" << kv.second << " ";
sstream << ")";
}
if (r.paths != f.paths) {
sstream << " paths=(";
for (auto &kv : r.paths)
for (const auto &kv : r.paths)
sstream << kv.first << "=" << kv.second << " ";
sstream << ", ";
for (auto &kv : f.paths)
for (const auto &kv : f.paths)
sstream << kv.first << "=" << kv.second << " ";
sstream << ")";
}
Expand Down Expand Up @@ -455,7 +455,7 @@ vtx_t resource_reader_jgf_t::create_vtx (resource_graph_t &g,
g[v].paths = fetcher.paths;
g[v].schedule.plans = plans;
g[v].idata.x_checker = x_checker;
for (auto kv : g[v].paths)
for (const auto &kv : g[v].paths)
g[v].idata.member_of[kv.first] = "*";

done:
Expand All @@ -468,10 +468,10 @@ vtx_t resource_reader_jgf_t::vtx_in_graph (const resource_graph_t &g,
std::string> &paths,
int rank)
{
for (auto const &paths_it : paths) {
for (const auto &paths_it : paths) {
auto iter = m.by_path.find (paths_it.second);
if (iter != m.by_path.end ()) {
for (auto &v : iter->second) {
for (const auto &v : iter->second) {
if (g[v].rank == rank) {
return v;
}
Expand All @@ -492,7 +492,7 @@ int resource_reader_jgf_t::check_root (vtx_t v, resource_graph_t &g,
{
int rc = -1;
std::pair<std::map<std::string, bool>::iterator, bool> ptr;
for (auto kv : g[v].paths) {
for (const auto &kv : g[v].paths) {
if (is_root (kv.second)) {
ptr = is_roots.emplace (kv.first, true);
if (!ptr.second)
Expand All @@ -512,7 +512,7 @@ int resource_reader_jgf_t::add_graph_metadata (vtx_t v,
int rc = -1;
std::pair<std::map<std::string, vtx_t>::iterator, bool> ptr;

for (auto kv : g[v].paths) {
for (const auto &kv : g[v].paths) {
if (is_root (kv.second)) {
ptr = m.roots.emplace (kv.first, v);
if (!ptr.second) {
Expand Down Expand Up @@ -540,27 +540,30 @@ int resource_reader_jgf_t::remove_graph_metadata (vtx_t v,
resource_graph_metadata_t &m)
{
int rc = -1;
for (auto kv : g[v].paths) {
for (auto &kv : g[v].paths) {
m.by_path.erase (kv.second);
}

m.by_outedges.erase (v);

for (auto it = m.by_type[g[v].type].begin (); it != m.by_type[g[v].type].end (); ++it) {
for (auto it = m.by_type[g[v].type].begin ();
it != m.by_type[g[v].type].end (); ++it) {
if (*it == v) {
m.by_type[g[v].type].erase (it);
break;
}
}

for (auto it = m.by_name[g[v].name].begin (); it != m.by_name[g[v].name].end (); ++it) {
for (auto it = m.by_name[g[v].name].begin ();
it != m.by_name[g[v].name].end (); ++it) {
if (*it == v) {
m.by_name[g[v].name].erase (it);
break;
}
}

for (auto it = m.by_rank[g[v].rank].begin (); it != m.by_rank[g[v].rank].end (); ++it) {
for (auto it = m.by_rank[g[v].rank].begin ();
it != m.by_rank[g[v].rank].end (); ++it) {
if (*it == v) {
m.by_rank[g[v].rank].erase (it);
break;
Expand Down Expand Up @@ -657,7 +660,7 @@ int resource_reader_jgf_t::exist (resource_graph_t &g,
{
try {
auto &vect = m.by_path.at (path);
for (auto &u : vect) {
for (const auto &u : vect) {
if (g[u].rank == rank) {
v = u;
return 0;
Expand Down Expand Up @@ -694,7 +697,7 @@ int resource_reader_jgf_t::find_vtx (resource_graph_t &g,
goto done;
}

for (auto &kv : fetcher.paths) {
for (const auto &kv : fetcher.paths) {
if (exist (g, m, kv.second, fetcher.rank, fetcher.vertex_id, u) < 0)
goto done;
if (v == nullvtx) {
Expand Down Expand Up @@ -884,7 +887,7 @@ int resource_reader_jgf_t::undo_vertices (resource_graph_t &g,
planner_t *plans = NULL;
vtx_t v = boost::graph_traits<resource_graph_t>::null_vertex ();

for (auto &kv : vmap) {
for (const auto &kv : vmap) {
if (kv.second.exclusive != 1)
continue;
try {
Expand Down Expand Up @@ -1118,7 +1121,7 @@ int resource_reader_jgf_t::update_src_edge (resource_graph_t &g,
if (vmap[source].is_roots.empty ())
return 0;

for (auto &kv : vmap[source].is_roots)
for (const auto &kv : vmap[source].is_roots)
m.v_rt_edges[kv.first].set_for_trav_update (vmap[source].needs,
vmap[source].exclusive,
token);
Expand Down Expand Up @@ -1204,7 +1207,7 @@ int resource_reader_jgf_t::get_subgraph_vertices (resource_graph_t &g,
for (; ei != ei_end; ++ei) {
next_vtx = boost::target (*ei, g);

for (auto const &paths_it : g[next_vtx].paths) {
for (const auto &paths_it : g[next_vtx].paths) {
// check that we don't recurse on parent edges
if (paths_it.second.find (g[vtx].name) != std::string::npos &&
paths_it.second.find (g[vtx].name) < paths_it.second.find (g[next_vtx].name)) {
Expand All @@ -1230,7 +1233,7 @@ int resource_reader_jgf_t::get_parent_vtx (resource_graph_t &g,

for (; ei != ei_end; ++ei) {
next_vtx = boost::target (*ei, g);
for (auto const &paths_it : g[vtx].paths) {
for (const auto &paths_it : g[vtx].paths) {
// check that the parent's name exists in the child's path before the child's name
if (paths_it.second.find (g[next_vtx].name) != std::string::npos &&
paths_it.second.find (g[vtx].name) > paths_it.second.find (g[next_vtx].name)) {
Expand Down Expand Up @@ -1356,7 +1359,7 @@ int resource_reader_jgf_t::remove_subgraph (resource_graph_t &g,
return -1;
}

for (auto &v : iter->second) {
for (const auto &v : iter->second) {
subgraph_root_vtx = v;
}

Expand All @@ -1370,7 +1373,7 @@ int resource_reader_jgf_t::remove_subgraph (resource_graph_t &g,
if (remove_metadata_outedges (parent_vtx, subgraph_root_vtx, g, m) != 0)
return -1;

for (auto & vtx : vtx_list) {
for (auto &vtx : vtx_list) {
// clear vertex edges but don't delete vertex
boost::clear_vertex (vtx, g);
remove_graph_metadata (vtx, g, m);
Expand Down
4 changes: 2 additions & 2 deletions resource/readers/resource_reader_rv1exec.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ vtx_t resource_reader_rv1exec_t::find_vertex (resource_graph_t &g,
if (vtx_iter == m.by_path.end ())
return null_vtx;
// Found in by_path
for (vtx_t v : vtx_iter->second) {
for (const vtx_t &v : vtx_iter->second) {
if (g[v].rank == rank
&& g[v].id == id
&& g[v].size == size
Expand Down Expand Up @@ -334,7 +334,7 @@ int resource_reader_rv1exec_t::undo_vertices (resource_graph_t &g,
planner_t *plans = NULL;

for (auto &[rank, vertices] : update_data.updated_vertices) {
for (vtx_t vtx : vertices) {
for (const vtx_t &vtx : vertices) {
// Check plan
if ( (plans = g[vtx].schedule.plans) == NULL) {
errno = EINVAL;
Expand Down

0 comments on commit 86dc3b7

Please sign in to comment.