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/19 11:06:01 UTC

tinkerpop git commit: beautiful. yet another simplification of GroupStep and GroupSideEffectStep. preTraversal is no longer needed. No more internal cloning required. And generateFinalReduction() is way more efficient as it operates on the Map itself ins

Repository: tinkerpop
Updated Branches:
  refs/heads/TINKERPOP-1606 119780624 -> 5b9e81a4f


beautiful. yet another simplification of GroupStep and GroupSideEffectStep. preTraversal is no longer needed. No more internal cloning required. And generateFinalReduction() is way more efficient as it operates on the Map itself instead of copying all the data over to another reduced Map. I can't believe how simple I have gotten what is perhaps that most complex step in Gremlin. Added a test case that demonstrates that internal side-effect steps within a group() still register with global memory. This was not possible before.


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

Branch: refs/heads/TINKERPOP-1606
Commit: 5b9e81a4f667f071a51496b131a0fb3f64900f52
Parents: 1197806
Author: Marko A. Rodriguez <ok...@gmail.com>
Authored: Thu Jan 19 04:05:52 2017 -0700
Committer: Marko A. Rodriguez <ok...@gmail.com>
Committed: Thu Jan 19 04:05:52 2017 -0700

----------------------------------------------------------------------
 .../process/traversal/step/map/GroupStep.java   | 78 +++++---------------
 .../step/sideEffect/GroupSideEffectStep.java    | 44 +++++------
 .../gremlin/structure/io/gryo/GryoVersion.java  |  2 +-
 .../step/sideEffect/GroovyGroupTest.groovy      |  5 ++
 .../traversal/step/sideEffect/GroupTest.java    | 31 +++++++-
 5 files changed, 75 insertions(+), 85 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/5b9e81a4/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 b0e90a1..29f9537 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
@@ -20,7 +20,6 @@
 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;
 import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__;
@@ -37,7 +36,6 @@ 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.Serializable;
 import java.util.ArrayList;
@@ -54,16 +52,14 @@ public final class GroupStep<S, K, V> extends ReducingBarrierStep<S, Map<K, V>>
 
     private char state = 'k';
     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.barrierStep = TraversalHelper.getFirstStepOfAssignableClass(Barrier.class, this.valueTraversal).orElse(null);
+        this.setReducingBiOperator(new GroupBiOperator<>(null == this.barrierStep ? Operator.assign : this.barrierStep.getMemoryComputeKey().getReducer()));
         this.setSeedSupplier(HashMapSupplier.instance());
     }
 
@@ -74,9 +70,8 @@ public final class GroupStep<S, K, V> extends ReducingBarrierStep<S, Map<K, V>>
             this.state = '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.barrierStep = TraversalHelper.getFirstStepOfAssignableClass(Barrier.class, this.valueTraversal).orElse(null);
+            this.setReducingBiOperator(new GroupBiOperator<>(null == this.barrierStep ? Operator.assign : this.barrierStep.getMemoryComputeKey().getReducer()));
             this.state = 'x';
         } else {
             throw new IllegalStateException("The key and value traversals for group()-step have already been set: " + this);
@@ -86,17 +81,13 @@ public final class GroupStep<S, K, V> extends ReducingBarrierStep<S, Map<K, V>>
     @Override
     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) traverser.get());
-        } else {
-            this.preTraversal.reset();
-            this.preTraversal.addStart(traverser);
-            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.valueTraversal.reset();
+        this.valueTraversal.addStart(traverser);
+        if (null == this.barrierStep) {
+            if (this.valueTraversal.hasNext())
+                map.put(TraversalUtil.applyNullable(traverser, this.keyTraversal), (V) this.valueTraversal.next());
+        } else if (this.barrierStep.hasNextBarrier())
+            map.put(TraversalUtil.applyNullable(traverser, this.keyTraversal), (V) this.barrierStep.nextBarrier());
         return map;
     }
 
@@ -107,12 +98,10 @@ public final class GroupStep<S, K, V> extends ReducingBarrierStep<S, Map<K, V>>
 
     @Override
     public List<Traversal.Admin<?, ?>> getLocalChildren() {
-        final List<Traversal.Admin<?, ?>> children = new ArrayList<>(3);
+        final List<Traversal.Admin<?, ?>> children = new ArrayList<>(2);
         if (null != this.keyTraversal)
             children.add(this.keyTraversal);
         children.add(this.valueTraversal);
-        if (null != this.preTraversal)
-            children.add(this.preTraversal);
         return children;
     }
 
@@ -127,9 +116,7 @@ public final class GroupStep<S, K, V> extends ReducingBarrierStep<S, Map<K, V>>
         if (null != this.keyTraversal)
             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));
