You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tinkerpop.apache.org by ok...@apache.org on 2017/01/18 22:49:04 UTC

tinkerpop git commit: wow. I can't believe I did it. With this branch and the ProjectedTraverser branch, there are no reducers that take a Traversal. At most they are take a BinaryOperator and thus, are stateless. This is huge. So many errant bugs will

Repository: tinkerpop
Updated Branches:
  refs/heads/TINKERPOP-1606 f53327b7f -> fe470e7cc


wow. I can't believe I did it. With this branch and the ProjectedTraverser branch,  there are no reducers that take a Traversal. At most they are take a BinaryOperator and thus, are stateless. This is huge. So many errant bugs will go away. Best of all, GroupStep and GroupSideEffectStep are crazy simple now. Like basic (and way more efficient).


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

Branch: refs/heads/TINKERPOP-1606
Commit: fe470e7cc379e502eae7d14f9c8728ffd3762136
Parents: f53327b
Author: Marko A. Rodriguez <ok...@gmail.com>
Authored: Wed Jan 18 15:49:01 2017 -0700
Committer: Marko A. Rodriguez <ok...@gmail.com>
Committed: Wed Jan 18 15:49:01 2017 -0700

----------------------------------------------------------------------
 .../process/traversal/step/map/GroupStep.java   | 92 ++++++--------------
 .../step/sideEffect/GroupSideEffectStep.java    | 24 ++---
 2 files changed, 37 insertions(+), 79 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/fe470e7c/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/GroupStep.java
----------------------------------------------------------------------
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/GroupStep.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/GroupStep.java
index 4d229d1..b0e90a1 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/GroupStep.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/GroupStep.java
@@ -19,6 +19,7 @@
 
 package org.apache.tinkerpop.gremlin.process.traversal.step.map;
 
+import org.apache.tinkerpop.gremlin.process.traversal.Operator;
 import org.apache.tinkerpop.gremlin.process.traversal.Step;
 import org.apache.tinkerpop.gremlin.process.traversal.Traversal;
 import org.apache.tinkerpop.gremlin.process.traversal.Traverser;
@@ -29,21 +30,15 @@ import org.apache.tinkerpop.gremlin.process.traversal.lambda.IdentityTraversal;
 import org.apache.tinkerpop.gremlin.process.traversal.lambda.TokenTraversal;
 import org.apache.tinkerpop.gremlin.process.traversal.step.Barrier;
 import org.apache.tinkerpop.gremlin.process.traversal.step.ByModulating;
-import org.apache.tinkerpop.gremlin.process.traversal.step.LambdaHolder;
 import org.apache.tinkerpop.gremlin.process.traversal.step.TraversalParent;
-import org.apache.tinkerpop.gremlin.process.traversal.step.util.CollectingBarrierStep;
 import org.apache.tinkerpop.gremlin.process.traversal.step.util.ReducingBarrierStep;
 import org.apache.tinkerpop.gremlin.process.traversal.traverser.TraverserRequirement;
-import org.apache.tinkerpop.gremlin.process.traversal.traverser.util.TraverserSet;
 import org.apache.tinkerpop.gremlin.process.traversal.util.TraversalHelper;
 import org.apache.tinkerpop.gremlin.process.traversal.util.TraversalUtil;
 import org.apache.tinkerpop.gremlin.structure.util.StringFactory;
 import org.apache.tinkerpop.gremlin.util.function.HashMapSupplier;
 import org.apache.tinkerpop.gremlin.util.iterator.IteratorUtils;
 
-import java.io.IOException;
-import java.io.ObjectInputStream;
-import java.io.ObjectOutputStream;
 import java.io.Serializable;
 import java.util.ArrayList;
 import java.util.HashMap;
@@ -61,11 +56,13 @@ public final class GroupStep<S, K, V> extends ReducingBarrierStep<S, Map<K, V>>
     private Traversal.Admin<S, K> keyTraversal;
     private Traversal.Admin<S, ?> preTraversal;
     private Traversal.Admin<S, V> valueTraversal;
+    private Barrier barrierStep;
 
     public GroupStep(final Traversal.Admin traversal) {
         super(traversal);
         this.valueTraversal = this.integrateChild(__.fold().asAdmin());
         this.preTraversal = this.integrateChild(generatePreTraversal(this.valueTraversal));
+        this.barrierStep = TraversalHelper.getFirstStepOfAssignableClass(Barrier.class, this.preTraversal).orElse(null);
         this.setReducingBiOperator(new GroupBiOperator<>(this.valueTraversal));
         this.setSeedSupplier(HashMapSupplier.instance());
     }
