Optimise traversal to avoid checking for edge instances (typedb#2131)

* Add a param to `InIsaFragment` to go to edges * Optimise away edge traversal when unnecessary * Fix compile error
micheleorsi · Sep 26, 2017 · 6c0cdd1 · 6c0cdd1
1 parent b14211d
commit 6c0cdd1
Show file tree

Hide file tree

Showing 5 changed files with 77 additions and 20 deletions.
diff --git a/grakn-graql/src/main/java/ai/grakn/graql/internal/gremlin/fragment/Fragments.java b/grakn-graql/src/main/java/ai/grakn/graql/internal/gremlin/fragment/Fragments.java
@@ -90,8 +90,8 @@ public static Fragment outRelates(VarProperty varProperty, Var start, Var end) {
         return new AutoValue_OutRelatesFragment(varProperty, start, end);
     }
 
-    public static Fragment inIsa(VarProperty varProperty, Var start, Var end) {
-        return new AutoValue_InIsaFragment(varProperty, start, end);
+    public static Fragment inIsa(VarProperty varProperty, Var start, Var end, boolean mayHaveEdgeInstances) {
+        return new AutoValue_InIsaFragment(varProperty, start, end, mayHaveEdgeInstances);
     }
 
     public static Fragment outIsa(VarProperty varProperty, Var start, Var end) {

diff --git a/grakn-graql/src/main/java/ai/grakn/graql/internal/gremlin/fragment/InIsaFragment.java b/grakn-graql/src/main/java/ai/grakn/graql/internal/gremlin/fragment/InIsaFragment.java
@@ -54,22 +54,30 @@ abstract class InIsaFragment extends Fragment {
     @Override
     public abstract Var end();
 
+    abstract boolean mayHaveEdgeInstances();
+
     @Override
     public GraphTraversal<Vertex, ? extends Element> applyTraversalInner(
             GraphTraversal<Vertex, ? extends Element> traversal, GraknTx graph, Collection<Var> vars) {
 
-        GraphTraversal<Vertex, Vertex> isImplicitRelationType =
-                __.<Vertex>hasLabel(RELATIONSHIP_TYPE.name()).has(IS_IMPLICIT.name(), true);
+        GraphTraversal<Vertex, Vertex> vertexTraversal = Fragments.isVertex(traversal);
+
+        if (mayHaveEdgeInstances()) {
+            GraphTraversal<Vertex, Vertex> isImplicitRelationType =
+                    __.<Vertex>hasLabel(RELATIONSHIP_TYPE.name()).has(IS_IMPLICIT.name(), true);
 
-        GraphTraversal<Vertex, Element> toVertexAndEdgeInstances = Fragments.union(ImmutableSet.of(
-                toVertexInstances(__.identity()),
-                toEdgeInstances()
-        ));
+            GraphTraversal<Vertex, Element> toVertexAndEdgeInstances = Fragments.union(ImmutableSet.of(
+                    toVertexInstances(__.identity()),
+                    toEdgeInstances()
+            ));
 
-        return choose(Fragments.isVertex(traversal), isImplicitRelationType,
-                toVertexAndEdgeInstances,
-                toVertexInstances(__.identity())
-        );
+            return choose(vertexTraversal, isImplicitRelationType,
+                    toVertexAndEdgeInstances,
+                    toVertexInstances(__.identity())
+            );
+        } else {
+            return toVertexInstances(vertexTraversal);
+        }
     }
 
     /**
@@ -115,7 +123,7 @@ private GraphTraversal<Vertex, Edge> toEdgeInstances() {
 
     @Override
     public String name() {
-        return "<-[isa]-";
+        return String.format("<-[isa:%s]-", mayHaveEdgeInstances() ? "with-edges" : "");
     }
 
     @Override

diff --git a/grakn-graql/src/main/java/ai/grakn/graql/internal/gremlin/sets/EquivalentFragmentSets.java b/grakn-graql/src/main/java/ai/grakn/graql/internal/gremlin/sets/EquivalentFragmentSets.java
@@ -46,7 +46,8 @@ public class EquivalentFragmentSets {
             ResourceIndexFragmentSet.RESOURCE_INDEX_OPTIMISATION,
             RolePlayerFragmentSet.RELATION_TYPE_OPTIMISATION,
             LabelFragmentSet.REDUNDANT_LABEL_ELIMINATION_OPTIMISATION,
-            SubFragmentSet.SUB_TRAVERSAL_ELIMINATION_OPTIMISATION
+            SubFragmentSet.SUB_TRAVERSAL_ELIMINATION_OPTIMISATION,
+            IsaFragmentSet.SKIP_EDGE_INSTANCE_CHECK_OPTIMISATION
     );
 
     /**
@@ -92,7 +93,7 @@ public static EquivalentFragmentSet notInternalFragmentSet(VarProperty varProper
      * An {@link EquivalentFragmentSet} that indicates a variable is a direct instance of a type.
      */
     public static EquivalentFragmentSet isa(VarProperty varProperty, Var instance, Var type) {
-        return new IsaFragmentSet(varProperty, instance, type);
+        return new IsaFragmentSet(varProperty, instance, type, true);
     }
 
     /**

diff --git a/grakn-graql/src/main/java/ai/grakn/graql/internal/gremlin/sets/IsaFragmentSet.java b/grakn-graql/src/main/java/ai/grakn/graql/internal/gremlin/sets/IsaFragmentSet.java
@@ -19,23 +19,34 @@
 
 package ai.grakn.graql.internal.gremlin.sets;
 
+import ai.grakn.concept.SchemaConcept;
 import ai.grakn.graql.Var;
 import ai.grakn.graql.admin.VarProperty;
 import ai.grakn.graql.internal.gremlin.EquivalentFragmentSet;
 import ai.grakn.graql.internal.gremlin.fragment.Fragments;
 
+import static ai.grakn.graql.internal.gremlin.sets.EquivalentFragmentSets.fragmentSetOfType;
+import static ai.grakn.graql.internal.gremlin.sets.EquivalentFragmentSets.labelOf;
+
 /**
  * @author Felix Chapman
  */
 class IsaFragmentSet extends EquivalentFragmentSet {
 
+    private final VarProperty varProperty;
     private final Var instance;
     private final Var type;
+    private final boolean mayHaveEdgeInstances;
 
-    IsaFragmentSet(VarProperty varProperty, Var instance, Var type) {
-        super(Fragments.outIsa(varProperty, instance, type), Fragments.inIsa(varProperty, type, instance));
+    IsaFragmentSet(VarProperty varProperty, Var instance, Var type, boolean mayHaveEdgeInstances) {
+        super(
+                Fragments.outIsa(varProperty, instance, type),
+                Fragments.inIsa(varProperty, type, instance, mayHaveEdgeInstances)
+        );
+        this.varProperty = varProperty;
         this.instance = instance;
         this.type = type;
+        this.mayHaveEdgeInstances = mayHaveEdgeInstances;
     }
 
     Var instance() {
@@ -45,4 +56,41 @@ Var instance() {
     Var type() {
         return type;
     }
+
+    /**
+     * We can skip the mid-traversal check for edge instances in the following case:
+     *
+     * <ol>
+     *     <li>There is an {@link IsaFragmentSet} {@code $x-[isa:with-edges]->$X}
+     *     <li>There is a {@link LabelFragmentSet} {@code $X[label:foo,bar]}
+     *     <li>The labels {@code foo} and {@code bar} are all not types that may have edge instances</li>
+     * </ol>
+     */
+    static final FragmentSetOptimisation SKIP_EDGE_INSTANCE_CHECK_OPTIMISATION = (fragments, tx) -> {
+        Iterable<IsaFragmentSet> isaSets = fragmentSetOfType(IsaFragmentSet.class, fragments)::iterator;
+
+        for (IsaFragmentSet isaSet : isaSets) {
+            if (!isaSet.mayHaveEdgeInstances) continue;
+
+            LabelFragmentSet labelSet = labelOf(isaSet.type(), fragments);
+
+            if (labelSet == null) continue;
+
+            boolean mayHaveEdgeInstances = labelSet.labels().stream()
+                    .map(tx::<SchemaConcept>getSchemaConcept)
+                    .anyMatch(IsaFragmentSet::mayHaveEdgeInstances);
+
+            if (!mayHaveEdgeInstances) {
+                fragments.remove(isaSet);
+                fragments.add(new IsaFragmentSet(isaSet.varProperty, isaSet.instance, isaSet.type, false));
+                return true;
+            }
+        }
+
+        return false;
+    };
+
+    private static boolean mayHaveEdgeInstances(SchemaConcept concept) {
+        return concept.isRelationshipType() && concept.isImplicit();
+    }
 }
diff --git a/grakn-graql/src/test/java/ai/grakn/graql/internal/gremlin/GraqlTraversalTest.java b/grakn-graql/src/test/java/ai/grakn/graql/internal/gremlin/GraqlTraversalTest.java
@@ -90,7 +90,7 @@ public class GraqlTraversalTest {
     private static final Fragment xValue = value(null, x, eq("hello"));
     private static final Fragment yId = id(null, y, ConceptId.of("movie"));
     private static final Fragment xIsaY = outIsa(null, x, y);
-    private static final Fragment yTypeOfX = inIsa(null, y, x);
+    private static final Fragment yTypeOfX = inIsa(null, y, x, true);
 
     private static final GraqlTraversal fastIsaTraversal = traversal(yId, yTypeOfX);
     private static GraknTx tx;
@@ -136,7 +136,7 @@ public void testComplexityFastIsaVsSlowIsa() {
     @Test
     public void testComplexityConnectedVsDisconnected() {
         GraqlTraversal connectedDoubleIsa = traversal(xIsaY, outIsa(null, y, z));
-        GraqlTraversal disconnectedDoubleIsa = traversal(xIsaY, inIsa(null, z, y));
+        GraqlTraversal disconnectedDoubleIsa = traversal(xIsaY, inIsa(null, z, y, true));
         assertFaster(connectedDoubleIsa, disconnectedDoubleIsa);
     }
 
@@ -159,7 +159,7 @@ public void testResourceWithTypeFasterFromType() {
         GraqlTraversal fromInstance =
                 traversal(outIsa(null, x, xx), id(null, xx, ConceptId.of("_")), inRolePlayer(x, z), outRolePlayer(z, y));
         GraqlTraversal fromType =
-                traversal(id(null, xx, ConceptId.of("_")), inIsa(null, xx, x), inRolePlayer(x, z), outRolePlayer(z, y));
+                traversal(id(null, xx, ConceptId.of("_")), inIsa(null, xx, x, true), inRolePlayer(x, z), outRolePlayer(z, y));
         assertFaster(fromType, fromInstance);
     }