+        clone.barrierStep = TraversalHelper.getFirstStepOfAssignableClass(Barrier.class, clone.valueTraversal).orElse(null);
         return clone;
     }
 
@@ -138,7 +125,6 @@ public final class GroupStep<S, K, V> extends ReducingBarrierStep<S, Map<K, V>>
         super.setTraversal(parentTraversal);
         integrateChild(this.keyTraversal);
         integrateChild(this.valueTraversal);
-        integrateChild(this.preTraversal);
     }
 
     @Override
@@ -164,9 +150,8 @@ public final class GroupStep<S, K, V> extends ReducingBarrierStep<S, Map<K, V>>
             // no-arg constructor for serialization
         }
 
-        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();
+        public GroupBiOperator(final BinaryOperator<V> barrierAggregator) {
+            this.barrierAggregator = barrierAggregator;
         }
 
         @Override
@@ -193,42 +178,19 @@ public final class GroupStep<S, K, V> extends ReducingBarrierStep<S, Map<K, V>>
                 valueTraversal instanceof IdentityTraversal ||
                 valueTraversal.getStartStep() instanceof LambdaMapStep && ((LambdaMapStep) valueTraversal.getStartStep()).getMapFunction() instanceof FunctionTraverser) {
             return (Traversal.Admin<S, E>) __.map(valueTraversal).fold();
-        } else {
+        } else
             return valueTraversal;
-        }
-    }
-
-    public static Traversal.Admin<?, ?> generatePreTraversal(final Traversal.Admin<?, ?> valueTraversal) {
-        if (!TraversalHelper.hasStepOfAssignableClass(Barrier.class, valueTraversal))
-            return valueTraversal.clone();
-        final Traversal.Admin<?, ?> first = __.identity().asAdmin();
-        boolean updated = false;
-        for (final Step step : valueTraversal.getSteps()) {
-            first.addStep(step.clone());
-            updated = true;
-            if (step instanceof Barrier)
-                break;
-
-        }
-        return updated ? first : null;
     }
 
     public static <K, V> Map<K, V> doFinalReduction(final Map<K, Object> map, final Traversal.Admin<?, V> valueTraversal) {
-        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(), (V) entry.getValue());
-            else {
+        TraversalHelper.getFirstStepOfAssignableClass(Barrier.class, valueTraversal).ifPresent(barrierStep -> {
+            for (final K key : map.keySet()) {
                 valueTraversal.reset();
-                barrierStep.addBarrier(entry.getValue());
+                barrierStep.addBarrier(map.get(key));
                 if (valueTraversal.hasNext())
-                    reducedMap.put(entry.getKey(), valueTraversal.next());
+                    map.put(key, valueTraversal.next());
             }
         });
-        assert map.isEmpty();
-        map.clear();
-        map.putAll(reducedMap);
         return (Map<K, V>) map;
     }
 }

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/5b9e81a4/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 1993a8b..0c5b278 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
@@ -18,6 +18,7 @@
  */
 package org.apache.tinkerpop.gremlin.process.traversal.step.sideEffect;
 
+import org.apache.tinkerpop.gremlin.process.traversal.Operator;
 import org.apache.tinkerpop.gremlin.process.traversal.Traversal;
 import org.apache.tinkerpop.gremlin.process.traversal.Traverser;
 import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__;
@@ -45,7 +46,6 @@ public final class GroupSideEffectStep<S, K, V> extends SideEffectStep<S> implem
 
     private char state = 'k';
     private Traversal.Admin<S, K> keyTraversal;
-    private Traversal.Admin<S, ?> preTraversal;
     private Traversal.Admin<S, V> valueTraversal;
     private Barrier barrierStep;
     ///
@@ -55,9 +55,11 @@ public final class GroupSideEffectStep<S, K, V> extends SideEffectStep<S> implem
         super(traversal);
         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));
+        this.barrierStep = TraversalHelper.getFirstStepOfAssignableClass(Barrier.class, this.valueTraversal).orElse(null);
+        this.getTraversal().getSideEffects().registerIfAbsent(this.sideEffectKey, HashMapSupplier.instance(),
+                new GroupStep.GroupBiOperator<>(null == this.barrierStep ?
+                        Operator.assign :
+                        this.barrierStep.getMemoryComputeKey().getReducer()));
     }
 
     @Override
@@ -67,9 +69,11 @@ public final class GroupSideEffectStep<S, K, V> extends SideEffectStep<S> implem
             this.state = 'v';
         } 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.barrierStep = TraversalHelper.getFirstStepOfAssignableClass(Barrier.class, this.valueTraversal).orElse(null);
