Merge branch '3.7-dev' into 3.8-dev

Author: spmallette

CHANGELOG.asciidoc

@@ -86,6 +86,7 @@ image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima
 === TinkerPop 3.7.4 (NOT OFFICIALLY RELEASED YET)
 
 * Fixed bug in server `Settings` where it was referencing a property that was back in 3.3.0 and generating a warning log.
+* Improved performance of `Traversal.lock()` which was being called excessively.
 * Added log entry in `WsAndHttpChannelizerHandler` to catch general errors that escape the handlers.
 * Added a `MessageSizeEstimator` implementation to cover `Frame` allowing Gremlin Server to better estimate message sizes for the direct buffer.
 * Fixed bug in Gremlin Console for field accessor issue with JDK17.

gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/DefaultTraversal.java

@@ -335,8 +335,10 @@ public void lock() {
         // lock the parent before the children
         this.locked = true;
 
-        // now lock all the children
-        TraversalHelper.applyTraversalRecursively(Admin::lock, this, true);
+        // now lock all the children. since it's being called recursively here, we don't need each child to then
+        // also call this recursively TINKERPOP-3100
+        if (isRoot() || parent instanceof VertexProgramStep)
+            TraversalHelper.applyTraversalRecursively(Admin::lock, this, true);
     }
 
     /**

gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/util/DefaultTraversalTest.java

@@ -20,12 +20,16 @@
 package org.apache.tinkerpop.gremlin.process.traversal.util;
 
 import org.apache.tinkerpop.gremlin.process.traversal.Operator;
+import org.apache.tinkerpop.gremlin.process.traversal.P;
 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.dsl.graph.DefaultGraphTraversal;
+import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversalSource;
 import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__;
 import org.apache.tinkerpop.gremlin.process.traversal.step.TraversalParent;
+import org.apache.tinkerpop.gremlin.structure.Graph;
+import org.apache.tinkerpop.gremlin.structure.util.empty.EmptyGraph;
 import org.apache.tinkerpop.gremlin.util.function.ConstantSupplier;
 import org.apache.tinkerpop.gremlin.util.function.HashSetSupplier;
 import org.hamcrest.CoreMatchers;
@@ -42,13 +46,15 @@
 import java.util.stream.Collectors;
 import java.util.stream.IntStream;
 
+import static org.apache.tinkerpop.gremlin.process.traversal.AnonymousTraversalSource.traversal;
 import static org.hamcrest.number.OrderingComparison.greaterThan;
 import static org.hamcrest.number.OrderingComparison.lessThan;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotEquals;
 import static org.junit.Assert.assertNotSame;
 import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.core.Is.is;
 import static org.junit.Assert.assertTrue;
 
 /**
@@ -127,6 +133,53 @@ public void shouldBeTheSameSideEffectsThroughoutAllChildTraversals() {
         recursiveTestTraversals(traversal, sideEffects, new HashSet<>(Arrays.asList("marko", "bob", "x")), 13);
     }
 
+    @Test
+    public void shouldLock() {
+        final Traversal t = getBigDeepTraversal();
+        t.asAdmin().lock();
+        recursiveTestLock(t.asAdmin());
+    }
+
+
+    @Test
+    public void shouldLockAfterApplyingStrategies() {
+        final Traversal t = getBigDeepTraversal();
+        t.asAdmin().applyStrategies();
+        recursiveTestLock(t.asAdmin());
+    }
+
+    private static Traversal getBigDeepTraversal() {
+        final Graph graph = EmptyGraph.instance();
+        final GraphTraversalSource g = traversal().withEmbedded(graph);
+
+        final Traversal t = g.V().or(
+                __.has("name", P.within(new HashSet<>(Arrays.asList("DARK STAR", "ST. STEPHEN", "CHINA CAT SUNFLOWER")))),
+                __.has("songType", P.eq("cover"))).where(
+                __.coalesce(
+                        __.where(
+                                __.union(
+                                        __.as("a").inE("sungBy").choose(
+                                                __.has("weight"), __.has("weight", P.gt(1)), __.identity()
+                                        ).outV().filter(__.has("weight", P.lt(1))),
+                                        __.as("a").outE("followedBy").choose(
+                                                __.has("weight"), __.has("weight", P.gt(1)), __.identity()
+                                        ).inV().where(__.identity())
+                                ).dedup().select("a")
+                        )
+                ));
+        return t;
+    }
+
+    private void recursiveTestLock(final Traversal.Admin<?, ?> traversal) {
+        assertThat(traversal.isLocked(), is(true));
+        for (final Step<?, ?> step : traversal.getSteps()) {
+            if (step instanceof TraversalParent) {
+                ((TraversalParent) step).getGlobalChildren().forEach(this::recursiveTestLock);
+                ((TraversalParent) step).getLocalChildren().forEach(this::recursiveTestLock);
+            }
+        }
+    }
+
     private void recursiveTestTraversals(final Traversal.Admin<?, ?> traversal, final TraversalSideEffects sideEffects, final Set aValue, final int bValue) {
         assertTrue(traversal.getSideEffects() == sideEffects);
         assertEquals(sideEffects.keys().size(), traversal.getSideEffects().keys().size());