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 2016/09/13 19:04:55 UTC
tinkerpop git commit: fixed a JavaTranslator bug where Bytecode
instructions were being mutated during translation. This is because I was
accessing an Object[] and not cloning it first. Dar. Added BytecodeTest which
tests hashCode/equality/bindings/clone
Repository: tinkerpop
Updated Branches:
refs/heads/master d659e15be -> 13782aad5
fixed a JavaTranslator bug where Bytecode instructions were being mutated during translation. This is because I was accessing an Object[] and not cloning it first. Dar. Added BytecodeTest which tests hashCode/equality/bindings/clone/etc. Also, for @spmallette, I added a bytecode cache benchmark to TinkerGraphPlayTest.testPlay8(). CTR.
Project: http://git-wip-us.apache.org/repos/asf/tinkerpop/repo
Commit: http://git-wip-us.apache.org/repos/asf/tinkerpop/commit/13782aad
Tree: http://git-wip-us.apache.org/repos/asf/tinkerpop/tree/13782aad
Diff: http://git-wip-us.apache.org/repos/asf/tinkerpop/diff/13782aad
Branch: refs/heads/master
Commit: 13782aad50b3a41c252cab0f7d45ce76b359eb59
Parents: d659e15
Author: Marko A. Rodriguez <ok...@gmail.com>
Authored: Tue Sep 13 13:04:44 2016 -0600
Committer: Marko A. Rodriguez <ok...@gmail.com>
Committed: Tue Sep 13 13:04:44 2016 -0600
----------------------------------------------------------------------
CHANGELOG.asciidoc | 1 +
.../gremlin/jsr223/JavaTranslator.java | 37 ++++----
.../gremlin/process/traversal/BytecodeTest.java | 97 ++++++++++++++++++++
.../structure/TinkerGraphPlayTest.java | 59 +++++++-----
4 files changed, 155 insertions(+), 39 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/13782aad/CHANGELOG.asciidoc
----------------------------------------------------------------------
diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc
index 9998a24..24ed601 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.3 (Release Date: NOT OFFICIALLY RELEASED YET)
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+* Fixed a `JavaTranslator` bug where `Bytecode` instructions were being mutated during translation.
* Added `Path` to Gremlin-Python with respective GraphSON 2.0 deserializer.
* Added missing `InetAddress` to GraphSON extension module.
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/13782aad/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/jsr223/JavaTranslator.java
----------------------------------------------------------------------
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/jsr223/JavaTranslator.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/jsr223/JavaTranslator.java
index c39896c..dac6583 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/jsr223/JavaTranslator.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/jsr223/JavaTranslator.java
@@ -116,43 +116,46 @@ public final class JavaTranslator<S extends TraversalSource, T extends Traversal
}
}
}
- ///
+ // create a copy of the argument array so as not to mutate the original bytecode
+ final Object[] argumentsCopy = new Object[arguments.length];
for (int i = 0; i < arguments.length; i++) {
if (arguments[i] instanceof Bytecode.Binding)
- arguments[i] = ((Bytecode.Binding) arguments[i]).value();
+ argumentsCopy[i] = ((Bytecode.Binding) arguments[i]).value();
else if (arguments[i] instanceof Bytecode)
- arguments[i] = translateFromAnonymous((Bytecode) arguments[i]);
+ argumentsCopy[i] = translateFromAnonymous((Bytecode) arguments[i]);
+ else
+ argumentsCopy[i] = arguments[i];
}
try {
for (final Method method : methodCache.get(methodName)) {
if (returnType.isAssignableFrom(method.getReturnType())) {
- if (method.getParameterCount() == arguments.length || (method.getParameterCount() > 0 && method.getParameters()[method.getParameters().length - 1].isVarArgs())) {
+ if (method.getParameterCount() == argumentsCopy.length || (method.getParameterCount() > 0 && method.getParameters()[method.getParameters().length - 1].isVarArgs())) {
final Parameter[] parameters = method.getParameters();
final Object[] newArguments = new Object[parameters.length];
boolean found = true;
for (int i = 0; i < parameters.length; i++) {
if (parameters[i].isVarArgs()) {
final Class<?> parameterClass = parameters[i].getType().getComponentType();
- if (arguments.length > i && !parameterClass.isAssignableFrom(arguments[i].getClass())) {
+ if (argumentsCopy.length > i && !parameterClass.isAssignableFrom(argumentsCopy[i].getClass())) {
found = false;
break;
}
- Object[] varArgs = (Object[]) Array.newInstance(parameterClass, arguments.length - i);
+ Object[] varArgs = (Object[]) Array.newInstance(parameterClass, argumentsCopy.length - i);
int counter = 0;
- for (int j = i; j < arguments.length; j++) {
- varArgs[counter++] = arguments[j];
+ for (int j = i; j < argumentsCopy.length; j++) {
+ varArgs[counter++] = argumentsCopy[j];
}
newArguments[i] = varArgs;
break;
} else {
- if (i < arguments.length &&
- (parameters[i].getType().isAssignableFrom(arguments[i].getClass()) ||
+ if (i < argumentsCopy.length &&
+ (parameters[i].getType().isAssignableFrom(argumentsCopy[i].getClass()) ||
(parameters[i].getType().isPrimitive() &&
- (Number.class.isAssignableFrom(arguments[i].getClass()) ||
- arguments[i].getClass().equals(Boolean.class) ||
- arguments[i].getClass().equals(Byte.class) ||
- arguments[i].getClass().equals(Character.class))))) {
- newArguments[i] = arguments[i];
+ (Number.class.isAssignableFrom(argumentsCopy[i].getClass()) ||
+ argumentsCopy[i].getClass().equals(Boolean.class) ||
+ argumentsCopy[i].getClass().equals(Byte.class) ||
+ argumentsCopy[i].getClass().equals(Character.class))))) {
+ newArguments[i] = argumentsCopy[i];
} else {
found = false;
break;
@@ -166,8 +169,8 @@ public final class JavaTranslator<S extends TraversalSource, T extends Traversal
}
}
} catch (final Throwable e) {
- throw new IllegalStateException(e.getMessage() + ":" + methodName + "(" + Arrays.toString(arguments) + ")", e);
+ throw new IllegalStateException(e.getMessage() + ":" + methodName + "(" + Arrays.toString(argumentsCopy) + ")", e);
}
- throw new IllegalStateException("Could not locate method: " + delegate.getClass().getSimpleName() + "." + methodName + "(" + Arrays.toString(arguments) + ")");
+ throw new IllegalStateException("Could not locate method: " + delegate.getClass().getSimpleName() + "." + methodName + "(" + Arrays.toString(argumentsCopy) + ")");
}
}
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/13782aad/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/BytecodeTest.java
----------------------------------------------------------------------
diff --git a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/BytecodeTest.java b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/BytecodeTest.java
new file mode 100644
index 0000000..0b9c65a
--- /dev/null
+++ b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/BytecodeTest.java
@@ -0,0 +1,97 @@
+/*
+ * 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;
+
+import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversalSource;
+import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__;
+import org.apache.tinkerpop.gremlin.structure.Column;
+import org.apache.tinkerpop.gremlin.structure.util.empty.EmptyGraph;
+import org.apache.tinkerpop.gremlin.util.iterator.IteratorUtils;
+import org.junit.Test;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotEquals;
+
+/**
+ * @author Marko A. Rodriguez (http://markorodriguez.com)
+ */
+public class BytecodeTest {
+
+ @Test
+ public void shouldHaveProperHashAndEquality() {
+ final GraphTraversalSource g = EmptyGraph.instance().traversal();
+ final Traversal.Admin traversal1 = g.V().out().repeat(__.out().in()).times(2).groupCount().by(__.outE().count()).select(Column.keys).order().by(Order.decr).asAdmin();
+ final Traversal.Admin traversal2 = g.V().out().repeat(__.out().in()).times(2).groupCount().by(__.outE().count()).select(Column.keys).order().by(Order.decr).asAdmin();
+ final Traversal.Admin traversal3 = g.V().out().repeat(__.out().in()).times(2).groupCount().by(__.outE().count()).select(Column.values).order().by(Order.decr).asAdmin();
+
+ assertEquals(traversal1, traversal2);
+ assertNotEquals(traversal1, traversal3);
+ assertNotEquals(traversal2, traversal3);
+ //
+ assertEquals(traversal1.hashCode(), traversal2.hashCode());
+ assertNotEquals(traversal1.hashCode(), traversal3.hashCode());
+ assertNotEquals(traversal2.hashCode(), traversal3.hashCode());
+ //
+ assertEquals(traversal1.getBytecode(), traversal2.getBytecode());
+ assertNotEquals(traversal1.getBytecode(), traversal3.getBytecode());
+ assertNotEquals(traversal2.getBytecode(), traversal3.getBytecode());
+ //
+ assertEquals(traversal1.getBytecode().hashCode(), traversal2.getBytecode().hashCode());
+ assertNotEquals(traversal1.getBytecode().hashCode(), traversal3.getBytecode().hashCode());
+ assertNotEquals(traversal2.getBytecode().hashCode(), traversal3.getBytecode().hashCode());
+
+ }
+
+ @Test
+ public void shouldCloneCorrectly() {
+ final GraphTraversalSource g = EmptyGraph.instance().traversal();
+ final Bytecode bytecode = g.V().out().asAdmin().getBytecode();
+ final Bytecode bytecodeClone = bytecode.clone();
+ assertEquals(bytecode, bytecodeClone);
+ assertEquals(bytecode.hashCode(), bytecodeClone.hashCode());
+ bytecodeClone.addStep("in", "created"); // mutate the clone and the original should stay the same
+ assertNotEquals(bytecode, bytecodeClone);
+ assertNotEquals(bytecode.hashCode(), bytecodeClone.hashCode());
+ assertEquals(2, IteratorUtils.count(bytecode.getInstructions()));
+ assertEquals(3, IteratorUtils.count(bytecodeClone.getInstructions()));
+ }
+
+ @Test
+ public void shouldIncludeBindingsInEquality() {
+ final Bindings b = new Bindings();
+ final GraphTraversalSource g = EmptyGraph.instance().traversal().withBindings(b);
+
+ final Bytecode bytecode1 = g.V().out(b.of("a", "created")).asAdmin().getBytecode();
+ final Bytecode bytecode2 = g.V().out(b.of("a", "knows")).asAdmin().getBytecode();
+ final Bytecode bytecode3 = g.V().out(b.of("b", "knows")).asAdmin().getBytecode();
+ final Bytecode bytecode4 = g.V().out(b.of("b", "knows")).asAdmin().getBytecode();
+
+ assertNotEquals(bytecode1, bytecode2);
+ assertNotEquals(bytecode1, bytecode3);
+ assertNotEquals(bytecode2, bytecode3);
+ assertNotEquals(bytecode2, bytecode4);
+ assertNotEquals(bytecode1, bytecode4);
+ assertEquals(bytecode3, bytecode4);
+
+ assertEquals(1, bytecode1.getBindings().size());
+ assertEquals("created", bytecode1.getBindings().get("a"));
+
+ }
+}
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/13782aad/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerGraphPlayTest.java
----------------------------------------------------------------------
diff --git a/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerGraphPlayTest.java b/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerGraphPlayTest.java
index 9f2b12a..94fd250 100644
--- a/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerGraphPlayTest.java
+++ b/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerGraphPlayTest.java
@@ -22,29 +22,30 @@ import org.apache.tinkerpop.gremlin.jsr223.JavaTranslator;
import org.apache.tinkerpop.gremlin.process.computer.Computer;
import org.apache.tinkerpop.gremlin.process.traversal.Bytecode;
import org.apache.tinkerpop.gremlin.process.traversal.Operator;
-import org.apache.tinkerpop.gremlin.process.traversal.P;
+import org.apache.tinkerpop.gremlin.process.traversal.Order;
import org.apache.tinkerpop.gremlin.process.traversal.Scope;
import org.apache.tinkerpop.gremlin.process.traversal.Traversal;
import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversal;
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.strategy.optimization.PathRetractionStrategy;
+import org.apache.tinkerpop.gremlin.structure.Column;
import org.apache.tinkerpop.gremlin.structure.Graph;
import org.apache.tinkerpop.gremlin.structure.T;
import org.apache.tinkerpop.gremlin.structure.Vertex;
import org.apache.tinkerpop.gremlin.structure.io.graphml.GraphMLIo;
-import org.apache.tinkerpop.gremlin.structure.io.graphson.GraphSONReader;
-import org.apache.tinkerpop.gremlin.structure.io.graphson.GraphSONWriter;
import org.apache.tinkerpop.gremlin.util.TimeUtil;
import org.junit.Ignore;
import org.junit.Test;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-import java.io.ByteArrayInputStream;
-import java.io.ByteArrayOutputStream;
import java.util.Arrays;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.LinkedHashSet;
import java.util.List;
+import java.util.Map;
import java.util.function.Supplier;
import static org.apache.tinkerpop.gremlin.process.traversal.P.lt;
@@ -61,7 +62,6 @@ import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.outE;
import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.select;
import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.union;
import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.valueMap;
-import static org.apache.tinkerpop.gremlin.util.function.Lambda.function;
/**
* @author Stephen Mallette (http://stephen.genoprime.com)
@@ -73,24 +73,39 @@ public class TinkerGraphPlayTest {
@Ignore
public void testPlay8() throws Exception {
Graph graph = TinkerFactory.createModern();
- GraphTraversalSource g = graph.traversal().withComputer();
+ GraphTraversalSource g = graph.traversal();
- Traversal<?, ?> traversal1 = g.V().has("age", P.gt(10).and(P.lt(30))).out("knows", "created").repeat(__.as("a").out().as("b").hasLabel("software")).times(1).select("b").by(T.label).groupCount().map(function("a.get()"));
- Bytecode bytecode1 = traversal1.asAdmin().getBytecode();
- System.out.println("BYTECODE 1: \n " + bytecode1 + "\n");
- final ByteArrayOutputStream outputStream = new ByteArrayOutputStream();
- GraphSONWriter.build().create().writeObject(outputStream, bytecode1);
- //
- System.out.println("GRAPHSON BYTECODE: \n " + new String(outputStream.toByteArray()) + "\n");
- //
- Traversal<?, ?> traversal2 = JavaTranslator.of(graph.traversal()).translate(GraphSONReader.build().create().readObject(new ByteArrayInputStream(outputStream.toByteArray()), Bytecode.class));
- Bytecode bytecode2 = traversal2.asAdmin().getBytecode();
- System.out.println("BYTECODE 2: \n " + bytecode2 + "\n");
- assert traversal1.equals(traversal2);
- assert bytecode1.equals(bytecode2);
- System.out.println("RESULT: \n " + traversal2.toList());
- }
+ final Traversal<?, ?> traversal = g.V().repeat(out()).times(2).groupCount().by("name").select(Column.keys).order().by(Order.decr);
+ final Bytecode bytecode = traversal.asAdmin().getBytecode();
+
+ final Map<Bytecode, Traversal.Admin<?, ?>> cache = new HashMap<>();
+ cache.put(bytecode, traversal.asAdmin());
+ final HashSet<?> result = new LinkedHashSet<>(Arrays.asList("ripple", "lop"));
+ System.out.println("BYTECODE: " + bytecode + "\n");
+ System.out.println("Bytecode->Traversal.clone() cache: " + TimeUtil.clock(1000, () -> {
+ final Traversal.Admin<?, ?> t = cache.get(bytecode).clone();
+ //assertEquals(result, t.next());
+ }));
+
+ System.out.println("Bytecode->JavaTranslator call : " + TimeUtil.clock(1000, () -> {
+ final Traversal t = JavaTranslator.of(g).translate(bytecode);
+ //assertEquals(result, t.next());
+ }));
+
+ System.out.println("\n==Second test with reversed execution==\n");
+
+ System.out.println("BYTECODE: " + bytecode + "\n");
+ System.out.println("Bytecode->JavaTranslator call : " + TimeUtil.clock(1000, () -> {
+ final Traversal t = JavaTranslator.of(g).translate(bytecode);
+ //assertEquals(result, t.next());
+ }));
+
+ System.out.println("Bytecode->Traversal.clone() cache: " + TimeUtil.clock(1000, () -> {
+ final Traversal.Admin<?, ?> t = cache.get(bytecode).clone();
+ //assertEquals(result, t.next());
+ }));
+ }
@Test
@Ignore