Opt fixes and specifics strategy simplification (#665)

* Fixed bug with fastutil equals * Simplified specifics strategy * Renamed fastutil classes
jgrapht · Sep 3, 2018 · 32dd8f3 · 32dd8f3
1 parent 43dd81f
commit 32dd8f3
Show file tree

Hide file tree

Showing 19 changed files with 555 additions and 306 deletions.
diff --git a/jgrapht-core/src/main/java/org/jgrapht/graph/DefaultGraphSpecificsStrategy.java b/jgrapht-core/src/main/java/org/jgrapht/graph/DefaultGraphSpecificsStrategy.java
@@ -1,14 +1,14 @@
 package org.jgrapht.graph;
 
 import java.io.Serializable;
+import java.util.LinkedHashMap;
 import java.util.function.BiFunction;
+import java.util.function.Function;
 
 import org.jgrapht.Graph;
 import org.jgrapht.GraphType;
-import org.jgrapht.graph.specifics.DirectedEdgeContainer;
 import org.jgrapht.graph.specifics.DirectedSpecifics;
 import org.jgrapht.graph.specifics.Specifics;
-import org.jgrapht.graph.specifics.UndirectedEdgeContainer;
 import org.jgrapht.graph.specifics.UndirectedSpecifics;
 
 /**
@@ -26,29 +26,34 @@
  * @param <E> the graph edge type
  */
 public class DefaultGraphSpecificsStrategy<V, E>
-    implements GraphSpecificsStrategy<V, E>
+    implements
+    GraphSpecificsStrategy<V, E>
 {
     private static final long serialVersionUID = 7615319421753562075L;
 
-    /**
-     * Get a function which creates the specifics. The factory will accept the graph type as a
-     * parameter.
-     * 
-     * @return a function which creates intrusive edges specifics.
-     */
+    @Override
+    public Function<GraphType, IntrusiveEdgesSpecifics<V, E>> getIntrusiveEdgesSpecificsFactory()
+    {
+        return (Function<GraphType, IntrusiveEdgesSpecifics<V, E>> & Serializable) (type) -> {
+            if (type.isWeighted()) {
+                return new WeightedIntrusiveEdgesSpecifics<V, E>(new LinkedHashMap<>());
+            } else {
+                return new UniformIntrusiveEdgesSpecifics<>(new LinkedHashMap<>());
+            }
+        };
+    }
+
     @Override
     public BiFunction<Graph<V, E>, GraphType, Specifics<V, E>> getSpecificsFactory()
     {
         return (BiFunction<Graph<V, E>, GraphType,
             Specifics<V, E>> & Serializable) (graph, type) -> {
                 if (type.isDirected()) {
-                    return new DirectedSpecifics<>(graph, this
-                        .<V, DirectedEdgeContainer<V, E>> getPredictableOrderMapFactory()
-                        .get(), getEdgeSetFactory());
+                    return new DirectedSpecifics<V, E>(
+                        graph, new LinkedHashMap<>(), getEdgeSetFactory());
                 } else {
-                    return new UndirectedSpecifics<>(graph, this
-                        .<V, UndirectedEdgeContainer<V, E>> getPredictableOrderMapFactory()
-                        .get(), getEdgeSetFactory());
+                    return new UndirectedSpecifics<>(
+                        graph, new LinkedHashMap<>(), getEdgeSetFactory());
                 }
             };
     }

diff --git a/jgrapht-core/src/main/java/org/jgrapht/graph/FastLookupGraphSpecificsStrategy.java b/jgrapht-core/src/main/java/org/jgrapht/graph/FastLookupGraphSpecificsStrategy.java
@@ -1,17 +1,16 @@
 package org.jgrapht.graph;
 
 import java.io.Serializable;
-import java.util.Set;
+import java.util.HashMap;
+import java.util.LinkedHashMap;
 import java.util.function.BiFunction;
+import java.util.function.Function;
 
 import org.jgrapht.Graph;
 import org.jgrapht.GraphType;
-import org.jgrapht.alg.util.Pair;
-import org.jgrapht.graph.specifics.DirectedEdgeContainer;
 import org.jgrapht.graph.specifics.FastLookupDirectedSpecifics;
 import org.jgrapht.graph.specifics.FastLookupUndirectedSpecifics;
 import org.jgrapht.graph.specifics.Specifics;
-import org.jgrapht.graph.specifics.UndirectedEdgeContainer;
 
 /**
  * The fast lookup specifics strategy implementation.
@@ -32,27 +31,28 @@ public class FastLookupGraphSpecificsStrategy<V, E>
 {
     private static final long serialVersionUID = -5490869870275054280L;
 
-    /**
-     * Get a function which creates the specifics. The factory will accept the graph type as a
-     * parameter.
-     * 
-     * @return a function which creates intrusive edges specifics.
-     */
+    @Override
+    public Function<GraphType, IntrusiveEdgesSpecifics<V, E>> getIntrusiveEdgesSpecificsFactory() { 
+        return (Function<GraphType, IntrusiveEdgesSpecifics<V, E>> & Serializable) (type) -> {
+            if (type.isWeighted()) {
+                return new WeightedIntrusiveEdgesSpecifics<V, E>(new LinkedHashMap<>());
+            } else {
+                return new UniformIntrusiveEdgesSpecifics<>(new LinkedHashMap<>());
+            }
+        };
+    }
+
     @Override
     public BiFunction<Graph<V, E>, GraphType, Specifics<V, E>> getSpecificsFactory()
     {
         return (BiFunction<Graph<V, E>, GraphType,
             Specifics<V, E>> & Serializable) (graph, type) -> {
                 if (type.isDirected()) {
-                    return new FastLookupDirectedSpecifics<>(graph, this
-                        .<V, DirectedEdgeContainer<V, E>> getPredictableOrderMapFactory()
-                        .get(), this.<Pair<V, V>, Set<E>> getMapFactory().get(),
-                        getEdgeSetFactory());
+                    return new FastLookupDirectedSpecifics<>(
+                        graph, new LinkedHashMap<>(), new HashMap<>(), getEdgeSetFactory());
                 } else {
-                    return new FastLookupUndirectedSpecifics<>(graph, this
-                        .<V, UndirectedEdgeContainer<V, E>> getPredictableOrderMapFactory()
-                        .get(), this.<Pair<V, V>, Set<E>> getMapFactory().get(),
-                        getEdgeSetFactory());
+                    return new FastLookupUndirectedSpecifics<>(
+                        graph, new LinkedHashMap<>(), new HashMap<>(), getEdgeSetFactory());
                 }
             };
     }

diff --git a/jgrapht-core/src/main/java/org/jgrapht/graph/GraphSpecificsStrategy.java b/jgrapht-core/src/main/java/org/jgrapht/graph/GraphSpecificsStrategy.java
@@ -1,12 +1,8 @@
 package org.jgrapht.graph;
 
 import java.io.Serializable;
-import java.util.HashMap;
-import java.util.LinkedHashMap;
-import java.util.Map;
 import java.util.function.BiFunction;
 import java.util.function.Function;
-import java.util.function.Supplier;
 
 import org.jgrapht.Graph;
 import org.jgrapht.GraphType;
@@ -16,7 +12,8 @@
 /**
  * A graph specifics construction factory.
  * 
- * <p>Such a strategy can be used to adjust the internals of the default graph implementations. 
+ * <p>
+ * Such a strategy can be used to adjust the internals of the default graph implementations.
  * 
  * @author Dimitrios Michail
  *
@@ -26,26 +23,21 @@
  * @see FastLookupGraphSpecificsStrategy
  * @see DefaultGraphSpecificsStrategy
  */
-public interface GraphSpecificsStrategy<V, E> extends Serializable
+public interface GraphSpecificsStrategy<V, E>
+    extends
+    Serializable
 {
     /**
      * Get a function which creates the intrusive edges specifics. The factory will accept the graph
      * type as a parameter.
      * 
+     * <p>
+     * Note that it is very important to use a map implementation which respects iteration order.
+     * 
      * @return a function which creates intrusive edges specifics.
      */
-    default Function<GraphType, IntrusiveEdgesSpecifics<V, E>> getIntrusiveEdgesSpecificsFactory() { 
-        return (Function<GraphType, IntrusiveEdgesSpecifics<V, E>> & Serializable) (type) -> {
-            if (type.isWeighted()) {
-                return new WeightedIntrusiveEdgesSpecifics<V, E>(
-                    this.<E, IntrusiveWeightedEdge> getPredictableOrderMapFactory().get());
-            } else {
-                return new UniformIntrusiveEdgesSpecifics<>(
-                    this.<E, IntrusiveEdge> getPredictableOrderMapFactory().get());
-            }
-        };
-    }
-
+    Function<GraphType, IntrusiveEdgesSpecifics<V, E>> getIntrusiveEdgesSpecificsFactory();
+
     /**
      * Get a function which creates the specifics. The factory will accept the graph type as a
      * parameter.
@@ -54,30 +46,6 @@ default Function<GraphType, IntrusiveEdgesSpecifics<V, E>> getIntrusiveEdgesSpec
      */
     BiFunction<Graph<V, E>, GraphType, Specifics<V, E>> getSpecificsFactory();
 
-    /**
-     * Get a supplier of maps with a predictable iteration order.
-     * 
-     * @return a supplier of maps with a predictable iteration order.
-     * @param <K1> the key type
-     * @param <V1> the value type 
-     */
-    default <K1, V1> Supplier<Map<K1, V1>> getPredictableOrderMapFactory()
-    {
-        return (Supplier<Map<K1, V1>> & Serializable)() -> new LinkedHashMap<>();
-    }
-
-    /**
-     * Get a supplier of maps.
-     * 
-     * @return a supplier of maps.
-     * @param <K1> the key type
-     * @param <V1> the value type 
-     */
-    default <K1, V1> Supplier<Map<K1, V1>> getMapFactory()
-    {
-        return (Supplier<Map<K1, V1>> & Serializable)() -> new HashMap<>();
-    }
-
     /**
      * Get an edge set factory.
      * 

diff --git a/jgrapht-core/src/test/java/org/jgrapht/alg/decomposition/HeavyPathDecompositionTest.java b/jgrapht-core/src/test/java/org/jgrapht/alg/decomposition/HeavyPathDecompositionTest.java
@@ -312,6 +312,7 @@ public void testDisconnectedSmallGraph(){
     }
 
     @Test
+    @Category(SlowTests.class)
     public void testRandomTrees(){
         final int NUM_TESTS = 100;
         Random random = new Random(0x2882);
@@ -336,6 +337,7 @@ public void testRandomTrees(){
     }
 
     @Test
+    @Category(SlowTests.class)
     public void testRandomForests(){
         final int NUM_TESTS = 1000;
         Random random = new Random(0x1881);
@@ -361,8 +363,8 @@ public void testRandomForests(){
         }
     }
 
-    @Category(SlowTests.class)
     @Test
+    @Category(SlowTests.class)    
     public void testHugeTree(){
         Random random = new Random(0x118811);
 

diff --git a/...pht-core/src/test/java/org/jgrapht/alg/isomorphism/AHUForestIsomorphismInspectorTest.java b/...pht-core/src/test/java/org/jgrapht/alg/isomorphism/AHUForestIsomorphismInspectorTest.java
@@ -18,13 +18,15 @@
 package org.jgrapht.alg.isomorphism;
 
 import org.jgrapht.Graph;
+import org.jgrapht.SlowTests;
 import org.jgrapht.alg.connectivity.ConnectivityInspector;
 import org.jgrapht.alg.util.Pair;
 import org.jgrapht.graph.DefaultEdge;
 import org.jgrapht.graph.SimpleGraph;
 import org.jgrapht.util.SupplierUtil;
 import org.junit.Assert;
 import org.junit.Test;
+import org.junit.experimental.categories.Category;
 
 import java.util.*;
 import java.util.stream.Collectors;
@@ -166,6 +168,7 @@ public void testSmallForest2(){
     }
 
     @Test
+    @Category(SlowTests.class)
     public void testHugeNumberOfChildren(){
         final int N = 100_000;
         Graph<Integer, DefaultEdge> tree1 = new SimpleGraph<>(DefaultEdge.class);
@@ -194,6 +197,7 @@ public void testHugeNumberOfChildren(){
     }
 
     @Test
+    @Category(SlowTests.class)
     public void testRandomForests(){
         Random random = new Random(0x2312);
         final int NUM_TESTS = 1000;
@@ -223,6 +227,7 @@ public void testRandomForests(){
     }
 
     @Test
+    @Category(SlowTests.class)
     public void testHugeRandomForest(){
         final int N = 50_000;
         Graph<Integer, DefaultEdge> tree1 = generateForest(N, new Random(0x88));

diff --git a/...re/src/test/java/org/jgrapht/alg/isomorphism/AHUUnrootedTreeIsomorphismInspectorTest.java b/...re/src/test/java/org/jgrapht/alg/isomorphism/AHUUnrootedTreeIsomorphismInspectorTest.java
@@ -18,12 +18,14 @@
 package org.jgrapht.alg.isomorphism;
 
 import org.jgrapht.Graph;
+import org.jgrapht.SlowTests;
 import org.jgrapht.alg.util.Pair;
 import org.jgrapht.graph.DefaultEdge;
 import org.jgrapht.graph.SimpleGraph;
 import org.jgrapht.util.SupplierUtil;
 import org.junit.Assert;
 import org.junit.Test;
+import org.junit.experimental.categories.Category;
 
 import java.util.Arrays;
 import java.util.HashSet;
@@ -69,17 +71,15 @@ public void testSingleVertex(){
 
     @Test(expected = NullPointerException.class)
     public void testNullGraphs(){
-        AHUUnrootedTreeIsomorphismInspector<String, DefaultEdge> isomorphism =
-                new AHUUnrootedTreeIsomorphismInspector<>(null, null);
+        new AHUUnrootedTreeIsomorphismInspector<>(null, null);
     }
 
     @Test(expected = NullPointerException.class)
     public void testOnlyOneNullGraph(){
         Graph<String, DefaultEdge> tree1 = new SimpleGraph<>(DefaultEdge.class);
         tree1.addVertex("a");
 
-        AHUUnrootedTreeIsomorphismInspector<String, DefaultEdge> isomorphism =
-                new AHUUnrootedTreeIsomorphismInspector<>(tree1, null);
+        new AHUUnrootedTreeIsomorphismInspector<>(tree1, null);
     }
 
     @Test
@@ -348,6 +348,7 @@ public void testInvalidRoot(){
     }
 
     @Test
+    @Category(SlowTests.class)
     public void testLineGraph(){
         final int N = 20_000;
         Graph<Integer, DefaultEdge> tree1 = new SimpleGraph<>(DefaultEdge.class);
@@ -364,7 +365,7 @@ public void testLineGraph(){
                 generateIsomorphicGraph(tree1, new Random(0x88));
 
         Graph<Integer, DefaultEdge> tree2 = pair.getFirst();
-        Map<Integer, Integer> mapping = pair.getSecond();
+        pair.getSecond();
 
         AHUUnrootedTreeIsomorphismInspector<Integer, DefaultEdge> isomorphism =
                 new AHUUnrootedTreeIsomorphismInspector<>(tree1, tree2);
@@ -375,6 +376,7 @@ public void testLineGraph(){
     }
 
     @Test
+    @Category(SlowTests.class)
     public void testHugeNumberOfChildren(){
         final int N = 100_000;
         Graph<Integer, DefaultEdge> tree1 = new SimpleGraph<>(DefaultEdge.class);
@@ -401,6 +403,7 @@ public void testHugeNumberOfChildren(){
     }
 
     @Test
+    @Category(SlowTests.class)
     public void testHugeRandomTree(){
         final int N = 50_000;
         Graph<Integer, DefaultEdge> tree1 = generateTree(N, new Random(0x88));