TINKERPOP-1911 Refactored JavaTranslator

CHANGELOG.asciidoc

@@ -27,6 +27,7 @@ image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima
 * Modified `GremlinDslProcessor` so that it generated the `getAnonymousTraversalClass()` method to return the DSL version of `__`.
 * Added the "Kitchen Sink" test data set.
 * Fixed deserialization of `P.not()` for GraphSON.
+* Improved performance of `JavaTranslator` by caching reflected methods required for traversal construction.
 * Added `idleConnectionTimeout` and `keepAliveInterval` to Gremlin Server that enables a "ping" and auto-close for seemingly dead clients.
 * Fixed a bug in `NumberHelper` that led to wrong min/max results if numbers exceeded the Integer limits.
 * Delayed setting of the request identifier until `RequestMessage` construction by the builder.

gremlin-benchmark/src/main/java/org/apache/tinkerpop/jsr223/JavaTranslatorBenchmark.java

@@ -0,0 +1,76 @@
+/*
+ * 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.jsr223;
+
+import org.apache.tinkerpop.benchmark.util.AbstractBenchmarkBase;
+import org.apache.tinkerpop.gremlin.jsr223.JavaTranslator;
+import org.apache.tinkerpop.gremlin.process.traversal.P;
+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.strategy.decoration.SubgraphStrategy;
+import org.apache.tinkerpop.gremlin.process.traversal.strategy.verification.ReadOnlyStrategy;
+import org.apache.tinkerpop.gremlin.structure.Graph;
+import org.apache.tinkerpop.gremlin.structure.Vertex;
+import org.apache.tinkerpop.gremlin.structure.util.empty.EmptyGraph;
+import org.openjdk.jmh.annotations.Benchmark;
+import org.openjdk.jmh.annotations.Scope;
+import org.openjdk.jmh.annotations.State;
+
+import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.as;
+import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.hasLabel;
+import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.inE;
+import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.values;
+
+/**
+ * @author Stephen Mallette (http://stephen.genoprime.com)
+ */
+@State(Scope.Thread)
+public class JavaTranslatorBenchmark extends AbstractBenchmarkBase {
+
+    private final Graph graph = EmptyGraph.instance();
+    private final GraphTraversalSource g = graph.traversal();
+    private final JavaTranslator<GraphTraversalSource, GraphTraversal.Admin<Vertex,Vertex>> translator = JavaTranslator.of(g);
+
+    @Benchmark
+    public GraphTraversal.Admin<Vertex,Vertex> testTranslationShort() {
+        return translator.translate(g.V().asAdmin().getBytecode());
+    }
+
+    @Benchmark
+    public GraphTraversal.Admin<Vertex,Vertex> testTranslationMedium() {
+        return translator.translate(g.V().out().in("link").out().in("link").asAdmin().getBytecode());
+    }
+
+    @Benchmark
+    public GraphTraversal.Admin<Vertex,Vertex> testTranslationLong() {
+        return translator.translate(g.V().match(
+                as("a").has("song", "name", "HERE COMES SUNSHINE"),
+                as("a").map(inE("followedBy").values("weight").mean()).as("b"),
+                as("a").inE("followedBy").as("c"),
+                as("c").filter(values("weight").where(P.gte("b"))).outV().as("d")).
+                <String>select("d").by("name").asAdmin().getBytecode());
+    }
+
+    @Benchmark
+    public GraphTraversal.Admin<Vertex,Vertex> testTranslationWithStrategy() {
+        return translator.translate(g.withStrategies(ReadOnlyStrategy.instance())
+                .withStrategies(SubgraphStrategy.build().vertices(hasLabel("person")).create())
+                .V().out().in("link").out().in("link").asAdmin().getBytecode());
+    }
+}

gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/jsr223/JavaTranslator.java

@@ -53,13 +53,15 @@ public final class JavaTranslator<S extends TraversalSource, T extends Traversal
     private static final boolean IS_TESTING = Boolean.valueOf(System.getProperty("is.testing", "false"));
 
     private final S traversalSource;
-    private final Class anonymousTraversal;
+    private final Class<?> anonymousTraversal;
     private static final Map<Class<?>, Map<String, List<Method>>> GLOBAL_METHOD_CACHE = new ConcurrentHashMap<>();
