[01/17] tinkerpop git commit: Created OrdertedTraverser which allows us to move beyond the star graph for OrderGlobalStep by()-projections. Moreover, it reduces the complexity of ordering as the objects of comparison are already determined. Going to gene [Forced Update!]

[sp...@apache.org]

Repository: tinkerpop
Updated Branches:
  refs/heads/TINKERPOP-1560 061bbc42c -> 97265772d (forced update)


Created OrdertedTraverser which allows us to move beyond the star graph for OrderGlobalStep by()-projections. Moreover, it reduces the complexity of ordering as the objects of comparison are already determined. Going to generalize fully to a ProjectedTraverser which will allow us to do the same for SampleGlobalStep, DedupGlobalStep, and down the road maintain order even as the computation is re-distributed to workers.


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

Branch: refs/heads/TINKERPOP-1560
Commit: 501e97a1ecb23f76b2fddba8eaed1dba4a5a839e
Parents: e80a4cd
Author: Marko A. Rodriguez <ok...@gmail.com>
Authored: Wed Jan 18 09:08:24 2017 -0700
Committer: Marko A. Rodriguez <ok...@gmail.com>
Committed: Wed Jan 18 09:08:24 2017 -0700

----------------------------------------------------------------------
 CHANGELOG.asciidoc                              |   1 +
 .../traversal/step/map/OrderGlobalStep.java     |  61 ++---
 .../step/util/CollectingBarrierStep.java        |   4 +-
 .../ComputerVerificationStrategy.java           |   5 +-
 .../traversal/traverser/OrderedTraverser.java   | 235 +++++++++++++++++++
 .../gremlin/structure/io/gryo/GryoVersion.java  |   4 +-
 6 files changed, 277 insertions(+), 33 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/501e97a1/CHANGELOG.asciidoc
----------------------------------------------------------------------
diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc
index 4f3f9ce..86c6b4f 100644
--- a/CHANGELOG.asciidoc
+++ b/CHANGELOG.asciidoc
@@ -26,6 +26,7 @@ image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima
 TinkerPop 3.2.4 (Release Date: NOT OFFICIALLY RELEASED YET)
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
+* `OrderGlobalStep` now emits traversers with their `by()`-projections and thus, can move beyond the local star graph.
 * SASL negotiation supports both a byte array and Base64 encoded bytes as a string for authentication to Gremlin Server.
 * Deprecated `TinkerIoRegistry` replacing it with the more consistently named `TinkerIoRegistryV1d0`.
 * Made error messaging more consistent during result iteration timeouts in Gremlin Server.

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/501e97a1/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/OrderGlobalStep.java
----------------------------------------------------------------------
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/OrderGlobalStep.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/OrderGlobalStep.java
index a7d21b2..ac5df90 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/OrderGlobalStep.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/OrderGlobalStep.java
@@ -27,10 +27,10 @@ import org.apache.tinkerpop.gremlin.process.traversal.step.ByModulating;
 import org.apache.tinkerpop.gremlin.process.traversal.step.ComparatorHolder;
 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.traverser.OrderedTraverser;
 import org.apache.tinkerpop.gremlin.process.traversal.traverser.TraverserRequirement;
 import org.apache.tinkerpop.gremlin.process.traversal.traverser.util.TraverserSet;
 import org.apache.tinkerpop.gremlin.structure.util.StringFactory;
-import org.apache.tinkerpop.gremlin.util.function.ChainedComparator;
 import org.javatuples.Pair;
 
 import java.io.Serializable;
