From 2a2b59b15833d80587bb8127d6edfd38a17ab788 Mon Sep 17 00:00:00 2001 From: "Marko A. Rodriguez" Date: Tue, 13 Sep 2016 13:23:32 -0600 Subject: [PATCH 1/4] JavaTranslator now makes use of a static method cache. This speeds up Bytecode->JavaTranslator translation. From 0.098ms to 0.029ms. If a GremlinServer Bytecode cache is not desired, then, at minimum we should use this JavaTranslator update in its place. --- .../gremlin/jsr223/JavaTranslator.java | 19 +++++++++++++++---- .../structure/TinkerGraphPlayTest.java | 2 +- 2 files changed, 16 insertions(+), 5 deletions(-) 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 dac6583cf85..d00ad1cb3b7 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 @@ -33,6 +33,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; /** * @author Marko A. Rodriguez (http://markorodriguez.com) @@ -41,12 +42,11 @@ public final class JavaTranslator> traversalSourceMethodCache = new HashMap<>(); - private final Map> traversalMethodCache = new HashMap<>(); + private static final Map, Map>> TRAVERSAL_SOURCE_METHOD_CACHE = new ConcurrentHashMap<>(); + private static final Map, Map>> TRAVERSAL_METHOD_CACHE = new ConcurrentHashMap<>(); private JavaTranslator(final S traversalSource) { this.traversalSource = traversalSource; - // todo: could produce an NPE later on. need a good model for when a traversal species doesn't support nesting. this.anonymousTraversal = traversalSource.getAnonymousTraversalClass().orElse(null); } @@ -102,8 +102,12 @@ public String toString() { } 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 = delegate instanceof TraversalSource ? this.traversalSourceMethodCache : this.traversalMethodCache; + final Map> methodCache = delegate instanceof TraversalSource ? + this.TRAVERSAL_SOURCE_METHOD_CACHE.getOrDefault(delegate.getClass(), new HashMap<>()) : + this.TRAVERSAL_METHOD_CACHE.getOrDefault(delegate.getClass(), new HashMap<>()); if (methodCache.isEmpty()) { for (final Method method : delegate.getClass().getMethods()) { if (!(method.getName().equals("addV") && method.getParameterCount() == 1 && method.getParameters()[0].getType().equals(Object[].class))) { // hack cause its hard to tell Object[] vs. String :| @@ -115,7 +119,14 @@ private Object invokeMethod(final Object delegate, final Class returnType, final list.add(method); } } + if (delegate instanceof TraversalSource) + TRAVERSAL_SOURCE_METHOD_CACHE.put((Class) delegate.getClass(), methodCache); + else + TRAVERSAL_METHOD_CACHE.put((Class) delegate.getClass(), methodCache); } + ////////////////////////// + ////////////////////////// + // 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++) { 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 94fd250c1b2..37187febe82 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 @@ -77,7 +77,7 @@ public void testPlay8() throws Exception { 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 JavaTranslator translator = JavaTranslator.of(g); final Map> cache = new HashMap<>(); cache.put(bytecode, traversal.asAdmin()); final HashSet result = new LinkedHashSet<>(Arrays.asList("ripple", "lop")); From ed8feea52e2055edcd45cd5c97f8ef3d60cc6820 Mon Sep 17 00:00:00 2001 From: "Marko A. Rodriguez" Date: Tue, 13 Sep 2016 13:30:27 -0600 Subject: [PATCH 2/4] removed this. from a static field name call. minor nothing. --- .../org/apache/tinkerpop/gremlin/jsr223/JavaTranslator.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 d00ad1cb3b7..a4f97a1c38a 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 @@ -106,8 +106,8 @@ private Object invokeMethod(final Object delegate, final Class returnType, final ////////////////////////// // populate method cache for fast access to methods in subsequent calls final Map> methodCache = delegate instanceof TraversalSource ? - this.TRAVERSAL_SOURCE_METHOD_CACHE.getOrDefault(delegate.getClass(), new HashMap<>()) : - this.TRAVERSAL_METHOD_CACHE.getOrDefault(delegate.getClass(), new HashMap<>()); + TRAVERSAL_SOURCE_METHOD_CACHE.getOrDefault(delegate.getClass(), new HashMap<>()) : + TRAVERSAL_METHOD_CACHE.getOrDefault(delegate.getClass(), new HashMap<>()); if (methodCache.isEmpty()) { for (final Method method : delegate.getClass().getMethods()) { if (!(method.getName().equals("addV") && method.getParameterCount() == 1 && method.getParameters()[0].getType().equals(Object[].class))) { // hack cause its hard to tell Object[] vs. String :| From 1fc52cb2d208fb0949e66f2e1cf2c1307495a537 Mon Sep 17 00:00:00 2001 From: "Marko A. Rodriguez" Date: Tue, 13 Sep 2016 13:34:47 -0600 Subject: [PATCH 3/4] reduced complexity by having a single GLOBAL_METHOD_CACHE for both Traversal and TraversalSource methods. Cleaner code with less instanceof stuff. --- .../tinkerpop/gremlin/jsr223/JavaTranslator.java | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) 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 a4f97a1c38a..ed2def22c9e 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 @@ -42,8 +42,8 @@ public final class JavaTranslator, Map>> TRAVERSAL_SOURCE_METHOD_CACHE = new ConcurrentHashMap<>(); - private static final Map, Map>> TRAVERSAL_METHOD_CACHE = new ConcurrentHashMap<>(); + private static final Map, Map>> GLOBAL_METHOD_CACHE = new ConcurrentHashMap<>(); + private JavaTranslator(final S traversalSource) { this.traversalSource = traversalSource; @@ -105,9 +105,7 @@ private Object invokeMethod(final Object delegate, final Class returnType, final ////////////////////////// ////////////////////////// // populate method cache for fast access to methods in subsequent calls - final Map> methodCache = delegate instanceof TraversalSource ? - TRAVERSAL_SOURCE_METHOD_CACHE.getOrDefault(delegate.getClass(), new HashMap<>()) : - TRAVERSAL_METHOD_CACHE.getOrDefault(delegate.getClass(), new HashMap<>()); + final Map> methodCache = GLOBAL_METHOD_CACHE.getOrDefault(delegate.getClass(), new HashMap<>()); if (methodCache.isEmpty()) { for (final Method method : delegate.getClass().getMethods()) { if (!(method.getName().equals("addV") && method.getParameterCount() == 1 && method.getParameters()[0].getType().equals(Object[].class))) { // hack cause its hard to tell Object[] vs. String :| @@ -119,10 +117,7 @@ private Object invokeMethod(final Object delegate, final Class returnType, final list.add(method); } } - if (delegate instanceof TraversalSource) - TRAVERSAL_SOURCE_METHOD_CACHE.put((Class) delegate.getClass(), methodCache); - else - TRAVERSAL_METHOD_CACHE.put((Class) delegate.getClass(), methodCache); + GLOBAL_METHOD_CACHE.put(delegate.getClass(), methodCache); } ////////////////////////// ////////////////////////// @@ -133,7 +128,7 @@ private Object invokeMethod(final Object delegate, final Class returnType, final if (arguments[i] instanceof Bytecode.Binding) argumentsCopy[i] = ((Bytecode.Binding) arguments[i]).value(); else if (arguments[i] instanceof Bytecode) - argumentsCopy[i] = translateFromAnonymous((Bytecode) arguments[i]); + argumentsCopy[i] = this.translateFromAnonymous((Bytecode) arguments[i]); else argumentsCopy[i] = arguments[i]; } From c47217d26336ff68dc8dbb9a028664cb80bdaf42 Mon Sep 17 00:00:00 2001 From: Stephen Mallette Date: Thu, 15 Sep 2016 15:42:12 -0400 Subject: [PATCH 4/4] Improved thread safety when building the method cache in JavaTranslator --- .../gremlin/jsr223/JavaTranslator.java | 34 +++++++++---------- 1 file changed, 17 insertions(+), 17 deletions(-) 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 ed2def22c9e..5d3e82ddf7e 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 @@ -102,25 +102,9 @@ public String toString() { } 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<>()); - if (methodCache.isEmpty()) { - for (final Method method : delegate.getClass().getMethods()) { - if (!(method.getName().equals("addV") && method.getParameterCount() == 1 && method.getParameters()[0].getType().equals(Object[].class))) { // hack cause its hard to tell Object[] vs. String :| - List list = methodCache.get(method.getName()); - if (null == list) { - list = new ArrayList<>(); - methodCache.put(method.getName(), list); - } - list.add(method); - } - } - GLOBAL_METHOD_CACHE.put(delegate.getClass(), methodCache); - } - ////////////////////////// - ////////////////////////// + if (methodCache.isEmpty()) buildMethodCache(delegate, methodCache); // create a copy of the argument array so as not to mutate the original bytecode final Object[] argumentsCopy = new Object[arguments.length]; @@ -179,4 +163,20 @@ else if (arguments[i] instanceof Bytecode) } throw new IllegalStateException("Could not locate method: " + delegate.getClass().getSimpleName() + "." + methodName + "(" + Arrays.toString(argumentsCopy) + ")"); } + + private synchronized static void buildMethodCache(final Object delegate, final Map> methodCache) { + if (methodCache.isEmpty()) { + for (final Method method : delegate.getClass().getMethods()) { + if (!(method.getName().equals("addV") && method.getParameterCount() == 1 && method.getParameters()[0].getType().equals(Object[].class))) { // hack cause its hard to tell Object[] vs. String :| + List list = methodCache.get(method.getName()); + if (null == list) { + list = new ArrayList<>(); + methodCache.put(method.getName(), list); + } + list.add(method); + } + } + GLOBAL_METHOD_CACHE.put(delegate.getClass(), methodCache); + } + } }