From aa060298012cfe97e4f4a833a2ad84137c36a86b Mon Sep 17 00:00:00 2001 From: xlaussel Date: Wed, 25 Nov 2020 11:22:30 +0100 Subject: [PATCH] References removed for extracted heapNode: could lead to bugs because the same was sometimes modified after when relaxing outgoing edges --- .../routing_algorithms/routing_base_ch.hpp | 6 +- .../routing_algorithms/routing_base_mld.hpp | 8 +- src/contractor/contractor_search.cpp | 2 +- .../alternative_path_ch.cpp | 14 ++-- .../routing_algorithms/many_to_many_ch.cpp | 21 ++--- .../routing_algorithms/many_to_many_mld.cpp | 76 +++++++++---------- 6 files changed, 57 insertions(+), 70 deletions(-) diff --git a/include/engine/routing_algorithms/routing_base_ch.hpp b/include/engine/routing_algorithms/routing_base_ch.hpp index 4ec2cbeb9..b9f131671 100644 --- a/include/engine/routing_algorithms/routing_base_ch.hpp +++ b/include/engine/routing_algorithms/routing_base_ch.hpp @@ -35,7 +35,7 @@ bool stallAtNode(const DataFacade &facade, const NodeID to = facade.GetTarget(edge); const EdgeWeight edge_weight = data.weight; BOOST_ASSERT_MSG(edge_weight > 0, "edge_weight invalid"); - const auto& toHeapNode= query_heap.GetHeapNodeIfWasInserted(to); + const auto toHeapNode= query_heap.GetHeapNodeIfWasInserted(to); if (toHeapNode) { if (toHeapNode->weight + edge_weight < heapNode.weight) @@ -64,7 +64,7 @@ void relaxOutgoingEdges(const DataFacade &facade, BOOST_ASSERT_MSG(edge_weight > 0, "edge_weight invalid"); const EdgeWeight to_weight = heapNode.weight + edge_weight; - auto toHeapNode = heap.GetHeapNodeIfWasInserted(to); + const auto toHeapNode = heap.GetHeapNodeIfWasInserted(to); // New Node discovered -> Add to Heap + Node Info Storage if (!toHeapNode) { @@ -124,7 +124,7 @@ void routingStep(const DataFacade &facade, const bool force_loop_reverse) { auto heapNode = forward_heap.DeleteMinGetHeapNode(); - auto reverseHeapNode = reverse_heap.GetHeapNodeIfWasInserted(heapNode.node); + const auto reverseHeapNode = reverse_heap.GetHeapNodeIfWasInserted(heapNode.node); if (reverseHeapNode) { diff --git a/include/engine/routing_algorithms/routing_base_mld.hpp b/include/engine/routing_algorithms/routing_base_mld.hpp index b06b8976b..d4d8f3d0c 100644 --- a/include/engine/routing_algorithms/routing_base_mld.hpp +++ b/include/engine/routing_algorithms/routing_base_mld.hpp @@ -254,7 +254,7 @@ void relaxOutgoingEdges(const DataFacade &facade, { const EdgeWeight to_weight = heapNode.weight + shortcut_weight; BOOST_ASSERT(to_weight >= heapNode.weight); - auto toHeapNode = forward_heap.GetHeapNodeIfWasInserted(to); + const auto toHeapNode = forward_heap.GetHeapNodeIfWasInserted(to); if (!toHeapNode) { forward_heap.Insert(to, to_weight, {heapNode.node, true}); @@ -283,7 +283,7 @@ void relaxOutgoingEdges(const DataFacade &facade, { const EdgeWeight to_weight = heapNode.weight + shortcut_weight; BOOST_ASSERT(to_weight >= heapNode.weight); - auto toHeapNode = forward_heap.GetHeapNodeIfWasInserted(to); + const auto toHeapNode = forward_heap.GetHeapNodeIfWasInserted(to); if (!toHeapNode) { forward_heap.Insert(to, to_weight, {heapNode.node, true}); @@ -321,7 +321,7 @@ void relaxOutgoingEdges(const DataFacade &facade, const EdgeWeight to_weight = heapNode.weight + node_weight + turn_penalty; - auto toHeapNode = forward_heap.GetHeapNodeIfWasInserted(to); + const auto toHeapNode = forward_heap.GetHeapNodeIfWasInserted(to); if (!toHeapNode) { forward_heap.Insert(to, to_weight, {heapNode.node, false}); @@ -357,7 +357,7 @@ void routingStep(const DataFacade &facade, // is weight + reverse_weight // More tighter upper bound requires additional condition reverse_heap.WasRemoved(to) // with weight(to -> target) = reverse_weight and all weights ≥ 0 - auto reverseHeapNode = reverse_heap.GetHeapNodeIfWasInserted(heapNode.node); + const auto reverseHeapNode = reverse_heap.GetHeapNodeIfWasInserted(heapNode.node); if (reverseHeapNode) { auto reverse_weight = reverseHeapNode->weight; diff --git a/src/contractor/contractor_search.cpp b/src/contractor/contractor_search.cpp index 2eaacd848..a5441f806 100644 --- a/src/contractor/contractor_search.cpp +++ b/src/contractor/contractor_search.cpp @@ -32,7 +32,7 @@ void relaxNode(ContractorHeap &heap, } const EdgeWeight to_weight = node_weight + data.weight; - const auto& toHeapNode= heap.GetHeapNodeIfWasInserted(to); + const auto toHeapNode= heap.GetHeapNodeIfWasInserted(to); // New Node discovered -> Add to Heap + Node Info Storage if (!toHeapNode) { diff --git a/src/engine/routing_algorithms/alternative_path_ch.cpp b/src/engine/routing_algorithms/alternative_path_ch.cpp index 2f7d8b99c..f71d68e04 100644 --- a/src/engine/routing_algorithms/alternative_path_ch.cpp +++ b/src/engine/routing_algorithms/alternative_path_ch.cpp @@ -62,11 +62,11 @@ void alternativeRoutingStep(const DataFacade &facade, QueryHeap &forward_heap = DIRECTION == FORWARD_DIRECTION ? heap1 : heap2; QueryHeap &reverse_heap = DIRECTION == FORWARD_DIRECTION ? heap2 : heap1; - auto & heapNode=forward_heap.DeleteMinGetHeapNode(); - const EdgeWeight weight = heapNode.weight; + // Take a copy (no ref &) of the extracted node because otherwise could be modified later if toHeapNode is the same + const auto heapNode=forward_heap.DeleteMinGetHeapNode(); const auto scaled_weight = - static_cast((weight + min_edge_offset) / (1. + VIAPATH_EPSILON)); + static_cast((heapNode.weight + min_edge_offset) / (1. + VIAPATH_EPSILON)); if ((INVALID_EDGE_WEIGHT != *upper_bound_to_shortest_path_weight) && (scaled_weight > *upper_bound_to_shortest_path_weight)) { @@ -76,11 +76,11 @@ void alternativeRoutingStep(const DataFacade &facade, search_space.emplace_back(heapNode.data.parent, heapNode.node); - const auto& reverseHeapNode= reverse_heap.GetHeapNodeIfWasInserted(heapNode.node); + const auto reverseHeapNode= reverse_heap.GetHeapNodeIfWasInserted(heapNode.node); if (reverseHeapNode) { search_space_intersection.emplace_back(heapNode.node); - const EdgeWeight new_weight = reverseHeapNode->weight + weight; + const EdgeWeight new_weight = reverseHeapNode->weight + heapNode.weight; if (new_weight < *upper_bound_to_shortest_path_weight) { if (new_weight >= 0) @@ -112,9 +112,9 @@ void alternativeRoutingStep(const DataFacade &facade, const EdgeWeight edge_weight = data.weight; BOOST_ASSERT(edge_weight > 0); - const EdgeWeight to_weight = weight + edge_weight; + const EdgeWeight to_weight = heapNode.weight + edge_weight; - const auto& toHeapNode= forward_heap.GetHeapNodeIfWasInserted(to); + const auto toHeapNode= forward_heap.GetHeapNodeIfWasInserted(to); // New Node discovered -> Add to Heap + Node Info Storage if (!toHeapNode) { diff --git a/src/engine/routing_algorithms/many_to_many_ch.cpp b/src/engine/routing_algorithms/many_to_many_ch.cpp index 9c3d5bbb2..fba34981c 100644 --- a/src/engine/routing_algorithms/many_to_many_ch.cpp +++ b/src/engine/routing_algorithms/many_to_many_ch.cpp @@ -71,7 +71,7 @@ void relaxOutgoingEdges(const DataFacade &facade, const auto to_duration = heapNode.data.duration + edge_duration; const auto to_distance = heapNode.data.distance + edge_distance; - const auto& toHeapNode= query_heap.GetHeapNodeIfWasInserted(to); + const auto toHeapNode= query_heap.GetHeapNodeIfWasInserted(to); // New Node discovered -> Add to Heap + Node Info Storage if (!toHeapNode) { @@ -100,10 +100,8 @@ void forwardRoutingStep(const DataFacade &facade, std::vector &middle_nodes_table, const PhantomNode &phantom_node) { - const auto& heapNode=query_heap.DeleteMinGetHeapNode(); - const auto source_weight = heapNode.weight; - const auto source_duration = heapNode.data.duration; - const auto source_distance = heapNode.data.distance; + // Take a copy of the extracted node because otherwise could be modified later if toHeapNode is the same + const auto heapNode=query_heap.DeleteMinGetHeapNode(); // Check if each encountered node has an entry const auto &bucket_list = std::equal_range(search_space_with_buckets.begin(), @@ -128,9 +126,9 @@ void forwardRoutingStep(const DataFacade &facade, : distances_table[row_index * number_of_targets + column_index]; // Check if new weight is better - auto new_weight = source_weight + target_weight; - auto new_duration = source_duration + target_duration; - auto new_distance = source_distance + target_distance; + auto new_weight = heapNode.weight + target_weight; + auto new_duration = heapNode.data.duration + target_duration; + auto new_distance = heapNode.data.distance + target_distance; if (new_weight < 0) { @@ -161,15 +159,12 @@ void backwardRoutingStep(const DataFacade &facade, std::vector &search_space_with_buckets, const PhantomNode &phantom_node) { + // Take a copy (no ref &) of the extracted node because otherwise could be modified later if toHeapNode is the same const auto heapNode=query_heap.DeleteMinGetHeapNode(); - const auto target_weight = heapNode.weight; - const auto target_duration = heapNode.data.duration; - const auto target_distance = heapNode.data.distance; - const auto parent = heapNode.data.parent; // Store settled nodes in search space bucket search_space_with_buckets.emplace_back( - heapNode.node, parent, column_index, target_weight, target_duration, target_distance); + heapNode.node, heapNode.data.parent, column_index, heapNode.weight, heapNode.data.duration, heapNode.data.distance); relaxOutgoingEdges( facade, heapNode, query_heap, phantom_node); diff --git a/src/engine/routing_algorithms/many_to_many_mld.cpp b/src/engine/routing_algorithms/many_to_many_mld.cpp index e1d5d609f..292bc3d4b 100644 --- a/src/engine/routing_algorithms/many_to_many_mld.cpp +++ b/src/engine/routing_algorithms/many_to_many_mld.cpp @@ -71,7 +71,7 @@ void relaxBorderEdges(const DataFacade &facade, const auto to_distance = distance + node_distance; // New Node discovered -> Add to Heap + Node Info Storage - const auto& toHeapNode= query_heap.GetHeapNodeIfWasInserted(to); + const auto toHeapNode= query_heap.GetHeapNodeIfWasInserted(to); if (!toHeapNode) { query_heap.Insert(to, to_weight, {node, false, to_duration, to_distance}); @@ -93,11 +93,11 @@ void relaxBorderEdges(const DataFacade &facade, template void relaxOutgoingEdges(const DataFacade &facade, - const SearchEngineData::ManyToManyQueryHeap::HeapNode& heapNode, + const typename SearchEngineData::ManyToManyQueryHeap::HeapNode& heapNode, typename SearchEngineData::ManyToManyQueryHeap &query_heap, Args... args) { - BOOST_ASSERT(!facade.ExcludeNode(heapNode.node)); + BOOST_ASSERT(!facade.ExcludeNode(node)); const auto &partition = facade.GetMultiLevelPartition(); @@ -109,40 +109,40 @@ void relaxOutgoingEdges(const DataFacade &facade, const auto &cells = facade.GetCellStorage(); const auto &metric = facade.GetCellMetric(); - const auto node=heapNode.node; + if (level >= 1 && !heapNode.data.from_clique_arc) { - const auto &cell = cells.GetCell(metric, level, partition.GetCell(level, node)); + const auto &cell = cells.GetCell(metric, level, partition.GetCell(level, heapNode.node)); if (DIRECTION == FORWARD_DIRECTION) { // Shortcuts in forward direction auto destination = cell.GetDestinationNodes().begin(); - auto shortcut_durations = cell.GetOutDuration(node); - auto shortcut_distances = cell.GetOutDistance(node); - for (auto shortcut_weight : cell.GetOutWeight(node)) + auto shortcut_durations = cell.GetOutDuration(heapNode.node); + auto shortcut_distances = cell.GetOutDistance(heapNode.node); + for (auto shortcut_weight : cell.GetOutWeight(heapNode.node)) { BOOST_ASSERT(destination != cell.GetDestinationNodes().end()); BOOST_ASSERT(!shortcut_durations.empty()); BOOST_ASSERT(!shortcut_distances.empty()); const NodeID to = *destination; - if (shortcut_weight != INVALID_EDGE_WEIGHT && node != to) + if (shortcut_weight != INVALID_EDGE_WEIGHT && heapNode.node != to) { const auto to_weight = heapNode.weight + shortcut_weight; const auto to_duration = heapNode.data.duration + shortcut_durations.front(); const auto to_distance = heapNode.data.distance + shortcut_distances.front(); - const auto& toHeapNode= query_heap.GetHeapNodeIfWasInserted(to); + const auto toHeapNode= query_heap.GetHeapNodeIfWasInserted(to); if (!toHeapNode) { - query_heap.Insert(to, to_weight, {node, true, to_duration, to_distance}); + query_heap.Insert(to, to_weight, {heapNode.node, true, to_duration, to_distance}); } - else if (std::tie(to_weight, to_duration, to_distance, node) < + else if (std::tie(to_weight, to_duration, to_distance, heapNode.node) < std::tie(toHeapNode->weight, toHeapNode->data.duration, toHeapNode->data.distance, toHeapNode->data.parent)) { - toHeapNode->data = {node, true, to_duration, to_distance}; + toHeapNode->data = {heapNode.node, true, to_duration, to_distance}; toHeapNode->weight=to_weight; query_heap.DecreaseKey(*toHeapNode); } @@ -157,32 +157,32 @@ void relaxOutgoingEdges(const DataFacade &facade, else { // Shortcuts in backward direction auto source = cell.GetSourceNodes().begin(); - auto shortcut_durations = cell.GetInDuration(node); - auto shortcut_distances = cell.GetInDistance(node); - for (auto shortcut_weight : cell.GetInWeight(node)) + auto shortcut_durations = cell.GetInDuration(heapNode.node); + auto shortcut_distances = cell.GetInDistance(heapNode.node); + for (auto shortcut_weight : cell.GetInWeight(heapNode.node)) { BOOST_ASSERT(source != cell.GetSourceNodes().end()); BOOST_ASSERT(!shortcut_durations.empty()); BOOST_ASSERT(!shortcut_distances.empty()); const NodeID to = *source; - if (shortcut_weight != INVALID_EDGE_WEIGHT && node != to) + if (shortcut_weight != INVALID_EDGE_WEIGHT && heapNode.node != to) { const auto to_weight = heapNode.weight + shortcut_weight; const auto to_duration = heapNode.data.duration + shortcut_durations.front(); const auto to_distance = heapNode.data.distance + shortcut_distances.front(); - const auto& toHeapNode= query_heap.GetHeapNodeIfWasInserted(to); + const auto toHeapNode= query_heap.GetHeapNodeIfWasInserted(to); if (!toHeapNode) { - query_heap.Insert(to, to_weight, {node, true, to_duration, to_distance}); + query_heap.Insert(to, to_weight, {heapNode.node, true, to_duration, to_distance}); } - else if (std::tie(to_weight, to_duration, to_distance, node) < + else if (std::tie(to_weight, to_duration, to_distance, heapNode.node) < std::tie(toHeapNode->weight, toHeapNode->data.duration, toHeapNode->data.distance, toHeapNode->data.parent)) { - toHeapNode->data = {node, true, to_duration, to_distance}; + toHeapNode->data = {heapNode.node, true, to_duration, to_distance}; toHeapNode->weight=to_weight; query_heap.DecreaseKey(*toHeapNode); } @@ -196,7 +196,7 @@ void relaxOutgoingEdges(const DataFacade &facade, } } - relaxBorderEdges(facade, node, heapNode.weight, heapNode.data.duration, heapNode.data.distance, query_heap, level); + relaxBorderEdges(facade, heapNode.node, heapNode.weight, heapNode.data.duration, heapNode.data.distance, query_heap, level); } // @@ -371,14 +371,12 @@ oneToManySearch(SearchEngineData &engine_working_data, while (!query_heap.Empty() && !target_nodes_index.empty()) { - // Extract node from the heap - const auto& heapNode=query_heap.DeleteMinGetHeapNode(); - const auto weight = heapNode.weight; - const auto duration = heapNode.data.duration; - const auto distance = heapNode.data.distance; + // Extract node from the heap. Take a copy (no ref) because otherwise can be modified later + // if toHeapNode is the same + const auto heapNode=query_heap.DeleteMinGetHeapNode(); // Update values - update_values(heapNode.node, weight, duration, distance); + update_values(heapNode.node, heapNode.weight, heapNode.data.duration, heapNode.data.distance); // Relax outgoing edges relaxOutgoingEdges(facade, @@ -408,10 +406,8 @@ void forwardRoutingStep(const DataFacade &facade, std::vector &middle_nodes_table, const PhantomNode &phantom_node) { - const auto& heapNode=query_heap.DeleteMinGetHeapNode(); - const auto source_weight = heapNode.weight; - const auto source_duration = heapNode.data.duration; - const auto source_distance = heapNode.data.distance; + // Take a copy of the extracted node because otherwise could be modified later if toHeapNode is the same + const auto heapNode=query_heap.DeleteMinGetHeapNode(); // Check if each encountered node has an entry const auto &bucket_list = std::equal_range(search_space_with_buckets.begin(), @@ -439,9 +435,9 @@ void forwardRoutingStep(const DataFacade &facade, auto ¤t_distance = distances_table.empty() ? nulldistance : distances_table[location]; // Check if new weight is better - auto new_weight = source_weight + target_weight; - auto new_duration = source_duration + target_duration; - auto new_distance = source_distance + target_distance; + auto new_weight = heapNode.weight + target_weight; + auto new_duration = heapNode.data.duration + target_duration; + auto new_distance = heapNode.data.distance + target_distance; if (new_weight >= 0 && std::tie(new_weight, new_duration, new_distance) < @@ -465,16 +461,12 @@ void backwardRoutingStep(const DataFacade &facade, std::vector &search_space_with_buckets, const PhantomNode &phantom_node) { - const auto& heapNode=query_heap.DeleteMinGetHeapNode(); - const auto target_weight = heapNode.weight; - const auto target_duration = heapNode.data.duration; - const auto target_distance = heapNode.data.distance; - const auto parent = heapNode.data.parent; - const auto from_clique_arc = heapNode.data.from_clique_arc; + // Take a copy of the extracted node because otherwise could be modified later if toHeapNode is the same + const auto heapNode=query_heap.DeleteMinGetHeapNode(); // Store settled nodes in search space bucket search_space_with_buckets.emplace_back( - heapNode.node, parent, from_clique_arc, column_idx, target_weight, target_duration, target_distance); + heapNode.node, heapNode.data.parent, heapNode.data.from_clique_arc, column_idx, heapNode.weight, heapNode.data.duration, heapNode.data.distance); const auto &partition = facade.GetMultiLevelPartition(); const auto maximal_level = partition.GetNumberOfLevels() - 1;