Repository: graphhopper/graphhopper
Pull request #932 (2017-01-12 14:21:45)

Improve javadoc comments
The javadoc changes reflect my learnings about Graphhopper internals when I was implementing graphhopper/map-matching#88.


Patch:
diff --git a/core/src/main/java/com/graphhopper/routing/QueryGraph.java b/core/src/main/java/com/graphhopper/routing/QueryGraph.java
index 8c98e8e62..af1e86376 100644
--- a/core/src/main/java/com/graphhopper/routing/QueryGraph.java
+++ b/core/src/main/java/com/graphhopper/routing/QueryGraph.java
@@ -43,6 +43,11 @@ import java.util.*;
  * introducing virtual nodes and edges. It is lightweight in order to be created every time a new
  * query comes in, which makes the behaviour thread safe.
  * <p>
+ * Calling any <tt>lookup</tt> method creates virtual edges between the tower nodes of the existing
+ * graph and new virtual tower nodes. Every virtual node has two adjacent nodes and is connected
+ * to each adjacent nodes via 2 virtual edges with opposite base node / adjacent node encoding.
+ * However, the edge explorer returned by {@link #createEdgeExplorer()} only returns two
+ * virtual edges per virtual node (the ones with correct base node).
  *
  * @author Peter Karich
  */
@@ -57,10 +62,8 @@ public class QueryGraph implements Graph {
     private final GraphExtension wrappedExtension;
     // TODO when spreading it on different threads we need multiple independent explorers
     private final Map<Integer, EdgeExplorer> cacheMap = new HashMap<Integer, EdgeExplorer>(4);
-    /**
-     * Virtual edges are created between existing graph and new virtual tower nodes. For every
-     * virtual node there are 4 edges: base-snap, snap-base, snap-adj, adj-snap.
-     */
+
+    // For every virtual node there are 4 edges: base-snap, snap-base, snap-adj, adj-snap.
     List<VirtualEdgeIteratorState> virtualEdges;
     private List<QueryResult> queryResults;
     /**
@@ -183,6 +186,8 @@ public class QueryGraph implements Graph {
 
     /**
      * Convenient method to initialize this QueryGraph with the two specified query results.
+     *
+     * @see #lookup(List)
      */
     public QueryGraph lookup(QueryResult fromRes, QueryResult toRes) {
         List<QueryResult> results = new ArrayList<QueryResult>(2);
@@ -193,8 +198,11 @@ public class QueryGraph implements Graph {
     }
 
     /**
-     * For all specified query results calculate snapped point and set closest node and edge to a
-     * virtual one if necessary. Additionally the wayIndex can change if an edge is swapped.
+     * For all specified query results calculate snapped point and if necessary set closest node
+     * to a virtual one and reverse closest edge. Additionally the wayIndex can change if an edge is
+     * swapped.
+     *
+     * @see QueryGraph
      */
     public void lookup(List<QueryResult> resList) {
         if (isInitialized())
@@ -694,6 +702,9 @@ public class QueryGraph implements Graph {
     }
 
     @Override
+    /**
+     * @see QueryGraph
+     */
     public EdgeExplorer createEdgeExplorer() {
         return createEdgeExplorer(EdgeFilter.ALL_EDGES);
     }
diff --git a/core/src/main/java/com/graphhopper/routing/util/FlagEncoder.java b/core/src/main/java/com/graphhopper/routing/util/FlagEncoder.java
index a9d8ac1a0..460550517 100644
--- a/core/src/main/java/com/graphhopper/routing/util/FlagEncoder.java
+++ b/core/src/main/java/com/graphhopper/routing/util/FlagEncoder.java
@@ -84,7 +84,8 @@ public interface FlagEncoder extends TurnCostEncoder {
     long setProperties(double speed, boolean forward, boolean backward);
 
     /**
-     * Reports whether the edge is available in forward direction for a certain vehicle
+     * Reports whether the edge is available in forward direction (i.e. from base node to adj node)
+     * for a certain vehicle.
      */
     boolean isForward(long flags);
 
@@ -94,7 +95,8 @@ public interface FlagEncoder extends TurnCostEncoder {
      * Currently this means starting from 100, and subclasses of this class start from 10000 and so on.
      */
     /**
-     * Reports whether the edge is available in backward direction for a certain vehicle
+     * Reports whether the edge is available in backward direction (i.e. from adj node to base node)
+     * for a certain vehicle.
      */
     boolean isBackward(long flags);
 
diff --git a/core/src/main/java/com/graphhopper/storage/index/QueryResult.java b/core/src/main/java/com/graphhopper/storage/index/QueryResult.java
index 7c1c90bfc..278b3822a 100644
--- a/core/src/main/java/com/graphhopper/storage/index/QueryResult.java
+++ b/core/src/main/java/com/graphhopper/storage/index/QueryResult.java
@@ -23,6 +23,8 @@ import com.graphhopper.util.PointList;
 import com.graphhopper.util.shapes.GHPoint;
 import com.graphhopper.util.shapes.GHPoint3D;
 
+import java.util.List;
+
 /**
  * Result of LocationIndex lookup.
  * <pre> X=query coordinates S=snapped coordinates: "snapping" real coords to road N=tower or pillar
@@ -49,8 +51,10 @@ public class QueryResult {
     }
 
     /**
-     * @return the closest matching node. -1 if nothing found, this should be avoided via a call of
-     * 'isValid'
+     * Returns the closest matching node. This is either a tower node of the base graph
+     * or a virtual node (see also {@link com.graphhopper.routing.QueryGraph#lookup(List)}).
+     *
+     * @return -1 if nothing found, this should be avoided via a call of 'isValid'
      */
     public int getClosestNode() {
         return closestNode;
@@ -165,6 +169,9 @@ public class QueryResult {
     }
 
     /**
+     * Whether the query point is projected onto a tower node, pillar node or somewhere within
+     * the closest edge.
+     *
      * Due to precision differences it is hard to define when something is exactly 90° or "on-node"
      * like TOWER or PILLAR or if it is more "on-edge" (EDGE). The default mechanism is to prefer
      * "on-edge" even if it could be 90°. To prefer "on-node" you could use e.g. GHPoint.equals with
diff --git a/core/src/main/java/com/graphhopper/util/Helper.java b/core/src/main/java/com/graphhopper/util/Helper.java
index 108ed8a8e..9ec13d971 100644
--- a/core/src/main/java/com/graphhopper/util/Helper.java
+++ b/core/src/main/java/com/graphhopper/util/Helper.java
@@ -24,6 +24,10 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import java.io.*;
+import java.lang.invoke.MethodHandle;
+import java.lang.invoke.MethodHandles;
+import java.lang.invoke.MethodType;
+import java.lang.reflect.Field;
 import java.lang.reflect.Method;
 import java.nio.ByteBuffer;
 import java.nio.MappedByteBuffer;
@@ -361,44 +365,59 @@ public class Helper {
     }
 
     public static void cleanMappedByteBuffer(final ByteBuffer buffer) {
-
+        // TODO avoid reflection on every call
         try {
             AccessController.doPrivileged(new PrivilegedExceptionAction<Object>() {
                 @Override
                 public Object run() throws Exception {
-                    try {
-                        // <=JDK8 class DirectByteBuffer {         sun.misc.Cleaner cleaner(Buffer buf) }
-                        // >=JDK9 class DirectByteBuffer { jdk.internal.ref.Cleaner cleaner(Buffer buf) }
-                        // Regarding MappedByteBufferAdapter, see #914
-                        final Class<?> directByteBufferClass =
-                                buffer.getClass().getSimpleName().equals("MappedByteBufferAdapter") ?
-                                        Class.forName("java.nio.MappedByteBufferAdapter") : Class.forName("java.nio.DirectByteBuffer");
-                        if (Constants.ANDROID) {
-                            final Method dbbFreeMethod = directByteBufferClass.getMethod("free");
-                            dbbFreeMethod.setAccessible(true);
-                            // call DirectByteBuffer.free(buffer)
-                            dbbFreeMethod.invoke(buffer);
-                            return null;
-                        }
+                    // Regarding MappedByteBufferAdapter, see #914
+                    final Class<?> directByteBufferClass = buffer.getClass().getSimpleName().equals("MappedByteBufferAdapter") ?
+                            Class.forName("java.nio.MappedByteBufferAdapter") : Class.forName("java.nio.DirectByteBuffer");
+                    if (Constants.ANDROID) {
+                        final Method dbbFreeMethod = directByteBufferClass.getMethod("free");
+                        dbbFreeMethod.setAccessible(true);
+                        // call DirectByteBuffer.free(buffer)
+                        dbbFreeMethod.invoke(buffer);
+                        return null;
+                    }
 
+                    // <=JDK8 class DirectByteBuffer { sun.misc.Cleaner cleaner(Buffer buf) }
+                    //        then call sun.misc.Cleaner.clean
+                    try {
                         final Method dbbCleanerMethod = directByteBufferClass.getMethod("cleaner");
                         dbbCleanerMethod.setAccessible(true);
-                        // call DirectByteBuffer.cleaner(buffer)
+                        // now call DirectByteBuffer.cleaner(buffer)
                         final Object cleaner = dbbCleanerMethod.invoke(buffer);
                         if (cleaner != null) {
                             final Class<?> cleanerMethodReturnType = dbbCleanerMethod.getReturnType();
-                            final Method cleanMethod;
-                            if (Runnable.class.isAssignableFrom(cleanerMethodReturnType)) {
-                                // >=JDK9 
-                                cleanMethod = cleanerMethodReturnType.getDeclaredMethod("run");
-                            } else {
-                                cleanMethod = cleanerMethodReturnType.getDeclaredMethod("clean");
-                            }
+                            final Method cleanMethod = cleanerMethodReturnType.getDeclaredMethod("clean");
                             cleanMethod.setAccessible(true);
+                            // now call sun.misc.Cleaner.clean
                             cleanMethod.invoke(cleaner);
                         }
-                    } catch (NoSuchMethodException ex) {
-                        // ignore if method cleaner or clean is not available, like on Android
+                        System.out.println("JDK8 " + Constants.JAVA_VERSION + " " + cleaner);
+                    } catch (NoSuchMethodException ex2) {
+                        // ignore if method cleaner or clean is not available
+
+                    } catch (RuntimeException ex) {
+                        // a bit ugly as InaccessibleObjectException is available in JDK9 and not before
+                        if (!ex.getClass().getSimpleName().equals("InaccessibleObjectException"))
+                            throw ex;
+
+                        // >=JDK9 class sun.misc.Unsafe { void invokeCleaner(ByteBuffer buf) }
+                        try {
+                            final Class<?> unsafeClass = Class.forName("sun.misc.Unsafe");
+                            // we do not need to check for a specific class, we can call the Unsafe method with any buffer class
+                            MethodHandle unmapper = MethodHandles.lookup().findVirtual(unsafeClass, "invokeCleaner",
+                                    MethodType.methodType(void.class, ByteBuffer.class));
+                            // fetch the unsafe instance and bind it to the virtual MethodHandle
+                            final Field f = unsafeClass.getDeclaredField("theUnsafe");
+                            f.setAccessible(true);
+                            final Object theUnsafe = f.get(null);
+                            unmapper.bindTo(theUnsafe).invokeExact(buffer);
+                        } catch (Throwable ex2) {
+                            throw new RuntimeException(ex);
+                        }
                     }
                     return null;
                 }


Test patch:
diff --git a/core/src/test/java/com/graphhopper/reader/dem/SRTMProviderTest.java b/core/src/test/java/com/graphhopper/reader/dem/SRTMProviderTest.java
index 9d277b0a4..83fd21588 100644
--- a/core/src/test/java/com/graphhopper/reader/dem/SRTMProviderTest.java
+++ b/core/src/test/java/com/graphhopper/reader/dem/SRTMProviderTest.java
@@ -18,6 +18,7 @@
 package com.graphhopper.reader.dem;
 
 import com.graphhopper.storage.DAType;
+import com.graphhopper.util.Constants;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
@@ -96,7 +97,6 @@ public class SRTMProviderTest {
     @Test
     public void testGetHeightMMap() throws IOException {
         instance.setCacheDir(new File("./files/"));
-        instance.setDAType(DAType.MMAP);
         assertEquals(161, instance.getEle(55.8943144, -3), 1e-1);
     }
 }