TINKERPOP-1644 Use Caffeine cache
Project: http://git-wip-us.apache.org/repos/asf/tinkerpop/repo Commit: http://git-wip-us.apache.org/repos/asf/tinkerpop/commit/18778e40 Tree: http://git-wip-us.apache.org/repos/asf/tinkerpop/tree/18778e40 Diff: http://git-wip-us.apache.org/repos/asf/tinkerpop/diff/18778e40 Branch: refs/heads/TINKERPOP-1642 Commit: 18778e405981523355319fa56bd04fd6cc07b845 Parents: deb6828 Author: BrynCooke <[email protected]> Authored: Wed Mar 8 12:24:46 2017 +0000 Committer: Stephen Mallette <[email protected]> Committed: Fri Mar 10 11:12:36 2017 -0500 ---------------------------------------------------------------------- gremlin-groovy/pom.xml | 5 ++ .../jsr223/GremlinGroovyScriptEngine.java | 75 +++++++++++--------- 2 files changed, 48 insertions(+), 32 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/18778e40/gremlin-groovy/pom.xml ---------------------------------------------------------------------- diff --git a/gremlin-groovy/pom.xml b/gremlin-groovy/pom.xml index 511952a..474b458 100644 --- a/gremlin-groovy/pom.xml +++ b/gremlin-groovy/pom.xml @@ -70,6 +70,11 @@ limitations under the License. <artifactId>jbcrypt</artifactId> <version>0.4</version> </dependency> + <dependency> + <groupId>com.github.ben-manes.caffeine</groupId> + <artifactId>caffeine</artifactId> + <version>2.3.1</version> + </dependency> <!-- TEST --> <dependency> <groupId>org.slf4j</groupId> http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/18778e40/gremlin-groovy/src/main/java/org/apache/tinkerpop/gremlin/groovy/jsr223/GremlinGroovyScriptEngine.java ---------------------------------------------------------------------- diff --git a/gremlin-groovy/src/main/java/org/apache/tinkerpop/gremlin/groovy/jsr223/GremlinGroovyScriptEngine.java b/gremlin-groovy/src/main/java/org/apache/tinkerpop/gremlin/groovy/jsr223/GremlinGroovyScriptEngine.java index 67275f8..51850ad 100644 --- a/gremlin-groovy/src/main/java/org/apache/tinkerpop/gremlin/groovy/jsr223/GremlinGroovyScriptEngine.java +++ b/gremlin-groovy/src/main/java/org/apache/tinkerpop/gremlin/groovy/jsr223/GremlinGroovyScriptEngine.java @@ -18,6 +18,9 @@ */ package org.apache.tinkerpop.gremlin.groovy.jsr223; +import com.github.benmanes.caffeine.cache.CacheLoader; +import com.github.benmanes.caffeine.cache.Caffeine; +import com.github.benmanes.caffeine.cache.LoadingCache; import groovy.grape.Grape; import groovy.lang.Binding; import groovy.lang.Closure; @@ -80,6 +83,8 @@ import java.util.Map; import java.util.ServiceLoader; import java.util.Set; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.Executor; import java.util.concurrent.Future; import java.util.concurrent.atomic.AtomicLong; import java.util.regex.Pattern; @@ -154,17 +159,44 @@ public class GremlinGroovyScriptEngine extends GroovyScriptEngineImpl } }; + private GremlinGroovyClassLoader loader; + /** * Script to generated Class map. */ - private ManagedConcurrentValueMap<String, Future<Class>> classMap = new ManagedConcurrentValueMap<>(ReferenceBundle.getSoftBundle()); + private LoadingCache<String, Future<Class>> classMap = Caffeine.newBuilder().softValues().build(new CacheLoader<String, Future<Class>>() { + @Override + public Future<Class> load(String script) throws Exception { + long start = System.currentTimeMillis(); + try { + return CompletableFuture.supplyAsync(() -> loader.parseClass(script, generateScriptName()), command -> command.run()); + } catch (Exception e) { + if (e.getCause() instanceof CompilationFailedException) { + long finish = System.currentTimeMillis(); + log.error("Script compilation FAILED {} took {}ms {}", script, finish - start, e.getCause()); + throw (CompilationFailedException) e.getCause(); + } else { + throw new AssertionError("Unexpected exception when compiling script", e); + } + } finally { + long time = System.currentTimeMillis() - start; + if (time > 5000) { + //We warn if a script took longer than a few seconds. Repeatedly seeing these warnings is a sign that something is wrong. + //Scripts with a large numbers of parameters often trigger this and should be avoided. + log.warn("Script compilation {} took {}ms", script, time); + } else { + log.debug("Script compilation {} took {}ms", script, time); + } + } + } + + }); /** * Global closures map - this is used to simulate a single global functions namespace */ private ManagedConcurrentValueMap<String, Closure> globalClosures = new ManagedConcurrentValueMap<>(ReferenceBundle.getHardBundle()); - private GremlinGroovyClassLoader loader; private AtomicLong counter = new AtomicLong(0L); @@ -420,7 +452,7 @@ public class GremlinGroovyScriptEngine extends GroovyScriptEngineImpl // must clear the local cache here because the classloader has been reset. therefore, classes previously // referenced before that might not have evaluated might cleanly evaluate now. - classMap.clear(); + classMap.invalidateAll(); globalClosures.clear(); final Set<Artifact> toReuse = new HashSet<>(artifactsToUse); @@ -535,41 +567,20 @@ public class GremlinGroovyScriptEngine extends GroovyScriptEngineImpl } Class getScriptClass(final String script) throws CompilationFailedException { - Future<Class> clazz = classMap.get(script); - - long start = System.currentTimeMillis(); try { - if (clazz != null) { - return clazz.get(); - } - - clazz = CompletableFuture.supplyAsync(() -> loader.parseClass(script, generateScriptName())); - classMap.put(script, clazz); - return clazz.get(); - - } catch (Exception e) { - if (e.getCause() instanceof CompilationFailedException) { - long finish = System.currentTimeMillis(); - log.error("Script compilation FAILED {} took {}ms {}", script, finish - start, e.getCause()); - throw (CompilationFailedException) e.getCause(); - } else { - throw new AssertionError("Unexpected exception when compiling script", e); - } - } finally { - long time = System.currentTimeMillis() - start; - if (time > 5000) { - //We warn if a script took longer than a few seconds. Repeatedly seeing these warnings is a sign that something is wrong. - //Scripts with a large numbers of parameters often trigger this and should be avoided. - log.warn("Script compilation {} took {}ms", script, time); - } else { - log.debug("Script compilation {} took {}ms", script, time); - } + return classMap.get(script).get(); + } catch (ExecutionException e) { + throw ((CompilationFailedException)e.getCause()); + } catch (InterruptedException e) { + //This should never happen as the future should completed before it is returned to the us. + throw new AssertionError(); } + } boolean isCached(final String script) { - return classMap.get(script) != null; + return classMap.getIfPresent(script) != null; } Object eval(final Class scriptClass, final ScriptContext context) throws ScriptException {
