remove default method in XFirstSearch to force thinking about the bit…

…set implementation, fixes #1648 (#1649)
graphhopper · Jun 27, 2019 · 653b108 · 653b108
1 parent 4591d8d
commit 653b108
Show file tree

Hide file tree

Showing 12 changed files with 72 additions and 21 deletions.
diff --git a/core/src/main/java/com/graphhopper/reader/dem/EdgeElevationInterpolator.java b/core/src/main/java/com/graphhopper/reader/dem/EdgeElevationInterpolator.java
@@ -21,6 +21,7 @@
 import com.graphhopper.coll.GHBitSet;
 import com.graphhopper.coll.GHBitSetImpl;
 import com.graphhopper.coll.GHIntHashSet;
+import com.graphhopper.coll.GHTBitSet;
 import com.graphhopper.routing.profiles.EnumEncodedValue;
 import com.graphhopper.routing.profiles.RoadEnvironment;
 import com.graphhopper.routing.util.AllEdgesIterator;
@@ -99,16 +100,19 @@ private void interpolateEdge(final EdgeIteratorState interpolatableEdge,
                                  final GHBitSet visitedEdgeIds, final EdgeExplorer edgeExplorer) {
         final IntSet outerNodeIds = new GHIntHashSet();
         final GHIntHashSet innerNodeIds = new GHIntHashSet();
-        gatherOuterAndInnerNodeIds(edgeExplorer, interpolatableEdge, visitedEdgeIds, outerNodeIds,
-                innerNodeIds);
-        nodeElevationInterpolator.interpolateElevationsOfInnerNodes(outerNodeIds.toArray(),
-                innerNodeIds.toArray());
+        gatherOuterAndInnerNodeIds(edgeExplorer, interpolatableEdge, visitedEdgeIds, outerNodeIds, innerNodeIds);
+        nodeElevationInterpolator.interpolateElevationsOfInnerNodes(outerNodeIds.toArray(), innerNodeIds.toArray());
     }
 
     public void gatherOuterAndInnerNodeIds(final EdgeExplorer edgeExplorer,
                                            final EdgeIteratorState interpolatableEdge, final GHBitSet visitedEdgesIds,
                                            final IntSet outerNodeIds, final GHIntHashSet innerNodeIds) {
         final BreadthFirstSearch gatherOuterAndInnerNodeIdsSearch = new BreadthFirstSearch() {
+            @Override
+            protected GHBitSet createBitSet() {
+                return new GHTBitSet();
+            }
+
             @Override
             protected boolean checkAdjacent(EdgeIteratorState edge) {
                 visitedEdgesIds.add(edge.getEdge());

diff --git a/core/src/main/java/com/graphhopper/reader/dem/ElevationInterpolator.java b/core/src/main/java/com/graphhopper/reader/dem/ElevationInterpolator.java
@@ -25,7 +25,6 @@
  * Elevation interpolator calculates elevation for the given lat/lon coordinates
  * based on lat/lon/ele coordinates of the given points.
  * <p>
- * <p>
  * In case of two points, elevation is calculated using linear interpolation
  * (see
  * {@link #calculateElevationBasedOnTwoPoints(double, double, double, double, double, double, double, double)}).

diff --git a/core/src/main/java/com/graphhopper/reader/dem/NodeElevationInterpolator.java b/core/src/main/java/com/graphhopper/reader/dem/NodeElevationInterpolator.java
@@ -22,7 +22,7 @@
 import com.graphhopper.util.PointList;
 
 /**
- * Interpolates elevations of inner nodes based on elevations of outer nodes.
+ * Interpolates elevations of pillar nodes based on elevations of tower nodes.
  *
  * @author Alexey Valikov
  */

diff --git a/core/src/main/java/com/graphhopper/storage/GraphEdgeIdFinder.java b/core/src/main/java/com/graphhopper/storage/GraphEdgeIdFinder.java
@@ -17,18 +17,17 @@
  */
 package com.graphhopper.storage;
 
+import com.graphhopper.coll.GHBitSet;
 import com.graphhopper.coll.GHIntHashSet;
+import com.graphhopper.coll.GHTBitSet;
 import com.graphhopper.routing.util.EdgeFilter;
-import com.graphhopper.util.shapes.Polygon;
 import com.graphhopper.storage.index.LocationIndex;
 import com.graphhopper.storage.index.QueryResult;
 import com.graphhopper.util.BreadthFirstSearch;
 import com.graphhopper.util.EdgeIteratorState;
 import com.graphhopper.util.PointList;
-import com.graphhopper.util.shapes.BBox;
-import com.graphhopper.util.shapes.Circle;
-import com.graphhopper.util.shapes.GHPoint;
-import com.graphhopper.util.shapes.Shape;
+import com.graphhopper.util.shapes.Polygon;
+import com.graphhopper.util.shapes.*;
 import org.locationtech.jts.geom.*;
 
 import java.util.ArrayList;
@@ -84,6 +83,11 @@ public void findEdgesInShape(final GHIntHashSet edgeIds, final Shape shape, Edge
             final NodeAccess na = graph.getNodeAccess();
             final Shape localShape = shape;
 
+            @Override
+            protected GHBitSet createBitSet() {
+                return new GHTBitSet();
+            }
+
             @Override
             protected boolean goFurther(int nodeId) {
                 if (isPolygon) return isInsideBBox(nodeId);
@@ -137,6 +141,7 @@ public void fillEdgeIDs(GHIntHashSet edgeIds, Geometry geometry, EdgeFilter filt
 
     /**
      * This method reads the blockAreaString and creates a Collection of Shapes or a set of found edges if area is small enough.
+     *
      * @param useEdgeIdsUntilAreaSize until the specified area (specified in m²) use the findEdgesInShape method
      */
     public BlockArea parseBlockArea(String blockAreaString, EdgeFilter filter, double useEdgeIdsUntilAreaSize) {

diff --git a/core/src/main/java/com/graphhopper/util/BreadthFirstSearch.java b/core/src/main/java/com/graphhopper/util/BreadthFirstSearch.java
@@ -21,11 +21,10 @@
 
 /**
  * Implementation of breadth first search (BFS)
- * <p>
  *
  * @author Peter Karich
  */
-public class BreadthFirstSearch extends XFirstSearch {
+public abstract class BreadthFirstSearch extends XFirstSearch {
     @Override
     public void start(EdgeExplorer explorer, int startNode) {
         SimpleIntDeque fifo = new SimpleIntDeque();

diff --git a/core/src/main/java/com/graphhopper/util/DepthFirstSearch.java b/core/src/main/java/com/graphhopper/util/DepthFirstSearch.java
@@ -22,12 +22,11 @@
 
 /**
  * Implementation of depth first search (DFS) by LIFO queue
- * <p>
  *
  * @author Peter Karich
  * @author Jan Sölter
  */
-public class DepthFirstSearch extends XFirstSearch {
+public abstract class DepthFirstSearch extends XFirstSearch {
     /**
      * beginning with startNode add all following nodes to LIFO queue. If node has been already
      * explored before, skip reexploration.

diff --git a/core/src/main/java/com/graphhopper/util/EngineWarmUp.java b/core/src/main/java/com/graphhopper/util/EngineWarmUp.java
@@ -19,6 +19,8 @@
 
 import com.graphhopper.GHRequest;
 import com.graphhopper.GraphHopper;
+import com.graphhopper.coll.GHBitSet;
+import com.graphhopper.coll.GHTBitSet;
 import com.graphhopper.storage.GraphHopperStorage;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -79,6 +81,11 @@ private static void warmUpNonCHSubNetwork(final GraphHopper graphHopper, int ite
             BreadthFirstSearch bfs = new BreadthFirstSearch() {
                 int counter = 0;
 
+                @Override
+                protected GHBitSet createBitSet() {
+                    return new GHTBitSet(graphHopper.getMaxVisitedNodes());
+                }
+
                 @Override
                 public boolean goFurther(int nodeId) {
                     counter++;

diff --git a/core/src/main/java/com/graphhopper/util/GHUtility.java b/core/src/main/java/com/graphhopper/util/GHUtility.java
@@ -21,6 +21,7 @@
 import com.graphhopper.coll.GHBitSet;
 import com.graphhopper.coll.GHBitSetImpl;
 import com.graphhopper.coll.GHIntArrayList;
+import com.graphhopper.coll.GHTBitSet;
 import com.graphhopper.routing.profiles.BooleanEncodedValue;
 import com.graphhopper.routing.profiles.DecimalEncodedValue;
 import com.graphhopper.routing.profiles.EnumEncodedValue;
@@ -260,6 +261,11 @@ public static void printInfo(final Graph g, int startNode, final int counts, fin
         new BreadthFirstSearch() {
             int counter = 0;
 
+            @Override
+            protected GHBitSet createBitSet() {
+                return new GHTBitSet();
+            }
+
             @Override
             protected boolean goFurther(int nodeId) {
                 System.out.println(getNodeInfo(g, nodeId, filter));

diff --git a/core/src/main/java/com/graphhopper/util/XFirstSearch.java b/core/src/main/java/com/graphhopper/util/XFirstSearch.java
@@ -18,18 +18,18 @@
 package com.graphhopper.util;
 
 import com.graphhopper.coll.GHBitSet;
-import com.graphhopper.coll.GHBitSetImpl;
 
 /**
  * This abstract class defines commonalities for BFS and DFS
- * <p>
  *
  * @author Jan Sölter
  */
 public abstract class XFirstSearch {
-    protected GHBitSet createBitSet() {
-        return new GHBitSetImpl();
-    }
+    /**
+     * Pick the BitSet implementation wisely. Use GHBitSetImpl only if we are sure you visit a large portion of the graph.
+     * And if you choose GHTBitSet the initial capacity can be also important for performance.
+     */
+    protected abstract GHBitSet createBitSet();
 
     public abstract void start(EdgeExplorer explorer, int startNode);
 

diff --git a/core/src/test/java/com/graphhopper/util/BreadthFirstSearchTest.java b/core/src/test/java/com/graphhopper/util/BreadthFirstSearchTest.java
@@ -18,7 +18,9 @@
 package com.graphhopper.util;
 
 import com.carrotsearch.hppc.IntArrayList;
+import com.graphhopper.coll.GHBitSet;
 import com.graphhopper.coll.GHIntHashSet;
+import com.graphhopper.coll.GHTBitSet;
 import com.graphhopper.routing.util.EncodingManager;
 import com.graphhopper.storage.Graph;
 import com.graphhopper.storage.GraphBuilder;
@@ -44,6 +46,11 @@ public void setup() {
     @Test
     public void testBFS() {
         BreadthFirstSearch bfs = new BreadthFirstSearch() {
+            @Override
+            protected GHBitSet createBitSet() {
+                return new GHTBitSet();
+            }
+
             @Override
             public boolean goFurther(int v) {
                 counter++;
@@ -78,6 +85,11 @@ public boolean goFurther(int v) {
     @Test
     public void testBFS2() {
         BreadthFirstSearch bfs = new BreadthFirstSearch() {
+            @Override
+            protected GHBitSet createBitSet() {
+                return new GHTBitSet();
+            }
+
             @Override
             public boolean goFurther(int v) {
                 counter++;

diff --git a/core/src/test/java/com/graphhopper/util/DepthFirstSearchTest.java b/core/src/test/java/com/graphhopper/util/DepthFirstSearchTest.java
@@ -18,7 +18,10 @@
 package com.graphhopper.util;
 
 import com.carrotsearch.hppc.IntArrayList;
+import com.graphhopper.coll.GHBitSet;
+import com.graphhopper.coll.GHBitSetImpl;
 import com.graphhopper.coll.GHIntHashSet;
+import com.graphhopper.coll.GHTBitSet;
 import com.graphhopper.routing.util.DefaultEdgeFilter;
 import com.graphhopper.routing.util.EncodingManager;
 import com.graphhopper.routing.util.FlagEncoder;
@@ -47,6 +50,11 @@ public void setup() {
     @Test
     public void testDFS1() {
         DepthFirstSearch dfs = new DepthFirstSearch() {
+            @Override
+            protected GHBitSet createBitSet() {
+                return new GHBitSetImpl();
+            }
+
             @Override
             public boolean goFurther(int v) {
                 counter++;
@@ -77,6 +85,11 @@ public boolean goFurther(int v) {
     @Test
     public void testDFS2() {
         DepthFirstSearch dfs = new DepthFirstSearch() {
+            @Override
+            protected GHBitSet createBitSet() {
+                return new GHBitSetImpl();
+            }
+
             @Override
             public boolean goFurther(int v) {
                 counter++;

diff --git a/reader-osm/src/test/java/com/graphhopper/reader/osm/GraphHopperOSMTest.java b/reader-osm/src/test/java/com/graphhopper/reader/osm/GraphHopperOSMTest.java
@@ -22,6 +22,8 @@
 import com.graphhopper.GHResponse;
 import com.graphhopper.GraphHopper;
 import com.graphhopper.PathWrapper;
+import com.graphhopper.coll.GHBitSet;
+import com.graphhopper.coll.GHBitSetImpl;
 import com.graphhopper.reader.DataReader;
 import com.graphhopper.routing.*;
 import com.graphhopper.routing.ch.CHAlgoFactoryDecorator;
@@ -147,7 +149,7 @@ public void testLoadOSMNoCH() {
 
     @Test
     public void testQueryLocationIndexWithBBox() {
-        GraphHopper gh = new GraphHopperOSM().setStoreOnFlush(true).
+        final GraphHopper gh = new GraphHopperOSM().setStoreOnFlush(true).
                 setEncodingManager(EncodingManager.create("car")).
                 setCHEnabled(false).
                 setGraphHopperLocation("./target/monacotmp-gh").
@@ -184,6 +186,11 @@ public void onEdge(EdgeIteratorState edge, int nodeA, int nodeB) {
 
         final Collection<Integer> bfsNodeList = new TreeSet<>();
         new BreadthFirstSearch() {
+            @Override
+            protected GHBitSet createBitSet() {
+                return new GHBitSetImpl(gh.getGraphHopperStorage().getNodes());
+            }
+
             @Override
             protected boolean goFurther(int nodeId) {
                 double lat = na.getLatitude(nodeId);