@@ -49,8 +49,8 @@ import java.util.stream.Collectors;
 public final class OrderGlobalStep<S, C extends Comparable> extends CollectingBarrierStep<S> implements ComparatorHolder<S, C>, TraversalParent, ByModulating {
 
     private List<Pair<Traversal.Admin<S, C>, Comparator<C>>> comparators = new ArrayList<>();
-    private ChainedComparator<S, C> chainedComparator = null;
     private long limit = Long.MAX_VALUE;
+    private boolean isShuffle = false;
 
     public OrderGlobalStep(final Traversal.Admin traversal) {
         super(traversal);
@@ -58,12 +58,30 @@ public final class OrderGlobalStep<S, C extends Comparable> extends CollectingBa
 
     @Override
     public void barrierConsumer(final TraverserSet<S> traverserSet) {
-        if (null == this.chainedComparator)
-            this.chainedComparator = new ChainedComparator<>(true, this.comparators);
-        if (this.chainedComparator.isShuffle())
+        if (this.isShuffle)
             traverserSet.shuffle();
         else
-            traverserSet.sort((Comparator) this.chainedComparator);
+            traverserSet.sort(Comparator.naturalOrder());
+    }
+
+    @Override
+    public void processAllStarts() {
+        if (this.starts.hasNext()) {
+            while (this.starts.hasNext()) {
+                this.traverserSet.add(new OrderedTraverser<S>(this.starts.next(), (List) this.comparators));
+            }
+            this.barrierConsumer(this.traverserSet);
+        }
+    }
+
+    @Override
+    public Traverser.Admin<S> processNextStart() {
+        if (!this.traverserSet.isEmpty()) {
+            return this.traverserSet.remove();
+        } else if (this.starts.hasNext()) {
+            this.processAllStarts();
+        }
+        return ((OrderedTraverser) this.traverserSet.remove()).getInternal();
     }
 
     public void setLimit(final long limit) {
@@ -76,6 +94,7 @@ public final class OrderGlobalStep<S, C extends Comparable> extends CollectingBa
 
     @Override
     public void addComparator(final Traversal.Admin<S, C> traversal, final Comparator<C> comparator) {
+        this.isShuffle = Order.shuffle == (Comparator) comparator;
         this.comparators.add(new Pair<>(this.integrateChild(traversal), comparator));
     }
 
@@ -125,7 +144,6 @@ public final class OrderGlobalStep<S, C extends Comparable> extends CollectingBa
         for (final Pair<Traversal.Admin<S, C>, Comparator<C>> comparator : this.comparators) {
             clone.comparators.add(new Pair<>(comparator.getValue0().clone(), comparator.getValue1()));
         }
-        clone.chainedComparator = null;
         return clone;
     }
 
@@ -137,47 +155,34 @@ public final class OrderGlobalStep<S, C extends Comparable> extends CollectingBa
 
     @Override
     public MemoryComputeKey<TraverserSet<S>> getMemoryComputeKey() {
-        if (null == this.chainedComparator)
-            this.chainedComparator = new ChainedComparator<>(true, this.comparators);
-        return MemoryComputeKey.of(this.getId(), new OrderBiOperator<>(this.chainedComparator, this.limit), false, true);
+        return MemoryComputeKey.of(this.getId(), new OrderBiOperator<>(this.limit, this.isShuffle), false, true);
     }
 
     ////////////////
 
-    public static final class OrderBiOperator<S> implements BinaryOperator<TraverserSet<S>>, Serializable, Cloneable {
+    public static final class OrderBiOperator<S> implements BinaryOperator<TraverserSet<S>>, Serializable {
 
-        private ChainedComparator chainedComparator;
         private long limit;
+        private boolean isShuffle;
 
         private OrderBiOperator() {
             // for serializers that need a no-arg constructor
         }
 
-        public OrderBiOperator(final ChainedComparator<S, ?> chainedComparator, final long limit) {
-            this.chainedComparator = chainedComparator;
+        public OrderBiOperator(final long limit, final boolean isShuffle) {
             this.limit = limit;
-        }
-
-        @Override
-        public OrderBiOperator<S> clone() {
-            try {
-                final OrderBiOperator<S> clone = (OrderBiOperator<S>) super.clone();
-                clone.chainedComparator = this.chainedComparator.clone();
-                return clone;
-            } catch (final CloneNotSupportedException e) {
-                throw new IllegalStateException(e.getMessage(), e);
-            }
+            this.isShuffle = isShuffle;
         }
 
         @Override
         public TraverserSet<S> apply(final TraverserSet<S> setA, final TraverserSet<S> setB) {
             setA.addAll(setB);
             if (Long.MAX_VALUE != this.limit && setA.bulkSize() > this.limit) {
-                if (this.chainedComparator.isShuffle())
+                if (this.isShuffle)
                     setA.shuffle();
                 else
-                    setA.sort(this.chainedComparator);
-                long counter = 0l;
+                    setA.sort(Comparator.naturalOrder());
+                long counter = 0L;
                 final Iterator<Traverser.Admin<S>> traversers = setA.iterator();
                 while (traversers.hasNext()) {
                     final Traverser.Admin<S> traverser = traversers.next();

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/501e97a1/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/CollectingBarrierStep.java
----------------------------------------------------------------------
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/CollectingBarrierStep.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/CollectingBarrierStep.java
index f9c85a2..b0cce80 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/CollectingBarrierStep.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/CollectingBarrierStep.java
@@ -39,8 +39,8 @@ import java.util.function.BinaryOperator;
  */
 public abstract class CollectingBarrierStep<S> extends AbstractStep<S, S> implements Barrier<TraverserSet<S>> {
 
-    private TraverserSet<S> traverserSet = new TraverserSet<>();
-    private int maxBarrierSize;
+    protected TraverserSet<S> traverserSet = new TraverserSet<>();
+    protected int maxBarrierSize;
 
     public CollectingBarrierStep(final Traversal.Admin traversal) {
         this(traversal, Integer.MAX_VALUE);

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/501e97a1/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/verification/ComputerVerificationStrategy.java
----------------------------------------------------------------------
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/verification/ComputerVerificationStrategy.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/verification/ComputerVerificationStrategy.java
index fc73fc3..5777adb 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/verification/ComputerVerificationStrategy.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/verification/ComputerVerificationStrategy.java
@@ -28,6 +28,7 @@ import org.apache.tinkerpop.gremlin.process.traversal.step.GraphComputing;
 import org.apache.tinkerpop.gremlin.process.traversal.step.Mutating;
 import org.apache.tinkerpop.gremlin.process.traversal.step.PathProcessor;
 import org.apache.tinkerpop.gremlin.process.traversal.step.TraversalParent;
+import org.apache.tinkerpop.gremlin.process.traversal.step.filter.SampleGlobalStep;
 import org.apache.tinkerpop.gremlin.process.traversal.step.map.GraphStep;
 import org.apache.tinkerpop.gremlin.process.traversal.step.sideEffect.InjectStep;
 import org.apache.tinkerpop.gremlin.process.traversal.step.sideEffect.SubgraphStep;
@@ -86,8 +87,8 @@ public final class ComputerVerificationStrategy extends AbstractTraversalStrateg
                     throw new VerificationException("Local traversals may not traverse past the local star-graph on GraphComputer: " + traversalOptional.get(), traversal);
             }
 
-            // collecting barriers and dedup global use can only operate on the element and its properties (no incidences)
-            if (step instanceof CollectingBarrierStep && step instanceof TraversalParent) {
+            // sample step use can only operate on the element and its properties (no incidences)
+            if (step instanceof SampleGlobalStep) {
                 if (((TraversalParent) step).getLocalChildren().stream().filter(t -> !TraversalHelper.isLocalProperties(t)).findAny().isPresent())
                     throw new VerificationException("The following barrier step can not process the incident edges of a vertex on GraphComputer: " + step, traversal);
             }

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/501e97a1/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/traverser/OrderedTraverser.java
----------------------------------------------------------------------
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/traverser/OrderedTraverser.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/traverser/OrderedTraverser.java
new file mode 100644
index 0000000..4dddaa3
--- /dev/null
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/traverser/OrderedTraverser.java
@@ -0,0 +1,235 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements.  See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership.  The ASF licenses this file
+ *  to you under the Apache License, Version 2.0 (the
+ *  "License"); you may not use this file except in compliance
+ *  with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing,
+ *  software distributed under the License is distributed on an
+ *  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *  KIND, either express or implied.  See the License for the
+ *  specific language governing permissions and limitations
+ *  under the License.
+ */
+
+package org.apache.tinkerpop.gremlin.process.traversal.traverser;
+
+import org.apache.tinkerpop.gremlin.process.traversal.Order;
+import org.apache.tinkerpop.gremlin.process.traversal.Path;
+import org.apache.tinkerpop.gremlin.process.traversal.Step;
+import org.apache.tinkerpop.gremlin.process.traversal.Traversal;
+import org.apache.tinkerpop.gremlin.process.traversal.TraversalSideEffects;
+import org.apache.tinkerpop.gremlin.process.traversal.Traverser;
+import org.apache.tinkerpop.gremlin.process.traversal.util.TraversalUtil;
+import org.apache.tinkerpop.gremlin.structure.util.Attachable;
+import org.javatuples.Pair;
+
+import java.util.ArrayList;
+import java.util.Comparator;
+import java.util.List;
+import java.util.Set;
+import java.util.function.Function;
+
+/**
+ * @author Marko A. Rodriguez (http://markorodriguez.com)
+ */
+public final class OrderedTraverser<T> implements Traverser.Admin<T> {
+
+    private Traverser.Admin<T> internal;
+    private int order;
+    private final List<Pair<Object, Comparator<?>>> orderChecks = new ArrayList<>();
+
+    private OrderedTraverser() {
+        // for serialization
+    }
+
+    public OrderedTraverser(final Traverser.Admin<T> internal, final List<Pair<Traversal.Admin<T, ?>, Comparator>> checks) {
+        this(internal, 0);
+        for (final Pair<Traversal.Admin<T, ?>, Comparator> pairs : checks) {
+            this.orderChecks.add(Pair.with(TraversalUtil.apply(this.internal, pairs.getValue0()), pairs.getValue1()));
+        }
+    }
+
+    public OrderedTraverser(final Traverser.Admin<T> internal, final int order) {
+        this.internal = internal instanceof OrderedTraverser ? ((OrderedTraverser) internal).internal : internal;
+        this.order = order;
+    }
+
+    public Traverser.Admin<T> getInternal() {
+        return this.internal;
+    }
+
+    public int order() {
+        return this.order;
+    }
+
+    @Override
+    public void merge(final Admin<?> other) {
+        this.internal.merge(other);
+    }
+
+    @Override
+    public <R> Admin<R> split(R r, Step<T, R> step) {
+        return new OrderedTraverser<>(this.internal.split(r, step), this.order);
+    }
+
+    @Override
+    public Admin<T> split() {
+        return new OrderedTraverser<>(this.internal.split(), this.order);
+    }
+
+    @Override
+    public void addLabels(final Set<String> labels) {
+        this.internal.addLabels(labels);
+    }
+
+    @Override
+    public void keepLabels(final Set<String> labels) {
+        this.internal.keepLabels(labels);
+    }
+
+    @Override
+    public void dropLabels(final Set<String> labels) {
+        this.internal.dropLabels(labels);
+    }
+
+    @Override
+    public void dropPath() {
+        this.internal.dropPath();
+    }
+
+    @Override
+    public void set(final T t) {
+        this.internal.set(t);
+    }
+
+    @Override
+    public void incrLoops(final String stepLabel) {
+        this.internal.incrLoops(stepLabel);
+    }
+
+    @Override
+    public void resetLoops() {
+        this.internal.resetLoops();
+    }
+
+    @Override
+    public String getStepId() {
+        return this.internal.getStepId();
+    }
+
+    @Override
+    public void setStepId(final String stepId) {
+        this.internal.setStepId(stepId);
+    }
+
+    @Override
+    public void setBulk(final long count) {
+        this.internal.setBulk(count);
+    }
+
+    @Override
+    public Admin<T> detach() {
+        this.internal = this.internal.detach();
+        return this;
+    }
+
+    @Override
+    public T attach(final Function<Attachable<T>, T> method) {
+        return this.internal.attach(method);
+    }
+
+    @Override
+    public void setSideEffects(final TraversalSideEffects sideEffects) {
+        this.internal.setSideEffects(sideEffects);
+    }
+
+    @Override
+    public TraversalSideEffects getSideEffects() {
+        return this.internal.getSideEffects();
+    }
+
+    @Override
+    public Set<String> getTags() {
+        return this.internal.getTags();
+    }
+
+    @Override
+    public T get() {
+        return this.internal.get();
+    }
+
+    @Override
+    public <S> S sack() {
+        return this.internal.sack();
+    }
+
+    @Override
+    public <S> void sack(final S object) {
+        this.internal.sack(object);
+    }
+
+    @Override
+    public Path path() {
+        return this.internal.path();
+    }
+
+    @Override
+    public int loops() {
+        return this.internal.loops();
+    }
+
+    @Override
+    public long bulk() {
+        return this.internal.bulk();
+    }
+
+    @Override
+    public int hashCode() {
+        return this.internal.hashCode();
+    }
+
+    @Override
+    public boolean equals(final Object object) {
+        return object instanceof OrderedTraverser && ((OrderedTraverser) object).internal.equals(this.internal);
+    }
+
+    @Override
+    public OrderedTraverser<T> clone() {
+        try {
+            final OrderedTraverser<T> clone = (OrderedTraverser<T>) super.clone();
+            clone.internal = (Traverser.Admin<T>) this.internal.clone();
+            return clone;
+        } catch (final CloneNotSupportedException e) {
+            throw new IllegalStateException(e.getMessage(), e);
+        }
+    }
+
+    @Override
+    public int compareTo(final Traverser<T> o) {
+        if (!(o instanceof OrderedTraverser))
+            return 0;
+        else {
+            if (this.orderChecks.isEmpty()) {
+                return Order.incr.compare(this.get(), o.get());
+            } else {
+                final OrderedTraverser<T> other = (OrderedTraverser<T>) o;
+                for (int i = 0; i < this.orderChecks.size(); i++) {
+                    final Comparator comparator = this.orderChecks.get(i).getValue1();
+                    final Object thisObject = this.orderChecks.get(i).getValue0();
+                    final Object otherObject = other.orderChecks.get(i).getValue0();
+                    final int comparison = comparator.compare(thisObject, otherObject);
+                    if (comparison != 0)
+                        return comparison;
+
+                }
+                return 0;
+            }
+        }
+    }
+}
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/501e97a1/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 0bd9e87..a04f2d9 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
@@ -51,6 +51,7 @@ import org.apache.tinkerpop.gremlin.process.traversal.traverser.LP_O_OB_P_S_SE_S
 import org.apache.tinkerpop.gremlin.process.traversal.traverser.LP_O_OB_S_SE_SL_Traverser;
 import org.apache.tinkerpop.gremlin.process.traversal.traverser.O_OB_S_SE_SL_Traverser;
 import org.apache.tinkerpop.gremlin.process.traversal.traverser.O_Traverser;
+import org.apache.tinkerpop.gremlin.process.traversal.traverser.OrderedTraverser;
 import org.apache.tinkerpop.gremlin.process.traversal.traverser.util.TraverserSet;
 import org.apache.tinkerpop.gremlin.process.traversal.util.AndP;
 import org.apache.tinkerpop.gremlin.process.traversal.util.DefaultTraversalMetrics;
@@ -221,7 +222,7 @@ public enum GryoVersion {
             add(GryoTypeReg.of(AbstractMap.SimpleImmutableEntry.class, 121));
             add(GryoTypeReg.of(java.sql.Timestamp.class, 161));
             add(GryoTypeReg.of(InetAddress.class, 162, new UtilSerializers.InetAddressSerializer()));
-            add(GryoTypeReg.of(ByteBuffer.class, 163, new UtilSerializers.ByteBufferSerializer()));  // ***LAST ID***
+            add(GryoTypeReg.of(ByteBuffer.class, 163, new UtilSerializers.ByteBufferSerializer()));
 
             add(GryoTypeReg.of(ReferenceEdge.class, 81));
             add(GryoTypeReg.of(ReferenceVertexProperty.class, 82));
@@ -245,6 +246,7 @@ public enum GryoVersion {
             add(GryoTypeReg.of(O_OB_S_SE_SL_Traverser.class, 89));
             add(GryoTypeReg.of(LP_O_OB_S_SE_SL_Traverser.class, 90));
             add(GryoTypeReg.of(LP_O_OB_P_S_SE_SL_Traverser.class, 91));
+            add(GryoTypeReg.of(OrderedTraverser.class, 164));   // ***LAST ID***
             add(GryoTypeReg.of(DefaultRemoteTraverser.class, 123, new GryoSerializers.DefaultRemoteTraverserSerializer()));
 
             add(GryoTypeReg.of(Bytecode.class, 122, new GryoSerializers.BytecodeSerializer()));

[17/17] tinkerpop git commit: TINKERPOP-1560 Used ManagedConcurrentValueMap in GremlinGroovyClassLoader

[sp...@apache.org]

TINKERPOP-1560 Used ManagedConcurrentValueMap in GremlinGroovyClassLoader

By configuring the ManagedConcurrentValueMap to have weak references, the GremlinGroovyClassLoader, and therefore the GremlinGroovyScriptEngine, are now able to "forget" classes that are no longer used. It was determined that the cache of these classes would grow indefinitely for each script passed to the GremlinGroovyScriptEngine, thus allowing the metaspace to continue to grow. It isn't really possible to write tests to verify that this change works, but I did test manually by watching memory usage in a profiler and could see that metaspace memory stayed stable and that classes were unloading from the classloader over time.


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

Branch: refs/heads/TINKERPOP-1560
Commit: 97265772db79618cd5de83bb45d22f3e2bb6be9e
Parents: f84f454
Author: Stephen Mallette <sp...@genoprime.com>
Authored: Fri Jan 20 11:54:45 2017 -0500
Committer: Stephen Mallette <sp...@genoprime.com>
Committed: Mon Jan 23 07:45:26 2017 -0500

----------------------------------------------------------------------
 CHANGELOG.asciidoc                              |  1 +
 .../groovy/jsr223/GremlinGroovyClassLoader.java | 24 ++++++++-
 .../jsr223/ManagedConcurrentValueMap.java       |  4 ++
 .../GremlinGroovyScriptEngineIntegrateTest.java | 55 ++++++++++++++++++++
 4 files changed, 82 insertions(+), 2 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/97265772/CHANGELOG.asciidoc
----------------------------------------------------------------------
diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc
index fb0f8da..5e06a1c 100644
--- a/CHANGELOG.asciidoc
+++ b/CHANGELOG.asciidoc
@@ -34,6 +34,7 @@ TinkerPop 3.2.4 (Release Date: NOT OFFICIALLY RELEASED YET)
 * SASL negotiation supports both a byte array and Base64 encoded bytes as a string for authentication to Gremlin Server.
 * Deprecated `TinkerIoRegistry` replacing it with the more consistently named `TinkerIoRegistryV1d0`.
 * Made error messaging more consistent during result iteration timeouts in Gremlin Server.
+* Fixed a memory leak in the classloader for the `GremlinGroovyScriptEngine` where classes in the loader were not releasing from memory as a strong reference was always maintained.
 * `PathRetractionStrategy` does not add a `NoOpBarrierStep` to the end of local children as its wasted computation in 99% of traversals.
 * Fixed a bug in `AddVertexStartStep` where if a side-effect was being used in the parametrization, an NPE occurred.
 * Fixed a bug in `LazyBarrierStrategy` where `profile()` was deactivating it accidentally.

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/97265772/gremlin-groovy/src/main/java/org/apache/tinkerpop/gremlin/groovy/jsr223/GremlinGroovyClassLoader.java
----------------------------------------------------------------------
diff --git a/gremlin-groovy/src/main/java/org/apache/tinkerpop/gremlin/groovy/jsr223/GremlinGroovyClassLoader.java b/gremlin-groovy/src/main/java/org/apache/tinkerpop/gremlin/groovy/jsr223/GremlinGroovyClassLoader.java
index e987ce0..2bc122c 100644
--- a/gremlin-groovy/src/main/java/org/apache/tinkerpop/gremlin/groovy/jsr223/GremlinGroovyClassLoader.java
+++ b/gremlin-groovy/src/main/java/org/apache/tinkerpop/gremlin/groovy/jsr223/GremlinGroovyClassLoader.java
@@ -20,19 +20,39 @@ package org.apache.tinkerpop.gremlin.groovy.jsr223;
 
 import groovy.lang.GroovyClassLoader;
 import org.codehaus.groovy.control.CompilerConfiguration;
+import org.codehaus.groovy.util.ReferenceBundle;
+
+import java.util.Map;
+import java.util.concurrent.ConcurrentMap;
 
 /**
- * A {@code GroovyClassLoader} extension that provides access to the {@code removeClassCacheEntry(String)} method.
+ * A {@code GroovyClassLoader} extension that uses a {@link ManagedConcurrentValueMap} configured with soft references.
+ * In this way, the classloader will "forget" scripts allowing them to be garbage collected and thus prevent memory
+ * issues in JVM metaspace.
  *
  * @author Stephen Mallette (http://stephen.genoprime.com)
  */
 class GremlinGroovyClassLoader extends GroovyClassLoader {
+    private final ManagedConcurrentValueMap<String, Class> classSoftCache;
+
     public GremlinGroovyClassLoader(final ClassLoader parent, final CompilerConfiguration conf) {
         super(parent, conf);
+        classSoftCache = new ManagedConcurrentValueMap<>(ReferenceBundle.getSoftBundle());
     }
 
     @Override
     protected void removeClassCacheEntry(final String name) {
-        super.removeClassCacheEntry(name);
+        this.classSoftCache.remove(name);
+    }
+
+    @Override
+    protected void setClassCacheEntry(final Class cls) {
+        this.classSoftCache.put(cls.getName(), cls);
+    }
+
+    @Override
+    protected Class getClassCacheEntry(final String name) {
+        if(null == name)  return null;
+        return this.classSoftCache.get(name);
     }
 }

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/97265772/gremlin-groovy/src/main/java/org/apache/tinkerpop/gremlin/groovy/jsr223/ManagedConcurrentValueMap.java
----------------------------------------------------------------------
diff --git a/gremlin-groovy/src/main/java/org/apache/tinkerpop/gremlin/groovy/jsr223/ManagedConcurrentValueMap.java b/gremlin-groovy/src/main/java/org/apache/tinkerpop/gremlin/groovy/jsr223/ManagedConcurrentValueMap.java
index dce056b..f1ee00d 100644
--- a/gremlin-groovy/src/main/java/org/apache/tinkerpop/gremlin/groovy/jsr223/ManagedConcurrentValueMap.java
+++ b/gremlin-groovy/src/main/java/org/apache/tinkerpop/gremlin/groovy/jsr223/ManagedConcurrentValueMap.java
@@ -76,6 +76,10 @@ class ManagedConcurrentValueMap<K, V> {
         internalMap.put(key, ref);
     }
 
+    public void remove(final K key) {
+        internalMap.remove(key);
+    }
+
     /**
      * Clear the map.
      */

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/97265772/gremlin-groovy/src/test/java/org/apache/tinkerpop/gremlin/jsr223/GremlinGroovyScriptEngineIntegrateTest.java
----------------------------------------------------------------------
diff --git a/gremlin-groovy/src/test/java/org/apache/tinkerpop/gremlin/jsr223/GremlinGroovyScriptEngineIntegrateTest.java b/gremlin-groovy/src/test/java/org/apache/tinkerpop/gremlin/jsr223/GremlinGroovyScriptEngineIntegrateTest.java
new file mode 100644
index 0000000..5242d3b
--- /dev/null
+++ b/gremlin-groovy/src/test/java/org/apache/tinkerpop/gremlin/jsr223/GremlinGroovyScriptEngineIntegrateTest.java
@@ -0,0 +1,55 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.tinkerpop.gremlin.jsr223;
+
+import org.apache.tinkerpop.gremlin.groovy.jsr223.GremlinGroovyScriptEngine;
+import org.javatuples.Pair;
+import org.junit.Ignore;
+import org.junit.Test;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Random;
+
+import static org.junit.Assert.assertEquals;
+
+/**
+ * @author Stephen Mallette (http://stephen.genoprime.com)
+ */
+public class GremlinGroovyScriptEngineIntegrateTest {
+    @Test
+    @Ignore("This is not a test that needs to run on build - it's more for profiling the GremlinGroovyScriptEngine")
+    public void shouldTest() throws Exception {
+        final Random r = new Random();
+        final List<Pair<String, Integer>> scripts = new ArrayList<>();
+        final GremlinGroovyScriptEngine engine = new GremlinGroovyScriptEngine();
+        for (int ix = 0; ix < 1000000; ix++) {
+            final String script = "1 + " + ix;
+            final int output = (int) engine.eval(script);
+            assertEquals(1 + ix, output);
+
+            if (ix % 1000 == 0) scripts.add(Pair.with(script, output));
+
+            if (ix % 25 == 0) {
+                final Pair<String,Integer> p = scripts.get(r.nextInt(scripts.size()));
+                assertEquals(p.getValue1().intValue(), (int) engine.eval(p.getValue0()));
+            }
+        }
+    }
+}

[02/17] tinkerpop git commit: remember user's working directory when script files are being loaded in execution mode

[sp...@apache.org]

remember user's working directory when script files are being loaded in execution mode


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

Branch: refs/heads/TINKERPOP-1560
Commit: 8fd6dbbe9cc0b98d124cd3fdf739377289300ce8
Parents: e80a4cd
Author: Daniel Kuppitz <da...@hotmail.com>
Authored: Wed Jan 18 17:18:46 2017 +0100
Committer: Daniel Kuppitz <da...@hotmail.com>
Committed: Wed Jan 18 17:18:46 2017 +0100

----------------------------------------------------------------------
 gremlin-console/src/main/bin/gremlin.sh                      | 4 +++-
 .../org/apache/tinkerpop/gremlin/console/Console.groovy      | 8 +++++++-
 2 files changed, 10 insertions(+), 2 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/8fd6dbbe/gremlin-console/src/main/bin/gremlin.sh
----------------------------------------------------------------------
diff --git a/gremlin-console/src/main/bin/gremlin.sh b/gremlin-console/src/main/bin/gremlin.sh
index 901610b..d8c94db 100755
--- a/gremlin-console/src/main/bin/gremlin.sh
+++ b/gremlin-console/src/main/bin/gremlin.sh
@@ -22,6 +22,8 @@
 set -e
 set -u
 
+USER_DIR=`pwd`
+
 cd $(dirname $0)
 DIR=`pwd`
 
@@ -90,7 +92,7 @@ while getopts ":l" opt; do
     esac
 done
 
-JAVA_OPTIONS="${JAVA_OPTIONS} -Dtinkerpop.ext=${USER_EXT_DIR:-${SYSTEM_EXT_DIR}} -Dlog4j.configuration=conf/log4j-console.properties -Dgremlin.log4j.level=$GREMLIN_LOG_LEVEL"
+JAVA_OPTIONS="${JAVA_OPTIONS} -Duser.working_dir=${USER_DIR} -Dtinkerpop.ext=${USER_EXT_DIR:-${SYSTEM_EXT_DIR}} -Dlog4j.configuration=conf/log4j-console.properties -Dgremlin.log4j.level=$GREMLIN_LOG_LEVEL"
 JAVA_OPTIONS=$(awk -v RS=' ' '!/^$/ {if (!x[$0]++) print}' <<< "${JAVA_OPTIONS}" | grep -v '^$' | paste -sd ' ' -)
 
 if [ -n "$SCRIPT_DEBUG" ]; then

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/8fd6dbbe/gremlin-console/src/main/groovy/org/apache/tinkerpop/gremlin/console/Console.groovy
----------------------------------------------------------------------
diff --git a/gremlin-console/src/main/groovy/org/apache/tinkerpop/gremlin/console/Console.groovy b/gremlin-console/src/main/groovy/org/apache/tinkerpop/gremlin/console/Console.groovy
index a2e518c..0788f42 100644
--- a/gremlin-console/src/main/groovy/org/apache/tinkerpop/gremlin/console/Console.groovy
+++ b/gremlin-console/src/main/groovy/org/apache/tinkerpop/gremlin/console/Console.groovy
@@ -378,7 +378,13 @@ class Console {
                 groovy.execute("args = []")
             }
 
-            final File file = new File(scriptFile)
+            File file = new File(scriptFile)
+            if (!file.exists() && !file.isAbsolute()) {
+                final String userWorkingDir = System.getProperty("user.working_dir");
+                if (userWorkingDir != null) {
+                    file = new File(userWorkingDir, scriptFile);
+                }
+            }
             int lineNumber = 0
             def lines = file.readLines()
             for (String line : lines) {

[14/17] tinkerpop git commit: Merge branch 'TINKERPOP-1605' into tp32

[sp...@apache.org]

Merge branch 'TINKERPOP-1605' into tp32


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

Branch: refs/heads/TINKERPOP-1560
Commit: 67a10923d77098f403efc187ba43781443d1ed8f
Parents: 8ad2911 8fd6dbb
Author: Daniel Kuppitz <da...@hotmail.com>
Authored: Sat Jan 21 12:04:46 2017 +0100
Committer: Daniel Kuppitz <da...@hotmail.com>
Committed: Sat Jan 21 12:04:46 2017 +0100

----------------------------------------------------------------------
 gremlin-console/src/main/bin/gremlin.sh                      | 4 +++-
 .../org/apache/tinkerpop/gremlin/console/Console.groovy      | 8 +++++++-
 2 files changed, 10 insertions(+), 2 deletions(-)
----------------------------------------------------------------------

[03/17] tinkerpop git commit: We now have ProjectedTraveser which is a Traverser with List