Contractor rewrite

features/bicycle/distance.feature

@@ -32,4 +32,8 @@ Feature: Bike - Use distance weight
         When I route I should get
             | from | to | route   | weight | time | distance |
             | a    | b  | abc,abc |    200 | 48s  | 200m +-1 |
+            | b    | c  | abc,abc |    200 | 48s  | 200m +-1 |
             | a    | c  | abc,abc |    400 | 96s  | 400m +-1 |
+            | b    | a  | abc,abc |    200 | 48s  | 200m +-1 |
+            | c    | b  | abc,abc |    200 | 48s  | 200m +-1 |
+            | c    | a  | abc,abc |    400 | 96s  | 400m +-1 |

features/foot/distance.feature

@@ -32,4 +32,8 @@ Feature: Foot - Use distance weight
         When I route I should get
             | from | to | route   | weight | time | distance |
             | a    | b  | abc,abc |    200 | 144s | 200m +-1 |
+            | b    | c  | abc,abc |    200 | 144s | 200m +-1 |
             | a    | c  | abc,abc |    400 | 288s | 400m +-1 |
+            | b    | a  | abc,abc |    200 | 144s | 200m +-1 |
+            | c    | b  | abc,abc |    200 | 144s | 200m +-1 |
+            | c    | a  | abc,abc |    400 | 288s | 400m +-1 |

features/lib/osrm_loader.js