@@ -78,6 +75,7 @@ public final class GroupStep<S, K, V> extends ReducingBarrierStep<S, Map<K, V>>
         } else if ('v' == this.state) {
             this.valueTraversal = this.integrateChild(convertValueTraversal(kvTraversal));
             this.preTraversal = this.integrateChild(generatePreTraversal(this.valueTraversal));
+            this.barrierStep = TraversalHelper.getFirstStepOfAssignableClass(Barrier.class, this.preTraversal).orElse(null);
             this.setReducingBiOperator(new GroupBiOperator<>(this.valueTraversal));
             this.state = 'x';
         } else {
@@ -89,19 +87,15 @@ public final class GroupStep<S, K, V> extends ReducingBarrierStep<S, Map<K, V>>
     public Map<K, V> projectTraverser(final Traverser.Admin<S> traverser) {
         final Map<K, V> map = new HashMap<>(1);
         if (null == this.preTraversal) {
-            map.put(TraversalUtil.applyNullable(traverser, this.keyTraversal), (V) new TraverserSet<>(traverser));
+            map.put(TraversalUtil.applyNullable(traverser, this.keyTraversal), (V) traverser.get());
         } else {
             this.preTraversal.reset();
             this.preTraversal.addStart(traverser);
-            final Barrier barrierStep = TraversalHelper.getFirstStepOfAssignableClass(Barrier.class, this.preTraversal).orElse(null);
-            if (null == barrierStep) {
-                final TraverserSet set = new TraverserSet();
-                while (this.preTraversal.hasNext()) {
-                    set.add(this.preTraversal.nextTraverser());
-                }
-                map.put(TraversalUtil.applyNullable(traverser, this.keyTraversal), (V) set);
-            } else if (barrierStep.hasNextBarrier())
-                map.put(TraversalUtil.applyNullable(traverser, this.keyTraversal), (V) barrierStep.nextBarrier());
+            if (null == this.barrierStep) {
+                if (this.preTraversal.hasNext())
+                    map.put(TraversalUtil.applyNullable(traverser, this.keyTraversal), (V) this.preTraversal.next());
+            } else if (this.barrierStep.hasNextBarrier())
+                map.put(TraversalUtil.applyNullable(traverser, this.keyTraversal), (V) this.barrierStep.nextBarrier());
         }
         return map;
     }
@@ -134,6 +128,7 @@ public final class GroupStep<S, K, V> extends ReducingBarrierStep<S, Map<K, V>>
             clone.keyTraversal = this.keyTraversal.clone();
         clone.valueTraversal = this.valueTraversal.clone();
         clone.preTraversal = (Traversal.Admin<S, ?>) GroupStep.generatePreTraversal(clone.valueTraversal);
+        clone.barrierStep = TraversalHelper.getFirstStepOfAssignableClass(Barrier.class, clone.preTraversal).orElse(null);
         clone.setReducingBiOperator(new GroupBiOperator<>(clone.valueTraversal));
         return clone;
     }
@@ -161,69 +156,32 @@ public final class GroupStep<S, K, V> extends ReducingBarrierStep<S, Map<K, V>>
 
     ///////////////////////
 
