You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tinkerpop.apache.org by sp...@apache.org on 2017/03/09 12:46:32 UTC
[06/12] tinkerpop git commit: TINKERPOP-1644 Improve script
compilation syncronisation Script compilation is synchronised. Script
compilation times are placed in to logs. Failed scripts will not be
recompiled. Scripts that take over 5 seconds to compile
TINKERPOP-1644 Improve script compilation syncronisation
Script compilation is synchronised.
Script compilation times are placed in to logs.
Failed scripts will not be recompiled.
Scripts that take over 5 seconds to compile are logged as a warning.
Project: http://git-wip-us.apache.org/repos/asf/tinkerpop/repo
Commit: http://git-wip-us.apache.org/repos/asf/tinkerpop/commit/37354c04
Tree: http://git-wip-us.apache.org/repos/asf/tinkerpop/tree/37354c04
Diff: http://git-wip-us.apache.org/repos/asf/tinkerpop/diff/37354c04
Branch: refs/heads/TINKERPOP-1644
Commit: 37354c04834a889c3bd822efcadfc69e48946c4c
Parents: 3fe223b
Author: BrynCooke <br...@gmail.com>
Authored: Thu Mar 2 19:07:28 2017 +0000
Committer: Stephen Mallette <sp...@genoprime.com>
Committed: Thu Mar 9 07:45:57 2017 -0500
----------------------------------------------------------------------
CHANGELOG.asciidoc | 4 ++
.../jsr223/GremlinGroovyScriptEngine.java | 57 ++++++++++++++++++--
2 files changed, 56 insertions(+), 5 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/37354c04/CHANGELOG.asciidoc
----------------------------------------------------------------------
diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc
index 04541d6..f14412e 100644
--- a/CHANGELOG.asciidoc
+++ b/CHANGELOG.asciidoc
@@ -26,6 +26,10 @@ image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima
TinkerPop 3.2.5 (Release Date: NOT OFFICIALLY RELEASED YET)
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+* Script compilation is synchronised.
+* Script compilation times are placed in to logs.
+* Failed scripts will not be recompiled.
+* Scripts that take over 5 seconds to compile are logged as a warning.
* Split `ComputerVerificationStrategy` into two strategies: `ComputerVerificationStrategy` and `ComputerFinalizationStrategy`.
* Removed `HasTest.g_V_hasId_compilationEquality` from process test suite as it makes too many assumptions about provider compilation.
* Deprecated `CustomizerProvider` infrastructure.
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/37354c04/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 1fb2efc..a8365a2 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
@@ -56,6 +56,8 @@ import org.codehaus.groovy.runtime.InvokerHelper;
import org.codehaus.groovy.runtime.MetaClassHelper;
import org.codehaus.groovy.runtime.MethodClosure;
import org.codehaus.groovy.util.ReferenceBundle;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
import javax.script.Bindings;
import javax.script.CompiledScript;
@@ -93,6 +95,7 @@ import java.util.stream.Collectors;
public class GremlinGroovyScriptEngine extends GroovyScriptEngineImpl
implements DependencyManager, AutoCloseable, GremlinScriptEngine {
+ private static final Logger log = LoggerFactory.getLogger(GremlinGroovyScriptEngine.class);
/**
* An "internal" key for sandboxing the script engine - technically not for public use.
*/
@@ -153,6 +156,7 @@ public class GremlinGroovyScriptEngine extends GroovyScriptEngineImpl
* Script to generated Class map.
*/
private ManagedConcurrentValueMap<String, Class> classMap = new ManagedConcurrentValueMap<>(ReferenceBundle.getSoftBundle());
+ private ManagedConcurrentValueMap<String, CompilationFailedException> failedClassMap = new ManagedConcurrentValueMap<>(ReferenceBundle.getSoftBundle());
/**
* Global closures map - this is used to simulate a single global functions namespace
@@ -529,13 +533,56 @@ public class GremlinGroovyScriptEngine extends GroovyScriptEngineImpl
return makeInterface(thiz, clazz);
}
+ private Class getScriptClassFromMap(String script) throws CompilationFailedException {
+ CompilationFailedException exception = failedClassMap.get(script);
+ if(exception != null) {
+ throw exception;
+ }
+ return classMap.get(script);
+
+ }
+
Class getScriptClass(final String script) throws CompilationFailedException {
- Class clazz = classMap.get(script);
- if (clazz != null) return clazz;
+ Class clazz = getScriptClassFromMap(script);
+ if (clazz != null) {
+ return clazz;
+ }
+ synchronized (this) {
+ clazz = getScriptClassFromMap(script);
+ if (clazz != null) {
+ return clazz;
+ }
- clazz = loader.parseClass(script, generateScriptName());
- classMap.put(script, clazz);
- return clazz;
+ long start = System.currentTimeMillis();
+ try {
+ clazz = loader.parseClass(script, generateScriptName());
+ 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);
+ }
+
+
+ }
+ catch(CompilationFailedException t) {
+ long finish = System.currentTimeMillis();
+ log.error("Script compilation FAILED {} took {}ms {}", script, finish - start, t);
+ failedClassMap.put(script, t);
+ throw t;
+ }
+ catch(Throwable t) {
+ long finish = System.currentTimeMillis();
+ log.error("Script compilation FAILED with throwable {} took {}ms {}", script, finish - start, t);
+ throw t;
+ }
+ classMap.put(script, clazz);
+
+ return clazz;
+ }
}
boolean isCached(final String script) {