-
+    private final Map<Class<?>, Map<String,Method>> localMethodCache = new ConcurrentHashMap<>();
+    private final Method anonymousTraversalStart;
 
     private JavaTranslator(final S traversalSource) {
         this.traversalSource = traversalSource;
         this.anonymousTraversal = traversalSource.getAnonymousTraversalClass().orElse(null);
+        this.anonymousTraversalStart = getStartMethodFromAnonymousTraversal();
     }
 
     public static <S extends TraversalSource, T extends Traversal.Admin<?, ?>> JavaTranslator<S, T> of(final S traversalSource) {
@@ -110,7 +112,7 @@ private Object translateObject(final Object object) {
             return translateObject(((Bytecode.Binding) object).value());
         else if (object instanceof Bytecode) {
             try {
-                final Traversal.Admin<?, ?> traversal = (Traversal.Admin) this.anonymousTraversal.getMethod("start").invoke(null);
+                final Traversal.Admin<?, ?> traversal = (Traversal.Admin) this.anonymousTraversalStart.invoke(null);
                 for (final Bytecode.Instruction instruction : ((Bytecode) object).getStepInstructions()) {
                     invokeMethod(traversal, Traversal.class, instruction.getOperator(), instruction.getArguments());
                 }
@@ -122,13 +124,7 @@ else if (object instanceof Bytecode) {
             final Map<String, Object> map = new HashMap<>();
             final Configuration configuration = ((TraversalStrategyProxy) object).getConfiguration();
             configuration.getKeys().forEachRemaining(key -> map.put(key, translateObject(configuration.getProperty(key))));
-            try {
-                return map.isEmpty() ?
-                        ((TraversalStrategyProxy) object).getStrategyClass().getMethod("instance").invoke(null) :
-                        ((TraversalStrategyProxy) object).getStrategyClass().getMethod("create", Configuration.class).invoke(null, new MapConfiguration(map));
-            } catch (final NoSuchMethodException | InvocationTargetException | IllegalAccessException e) {
-                throw new IllegalStateException(e.getMessage(), e);
-            }
+            return invokeStrategyCreationMethod(object, map);
         } else if (object instanceof Map) {
             final Map<Object, Object> map = object instanceof Tree ?
                     new Tree() :
@@ -163,6 +159,37 @@ else if (object instanceof Bytecode) {
             return object;
     }
 
+    private Object invokeStrategyCreationMethod(final Object delegate, final Map<String, Object> map) {
+        final Class<?> strategyClass = ((TraversalStrategyProxy) delegate).getStrategyClass();
+        final Map<String, Method> methodCache = localMethodCache.computeIfAbsent(strategyClass, k -> {
+            final Map<String, Method> cacheEntry = new HashMap<>();
+            try {
+                cacheEntry.put("instance", strategyClass.getMethod("instance"));
+            } catch (NoSuchMethodException ignored) {
+                // nothing - the strategy may not be constructed this way
+            }
+
+            try {
+                cacheEntry.put("create", strategyClass.getMethod("create", Configuration.class));
+            } catch (NoSuchMethodException ignored) {
+                // nothing - the strategy may not be constructed this way
+            }
+
+            if (cacheEntry.isEmpty())
+                throw new IllegalStateException(String.format("%s does can only be constructed with instance() or create(Configuration)", strategyClass.getSimpleName()));
+
+            return cacheEntry;
+        });
+
+        try {
+            return map.isEmpty() ?
+                    methodCache.get("instance").invoke(null) :
+                    methodCache.get("create").invoke(null, new MapConfiguration(map));
+        } catch (final InvocationTargetException | IllegalAccessException e) {
+            throw new IllegalStateException(e.getMessage(), e);
+        }
+    }
+
     private Object invokeMethod(final Object delegate, final Class returnType, final String methodName, final Object... arguments) {
         // populate method cache for fast access to methods in subsequent calls
         final Map<String, List<Method>> methodCache = GLOBAL_METHOD_CACHE.getOrDefault(delegate.getClass(), new HashMap<>());
@@ -247,4 +274,15 @@ private synchronized static void buildMethodCache(final Object delegate, final M
             GLOBAL_METHOD_CACHE.put(delegate.getClass(), methodCache);
         }
     }
+
+    private Method getStartMethodFromAnonymousTraversal() {
+        if (this.anonymousTraversal != null) {
+            try {
+                return this.anonymousTraversal.getMethod("start");
+            } catch (NoSuchMethodException ignored) {
+            }
+        }
+
+        return null;
+    }
 }