This is an automated email from the ASF dual-hosted git repository.
spmallette pushed a commit to branch TINKERPOP-2365
in repository https://gitbox.apache.org/repos/asf/tinkerpop.git
commit 1e2d7b84b0795de359b061d649dd3cb761333721
Author: Stephen Mallette <sp...@genoprime.com>
AuthorDate: Fri May 1 08:04:53 2020 -0400
TINKERPOP-2365 Prevented extra barrier being added with profile()
LazyBarrierStrategy was improperly adding a barrier step to the end of a
traversal when profile() was called. This wasn't noticeable in testing because
of an overly zealous choice to disable the strategy in testing if profile() was
present. Modified the strategy to more properly handle this situation and to
limit the removal of LazyBarrierStrategy to ProfileTest only.
---
CHANGELOG.asciidoc | 3 ++-
.../strategy/optimization/LazyBarrierStrategy.java | 13 +++++++++----
.../strategy/optimization/LazyBarrierStrategyTest.java | 6 +++++-
.../gremlin/process/traversal/step/map/ProfileTest.java | 10 +++++++++-
4 files changed, 25 insertions(+), 7 deletions(-)
diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc
index ce200c1..4f2d06a 100644
--- a/CHANGELOG.asciidoc
+++ b/CHANGELOG.asciidoc
@@ -33,7 +33,8 @@
image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima
* Bumped to Jackson 2.9.10.4.
* Remove invalid service descriptors from gremlin-shaded.
* Fixed bug in Python and .NET traversal `clone()` where deep copies of
bytecode were not occurring.
-* Fixed bug in Python about integer serializer which was out of range of
`g:Int32`
+* Fixed bug in Python about integer serializer which was out of range of
`g:Int32`.
+* Fixed bug where `profile()` was forcing `LazyBarrierStrategy` to add an
extra `barrier()` to the end of traversals.
[[release-3-3-10]]
=== TinkerPop 3.3.10 (Release Date: February 3, 2020)
diff --git
a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/LazyBarrierStrategy.java
b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/LazyBarrierStrategy.java
index 1313d3e..0f30e33 100644
---
a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/LazyBarrierStrategy.java
+++
b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/LazyBarrierStrategy.java
@@ -66,9 +66,7 @@ public final class LazyBarrierStrategy extends
AbstractTraversalStrategy<Travers
@Override
public void apply(final Traversal.Admin<?, ?> traversal) {
if (TraversalHelper.onGraphComputer(traversal) ||
-
traversal.getTraverserRequirements().contains(TraverserRequirement.PATH) ||
- (IS_TESTING &&
((TraversalHelper.hasStepOfAssignableClass(ProfileStep.class,
TraversalHelper.getRootTraversal(traversal)) ||
-
TraversalHelper.hasStepOfAssignableClass(ProfileSideEffectStep.class,
TraversalHelper.getRootTraversal(traversal)))))) // necessary cause ProfileTest
analyzes counts
+
traversal.getTraverserRequirements().contains(TraverserRequirement.PATH))
return;
boolean foundFlatMap = false;
@@ -86,10 +84,17 @@ public final class LazyBarrierStrategy extends
AbstractTraversalStrategy<Travers
(step instanceof GraphStep &&
(i > 0 || ((GraphStep) step).getIds().length >=
BIG_START_SIZE ||
(((GraphStep) step).getIds().length == 0
&& !(step.getNextStep() instanceof HasStep))))) {
+
+ // NoneStep, EmptyStep signify the end of the traversal where
no barriers are really going to be
+ // helpful after that. ProfileSideEffectStep means the
traversal had profile() called on it and if
+ // we don't account for that a barrier will inject at the end
of the traversal where it wouldn't
+ // be otherwise. LazyBarrierStrategy executes before the
finalization strategy of ProfileStrategy
+ // so additionally injected ProfileSideEffectStep instances
should not have effect here.
if (foundFlatMap && !labeledPath &&
!(step.getNextStep() instanceof Barrier) &&
!(step.getNextStep() instanceof NoneStep) &&
- !(step.getNextStep() instanceof EmptyStep)) {
+ !(step.getNextStep() instanceof EmptyStep) &&
+ !(step.getNextStep() instanceof
ProfileSideEffectStep)) {
final Step noOpBarrierStep = new
NoOpBarrierStep<>(traversal, MAX_BARRIER_SIZE);
TraversalHelper.copyLabels(step, noOpBarrierStep, true);
TraversalHelper.insertAfterStep(noOpBarrierStep, step,
traversal);
diff --git
a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/LazyBarrierStrategyTest.java
b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/LazyBarrierStrategyTest.java
index b2bbd94..760a0d0 100644
---
a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/LazyBarrierStrategyTest.java
+++
b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/LazyBarrierStrategyTest.java
@@ -100,7 +100,11 @@ public class LazyBarrierStrategyTest {
{__.out().as("a").out().as("b").in().where(P.neq("a")).out().select("b").out(),
__.out().as("a").out().as("b").in().where(P.neq("a")).barrier(PATH_SIZE).out().select("b").barrier(PATH_SIZE).out(),
Collections.singletonList(PathRetractionStrategy.instance())},
{__.out().as("a").out().as("b").in().where(P.neq("a")).out().select("b").out().out(),
__.out().as("a").out().as("b").in().where(P.neq("a")).barrier(PATH_SIZE).out().select("b").barrier(PATH_SIZE).out().barrier(LAZY_SIZE).out(),
Collections.singletonList(PathRetractionStrategy.instance())},
{__.V().out().out().groupCount().by(__.out().out().out()).out(),
__.V().out().barrier(LAZY_SIZE).out().groupCount().by(__.out().out().barrier(LAZY_SIZE).out()).out(),
Collections.emptyList()},
-
{__.V().out().out().groupCount().by(__.out().out().out()).out().as("a"),
__.V().out().barrier(LAZY_SIZE).out().groupCount().by(__.out().out().barrier(LAZY_SIZE).out()).out().as("a"),
Collections.emptyList()}
+
{__.V().out().out().groupCount().by(__.out().out().out()).out().as("a"),
__.V().out().barrier(LAZY_SIZE).out().groupCount().by(__.out().out().barrier(LAZY_SIZE).out()).out().as("a"),
Collections.emptyList()},
+ {__.V().both().profile(), __.V().both().profile(),
Collections.emptyList() },
+ {__.V().both().both().profile(),
__.V().both().barrier(LAZY_SIZE).both().profile(), Collections.emptyList() },
+ {__.V().both().local(__.both().both().out()).profile(),
__.V().both().barrier(LAZY_SIZE).local(__.both().both().barrier(LAZY_SIZE).out()).profile(),
Collections.emptyList() },
+ {__.V().both().local(__.both().both().out()).in().profile(),
__.V().both().barrier(LAZY_SIZE).local(__.both().both().barrier(LAZY_SIZE).out()).in().profile(),
Collections.emptyList() },
});
}
}
diff --git
a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/ProfileTest.java
b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/ProfileTest.java
index 77e0213..b7985e6 100644
---
a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/ProfileTest.java
+++
b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/ProfileTest.java
@@ -33,6 +33,7 @@ import
org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__;
import org.apache.tinkerpop.gremlin.process.traversal.step.Profiling;
import org.apache.tinkerpop.gremlin.process.traversal.step.util.ProfileStep;
import
org.apache.tinkerpop.gremlin.process.traversal.strategy.optimization.CountStrategy;
+import
org.apache.tinkerpop.gremlin.process.traversal.strategy.optimization.LazyBarrierStrategy;
import
org.apache.tinkerpop.gremlin.process.traversal.strategy.optimization.RepeatUnrollStrategy;
import
org.apache.tinkerpop.gremlin.process.traversal.strategy.verification.ComputerVerificationStrategy;
import
org.apache.tinkerpop.gremlin.process.traversal.strategy.verification.VerificationException;
@@ -40,7 +41,9 @@ import
org.apache.tinkerpop.gremlin.process.traversal.util.Metrics;
import org.apache.tinkerpop.gremlin.process.traversal.util.MutableMetrics;
import org.apache.tinkerpop.gremlin.process.traversal.util.TraversalHelper;
import org.apache.tinkerpop.gremlin.process.traversal.util.TraversalMetrics;
+import org.apache.tinkerpop.gremlin.structure.Graph;
import org.apache.tinkerpop.gremlin.structure.Vertex;
+import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
@@ -54,7 +57,6 @@ 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.hamcrest.core.IsEqual.equalTo;
-import static org.hamcrest.core.IsInstanceOf.instanceOf;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotEquals;
@@ -94,6 +96,12 @@ public abstract class ProfileTest extends
AbstractGremlinProcessTest {
public abstract Traversal<Vertex, TraversalMetrics>
get_g_V_groupXmX_profile();
+ @Override
+ protected void afterLoadGraphWith(final Graph graph) throws Exception {
+ // profile() does some explicit counting which goes off with
LazyBarrierStrategy in place.
+ g = g.withoutStrategies(LazyBarrierStrategy.class);
+ }
+
@Test
@LoadGraphWith(MODERN)
public void modern_V_out_out_profile() {