+            this.getTraversal().getSideEffects().register(this.sideEffectKey, null,
+                    new GroupStep.GroupBiOperator<>(null == this.barrierStep ?
+                            Operator.assign :
+                            this.barrierStep.getMemoryComputeKey().getReducer()));
             this.state = 'x';
         } else {
             throw new IllegalStateException("The key and value traversals for group()-step have already been set: " + this);
@@ -79,17 +83,13 @@ public final class GroupSideEffectStep<S, K, V> extends SideEffectStep<S> implem
     @Override
     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) traverser.get());
-        } else {
-            this.preTraversal.reset();
-            this.preTraversal.addStart(traverser);
-            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.valueTraversal.reset();
+        this.valueTraversal.addStart(traverser);
+        if (null == this.barrierStep) {
+            if (this.valueTraversal.hasNext())
+                map.put(TraversalUtil.applyNullable(traverser, this.keyTraversal), (V) this.valueTraversal.next());
+        } else if (this.barrierStep.hasNextBarrier())
+            map.put(TraversalUtil.applyNullable(traverser, this.keyTraversal), (V) this.barrierStep.nextBarrier());
         if (!map.isEmpty())
             this.getTraversal().getSideEffects().add(this.sideEffectKey, map);
     }
@@ -106,12 +106,10 @@ public final class GroupSideEffectStep<S, K, V> extends SideEffectStep<S> implem
 
     @Override
     public List<Traversal.Admin<?, ?>> getLocalChildren() {
-        final List<Traversal.Admin<?, ?>> children = new ArrayList<>(3);
+        final List<Traversal.Admin<?, ?>> children = new ArrayList<>(2);
         if (null != this.keyTraversal)
             children.add(this.keyTraversal);
         children.add(this.valueTraversal);
-        if (null != this.preTraversal)
-            children.add(this.preTraversal);
         return children;
     }
 
@@ -126,8 +124,7 @@ public final class GroupSideEffectStep<S, K, V> extends SideEffectStep<S> implem
         if (null != this.keyTraversal)
             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.barrierStep = TraversalHelper.getFirstStepOfAssignableClass(Barrier.class, clone.valueTraversal).orElse(null);
         return clone;
     }
 
@@ -136,7 +133,6 @@ public final class GroupSideEffectStep<S, K, V> extends SideEffectStep<S> implem
         super.setTraversal(parentTraversal);
         this.integrateChild(this.keyTraversal);
         this.integrateChild(this.valueTraversal);
-        this.integrateChild(this.preTraversal);
     }
 
     @Override

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/5b9e81a4/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/gryo/GryoVersion.java
----------------------------------------------------------------------
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/gryo/GryoVersion.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/gryo/GryoVersion.java
index 837095b..0dc1725 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/gryo/GryoVersion.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/gryo/GryoVersion.java
@@ -340,7 +340,7 @@ public enum GryoVersion {
             add(GryoTypeReg.of(Operator.class, 107));
             add(GryoTypeReg.of(FoldStep.FoldBiOperator.class, 108));
             add(GryoTypeReg.of(GroupCountStep.GroupCountBiOperator.class, 109));
-            add(GryoTypeReg.of(GroupStep.GroupBiOperator.class, 117, new JavaSerializer())); // because they contain traversals
+            add(GryoTypeReg.of(GroupStep.GroupBiOperator.class, 117));
             add(GryoTypeReg.of(MeanGlobalStep.MeanGlobalBiOperator.class, 110));
             add(GryoTypeReg.of(MeanGlobalStep.MeanNumber.class, 111));
             add(GryoTypeReg.of(TreeStep.TreeBiOperator.class, 112));

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/5b9e81a4/gremlin-groovy-test/src/main/groovy/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/GroovyGroupTest.groovy
----------------------------------------------------------------------
diff --git a/gremlin-groovy-test/src/main/groovy/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/GroovyGroupTest.groovy b/gremlin-groovy-test/src/main/groovy/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/GroovyGroupTest.groovy
index 84da296..3ce9efe 100644
--- a/gremlin-groovy-test/src/main/groovy/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/GroovyGroupTest.groovy
+++ b/gremlin-groovy-test/src/main/groovy/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/GroovyGroupTest.groovy
@@ -123,5 +123,10 @@ public abstract class GroovyGroupTest {
         public Traversal<Vertex, Map<String, String>> get_g_V_groupXmX_byXnameX_byXinXknowsX_nameX_capXmX() {
             new ScriptTraversal<>(g, "gremlin-groovy", "g.V.group('m').by('name').by(__.in('knows').name).cap('m')")
         }
+
+        @Override
+        public Traversal<Vertex, Map<String, Number>> get_g_V_group_byXlabelX_byXbothE_groupXaX_byXlabelX_byXweight_sumX_weight_sumX() {
+            new ScriptTraversal<>(g, "gremlin-groovy", "g.V.group().by(label).by(bothE().group('a').by(label).by(values('weight').sum).weight.sum)")
+        }
     }
 }

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/5b9e81a4/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/GroupTest.java
----------------------------------------------------------------------
diff --git a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/GroupTest.java b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/GroupTest.java
index 036c8c8..68531fb 100644
--- a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/GroupTest.java
+++ b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/GroupTest.java
@@ -37,10 +37,12 @@ import java.util.Map;
 import static org.apache.tinkerpop.gremlin.LoadGraphWith.GraphData.GRATEFUL;
 import static org.apache.tinkerpop.gremlin.LoadGraphWith.GraphData.MODERN;
 import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.both;