@@ -38,12 +38,12 @@ class OSRMBaseLoader {
 
       this.child.stdout.on('data', (data) => {
         if (data.includes('running and waiting for requests')) {
-          log('Routed running and waiting for requests');
-          resolve();
+            resolve();
         }
+        log(`${data}`.trim());
       });
 
-      this.child.on('exit', (code) => {
+      this.child.on('close', (code) => {
         log(`osrm-routed completed with exit code ${code}`);
         this.child = null;
       });
@@ -209,7 +209,9 @@ export class OSRMDatastoreLoader extends OSRMBaseLoader {
       // we MUST consume stdout and stderr or the osrm-routed process will block eventually
       this.child.stderr.on('data', (data) => this.logSync(`osrm-routed stderr:\n${data}`));
 
-      this.child.on('exit', (code, signal) => {
+      this.child.on('close', (code, signal) => {
+        this.child.stdout.read();
+        this.child.stderr.read();
         this.child = null;
         if (signal != null) {
           const msg = `osrm-routed aborted with signal ${signal}`;

features/support/run.js

@@ -39,7 +39,7 @@ export function runBin(bin, args, options, log) {
     log(`${bin} aborted with error ${err}`);
     throw(err);
   });
-  child.on('exit', (code, signal) => {
+  child.on('close', (code, signal) => {
     if (signal != null) {
       const msg = `${bin} aborted with signal ${child.signal}`;
       log(msg);
@@ -76,9 +76,9 @@ export function runBinSync(bin, args, options, log) {
   if (child.stderr)
     log(`${bin} stderr:\n${child.stderr}`);
   if (child.status != null)
-    log(`${bin} completed with exit code ${child.status}`);
+    log(`${bin} sync completed with exit code ${child.status}`);
   if (child.signal != null) {
-    const msg = `${bin} aborted with signal ${child.signal}`;
+    const msg = `${bin} sync aborted with signal ${child.signal}`;
     log(msg);
     if (child.signal != 'SIGABRT') // some tests deliberately fail
       throw new Error(msg);

include/contractor/contractor_graph.hpp

@@ -18,7 +18,7 @@ struct ContractorEdgeData
                        EdgeDuration duration,
                        EdgeDistance distance,
                        unsigned original_edges,
-                       unsigned id,
+                       NodeID id,
                        bool shortcut,
                        bool forward,
                        bool backward)
@@ -30,7 +30,8 @@ struct ContractorEdgeData
     EdgeWeight weight;
     EdgeDuration duration;
     EdgeDistance distance;
-    unsigned id;
+    NodeID id;
+    /** Recursive count of how many edges are replaced by this shortcut. */
     unsigned originalEdges : 29;
     bool shortcut : 1;
     bool forward : 1;

include/contractor/contractor_heap.hpp

@@ -7,20 +7,9 @@
 
 namespace osrm::contractor
 {
-struct ContractorHeapData
-{
-    ContractorHeapData() {}
-    ContractorHeapData(short hop_, bool target_) : hop(hop_), target(target_) {}
-
-    short hop = 0;
-    bool target = false;
-};
-
-using ContractorHeap = util::QueryHeap<NodeID,
-                                       NodeID,
-                                       EdgeWeight,
-                                       ContractorHeapData,
-                                       util::LinearHashStorage<NodeID, NodeID>>;
+// Data: if true signals that node is a target
+using ContractorHeap =
+    util::QueryHeap<NodeID, NodeID, EdgeWeight, bool, util::LinearHashStorage<NodeID, NodeID>>;
 
 } // namespace osrm::contractor
 

include/contractor/contractor_search.hpp

@@ -13,6 +13,7 @@ namespace osrm::contractor
 
 void search(ContractorHeap &heap,
             const ContractorGraph &graph,
+            const NodeID start,
             const std::vector<bool> &contractable,
             const unsigned number_of_targets,
             const int node_limit,

include/contractor/graph_contractor.hpp

@@ -4,44 +4,35 @@
 #include "contractor/contractor_graph.hpp"
 #include "contractor/query_graph.hpp"
 
-#include "util/filtered_graph.hpp"
-
 #include <vector>
 
 namespace osrm::contractor
 {
 
 using GraphAndFilter = std::tuple<QueryGraph, std::vector<std::vector<bool>>>;
 
-GraphAndFilter contractFullGraph(ContractorGraph contractor_graph,
-                                 std::vector<EdgeWeight> node_weights);
+GraphAndFilter contractFullGraph(ContractorGraph contractor_graph);
 
 GraphAndFilter contractExcludableGraph(ContractorGraph contractor_graph_,
-                                       std::vector<EdgeWeight> node_weights,
                                        const std::vector<std::vector<bool>> &filters);
 
 std::vector<bool> contractGraph(ContractorGraph &graph,
                                 std::vector<bool> node_is_uncontracted,
                                 std::vector<bool> node_is_contractable,
-                                std::vector<EdgeWeight> node_weights,
                                 double core_factor = 1.0);
 
 // Overload for contracting all nodes
-inline auto contractGraph(ContractorGraph &graph,
-                          std::vector<EdgeWeight> node_weights,
-                          double core_factor = 1.0)
+inline auto contractGraph(ContractorGraph &graph, double core_factor = 1.0)
 {
-    return contractGraph(graph, {}, {}, std::move(node_weights), core_factor);
+    return contractGraph(graph, {}, {}, core_factor);
 }
 
 // Overload no contracted nodes
 inline auto contractGraph(ContractorGraph &graph,
                           std::vector<bool> node_is_contractable,
-                          std::vector<EdgeWeight> node_weights,
                           double core_factor = 1.0)
 {
-    return contractGraph(
-        graph, {}, std::move(node_is_contractable), std::move(node_weights), core_factor);
+    return contractGraph(graph, {}, std::move(node_is_contractable), core_factor);
 }
 
 } // namespace osrm::contractor

include/engine/guidance/assemble_leg.hpp

@@ -7,6 +7,7 @@
 #include "engine/guidance/route_step.hpp"
 #include "engine/internal_route_result.hpp"
 #include "util/coordinate_calculation.hpp"
+#include "util/log.hpp"
 #include "util/typedefs.hpp"
 
 #include <boost/algorithm/string/join.hpp>

include/engine/guidance/assemble_steps.hpp

@@ -15,6 +15,7 @@
 #include "util/coordinate_calculation.hpp"
 #include "util/guidance/entry_class.hpp"
 #include "util/guidance/turn_lanes.hpp"
+#include "util/log.hpp"
 #include "util/typedefs.hpp"
 
 #include <cstddef>
@@ -153,6 +154,9 @@ inline std::vector<RouteStep> assembleSteps(const datafacade::BaseDataFacade &fa
                                           {intersection},
                                           is_left_hand_driving});
 
+                util::Log(logDEBUG) << "pushing RouteStep of duration "
+                                    << from_alias<double>(segment_duration) / 10.;
+
                 if (leg_data_index + 1 < leg_data.size())
                 {
                     step_name_id =
@@ -272,6 +276,13 @@ inline std::vector<RouteStep> assembleSteps(const datafacade::BaseDataFacade &fa
         // u-------------v
         // |   |---------| source_weight
         // |         |---| target_weight
+
+#ifndef NDEBUG
+        if (source_weight > target_weight)
+            util::Log(logDEBUG) << "error: source_weight = " << source_weight
+                                << " target_weight = " << target_weight;
+#endif
+
         BOOST_ASSERT(target_weight >= source_weight);
         const EdgeWeight weight = target_weight - source_weight;
 

include/engine/routing_algorithms/routing_base_ch.hpp

@@ -6,6 +6,7 @@
 #include "engine/routing_algorithms/routing_base.hpp"
 #include "engine/search_engine_data.hpp"
 
+#include "util/log.hpp"
 #include "util/typedefs.hpp"
 
 #include <boost/assert.hpp>
@@ -112,49 +113,44 @@ void routingStep(const DataFacade<Algorithm> &facade,
                  NodeID &middle_node_id,
                  EdgeWeight &upper_bound,
                  EdgeWeight min_edge_offset,
-                 const std::vector<NodeID> &force_step_nodes)
+                 [[maybe_unused]] const std::vector<NodeID> &force_step_nodes)
 {
     auto heapNode = forward_heap.DeleteMinGetHeapNode();
     const auto reverseHeapNode = reverse_heap.GetHeapNodeIfWasInserted(heapNode.node);
 
     if (reverseHeapNode)
     {
         const EdgeWeight new_weight = reverseHeapNode->weight + heapNode.weight;
-        if (new_weight < upper_bound)
+
+        if (new_weight >= EdgeWeight{0} && new_weight <= upper_bound &&
+            !shouldForceStep(force_step_nodes, heapNode, *reverseHeapNode))
+        {
+            middle_node_id = heapNode.node;
+            upper_bound = new_weight;
+        }
+        else
         {
-            if (shouldForceStep(force_step_nodes, heapNode, *reverseHeapNode) ||
-                // in this case we are looking at a bi-directional way where the source
-                // and target phantom are on the same edge based node
-                new_weight < EdgeWeight{0})
+            // The two identical nodes did not match after all.
+            // Before forcing step, check whether there is a loop present at the node.
+            // We may find a valid weight path by following the loop.
+            for (const auto edge : facade.GetAdjacentEdgeRange(heapNode.node))
             {
-                // Before forcing step, check whether there is a loop present at the node.
-                // We may find a valid weight path by following the loop.
-                for (const auto edge : facade.GetAdjacentEdgeRange(heapNode.node))
+                const auto &data = facade.GetEdgeData(edge);
+                if (DIRECTION == FORWARD_DIRECTION ? data.forward : data.backward)
                 {
-                    const auto &data = facade.GetEdgeData(edge);
-                    if (DIRECTION == FORWARD_DIRECTION ? data.forward : data.backward)
+                    const NodeID to = facade.GetTarget(edge);
+                    if (to == heapNode.node)
                     {
-                        const NodeID to = facade.GetTarget(edge);
-                        if (to == heapNode.node)
+                        const EdgeWeight edge_weight = data.weight;
+                        const EdgeWeight loop_weight = new_weight + edge_weight;
+                        if (loop_weight >= EdgeWeight{0} && loop_weight < upper_bound)
                         {
-                            const EdgeWeight edge_weight = data.weight;
-                            const EdgeWeight loop_weight = new_weight + edge_weight;
-                            if (loop_weight >= EdgeWeight{0} && loop_weight < upper_bound)
-                            {
-                                middle_node_id = heapNode.node;
-                                upper_bound = loop_weight;
-                            }
+                            middle_node_id = heapNode.node;
+                            upper_bound = loop_weight;
                         }
                     }
                 }
             }
-            else
-            {
-                BOOST_ASSERT(new_weight >= EdgeWeight{0});
-
-                middle_node_id = heapNode.node;
-                upper_bound = new_weight;
-            }
         }
     }
 
@@ -389,6 +385,17 @@ void unpackPath(const FacadeT &facade,
     }
 
     annotatePath(facade, route_endpoints, unpacked_nodes, unpacked_edges, unpacked_path);
+
+#ifndef NDEBUG
+    {
+        auto log = util::Log(logDEBUG);
+        log << "unpacked path =";
+        for (const auto &p : unpacked_path)
+        {
+            log << " " << p.from_edge_based_node;
+        }
+    }
+#endif
 }
 
 /**

include/util/d_ary_heap.hpp

@@ -40,6 +40,15 @@ template <typename HeapData, int Arity, typename Comparator = std::less<HeapData
         heapifyUp(handle, std::forward<ReorderHandler>(reorderHandler));
     }
 
+    template <typename ReorderHandler>
+    void increase(HeapHandle handle, HeapData &&data, ReorderHandler &&reorderHandler)
+    {
+        BOOST_ASSERT(handle < heap.size());
+
+        heap[handle] = std::forward<HeapData>(data);
+        heapifyDown(handle, std::forward<ReorderHandler>(reorderHandler));
+    }
+
     void clear() { heap.clear(); }
 
     template <typename ReorderHandler> void pop(ReorderHandler &&reorderHandler)

include/util/dynamic_graph.hpp

@@ -415,8 +415,11 @@ template <typename EdgeDataT> class DynamicGraph
 
     void Renumber(const std::vector<NodeID> &old_to_new_node)
     {
+        bool renumber = old_to_new_node.size() != 0;
+
         // permutate everything but the sentinel
-        util::inplacePermutation(node_array.begin(), node_array.end(), old_to_new_node);
+        if (renumber)
+            util::inplacePermutation(node_array.begin(), node_array.end(), old_to_new_node);
 
         // Build up edge permutation
         if (edge_list.size() >= std::numeric_limits<EdgeID>::max())
@@ -432,7 +435,8 @@ template <typename EdgeDataT> class DynamicGraph
             // move all filled edges
             for (auto edge : GetAdjacentEdgeRange(node))
             {
-                edge_list[edge].target = old_to_new_node[edge_list[edge].target];
+                if (renumber)
+                    edge_list[edge].target = old_to_new_node[edge_list[edge].target];
                 BOOST_ASSERT(edge_list[edge].target != SPECIAL_NODEID);
                 old_to_new_edge[edge] = new_edge_index++;
             }

include/util/query_heap.hpp

@@ -7,7 +7,6 @@
 #include "d_ary_heap.hpp"
 #include <algorithm>
 #include <limits>
-#include <optional>
 #include <unordered_map>
 #include <vector>
 
@@ -333,6 +332,15 @@ class QueryHeap
                       { inserted_nodes[heapData.index].handle = new_handle; });
     }
 
+    void IncreaseKey(const HeapNode &heapNode)
+    {
+        BOOST_ASSERT(!WasRemoved(heapNode.node));
+        heap.increase(heapNode.handle,
+                      HeapData{heapNode.weight, heap[heapNode.handle].index},
+                      [this](const auto &heapData, auto new_handle)
+                      { inserted_nodes[heapData.index].handle = new_handle; });
+    }
+
   private:
     std::vector<HeapNode> inserted_nodes;
     HeapContainer heap;