TINKERPOP-1095 Added a custom ScriptContext Used the custom ScriptContext, GremlinScriptContext, to prevent usage of the SimpleScriptContext which creates unnecessary ByteBuffer instances on every call to eval().
Project: http://git-wip-us.apache.org/repos/asf/tinkerpop/repo Commit: http://git-wip-us.apache.org/repos/asf/tinkerpop/commit/48daf8d2 Tree: http://git-wip-us.apache.org/repos/asf/tinkerpop/tree/48daf8d2 Diff: http://git-wip-us.apache.org/repos/asf/tinkerpop/diff/48daf8d2 Branch: refs/heads/TINKERPOP-1095 Commit: 48daf8d25fcf3abb9d2a3c307e65ec6613595959 Parents: e3e1dca Author: Stephen Mallette <[email protected]> Authored: Fri Mar 24 12:23:46 2017 -0400 Committer: Stephen Mallette <[email protected]> Committed: Fri Mar 24 13:26:59 2017 -0400 ---------------------------------------------------------------------- CHANGELOG.asciidoc | 9 +- .../gremlin/jsr223/GremlinScriptContext.java | 234 +++++++++++++++++++ .../jsr223/GremlinGroovyScriptEngine.java | 22 ++ 3 files changed, 261 insertions(+), 4 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/48daf8d2/CHANGELOG.asciidoc ---------------------------------------------------------------------- diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index ed26f7a..e6216c0 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -32,10 +32,11 @@ TinkerPop 3.2.5 (Release Date: NOT OFFICIALLY RELEASED YET) * Added various metrics to the `GremlinGroovyScriptEngine` around script compilation and exposed them in Gremlin Server. * Moved the `caffeine` dependency down to `gremlin-groovy` and out of `gremlin-server`. * Improved script compilation in `GremlinGroovyScriptEngine to use better caching, log long compile times and prevent failed compilations from recompiling on future requests. -* 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. +* Synchronized script compilation. +* Logged Script compilation times. +* Prevented failed scripts from recompiling. +* Logged warnings for scripts that take "too long" to compile. +* Improved memory usage of the `GremlinGroovyScriptEngine`. * Added `cyclicPath().from().to().by()` support to `GraphTraversal`. * Added `simplePath().from().to().by()` support to `GraphTraversal`. * Added `path().from().to()` support to `GraphTraversal` so sub-paths can be isolated from the current path. http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/48daf8d2/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/jsr223/GremlinScriptContext.java ---------------------------------------------------------------------- diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/jsr223/GremlinScriptContext.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/jsr223/GremlinScriptContext.java new file mode 100644 index 0000000..5f94225 --- /dev/null +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/jsr223/GremlinScriptContext.java @@ -0,0 +1,234 @@ +/* + * 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.gremlin.jsr223; + +import javax.script.Bindings; +import javax.script.ScriptContext; +import javax.script.SimpleBindings; +import java.io.Reader; +import java.io.Writer; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.Objects; + +/** + * A {@code ScriptContext} that doesn't create new instances of {@code Reader} and {@code Writer} classes on + * initialization. + * + * @author Stephen Mallette (http://stephen.genoprime.com) + */ +public class GremlinScriptContext implements ScriptContext { + + private Writer writer; + private Writer errorWriter; + private Reader reader; + private Bindings engineScope; + private Bindings globalScope; + private static List<Integer> scopes; + + static { + scopes = new ArrayList<>(2); + scopes.add(ENGINE_SCOPE); + scopes.add(GLOBAL_SCOPE); + scopes = Collections.unmodifiableList(scopes); + } + + /** + * Create a {@code GremlinScriptContext}. + */ + public GremlinScriptContext(final Reader in, final Writer out, final Writer error) { + engineScope = new SimpleBindings(); + globalScope = null; + reader = in; + writer = out; + errorWriter = error; + } + + /** + * {@inheritDoc} + */ + @Override + public void setBindings(final Bindings bindings, final int scope) { + switch (scope) { + case ENGINE_SCOPE: + if (null == bindings) throw new NullPointerException("Engine scope cannot be null."); + engineScope = bindings; + break; + case GLOBAL_SCOPE: + globalScope = bindings; + break; + default: + throw new IllegalArgumentException("Invalid scope value."); + } + } + + /** + * {@inheritDoc} + */ + @Override + public Object getAttribute(final String name) { + checkName(name); + if (engineScope.containsKey(name)) { + return getAttribute(name, ENGINE_SCOPE); + } else if (globalScope != null && globalScope.containsKey(name)) { + return getAttribute(name, GLOBAL_SCOPE); + } + + return null; + } + + /** + * {@inheritDoc} + */ + @Override + public Object getAttribute(final String name, final int scope) { + checkName(name); + switch (scope) { + case ENGINE_SCOPE: + return engineScope.get(name); + case GLOBAL_SCOPE: + return globalScope != null ? globalScope.get(name) : null; + default: + throw new IllegalArgumentException("Illegal scope value."); + } + } + + /** + * {@inheritDoc} + */ + @Override + public Object removeAttribute(final String name, final int scope) { + checkName(name); + switch (scope) { + case ENGINE_SCOPE: + return getBindings(ENGINE_SCOPE) != null ? getBindings(ENGINE_SCOPE).remove(name) : null; + case GLOBAL_SCOPE: + return getBindings(GLOBAL_SCOPE) != null ? getBindings(GLOBAL_SCOPE).remove(name) : null; + default: + throw new IllegalArgumentException("Illegal scope value."); + } + } + + /** + * {@inheritDoc} + */ + @Override + public void setAttribute(final String name, final Object value, final int scope) { + checkName(name); + switch (scope) { + case ENGINE_SCOPE: + engineScope.put(name, value); + return; + case GLOBAL_SCOPE: + if (globalScope != null) globalScope.put(name, value); + return; + default: + throw new IllegalArgumentException("Illegal scope value."); + } + } + + /** + * {@inheritDoc} + */ + @Override + public int getAttributesScope(final String name) { + checkName(name); + if (engineScope.containsKey(name)) + return ENGINE_SCOPE; + else if (globalScope != null && globalScope.containsKey(name)) + return GLOBAL_SCOPE; + else + return -1; + } + + /** + * {@inheritDoc} + */ + @Override + public Bindings getBindings(final int scope) { + if (scope == ENGINE_SCOPE) + return engineScope; + else if (scope == GLOBAL_SCOPE) + return globalScope; + else + throw new IllegalArgumentException("Illegal scope value."); + } + + /** + * {@inheritDoc} + */ + @Override + public List<Integer> getScopes() { + return scopes; + } + + /** + * {@inheritDoc} + */ + @Override + public Writer getWriter() { + return writer; + } + + /** + * {@inheritDoc} + */ + @Override + public Reader getReader() { + return reader; + } + + /** + * {@inheritDoc} + */ + @Override + public void setReader(final Reader reader) { + this.reader = reader; + } + + /** + * {@inheritDoc} + */ + @Override + public void setWriter(final Writer writer) { + this.writer = writer; + } + + /** + * {@inheritDoc} + */ + @Override + public Writer getErrorWriter() { + return errorWriter; + } + + /** + * {@inheritDoc} + */ + @Override + public void setErrorWriter(final Writer writer) { + this.errorWriter = writer; + } + + private void checkName(final String name) { + Objects.requireNonNull(name); + if (name.isEmpty()) throw new IllegalArgumentException("name cannot be empty"); + } +} http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/48daf8d2/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 7a39f43..d52e104 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 @@ -44,6 +44,7 @@ import org.apache.tinkerpop.gremlin.groovy.plugin.GremlinPlugin; import org.apache.tinkerpop.gremlin.groovy.plugin.GremlinPluginException; import org.apache.tinkerpop.gremlin.jsr223.CoreGremlinPlugin; import org.apache.tinkerpop.gremlin.jsr223.Customizer; +import org.apache.tinkerpop.gremlin.jsr223.GremlinScriptContext; import org.apache.tinkerpop.gremlin.jsr223.GremlinScriptEngine; import org.apache.tinkerpop.gremlin.jsr223.GremlinScriptEngineFactory; import org.apache.tinkerpop.gremlin.jsr223.ImportCustomizer; @@ -474,6 +475,27 @@ public class GremlinGroovyScriptEngine extends GroovyScriptEngineImpl } /** + * Creates the {@code ScriptContext} using a {@link GremlinScriptContext} which avoids a significant amount of + * additional object creation on script evaluation. + */ + @Override + protected ScriptContext getScriptContext(final Bindings nn) { + final GremlinScriptContext ctxt = new GremlinScriptContext(context.getReader(), context.getWriter(), context.getErrorWriter()); + final Bindings gs = getBindings(ScriptContext.GLOBAL_SCOPE); + + if (gs != null) ctxt.setBindings(gs, ScriptContext.GLOBAL_SCOPE); + + if (nn != null) { + ctxt.setBindings(nn, + ScriptContext.ENGINE_SCOPE); + } else { + throw new NullPointerException("Engine scope Bindings may not be null."); + } + + return ctxt; + } + + /** * Resets the {@code ScriptEngine} but does not clear the loaded plugins or bindings. Typically called by * {@link DependencyManager} methods that need to just force the classloader to be recreated and script caches * cleared.
