tinkerpop-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From spmalle...@apache.org
Subject [34/45] tinkerpop git commit: TINKERPOP-1642 Improved performance of addV() and chained mutating statements
Date Mon, 20 Mar 2017 18:10:07 GMT
TINKERPOP-1642 Improved performance of addV() and chained mutating statements

Added more tests for Parameters. Performance improved by about 2.5x given the benchmarks.
Also long chained mutating traversals are now inserting roughly on par with single insert
traversals (prior to this change they were about 25% slower).


Project: http://git-wip-us.apache.org/repos/asf/tinkerpop/repo
Commit: http://git-wip-us.apache.org/repos/asf/tinkerpop/commit/8621401b
Tree: http://git-wip-us.apache.org/repos/asf/tinkerpop/tree/8621401b
Diff: http://git-wip-us.apache.org/repos/asf/tinkerpop/diff/8621401b

Branch: refs/heads/TINKERPOP-1642
Commit: 8621401b9a7a7a1b622fed3daefdabd0469cc0b0
Parents: 2138c8e
Author: Stephen Mallette <spmva@genoprime.com>
Authored: Thu Mar 2 10:44:58 2017 -0500
Committer: Stephen Mallette <spmva@genoprime.com>
Committed: Mon Mar 20 14:07:22 2017 -0400

----------------------------------------------------------------------
 .../traversal/dsl/graph/GraphTraversal.java     |   9 +-
 .../process/traversal/step/util/Parameters.java |  33 ++++-
 .../traversal/step/util/ParametersTest.java     | 119 ++++++++++++++++++-
 3 files changed, 146 insertions(+), 15 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/8621401b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversal.java
----------------------------------------------------------------------
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversal.java
b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversal.java
index 8070538..c89c7be 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversal.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversal.java
@@ -1010,7 +1010,6 @@ public interface GraphTraversal<S, E> extends Traversal<S,
E> {
         for (int i = 0; i < propertyKeyValues.length; i = i + 2) {
             this.property(propertyKeyValues[i], propertyKeyValues[i + 1]);
         }
-        //((AddVertexStep) this.asAdmin().getEndStep()).addPropertyMutations(propertyKeyValues);
         return (GraphTraversal<S, Vertex>) this;
     }
 