+import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.bothE;
 import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.constant;
 import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.count;
 import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.out;
 import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.outE;
+import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.values;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
@@ -90,6 +92,8 @@ public abstract class GroupTest extends AbstractGremlinProcessTest {
 
     public abstract Traversal<Vertex, Map<String, String>> get_g_V_groupXmX_byXnameX_byXinXknowsX_nameX_capXmX();
 
+    public abstract Traversal<Vertex, Map<String, Number>> get_g_V_group_byXlabelX_byXbothE_groupXaX_byXlabelX_byXweight_sumX_weight_sumX();
+
     @Test
     @LoadGraphWith(MODERN)
     public void g_V_group_byXnameX() {
@@ -441,6 +445,24 @@ public abstract class GroupTest extends AbstractGremlinProcessTest {
         checkSideEffects(traversal.asAdmin().getSideEffects(), "m", HashMap.class);
     }
 
+    @Test
+    @LoadGraphWith(MODERN)
+    public void g_V_group_byXlabelX_byXbothE_groupXaX_byXlabelX_byXweight_sumX_weight_sumX() {
+        final Traversal<Vertex, Map<String, Number>> traversal = get_g_V_group_byXlabelX_byXbothE_groupXaX_byXlabelX_byXweight_sumX_weight_sumX();
+        printTraversalForm(traversal);
+        final Map<String, Number> map = traversal.next();
+        assertFalse(traversal.hasNext());
+        assertEquals(2, map.size());
+        assertEquals(2.0d, map.get("software").doubleValue(), 0.01d);
+        assertEquals(5.0d, map.get("person").doubleValue(), 0.01d);
+        checkSideEffects(traversal.asAdmin().getSideEffects(), "a", HashMap.class);
+        final Map<String, Number> sideEffect = traversal.asAdmin().getSideEffects().get("a");
+        assertEquals(2, sideEffect.size());
+        assertEquals(4.0d, sideEffect.get("created").doubleValue(), 0.01d);
+        assertEquals(3.0d, sideEffect.get("knows").doubleValue(), 0.01d);
+    }
+
+
     public static class Traversals extends GroupTest {
 
         @Override
@@ -525,17 +547,22 @@ public abstract class GroupTest extends AbstractGremlinProcessTest {
 
         @Override
         public Traversal<Vertex, Map<Long, Map<String, List<Vertex>>>> get_g_V_group_byXbothE_countX_byXgroup_byXlabelXX() {
-            return g.V().<Long, Map<String, List<Vertex>>>group().by(__.bothE().count()).by(__.group().by(T.label));
+            return g.V().<Long, Map<String, List<Vertex>>>group().by(bothE().count()).by(__.group().by(T.label));
         }
 
         @Override
         public Traversal<Vertex, Map<String, Map<String, Number>>> get_g_V_outXfollowedByX_group_byXsongTypeX_byXbothE_group_byXlabelX_byXweight_sumXX() {
-            return g.V().out("followedBy").<String, Map<String, Number>>group().by("songType").by(__.bothE().group().by(T.label).by(__.values("weight").sum()));
+            return g.V().out("followedBy").<String, Map<String, Number>>group().by("songType").by(bothE().group().by(T.label).by(values("weight").sum()));
         }
 
         @Override
         public Traversal<Vertex, Map<String, String>> get_g_V_groupXmX_byXnameX_byXinXknowsX_nameX_capXmX() {
             return g.V().group("m").by("name").by(__.in("knows").values("name")).cap("m");
         }
+
+        @Override
+        public Traversal<Vertex, Map<String, Number>> get_g_V_group_byXlabelX_byXbothE_groupXaX_byXlabelX_byXweight_sumX_weight_sumX() {
+            return g.V().<String, Number>group().by(T.label).by(bothE().group("a").by(T.label).by(values("weight").sum()).values("weight").sum());
+        }
     }
 }