tinkerpop-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ok...@apache.org
Subject [11/19] tinkerpop git commit: TINKERPOP-1642 Pushed integrateChild() operations into Parameters.set()
Date Thu, 30 Mar 2017 21:03:55 GMT
TINKERPOP-1642 Pushed integrateChild() operations into Parameters.set()

There was no need to continually iterate the traversal list and doing integrateChild() over
and over again for each newly added traversal. Removing those wasteful cycles and only doing
that once, when the Traversal was added helped improve performance.


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

Branch: refs/heads/master
Commit: 181558cea654ad0f7e351755bd02d952408a0944
Parents: 12c7f01
Author: Stephen Mallette <spmva@genoprime.com>
Authored: Mon Mar 13 14:50:09 2017 -0400
Committer: Stephen Mallette <spmva@genoprime.com>
Committed: Wed Mar 29 11:22:57 2017 -0400

----------------------------------------------------------------------
 .../process/traversal/step/map/AddEdgeStep.java | 15 +++----
 .../traversal/step/map/AddVertexStartStep.java  |  6 +--
 .../traversal/step/map/AddVertexStep.java       |  6 +--
 .../step/sideEffect/AddPropertyStep.java        |  7 +--
 .../process/traversal/step/util/Parameters.java | 19 ++------
 .../strategy/decoration/ElementIdStrategy.java  |  2 +-
 .../traversal/step/util/ParametersTest.java     | 46 ++++++++++----------
 7 files changed, 39 insertions(+), 62 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/181558ce/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/AddEdgeStep.java
----------------------------------------------------------------------
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/AddEdgeStep.java
b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/AddEdgeStep.java
index 3a46c0d..03a2fa7 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/AddEdgeStep.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/AddEdgeStep.java
@@ -57,7 +57,7 @@ public final class AddEdgeStep<S> extends MapStep<S, Edge>
 
     public AddEdgeStep(final Traversal.Admin traversal, final String edgeLabel) {
         super(traversal);
-        this.parameters.set(T.label, edgeLabel);
+        this.parameters.set(this, T.label, edgeLabel);
     }
 
     @Override
@@ -77,20 +77,17 @@ public final class AddEdgeStep<S> extends MapStep<S, Edge>
 
     @Override
     public void addPropertyMutations(final Object... keyValues) {
-        this.parameters.set(keyValues);
-        this.parameters.integrateTraversals(this);
+        this.parameters.set(this, keyValues);
     }
 
     @Override