@@ -2058,11 +2057,13 @@ public interface GraphTraversal<S, E> extends Traversal<S,
E> {
             this.asAdmin().getBytecode().addStep(Symbols.property, key, value, keyValues);
         else
             this.asAdmin().getBytecode().addStep(Symbols.property, cardinality, key, value,
keyValues);
+
         // if it can be detected that this call to property() is related to an addV/E() then
we can attempt to fold
         // the properties into that step to gain an optimization for those graphs that support
such capabilities.
-        if ((this.asAdmin().getEndStep() instanceof AddVertexStep || this.asAdmin().getEndStep()
instanceof AddEdgeStep
-                || this.asAdmin().getEndStep() instanceof AddVertexStartStep) &&
keyValues.length == 0 && null == cardinality) {
-            ((Mutating) this.asAdmin().getEndStep()).addPropertyMutations(key, value);
+        final Step endStep = this.asAdmin().getEndStep();
+        if ((endStep instanceof AddVertexStep || endStep instanceof AddEdgeStep || endStep
instanceof AddVertexStartStep) &&
+                keyValues.length == 0 && null == cardinality) {
+            ((Mutating) endStep).addPropertyMutations(key, value);
         } else {
             this.asAdmin().addStep(new AddPropertyStep(this.asAdmin(), cardinality, key,
value));
             ((AddPropertyStep) this.asAdmin().getEndStep()).addPropertyMutations(keyValues);

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/8621401b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/Parameters.java
----------------------------------------------------------------------
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/Parameters.java
b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/Parameters.java
index 3584c87..67cb2f9 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/Parameters.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/Parameters.java
@@ -49,6 +49,11 @@ public final class Parameters implements Cloneable, Serializable {
     private Map<Object, List<Object>> parameters = new HashMap<>();
 
     /**
+     * Determines if there are traversals present in the parameters {@code Map}.
+     */
+    private boolean traversalsPresent = false;
+
+    /**
      * Checks for existence of key in parameter set.
      *
      * @param key the key to check
@@ -138,6 +143,11 @@ public final class Parameters implements Cloneable, Serializable {
         Parameters.legalPropertyKeyValueArray(keyValues);
         for (int i = 0; i < keyValues.length; i = i + 2) {
             if (keyValues[i + 1] != null) {
+                // track whether or not traversals are present so that elsewhere in Parameters
there is no need
+                // to iterate all values to not find any.
+                if (!traversalsPresent && keyValues[i + 1] instanceof Traversal.Admin)
+                    traversalsPresent = true;
+
                 List<Object> values = this.parameters.get(keyValues[i]);
                 if (null == values) {
                     values = new ArrayList<>();
@@ -150,18 +160,31 @@ public final class Parameters implements Cloneable, Serializable {
         }
     }
 
+    /**
+     * Calls {@link TraversalParent#integrateChild(Traversal.Admin)} on any traversal objects
in the parameter list.
+     * This method requires that {@link #set(Object...)} is called prior to this method as
the it will switch the
+     * {@code traversalsPresent} flag field if a {@link Traversal.Admin} object is present
and allow this method to
+     * do its work.
+     */
     public void integrateTraversals(final TraversalParent step) {
-        for (final List<Object> values : this.parameters.values()) {
-            for (final Object object : values) {
-                if (object instanceof Traversal.Admin) {
-                    step.integrateChild((Traversal.Admin) object);
+        if (traversalsPresent) {
+            for (final List<Object> values : this.parameters.values()) {
+                for (final Object object : values) {
+                    if (object instanceof Traversal.Admin) {
+                        step.integrateChild((Traversal.Admin) object);
+                    }
                 }
             }
         }
     }
 
+    /**
+     * Gets all the {@link Traversal.Admin} objects in the map of parameters.
+     */
     public <S, E> List<Traversal.Admin<S, E>> getTraversals() {
-        List<Traversal.Admin<S, E>> result = new ArrayList<>();
+        if (!traversalsPresent) return Collections.emptyList();
+
+        final List<Traversal.Admin<S, E>> result = new ArrayList<>();
         for (final List<Object> list : this.parameters.values()) {
             for (final Object object : list) {
                 if (object instanceof Traversal.Admin) {

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/8621401b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/ParametersTest.java
----------------------------------------------------------------------
diff --git a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/ParametersTest.java
b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/ParametersTest.java
index 7490dea..27b9293 100644
--- a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/ParametersTest.java
+++ b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/ParametersTest.java
@@ -18,8 +18,12 @@
  */
 package org.apache.tinkerpop.gremlin.process.traversal.step.util;
 
+import org.apache.tinkerpop.gremlin.process.traversal.Traverser;
+import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__;
+import org.apache.tinkerpop.gremlin.process.traversal.step.TraversalParent;
 import org.junit.Test;
 
+import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
@@ -28,12 +32,66 @@ import static org.hamcrest.CoreMatchers.is;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.contains;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotEquals;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
 
 /**
  * @author Stephen Mallette (http://stephen.genoprime.com)
  */
 public class ParametersTest {
     @Test
+    public void shouldGetKeyValuesEmpty() {
+        final Parameters parameters = new Parameters();
+        assertThat(Arrays.equals(parameters.getKeyValues(mock(Traverser.Admin.class)), new
Object[0]), is(true));
+    }
+
+    @Test
+    public void shouldGetKeyValues() {
+        final Parameters parameters = new Parameters();
+        parameters.set("a", "axe", "b", "bat", "c", "cat");
+
+        final Object[] params = parameters.getKeyValues(mock(Traverser.Admin.class));
+        assertEquals(6, params.length);
+        assertThat(Arrays.equals(new Object[] {"a", "axe", "b", "bat", "c", "cat"}, params),
is(true));
+    }
+
+    @Test
+    public void shouldGetKeyValuesIgnoringSomeKeys() {
+        final Parameters parameters = new Parameters();
+        parameters.set("a", "axe", "b", "bat", "c", "cat");
+
+        final Object[] params = parameters.getKeyValues(mock(Traverser.Admin.class), "b");
+        assertEquals(4, params.length);
+        assertThat(Arrays.equals(new Object[] {"a", "axe", "c", "cat"}, params), is(true));
+    }
+
+    @Test
+    public void shouldGetUsingTraverserOverload() {
+        final Parameters parameters = new Parameters();
+        parameters.set("a", "axe", "b", "bat", "c", "cat");
+
+        assertEquals(Collections.singletonList("axe"), parameters.get(mock(Traverser.Admin.class),
"a", () -> "x"));
+        assertEquals(Collections.singletonList("bat"), parameters.get(mock(Traverser.Admin.class),
"b", () -> "x"));
+        assertEquals(Collections.singletonList("cat"), parameters.get(mock(Traverser.Admin.class),
"c", () -> "x"));
+        assertEquals(Collections.singletonList("zebra"), parameters.get(mock(Traverser.Admin.class),
"z", () -> "zebra"));
+    }
+
+    @Test
+    public void shouldGetMultipleUsingTraverserOverload() {
+        final Parameters parameters = new Parameters();
+        parameters.set("a", "axe", "a", "ant", "b", "bat", "b", "ball", "c", "cat");
+
+        final Map<Object,List<Object>> params = parameters.getRaw();
+        assertEquals(3, params.size());
+        assertEquals(Arrays.asList("axe", "ant"), parameters.get(mock(Traverser.Admin.class),
"a", () -> "x"));
+        assertEquals(Arrays.asList("bat", "ball"), parameters.get(mock(Traverser.Admin.class),
"b", () -> "x"));
+        assertEquals(Collections.singletonList("cat"), parameters.get(mock(Traverser.Admin.class),
"c", () -> "x"));
+        assertEquals(Collections.singletonList("zebra"), parameters.get(mock(Traverser.Admin.class),
"z", () -> "zebra"));
+    }
+
+    @Test
     public void shouldGetRaw() {
         final Parameters parameters = new Parameters();
         parameters.set("a", "axe", "b", "bat", "c", "cat");
@@ -143,11 +201,60 @@ public class ParametersTest {
         final Parameters parameters = new Parameters();
         parameters.set("a", "axe", "b", "bat", "c", "cat");
 
-        final Map<Object,List<Object>> params = parameters.getRaw();
-        assertEquals(3, params.size());
-        assertEquals("axe", params.get("a").get(0));
-        assertEquals("bat", params.get("b").get(0));
-        assertEquals("cat", params.get("c").get(0));
-        assertEquals("zebra", parameters.get("z", () -> "zebra").get(0));
+        assertEquals(Collections.singletonList("axe"), parameters.get("a", () -> "x"));
+        assertEquals(Collections.singletonList("bat"), parameters.get("b", () -> "x"));
+        assertEquals(Collections.singletonList("cat"), parameters.get("c", () -> "x"));
+        assertEquals(Collections.singletonList("zebra"), parameters.get("z", () -> "zebra"));
+    }
+
+    @Test
+    public void shouldClone() {
+        final Parameters parameters = new Parameters();
+        parameters.set("a", "axe", "a", "ant", "b", "bat", "b", "ball", "c", "cat");
+
+        final Parameters parametersClone = parameters.clone();
+
+        assertEquals(parameters.getRaw(), parametersClone.getRaw());
+    }
+
+    @Test
+    public void shouldBeDifferent() {
+        final Parameters parameters = new Parameters();
+        parameters.set("a", "axe", "a", "ant", "b", "bat", "b", "ball", "c", "cat");
+
+        final Parameters parametersDifferent = new Parameters();
+        parametersDifferent.set("a", "ant", "a", "axe", "b", "bat", "b", "ball", "c", "cat");
+
+        assertNotEquals(parameters.getRaw(), parametersDifferent.getRaw());
+    }
+
+    @Test
+    public void shouldGetNoTraversals() {
+        final Parameters parameters = new Parameters();
+        parameters.set("a", "axe", "a", "ant", "b", "bat", "b", "ball", "c", "cat");
+        assertEquals(Collections.emptyList(), parameters.getTraversals());
+    }
+
+    @Test
+    public void shouldGetTraversals() {
+        final Parameters parameters = new Parameters();
+        parameters.set("a", "axe", "a", "ant", "b", "bat", "b", "ball", "c", "cat", "t",
__.outE("knows"));
+        assertEquals(Collections.singletonList(__.outE("knows")), parameters.getTraversals());
+    }
+
+    @Test
+    public void shouldIntegrateTraversals() {
+        final Parameters parameters = new Parameters();
+        parameters.set("a", "axe", "a", "ant", "b", "bat", "b", "ball", "c", "cat", "t",
__.outE("knows"));
+
+        final TraversalParent mock = mock(TraversalParent.class);
+
+        // the mock can return nothing of consequence as the return isn't used in this case.
we just need to
+        // validate that the method gets called as a result of calls to set/integrateTraversals()
+        when(mock.integrateChild(__.outE("knows").asAdmin())).thenReturn(null);
+
+        parameters.integrateTraversals(mock);
+
+        verify(mock).integrateChild(__.outE("knows").asAdmin());
     }
 }


Mime
View raw message