From commits-return-27072-archive-asf-public=cust-asf.ponee.io@tinkerpop.apache.org Tue Mar 13 19:33:30 2018 Return-Path: X-Original-To: archive-asf-public@cust-asf.ponee.io Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by mx-eu-01.ponee.io (Postfix) with SMTP id 4D97E18072F for ; Tue, 13 Mar 2018 19:33:28 +0100 (CET) Received: (qmail 36904 invoked by uid 500); 13 Mar 2018 18:33:27 -0000 Mailing-List: contact commits-help@tinkerpop.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@tinkerpop.apache.org Delivered-To: mailing list commits@tinkerpop.apache.org Received: (qmail 36849 invoked by uid 99); 13 Mar 2018 18:33:27 -0000 Received: from git1-us-west.apache.org (HELO git1-us-west.apache.org) (140.211.11.23) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 13 Mar 2018 18:33:27 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id 3AC27F65D8; Tue, 13 Mar 2018 18:33:27 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: dkuppitz@apache.org To: commits@tinkerpop.apache.org Date: Tue, 13 Mar 2018 18:33:31 -0000 Message-Id: In-Reply-To: References: X-Mailer: ASF-Git Admin Mailer Subject: [05/31] tinkerpop git commit: TINKERPOP-1911 Refactored JavaTranslator TINKERPOP-1911 Refactored JavaTranslator Cached methods that were being reflected on every translation which improved performance a bit. Project: http://git-wip-us.apache.org/repos/asf/tinkerpop/repo Commit: http://git-wip-us.apache.org/repos/asf/tinkerpop/commit/d9db27f4 Tree: http://git-wip-us.apache.org/repos/asf/tinkerpop/tree/d9db27f4 Diff: http://git-wip-us.apache.org/repos/asf/tinkerpop/diff/d9db27f4 Branch: refs/heads/TRAVIS-TEST Commit: d9db27f4aa7b8227a557cf5bbb31203612d7548f Parents: 2c6c151 Author: Stephen Mallette Authored: Tue Mar 6 13:00:30 2018 -0500 Committer: Stephen Mallette Committed: Tue Mar 6 13:00:30 2018 -0500 ---------------------------------------------------------------------- CHANGELOG.asciidoc | 1 + .../jsr223/JavaTranslatorBenchmark.java | 76 ++++++++++++++++++++ .../gremlin/jsr223/JavaTranslator.java | 58 ++++++++++++--- 3 files changed, 125 insertions(+), 10 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/d9db27f4/CHANGELOG.asciidoc ---------------------------------------------------------------------- diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index b9f22e1..54e5347 100644 --- a/CHANGELOG.asciidoc +++ b/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. http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/d9db27f4/gremlin-benchmark/src/main/java/org/apache/tinkerpop/jsr223/JavaTranslatorBenchmark.java ---------------------------------------------------------------------- diff --git a/gremlin-benchmark/src/main/java/org/apache/tinkerpop/jsr223/JavaTranslatorBenchmark.java b/gremlin-benchmark/src/main/java/org/apache/tinkerpop/jsr223/JavaTranslatorBenchmark.java new file mode 100644 index 0000000..d33b574 --- /dev/null +++ b/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> translator = JavaTranslator.of(g); + + @Benchmark + public GraphTraversal.Admin testTranslationShort() { + return translator.translate(g.V().asAdmin().getBytecode()); + } + + @Benchmark + public GraphTraversal.Admin testTranslationMedium() { + return translator.translate(g.V().out().in("link").out().in("link").asAdmin().getBytecode()); + } + + @Benchmark + public GraphTraversal.Admin 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")). + select("d").by("name").asAdmin().getBytecode()); + } + + @Benchmark + public GraphTraversal.Admin testTranslationWithStrategy() { + return translator.translate(g.withStrategies(ReadOnlyStrategy.instance()) + .withStrategies(SubgraphStrategy.build().vertices(hasLabel("person")).create()) + .V().out().in("link").out().in("link").asAdmin().getBytecode()); + } +} http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/d9db27f4/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 c39ee23..df12055 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 @@ -53,13 +53,15 @@ public final class JavaTranslator anonymousTraversal; private static final Map, Map>> GLOBAL_METHOD_CACHE = new ConcurrentHashMap<>(); - + private final Map, Map> 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 > JavaTranslator of(final S traversalSource) { @@ -110,7 +112,7 @@ public final class JavaTranslator 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 @@ public final class JavaTranslator 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 map = object instanceof Tree ? new Tree() : @@ -163,6 +159,37 @@ public final class JavaTranslator map) { + final Class strategyClass = ((TraversalStrategyProxy) delegate).getStrategyClass(); + final Map methodCache = localMethodCache.computeIfAbsent(strategyClass, k -> { + final Map 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> methodCache = GLOBAL_METHOD_CACHE.getOrDefault(delegate.getClass(), new HashMap<>()); @@ -247,4 +274,15 @@ public final class JavaTranslator