Skip to content

Commit

Permalink
C++ optimizations: using const auto& for iteration where possible, em…
Browse files Browse the repository at this point in the history
…pty() instead of size() == 0
  • Loading branch information
eli-b committed Feb 7, 2020
1 parent ba5ee70 commit d8acb31
Show file tree
Hide file tree
Showing 5 changed files with 31 additions and 30 deletions.
41 changes: 21 additions & 20 deletions ICBSSearch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -449,11 +449,11 @@ void ICBSSearch::collectConstraints(ICBSNode* curr, std::list<pair<int, Constrai
{
while (curr != nullptr)
{
for (auto con : curr->positive_constraints[curr->agent_id])
for (const auto& con : curr->positive_constraints[curr->agent_id])
{
constraints.push_back(make_pair(curr->agent_id, con));
}
for (auto con : curr->negative_constraints[curr->agent_id])
for (const auto& con : curr->negative_constraints[curr->agent_id])
{
constraints.push_back(make_pair(curr->agent_id, con));
}
Expand All @@ -479,8 +479,8 @@ int ICBSSearch::buildConstraintTable(ICBSNode* curr, int agent_id, int newConstr
list < Constraint > constraints_negative;
while (curr != nullptr)
{
for (auto con: curr->negative_constraints[agent_id]) {
auto [loc1, loc2, constraint_timestep, positive_constraint] = con;
for (const auto& con: curr->negative_constraints[agent_id]) {
const auto& [loc1, loc2, constraint_timestep, positive_constraint] = con;
constraints_negative.push_back(con);
if (loc1 == goal.first && loc2 < 0 && lastGoalConsTimestep < constraint_timestep)
lastGoalConsTimestep = constraint_timestep;
Expand All @@ -489,8 +489,8 @@ int ICBSSearch::buildConstraintTable(ICBSNode* curr, int agent_id, int newConstr
for (int j = 0; j < num_of_agents ; ++j) {
if (j == agent_id) // For our agent, those are landmarks
{
for (auto con: curr->positive_constraints[agent_id]) {
auto[loc1, loc2, constraint_timestep, positive_constraint] = con;
for (const auto& con: curr->positive_constraints[j]) {
const auto& [loc1, loc2, constraint_timestep, positive_constraint] = con;

if (loc2 < 0) // vertex constraint
{
Expand Down Expand Up @@ -520,8 +520,8 @@ int ICBSSearch::buildConstraintTable(ICBSNode* curr, int agent_id, int newConstr
}
}
else { // for the other agents, it is equivalent to a negative constraint
for (auto con: curr->positive_constraints[agent_id]) {
auto [loc1, loc2, constraint_timestep, positive_constraint] = con;
for (const auto& con: curr->positive_constraints[j]) {
const auto& [loc1, loc2, constraint_timestep, positive_constraint] = con;
if (loc1 == goal.first && loc2 < 0 && lastGoalConsTimestep < constraint_timestep) {
lastGoalConsTimestep = constraint_timestep;
}
Expand Down Expand Up @@ -717,7 +717,7 @@ void ICBSSearch::copyConflicts(const vector<bool>& unchanged,
{
for (const auto& conflict : from)
{
auto [agent1, agent2, loc1, loc2, timestep] = *conflict;
const auto& [agent1, agent2, loc1, loc2, timestep] = *conflict;
if (unchanged[agent1] && unchanged[agent2])
to.push_back(conflict);
}
Expand All @@ -729,7 +729,7 @@ void ICBSSearch::clearConflictsOfAffectedAgents(bool *unchanged,
{
auto it = lst.begin();
while (it != lst.end()) {
auto [agent1, agent2, loc1, loc2, timestep] = **it;
const auto& [agent1, agent2, loc1, loc2, timestep] = **it;
if (!unchanged[agent1] || !unchanged[agent2])
it = lst.erase(it);
else
Expand Down Expand Up @@ -805,13 +805,13 @@ void ICBSSearch::findConflicts(ICBSNode &curr, int a1, int a2)
int loc2 = a2_path[timestep].location;
if (loc1 == loc2)
{
curr.unknownConf.push_back(std::shared_ptr<Conflict>(new Conflict(a1, a2, loc1, -1, timestep)));
curr.unknownConf.push_back(std::make_shared<Conflict>(a1, a2, loc1, -1, timestep));
}
else if (timestep < min_path_length - 1
&& loc1 == a2_path[timestep + 1].location
&& loc2 == a1_path[timestep + 1].location)
{
curr.unknownConf.push_back(std::shared_ptr<Conflict>(new Conflict(a1, a2, loc1, loc2, timestep + 1))); // edge conflict
curr.unknownConf.push_back(std::make_shared<Conflict>(a1, a2, loc1, loc2, timestep + 1)); // edge conflict
}
}
if (a1_path.size() != a2_path.size()) // check whether there are conflicts that occur after one agent reaches its goal
Expand Down Expand Up @@ -1092,7 +1092,8 @@ std::shared_ptr<Conflict> ICBSSearch::getHighestPriorityConflict(ICBSNode &node,

for (const auto& conf : confs)
{
auto [agent1, agent2, loc1, loc2, timestep] = *conf;
const auto& [agent1, agent2, loc1, loc2, timestep] = *conf;

vector<PathEntry> & a1_path = *(*node.all_paths)[agent1];
vector<PathEntry> & a2_path = *(*node.all_paths)[agent2];

Expand Down Expand Up @@ -1830,7 +1831,7 @@ std::tuple<ICBSNode *, bool> ICBSSearch::generateChild(ICBSNode *child, Conflict
for (const auto& con : child->negative_constraints[child->agent_id]) // TODO: When rectangle conflict resolution is added, more than one negative constraint
// might be added at once. Replan once after all negative constraints are accounted for!
{
auto [loc1, loc2, timestep, positive_constraint] = con;
const auto& [loc1, loc2, timestep, positive_constraint] = con;
int a[2];
if (split == split_strategy::DISJOINT3)
{
Expand Down Expand Up @@ -1895,7 +1896,7 @@ std::tuple<ICBSNode *, bool> ICBSSearch::generateChild(ICBSNode *child, Conflict
for (const auto& con : child->positive_constraints[child->agent_id]) // TODO: Replan once after all positive constraints are applied instead of this way.
// This way is OK for now since only a single positive constraint is added at a time.
{
auto [loc1, loc2, timestep, positive_constraint] = con;
const auto& [loc1, loc2, timestep, positive_constraint] = con;
if (loc2 < 0 && // vertex constraint
getAgentLocation(parent_paths, timestep, ag) == loc1) // The agent is in violation of the positive constraint
{
Expand Down Expand Up @@ -2678,11 +2679,11 @@ void ICBSSearch::printConstraints(const ICBSNode* n) const
while (curr != nullptr)
{
for (int j = 0; j < num_of_agents; ++j) {
for (auto con: curr->positive_constraints[j])
for (const auto& con: curr->positive_constraints[j])
{
std::cout << curr->agent_id << ": " << con << std::endl;
}
for (auto con: curr->negative_constraints[j])
for (const auto& con: curr->negative_constraints[j])
{
std::cout << curr->agent_id << ": " << con << std::endl;
}
Expand Down Expand Up @@ -3079,7 +3080,7 @@ bool ICBSSearch::runICBSSearch()
// ICBSNode* n1 = new ICBSNode(curr);
// ICBSNode* n2 = new ICBSNode(curr);
// ICBSNode* n3 = new ICBSNode(curr);
// auto [agent1_id, agent2_id, location1, location2, timestep] = *curr->conflict;
// const auto& [agent1_id, agent2_id, location1, location2, timestep] = *curr->conflict;
// n1->agent_id = agent1_id;
// n2->agent_id = agent2_id;
// n3->agent_id = (1 + agent1_id) * num_of_agents + agent2_id;
Expand Down Expand Up @@ -3483,7 +3484,7 @@ std::tuple<bool, int> ICBSSearch::do_idcbsh_iteration(ICBSNode *curr,
}
}

if (curr->conflict == NULL) // Failed to find a conflict => no conflicts => found a solution
if (curr->conflict == nullptr) // Failed to find a conflict => no conflicts => found a solution
{
solution_found = true;
solution_cost = curr->g_val;
Expand All @@ -3506,7 +3507,7 @@ std::tuple<bool, int> ICBSSearch::do_idcbsh_iteration(ICBSNode *curr,
std::cout << "Chosen conflict: " << *curr->conflict << std::endl;
}

auto [agent1_id, agent2_id, location1, location2, timestep] = *curr->conflict;
const auto& [agent1_id, agent2_id, location1, location2, timestep] = *curr->conflict;

std::shared_ptr<Conflict> orig_conflict = curr->conflict; // TODO: Consider not storing the chosen conflict as a state of the node
int orig_agent_id = curr->agent_id;
Expand Down
12 changes: 6 additions & 6 deletions ICBSSingleAgentLLSearch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ bool ICBSSingleAgentLLSearch::findPath(vector<PathEntry> &path,
cat.num_conflicts_for_step(curr->loc, next_id, next_timestep);

// generate (maybe temporary) node
ICBSSingleAgentLLNode* next = new ICBSSingleAgentLLNode(next_id, next_g_val, next_h_val, curr,
auto next = new ICBSSingleAgentLLNode(next_id, next_g_val, next_h_val, curr,
next_timestep, next_internal_conflicts, false);
it = allNodes_table.find(next);// try to retrieve it from the hash table
if (it == allNodes_table.end())
Expand Down Expand Up @@ -203,7 +203,7 @@ bool ICBSSingleAgentLLSearch::findShortestPath(vector<PathEntry> &path,
hashtable_t::iterator it; // will be used for find()

// generate start and add it to the OPEN list
ICBSSingleAgentLLNode* root = new ICBSSingleAgentLLNode(start.first, 0, my_heuristic[start.first], NULL, start.second, 0, false);
auto root = new ICBSSingleAgentLLNode(start.first, 0, my_heuristic[start.first], nullptr, start.second, 0, false);
num_generated++;
root->open_handle = open_list.push(root);
root->focal_handle = focal_list.push(root);
Expand Down Expand Up @@ -281,7 +281,7 @@ bool ICBSSingleAgentLLSearch::findShortestPath(vector<PathEntry> &path,
} // end for loop that generates successors


if (open_list.size() == 0) // in case OPEN is empty, no path found...
if (open_list.empty()) // in case OPEN is empty, no path found...
break;
// update FOCAL if min f-val increased
ICBSSingleAgentLLNode* open_head = open_list.top();
Expand Down Expand Up @@ -311,10 +311,10 @@ bool ICBSSingleAgentLLSearch::findShortestPath(vector<PathEntry> &path,

inline void ICBSSingleAgentLLSearch::releaseClosedListNodes(hashtable_t& allNodes_table)
{
hashtable_t::iterator it;
for (auto it: allNodes_table)
for (const auto& pair: allNodes_table)
{
delete it.second;
const auto& [node_k, node] = pair;
delete node;
}
}

Expand Down
2 changes: 1 addition & 1 deletion MDD.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ bool MDD::buildMDD(const std::vector < std::unordered_map<int, ConstraintState >

bool MDD::updateMDD(const tuple<int, int, int> &constraint, int num_col)
{
auto [loc1, loc2, t] = constraint;
const auto& [loc1, loc2, t] = constraint;

if (loc2 < 0) // edge constraint - TODO: explain this hack. Looks like when loc2<0, loc1 and (-loc2-1) are indices
// in cell enumeration, and otherwise loc1 and loc2 are row and column values.
Expand Down
2 changes: 1 addition & 1 deletion XytHolder.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ class XytHolder {
}
for (int i = 0; i < other.xy_size ; ++i) {
if (other.data[i] != nullptr) {
for (auto pair: *other.data[i]) {
for (const auto& pair: *other.data[i]) {
set(i, pair[0], pair[1]);
}
}
Expand Down
4 changes: 2 additions & 2 deletions dynamic_constraints_manager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ void DynamicConstraintsManager::addDynConstraint(int from_id, int to_id, int ts)

void DynamicConstraintsManager::popDynConstraint(int from_id, int to_id, int ts) {
// Remove it from the list of dynamic constraints.
auto [back_from_id, back_to_id] = dyn_constraints_[ts].back();
const auto [back_from_id, back_to_id] = dyn_constraints_[ts].back(); // Not auto& because the constraint is about to be removed
VLOG_IF(1, back_from_id != from_id) << "ERROR: We assume constraints are popped in the same order as they're added, but in reverse";
assert(back_from_id == from_id);
VLOG_IF(1, back_to_id != to_id) << "ERROR: We assume constraints are popped in the same order as they're added, but in reverse";
Expand All @@ -89,7 +89,7 @@ bool DynamicConstraintsManager::isDynCons(int curr_id, int next_id, int next_ts)
VLOG(11) << "\t\t\tisDynConstrained: <from=" << curr_id << ", to=" << next_id << ", t=" << next_ts << ">";
// Check edge constraints (move from curr_id to next_id (getting to next_id at next_ts) is disallowed).
if ( next_ts > 0 && next_ts < static_cast<int>(dyn_constraints_.size()) ) {
for (auto c : dyn_constraints_[next_ts]) {
for (const auto& c : dyn_constraints_[next_ts]) {
if ( c.first == curr_id && c.second == next_id ) {
VLOG(11) << "\t\t\t\tYES DYN_CONS!";
return true;
Expand Down

0 comments on commit d8acb31

Please sign in to comment.