-    public static final class GroupBiOperator<K, V> implements BinaryOperator<Map<K, V>>, Serializable, Cloneable {
+    public static final class GroupBiOperator<K, V> implements BinaryOperator<Map<K, V>>, Serializable {
 
-        private Barrier barrierStep;
-
-        public GroupBiOperator(final Traversal.Admin<?, V> valueTraversal) {
-            // if there is a lambda that can not be serialized, then simply use TraverserSets
-            if (TraversalHelper.hasStepOfAssignableClassRecursively(LambdaHolder.class, valueTraversal))
-                this.barrierStep = null;
-            else {
-                this.barrierStep = TraversalHelper.getFirstStepOfAssignableClass(Barrier.class, valueTraversal.clone()).orElse(null);
-                if (this.barrierStep instanceof CollectingBarrierStep)
-                    this.barrierStep = null;
-                if (null != this.barrierStep)
-                    this.barrierStep = (Barrier) ((Step) this.barrierStep).clone();
-            }
-        }
+        private BinaryOperator<V> barrierAggregator;
 
         public GroupBiOperator() {
             // no-arg constructor for serialization
         }
 
-        @Override
-        public GroupBiOperator<K, V> clone() {
-            try {
-                final GroupBiOperator<K, V> clone = (GroupBiOperator<K, V>) super.clone();
-                if (null != this.barrierStep)
-                    clone.barrierStep = (Barrier) ((Step) this.barrierStep).clone();
-                return clone;
-            } catch (final CloneNotSupportedException e) {
-                throw new IllegalStateException(e.getMessage(), e);
-            }
+        public GroupBiOperator(final Traversal.Admin<?, V> valueTraversal) {
+            final Barrier barrierStep = TraversalHelper.getFirstStepOfAssignableClass(Barrier.class, valueTraversal).orElse(null);
+            this.barrierAggregator = null == barrierStep ? (BinaryOperator<V>) Operator.assign : barrierStep.getMemoryComputeKey().getReducer();
         }
 
         @Override
         public Map<K, V> apply(final Map<K, V> mapA, final Map<K, V> mapB) {
             for (final K key : mapB.keySet()) {
-                Object objectA = mapA.get(key);
-                final Object objectB = mapB.get(key);
-                if (null == objectA) {
+                V objectA = mapA.get(key);
+                final V objectB = mapB.get(key);
+                if (null == objectA)
                     objectA = objectB;
-                } else if (null == objectB) {
-
-                } else {
-                    if (null == this.barrierStep) {
-                        ((TraverserSet) objectA).addAll((TraverserSet) objectB);
-                    } else {
-                        this.barrierStep.addBarrier(objectA);
-                        this.barrierStep.addBarrier(objectB);
-                        objectA = this.barrierStep.nextBarrier();
-                    }
-                }
-                mapA.put(key, (V) objectA);
+                else if (null != objectB)
+                    objectA = this.barrierAggregator.apply(objectA, objectB);
+                mapA.put(key, objectA);
             }
             return mapA;
         }
-
-        private void writeObject(final ObjectOutputStream outputStream) throws IOException {
-            outputStream.writeObject(this.barrierStep); // todo: reset() instead?
-        }
-
-        private void readObject(final ObjectInputStream inputStream) throws IOException, ClassNotFoundException {
-            this.barrierStep = (Barrier) inputStream.readObject();
-        }
     }
 
 
@@ -259,9 +217,9 @@ public final class GroupStep<S, K, V> extends ReducingBarrierStep<S, Map<K, V>>
         final Map<K, V> reducedMap = new HashMap<>(map.size());
         final Barrier barrierStep = TraversalHelper.getFirstStepOfAssignableClass(Barrier.class, valueTraversal).orElse(null);
         IteratorUtils.removeOnNext(map.entrySet().iterator()).forEachRemaining(entry -> {
-            if (null == barrierStep) {
-                reducedMap.put(entry.getKey(), ((TraverserSet<V>) entry.getValue()).peek().get());
-            } else {
+            if (null == barrierStep)
+                reducedMap.put(entry.getKey(), (V) entry.getValue());
+            else {
                 valueTraversal.reset();
                 barrierStep.addBarrier(entry.getValue());
                 if (valueTraversal.hasNext())

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/fe470e7c/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/GroupSideEffectStep.java
----------------------------------------------------------------------
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/GroupSideEffectStep.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/GroupSideEffectStep.java
index bba795b..1993a8b 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/GroupSideEffectStep.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/GroupSideEffectStep.java
@@ -27,7 +27,6 @@ import org.apache.tinkerpop.gremlin.process.traversal.step.SideEffectCapable;
 import org.apache.tinkerpop.gremlin.process.traversal.step.TraversalParent;
 import org.apache.tinkerpop.gremlin.process.traversal.step.map.GroupStep;
 import org.apache.tinkerpop.gremlin.process.traversal.traverser.TraverserRequirement;
-import org.apache.tinkerpop.gremlin.process.traversal.traverser.util.TraverserSet;
 import org.apache.tinkerpop.gremlin.process.traversal.util.TraversalHelper;
 import org.apache.tinkerpop.gremlin.process.traversal.util.TraversalUtil;
 import org.apache.tinkerpop.gremlin.structure.util.StringFactory;
@@ -48,6 +47,7 @@ public final class GroupSideEffectStep<S, K, V> extends SideEffectStep<S> implem
     private Traversal.Admin<S, K> keyTraversal;
     private Traversal.Admin<S, ?> preTraversal;
     private Traversal.Admin<S, V> valueTraversal;
+    private Barrier barrierStep;
     ///
     private String sideEffectKey;
 
@@ -56,6 +56,7 @@ public final class GroupSideEffectStep<S, K, V> extends SideEffectStep<S> implem
         this.sideEffectKey = sideEffectKey;
         this.valueTraversal = this.integrateChild(__.fold().asAdmin());
         this.preTraversal = this.integrateChild(GroupStep.generatePreTraversal(this.valueTraversal));
+        this.barrierStep = TraversalHelper.getFirstStepOfAssignableClass(Barrier.class, this.preTraversal).orElse(null);
         this.getTraversal().getSideEffects().registerIfAbsent(this.sideEffectKey, HashMapSupplier.instance(), new GroupStep.GroupBiOperator<>(this.valueTraversal));
     }
 
@@ -67,6 +68,7 @@ public final class GroupSideEffectStep<S, K, V> extends SideEffectStep<S> implem
         } else if ('v' == this.state) {
             this.valueTraversal = this.integrateChild(GroupStep.convertValueTraversal(kvTraversal));
             this.preTraversal = this.integrateChild(GroupStep.generatePreTraversal(this.valueTraversal));
+            this.barrierStep = TraversalHelper.getFirstStepOfAssignableClass(Barrier.class, this.preTraversal).orElse(null);
             this.getTraversal().getSideEffects().register(this.sideEffectKey, null, new GroupStep.GroupBiOperator<>(this.valueTraversal));
             this.state = 'x';
         } else {
@@ -78,21 +80,18 @@ public final class GroupSideEffectStep<S, K, V> extends SideEffectStep<S> implem
     protected void sideEffect(final Traverser.Admin<S> traverser) {
         final Map<K, V> map = new HashMap<>(1);
         if (null == this.preTraversal) {
-            map.put(TraversalUtil.applyNullable(traverser, this.keyTraversal), (V) new TraverserSet<>(traverser));
+            map.put(TraversalUtil.applyNullable(traverser, this.keyTraversal), (V) traverser.get());
         } else {
             this.preTraversal.reset();
             this.preTraversal.addStart(traverser);
-            final Barrier barrierStep = TraversalHelper.getFirstStepOfAssignableClass(Barrier.class, this.preTraversal).orElse(null);
-            if (null == barrierStep) {
-                final TraverserSet traverserSet = new TraverserSet();
-                while (this.preTraversal.hasNext()) {
-                    traverserSet.add(this.preTraversal.nextTraverser());
-                }
-                map.put(TraversalUtil.applyNullable(traverser, this.keyTraversal), (V) traverserSet);
-            } else if (barrierStep.hasNextBarrier())
-                map.put(TraversalUtil.applyNullable(traverser, this.keyTraversal), (V) barrierStep.nextBarrier());
+            if (null == this.barrierStep) {
+                if (this.preTraversal.hasNext())
+                    map.put(TraversalUtil.applyNullable(traverser, this.keyTraversal), (V) this.preTraversal.next());
+            } else if (this.barrierStep.hasNextBarrier())
+                map.put(TraversalUtil.applyNullable(traverser, this.keyTraversal), (V) this.barrierStep.nextBarrier());
         }
-        this.getTraversal().getSideEffects().add(this.sideEffectKey, map);
+        if (!map.isEmpty())
+            this.getTraversal().getSideEffects().add(this.sideEffectKey, map);
     }
 
     @Override
@@ -128,6 +127,7 @@ public final class GroupSideEffectStep<S, K, V> extends SideEffectStep<S> implem
             clone.keyTraversal = this.keyTraversal.clone();
         clone.valueTraversal = this.valueTraversal.clone();
         clone.preTraversal = (Traversal.Admin<S, ?>) GroupStep.generatePreTraversal(clone.valueTraversal);
+        clone.barrierStep = TraversalHelper.getFirstStepOfAssignableClass(Barrier.class, clone.preTraversal).orElse(null);
         return clone;
     }