-    public void addTo(final Traversal.Admin<?, ?> toObject) {
-        this.parameters.set(TO, toObject);
-        this.parameters.integrateTraversals(this);
+    public void addTo(final Traversal.Admin<?,?> toObject) {
+        this.parameters.set(this, TO, toObject);
     }
 
     @Override
-    public void addFrom(final Traversal.Admin<?, ?> fromObject) {
-        this.parameters.set(FROM, fromObject);
-        this.parameters.integrateTraversals(this);
+    public void addFrom(final Traversal.Admin<?,?> fromObject) {
+        this.parameters.set(this, FROM, fromObject);
     }
 
     @Override

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/181558ce/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/AddVertexStartStep.java
----------------------------------------------------------------------
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/AddVertexStartStep.java
b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/AddVertexStartStep.java
index 064fc79..7de8839 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/AddVertexStartStep.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/AddVertexStartStep.java
@@ -54,8 +54,7 @@ public final class AddVertexStartStep extends AbstractStep<Vertex, Vertex>
 
     public AddVertexStartStep(final Traversal.Admin traversal, final String label) {
         super(traversal);
-        this.parameters.set(T.label, label);
-        this.parameters.integrateTraversals(this);
+        this.parameters.set(this, T.label, label);
     }
 
     @Override
@@ -75,8 +74,7 @@ public final class AddVertexStartStep extends AbstractStep<Vertex, Vertex>
 
     @Override
     public void addPropertyMutations(final Object... keyValues) {
-        this.parameters.set(keyValues);
-        this.parameters.integrateTraversals(this);
+        this.parameters.set(this, keyValues);
     }
 
     @Override

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/181558ce/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/AddVertexStep.java
----------------------------------------------------------------------
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/AddVertexStep.java
b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/AddVertexStep.java
index 69ee6e5..c35fa80 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/AddVertexStep.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/AddVertexStep.java
@@ -49,8 +49,7 @@ public final class AddVertexStep<S> extends MapStep<S, Vertex>
 
     public AddVertexStep(final Traversal.Admin traversal, final String label) {
         super(traversal);
-        this.parameters.set(T.label, label);
-        this.parameters.integrateTraversals(this);
+        this.parameters.set(this, T.label, label);
     }
 
     @Override
@@ -70,8 +69,7 @@ public final class AddVertexStep<S> extends MapStep<S, Vertex>
 
     @Override
     public void addPropertyMutations(final Object... keyValues) {
-        this.parameters.set(keyValues);
-        this.parameters.integrateTraversals(this);
+        this.parameters.set(this, keyValues);
     }
 
     @Override

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/181558ce/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/AddPropertyStep.java
----------------------------------------------------------------------
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/AddPropertyStep.java
b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/AddPropertyStep.java
index 85292b7..3de8932 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/AddPropertyStep.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/AddPropertyStep.java
@@ -55,10 +55,8 @@ public final class AddPropertyStep<S extends Element> extends SideEffectStep<S>
 
     public AddPropertyStep(final Traversal.Admin traversal, final VertexProperty.Cardinality
cardinality, final Object keyObject, final Object valueObject) {
         super(traversal);
-        this.parameters.set(T.key, keyObject);
-        this.parameters.set(T.value, valueObject);
+        this.parameters.set(this, T.key, keyObject, T.value, valueObject);
         this.cardinality = cardinality;
-        this.parameters.integrateTraversals(this);
     }
 
     @Override
@@ -78,8 +76,7 @@ public final class AddPropertyStep<S extends Element> extends SideEffectStep<S>
 
     @Override
     public void addPropertyMutations(final Object... keyValues) {
-        this.parameters.set(keyValues);
-        this.parameters.integrateTraversals(this);
+        this.parameters.set(this, keyValues);
     }
 
     @Override

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/181558ce/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 4b38d2e..0583ed2 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
@@ -55,8 +55,8 @@ public final class Parameters implements Cloneable, Serializable {
 
     /**
      * A cached list of traversals that serve as parameter values. The list is cached on
calls to
-     * {@link #set(Object...)} because when the parameter map is large the cost of iterating
it repeatedly on the
-     * high number of calls to {@link #getTraversals()} and {@link #integrateTraversals(TraversalParent)}
is great.
+     * {@link #set(TraversalParent, Object...)} because when the parameter map is large the
cost of iterating it repeatedly on the
+     * high number of calls to {@link #getTraversals()} is great.
      */
     private List<Traversal.Admin<?,?>> traversals = new ArrayList<>();
 
@@ -170,7 +170,7 @@ public final class Parameters implements Cloneable, Serializable {
     /**
      * Set parameters given key/value pairs.
      */
-    public void set(final Object... keyValues) {
+    public void set(final TraversalParent parent, final Object... keyValues) {
         Parameters.legalPropertyKeyValueArray(keyValues);
         for (int i = 0; i < keyValues.length; i = i + 2) {
             if (keyValues[i + 1] != null) {
@@ -179,6 +179,7 @@ public final class Parameters implements Cloneable, Serializable {
                 if (keyValues[i + 1] instanceof Traversal.Admin) {
                     final Traversal.Admin t = (Traversal.Admin) keyValues[i + 1];
                     addTraversal(t);
+                    if (parent != null) parent.integrateChild(t);
                 }
 
                 List<Object> values = this.parameters.get(keyValues[i]);
@@ -194,18 +195,6 @@ 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 (Traversal.Admin t : traversals) {
-            step.integrateChild(t);
-        }
-    }
-
-    /**
      * Gets all the {@link Traversal.Admin} objects in the map of parameters.
      */
     public <S, E> List<Traversal.Admin<S, E>> getTraversals() {

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/181558ce/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/decoration/ElementIdStrategy.java
----------------------------------------------------------------------
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/decoration/ElementIdStrategy.java
b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/decoration/ElementIdStrategy.java
index 32437e1..c8f9d22 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/decoration/ElementIdStrategy.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/decoration/ElementIdStrategy.java
@@ -111,7 +111,7 @@ public final class ElementIdStrategy extends AbstractTraversalStrategy<Traversal
                 if (parameterizing.getParameters().contains(T.id))
                     parameterizing.getParameters().rename(T.id, this.idPropertyKey);
                 else if (!parameterizing.getParameters().contains(this.idPropertyKey))
-                    parameterizing.getParameters().set(this.idPropertyKey, idMaker.get());
+                    parameterizing.getParameters().set(null, this.idPropertyKey, idMaker.get());
             }
         });
     }

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/181558ce/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 9c96c1c..ebe40af 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
@@ -52,7 +52,7 @@ public class ParametersTest {
     @Test
     public void shouldGetKeyValues() {
         final Parameters parameters = new Parameters();
-        parameters.set("a", "axe", "b", "bat", "c", "cat");
+        parameters.set(null, "a", "axe", "b", "bat", "c", "cat");
 
         final Object[] params = parameters.getKeyValues(mock(Traverser.Admin.class));
         assertEquals(6, params.length);
@@ -62,7 +62,7 @@ public class ParametersTest {
     @Test
     public void shouldGetKeyValuesIgnoringSomeKeys() {
         final Parameters parameters = new Parameters();
-        parameters.set("a", "axe", "b", "bat", "c", "cat");
+        parameters.set(null, "a", "axe", "b", "bat", "c", "cat");
 
         final Object[] params = parameters.getKeyValues(mock(Traverser.Admin.class), "b");
         assertEquals(4, params.length);
@@ -72,7 +72,7 @@ public class ParametersTest {
     @Test
     public void shouldGetUsingTraverserOverload() {
         final Parameters parameters = new Parameters();
-        parameters.set("a", "axe", "b", "bat", "c", "cat");
+        parameters.set(null, "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"));
@@ -83,7 +83,7 @@ public class ParametersTest {
     @Test
     public void shouldGetMultipleUsingTraverserOverload() {
         final Parameters parameters = new Parameters();
-        parameters.set("a", "axe", "a", "ant", "b", "bat", "b", "ball", "c", "cat");
+        parameters.set(null, "a", "axe", "a", "ant", "b", "bat", "b", "ball", "c", "cat");
 
         final Map<Object,List<Object>> params = parameters.getRaw();
         assertEquals(3, params.size());
@@ -96,7 +96,7 @@ public class ParametersTest {
     @Test
     public void shouldGetRaw() {
         final Parameters parameters = new Parameters();
-        parameters.set("a", "axe", "b", "bat", "c", "cat");
+        parameters.set(null, "a", "axe", "b", "bat", "c", "cat");
 
         final Map<Object,List<Object>> params = parameters.getRaw();
         assertEquals(3, params.size());
@@ -108,7 +108,7 @@ public class ParametersTest {
     @Test
     public void shouldGetRawWithMulti() {
         final Parameters parameters = new Parameters();
-        parameters.set("a", "axe", "b", "bat", "a", "ant", "c", "cat");
+        parameters.set(null, "a", "axe", "b", "bat", "a", "ant", "c", "cat");
 
         final Map<Object,List<Object>> params = parameters.getRaw();
         assertEquals(3, params.size());
@@ -127,7 +127,7 @@ public class ParametersTest {
     @Test
     public void shouldGetRawExcept() {
         final Parameters parameters = new Parameters();
-        parameters.set("a", "axe", "b", "bat", "c", "cat");
+        parameters.set(null, "a", "axe", "b", "bat", "c", "cat");
 
         final Map<Object,List<Object>> params = parameters.getRaw("b");
         assertEquals(2, params.size());
@@ -138,7 +138,7 @@ public class ParametersTest {
     @Test
     public void shouldRemove() {
         final Parameters parameters = new Parameters();
-        parameters.set("a", "axe", "b", "bat", "c", "cat");
+        parameters.set(null, "a", "axe", "b", "bat", "c", "cat");
 
         final Map<Object,List<Object>> before = parameters.getRaw();
         assertEquals(3, before.size());
@@ -157,7 +157,7 @@ public class ParametersTest {
     @Test
     public void shouldRemoveRefreshTraversalCache() {
         final Parameters parameters = new Parameters();
-        parameters.set("a", "axe", "b", "bat", "c", "cat", "c", mock(Traversal.Admin.class),
"t", mock(Traversal.Admin.class));
+        parameters.set(null, "a", "axe", "b", "bat", "c", "cat", "c", mock(Traversal.Admin.class),
"t", mock(Traversal.Admin.class));
 
         final Map<Object,List<Object>> before = parameters.getRaw();
         assertEquals(4, before.size());
@@ -190,7 +190,7 @@ public class ParametersTest {
     @Test
     public void shouldRename() {
         final Parameters parameters = new Parameters();
-        parameters.set("a", "axe", "b", "bat", "c", "cat");
+        parameters.set(null, "a", "axe", "b", "bat", "c", "cat");
 
         parameters.rename("a", "z");
 
@@ -204,7 +204,7 @@ public class ParametersTest {
     @Test
     public void shouldContainKey() {
         final Parameters parameters = new Parameters();
-        parameters.set("a", "axe", "b", "bat", "c", "cat");
+        parameters.set(null, "a", "axe", "b", "bat", "c", "cat");
 
         assertThat(parameters.contains("b"), is(true));
     }
@@ -212,7 +212,7 @@ public class ParametersTest {
     @Test
     public void shouldNotContainKey() {
         final Parameters parameters = new Parameters();
-        parameters.set("a", "axe", "b", "bat", "c", "cat");
+        parameters.set(null, "a", "axe", "b", "bat", "c", "cat");
 
         assertThat(parameters.contains("z"), is(false));
     }
@@ -220,7 +220,7 @@ public class ParametersTest {
     @Test
     public void shouldGetSetMultiple() {
         final Parameters parameters = new Parameters();
-        parameters.set("a", "axe", "a", "ant", "b", "bat", "b", "ball", "c", "cat");
+        parameters.set(null, "a", "axe", "a", "ant", "b", "bat", "b", "ball", "c", "cat");
 
         final Map<Object,List<Object>> params = parameters.getRaw();
         assertEquals(3, params.size());
@@ -234,7 +234,7 @@ public class ParametersTest {
     @Test
     public void shouldGetDefault() {
         final Parameters parameters = new Parameters();
-        parameters.set("a", "axe", "b", "bat", "c", "cat");
+        parameters.set(null, "a", "axe", "b", "bat", "c", "cat");
 
         assertEquals(Collections.singletonList("axe"), parameters.get("a", () -> "x"));
         assertEquals(Collections.singletonList("bat"), parameters.get("b", () -> "x"));
@@ -245,7 +245,7 @@ public class ParametersTest {
     @Test
     public void shouldClone() {
         final Parameters parameters = new Parameters();
-        parameters.set("a", "axe", "a", "ant", "b", "bat", "b", "ball", "c", "cat");
+        parameters.set(null, "a", "axe", "a", "ant", "b", "bat", "b", "ball", "c", "cat");
 
         final Parameters parametersClone = parameters.clone();
 
@@ -259,7 +259,7 @@ public class ParametersTest {
         when(t.clone()).thenReturn(t);
 
         final Parameters parameters = new Parameters();
-        parameters.set("a", "axe", "a", "ant", "b", "bat", "b", "ball", "c", "cat", "t",
t);
+        parameters.set(null, "a", "axe", "a", "ant", "b", "bat", "b", "ball", "c", "cat",
"t", t);
 
         final Parameters parametersClone = parameters.clone();
 
@@ -270,10 +270,10 @@ public class ParametersTest {
     @Test
     public void shouldBeDifferent() {
         final Parameters parameters = new Parameters();
-        parameters.set("a", "axe", "a", "ant", "b", "bat", "b", "ball", "c", "cat");
+        parameters.set(null, "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");
+        parametersDifferent.set(null, "a", "ant", "a", "axe", "b", "bat", "b", "ball", "c",
"cat");
 
         assertNotEquals(parameters.getRaw(), parametersDifferent.getRaw());
     }
@@ -281,29 +281,27 @@ public class ParametersTest {
     @Test
     public void shouldGetNoTraversals() {
         final Parameters parameters = new Parameters();
-        parameters.set("a", "axe", "a", "ant", "b", "bat", "b", "ball", "c", "cat");
+        parameters.set(null, "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"));
+        parameters.set(null, "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);
+        final Parameters parameters = new Parameters();
 
         // 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);
+        parameters.set(mock, "a", "axe", "a", "ant", "b", "bat", "b", "ball", "c", "cat",
"t", __.outE("knows"));
 
         verify(mock).integrateChild(__.outE("knows").asAdmin());
     }


Mime
View raw message