TINKERPOP-1644 Provided configuration options for GremlinGroovyScriptEngine
Introduced new customizers to pass in configuration options to the GremlinGroovyScriptEngine. Project: http://git-wip-us.apache.org/repos/asf/tinkerpop/repo Commit: http://git-wip-us.apache.org/repos/asf/tinkerpop/commit/4bdeac4b Tree: http://git-wip-us.apache.org/repos/asf/tinkerpop/tree/4bdeac4b Diff: http://git-wip-us.apache.org/repos/asf/tinkerpop/diff/4bdeac4b Branch: refs/heads/TINKERPOP-1642 Commit: 4bdeac4b796b38fb14d1be763e03cc837b57d3d7 Parents: de1d58a Author: Stephen Mallette <sp...@genoprime.com> Authored: Wed Mar 8 16:13:06 2017 -0500 Committer: Stephen Mallette <sp...@genoprime.com> Committed: Fri Mar 10 11:12:36 2017 -0500 ---------------------------------------------------------------------- .../jsr223/CompilationOptionsCustomizer.java | 39 ++++++++++ .../jsr223/GremlinGroovyScriptEngine.java | 36 +++++++-- .../jsr223/GroovyCompilerGremlinPlugin.java | 13 ++++ .../CompilationOptionsCustomizerProvider.java | 48 ++++++++++++ ...roovyScriptEngineCompilationOptionsTest.java | 80 ++++++++++++++++++++ .../jsr223/GroovyCompilerGremlinPluginTest.java | 24 ++++-- 6 files changed, 225 insertions(+), 15 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/4bdeac4b/gremlin-groovy/src/main/java/org/apache/tinkerpop/gremlin/groovy/jsr223/CompilationOptionsCustomizer.java ---------------------------------------------------------------------- diff --git a/gremlin-groovy/src/main/java/org/apache/tinkerpop/gremlin/groovy/jsr223/CompilationOptionsCustomizer.java b/gremlin-groovy/src/main/java/org/apache/tinkerpop/gremlin/groovy/jsr223/CompilationOptionsCustomizer.java new file mode 100644 index 0000000..2a22eb3 --- /dev/null +++ b/gremlin-groovy/src/main/java/org/apache/tinkerpop/gremlin/groovy/jsr223/CompilationOptionsCustomizer.java @@ -0,0 +1,39 @@ +/* + * 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.groovy.jsr223; + +import org.apache.tinkerpop.gremlin.jsr223.Customizer; + +/** + * Provides some custom compilation options to the {@link GremlinGroovyScriptEngine}. + * + * @author Stephen Mallette (http://stephen.genoprime.com) + */ +class CompilationOptionsCustomizer implements Customizer { + + private final int expectedCompilationTime; + + public CompilationOptionsCustomizer(final int expectedCompilationTime) { + this.expectedCompilationTime = expectedCompilationTime; + } + + public int getExpectedCompilationTime() { + return expectedCompilationTime; + } +} http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/4bdeac4b/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 42d954a..5168130 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 @@ -35,6 +35,7 @@ import org.apache.tinkerpop.gremlin.groovy.DefaultImportCustomizerProvider; import org.apache.tinkerpop.gremlin.groovy.EmptyImportCustomizerProvider; import org.apache.tinkerpop.gremlin.groovy.ImportCustomizerProvider; import org.apache.tinkerpop.gremlin.groovy.NoImportCustomizerProvider; +import org.apache.tinkerpop.gremlin.groovy.jsr223.customizer.CompilationOptionsCustomizerProvider; import org.apache.tinkerpop.gremlin.groovy.jsr223.customizer.ConfigurationCustomizerProvider; import org.apache.tinkerpop.gremlin.groovy.jsr223.customizer.InterpreterModeCustomizerProvider; import org.apache.tinkerpop.gremlin.groovy.loaders.GremlinLoader; @@ -80,11 +81,11 @@ import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Optional; 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; @@ -125,7 +126,6 @@ public class GremlinGroovyScriptEngine extends GroovyScriptEngineImpl */ public static final String REFERENCE_TYPE_PHANTOM = "phantom"; - /** * A value to the {@link #KEY_REFERENCE_TYPE} that marks the script as one that can be garbage collected * even when memory is abundant. @@ -178,26 +178,26 @@ public class GremlinGroovyScriptEngine extends GroovyScriptEngineImpl throw e; } finally { final long time = System.currentTimeMillis() - start; - if (time > 5000) { + if (time > expectedCompilationTime) { //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); + longRunCompilationCount.incrementAndGet(); } else { log.debug("Script compilation {} took {}ms", script, time); } } }, Runnable::run); } - }); /** * Global closures map - this is used to simulate a single global functions namespace */ - private ManagedConcurrentValueMap<String, Closure> globalClosures = new ManagedConcurrentValueMap<>(ReferenceBundle.getHardBundle()); - + private final ManagedConcurrentValueMap<String, Closure> globalClosures = new ManagedConcurrentValueMap<>(ReferenceBundle.getHardBundle()); - private AtomicLong counter = new AtomicLong(0L); + private final AtomicLong counter = new AtomicLong(0L); + private final AtomicLong longRunCompilationCount = new AtomicLong(0L); /** * The list of loaded plugins for the console. @@ -219,6 +219,7 @@ public class GremlinGroovyScriptEngine extends GroovyScriptEngineImpl private final Set<Artifact> artifactsToUse = new HashSet<>(); private final boolean interpreterModeEnabled; + private final int expectedCompilationTime; /** * Creates a new instance using the {@link DefaultImportCustomizerProvider}. @@ -255,6 +256,12 @@ public class GremlinGroovyScriptEngine extends GroovyScriptEngineImpl .map(p -> ((GroovyCustomizer) p)) .collect(Collectors.toList()); + final Optional<CompilationOptionsCustomizer> compilationOptionsCustomizerProvider = listOfCustomizers.stream() + .filter(p -> p instanceof CompilationOptionsCustomizer) + .map(p -> (CompilationOptionsCustomizer) p).findFirst(); + expectedCompilationTime = compilationOptionsCustomizerProvider.isPresent() ? + compilationOptionsCustomizerProvider.get().getExpectedCompilationTime() : 5000; + // determine if interpreter mode should be enabled interpreterModeEnabled = groovyCustomizers.stream() .anyMatch(p -> p.getClass().equals(InterpreterModeGroovyCustomizer.class)); @@ -286,9 +293,15 @@ public class GremlinGroovyScriptEngine extends GroovyScriptEngineImpl interpreterModeEnabled = providers.stream() .anyMatch(p -> p.getClass().equals(InterpreterModeCustomizerProvider.class)); + final Optional<CompilationOptionsCustomizerProvider> compilationOptionsCustomizerProvider = providers.stream() + .filter(p -> p instanceof CompilationOptionsCustomizerProvider) + .map(p -> (CompilationOptionsCustomizerProvider) p).findFirst(); + expectedCompilationTime = compilationOptionsCustomizerProvider.isPresent() ? + compilationOptionsCustomizerProvider.get().getExpectedCompilationTime() : 5000; + // remove used providers as the rest will be applied directly customizerProviders = providers.stream() - .filter(p -> p != null && !(p instanceof ImportCustomizerProvider)) + .filter(p -> p != null && !(p instanceof ImportCustomizerProvider) && !(p instanceof CompilationOptionsCustomizerProvider)) .collect(Collectors.toList()); // groovy customizers are not used here - set to empty list so that the customizerProviders get used @@ -565,6 +578,13 @@ public class GremlinGroovyScriptEngine extends GroovyScriptEngineImpl return makeInterface(thiz, clazz); } + /** + * Gets the number of compilations that extended beyond the {@link #expectedCompilationTime}. + */ + public long getLongRunCompilationCount() { + return longRunCompilationCount.longValue(); + } + Class getScriptClass(final String script) throws CompilationFailedException { try { return classMap.get(script).get(); http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/4bdeac4b/gremlin-groovy/src/main/java/org/apache/tinkerpop/gremlin/groovy/jsr223/GroovyCompilerGremlinPlugin.java ---------------------------------------------------------------------- diff --git a/gremlin-groovy/src/main/java/org/apache/tinkerpop/gremlin/groovy/jsr223/GroovyCompilerGremlinPlugin.java b/gremlin-groovy/src/main/java/org/apache/tinkerpop/gremlin/groovy/jsr223/GroovyCompilerGremlinPlugin.java index 71f5dc7..5680a4f 100644 --- a/gremlin-groovy/src/main/java/org/apache/tinkerpop/gremlin/groovy/jsr223/GroovyCompilerGremlinPlugin.java +++ b/gremlin-groovy/src/main/java/org/apache/tinkerpop/gremlin/groovy/jsr223/GroovyCompilerGremlinPlugin.java @@ -59,9 +59,19 @@ public class GroovyCompilerGremlinPlugin extends AbstractGremlinPlugin { private long timeInMillis = 0; private Compilation compilation = Compilation.NONE; private String extensions = null; + private int expectedCompilationTime = 5000; private Map<String,Object> keyValues = Collections.emptyMap(); + /** + * If the time it takes to compile a script exceeds the specified time then a warning is written to the logs. + * Defaults to 5000ms. + */ + public Builder expectedCompilationTime(final int timeInMillis) { + this.expectedCompilationTime = timeInMillis; + return this; + } + public Builder enableInterpreterMode(final boolean interpreterMode) { this.interpreterMode = interpreterMode; return this; @@ -111,6 +121,9 @@ public class GroovyCompilerGremlinPlugin extends AbstractGremlinPlugin { if (timeInMillis > 0) list.add(new TimedInterruptGroovyCustomizer(timeInMillis)); + if (expectedCompilationTime > 0) + list.add(new CompilationOptionsCustomizer(expectedCompilationTime)); + if (compilation == Compilation.COMPILE_STATIC) list.add(new CompileStaticGroovyCustomizer(extensions)); else if (compilation == Compilation.TYPE_CHECKED) http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/4bdeac4b/gremlin-groovy/src/main/java/org/apache/tinkerpop/gremlin/groovy/jsr223/customizer/CompilationOptionsCustomizerProvider.java ---------------------------------------------------------------------- diff --git a/gremlin-groovy/src/main/java/org/apache/tinkerpop/gremlin/groovy/jsr223/customizer/CompilationOptionsCustomizerProvider.java b/gremlin-groovy/src/main/java/org/apache/tinkerpop/gremlin/groovy/jsr223/customizer/CompilationOptionsCustomizerProvider.java new file mode 100644 index 0000000..0475511 --- /dev/null +++ b/gremlin-groovy/src/main/java/org/apache/tinkerpop/gremlin/groovy/jsr223/customizer/CompilationOptionsCustomizerProvider.java @@ -0,0 +1,48 @@ +/* + * 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.groovy.jsr223.customizer; + +import org.apache.tinkerpop.gremlin.groovy.CompilerCustomizerProvider; +import org.apache.tinkerpop.gremlin.groovy.jsr223.GremlinGroovyScriptEngine; +import org.codehaus.groovy.control.customizers.CompilationCustomizer; + +/** + * Provides compilation configuration options to the {@link GremlinGroovyScriptEngine}. + * + * @author Stephen Mallette (http://stephen.genoprime.com) + * @deprecated As of release 3.2.5, not replaced by a public class. + */ +@Deprecated +public class CompilationOptionsCustomizerProvider implements CompilerCustomizerProvider { + + private final int expectedCompilationTime; + + public CompilationOptionsCustomizerProvider(final int expectedCompilationTime) { + this.expectedCompilationTime = expectedCompilationTime; + } + + public int getExpectedCompilationTime() { + return expectedCompilationTime; + } + + @Override + public CompilationCustomizer create() { + throw new UnsupportedOperationException("This is a marker implementation that does not create a CompilationCustomizer instance"); + } +} http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/4bdeac4b/gremlin-groovy/src/test/java/org/apache/tinkerpop/gremlin/groovy/jsr223/GremlinGroovyScriptEngineCompilationOptionsTest.java ---------------------------------------------------------------------- diff --git a/gremlin-groovy/src/test/java/org/apache/tinkerpop/gremlin/groovy/jsr223/GremlinGroovyScriptEngineCompilationOptionsTest.java b/gremlin-groovy/src/test/java/org/apache/tinkerpop/gremlin/groovy/jsr223/GremlinGroovyScriptEngineCompilationOptionsTest.java new file mode 100644 index 0000000..f5dd9c5 --- /dev/null +++ b/gremlin-groovy/src/test/java/org/apache/tinkerpop/gremlin/groovy/jsr223/GremlinGroovyScriptEngineCompilationOptionsTest.java @@ -0,0 +1,80 @@ +/* + * 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.groovy.jsr223; + +import org.apache.tinkerpop.gremlin.groovy.jsr223.customizer.CompilationOptionsCustomizerProvider; +import org.junit.Test; + +import javax.script.Bindings; +import javax.script.SimpleBindings; + +import static org.junit.Assert.assertEquals; + +/** + * @author Stephen Mallette (http://stephen.genoprime.com) + */ +public class GremlinGroovyScriptEngineCompilationOptionsTest { + @Test + public void shouldRegisterLongCompilationDeprecated() throws Exception { + final GremlinGroovyScriptEngine engine = new GremlinGroovyScriptEngine(new CompilationOptionsCustomizerProvider(10)); + + final int numberOfParameters = 3000; + final Bindings b = new SimpleBindings(); + + // generate a script that takes a long time to compile + String script = "x = 0"; + for (int ix = 0; ix < numberOfParameters; ix++) { + if (ix > 0 && ix % 100 == 0) { + script = script + ";" + System.lineSeparator() + "x = x"; + } + script = script + " + x" + ix; + b.put("x" + ix, ix); + } + + assertEquals(0, engine.getLongRunCompilationCount()); + + engine.eval(script, b); + + assertEquals(1, engine.getLongRunCompilationCount()); + } + + @Test + public void shouldRegisterLongCompilation() throws Exception { + final GremlinGroovyScriptEngine engine = new GremlinGroovyScriptEngine(new CompilationOptionsCustomizer(10)); + + final int numberOfParameters = 3000; + final Bindings b = new SimpleBindings(); + + // generate a script that takes a long time to compile + String script = "x = 0"; + for (int ix = 0; ix < numberOfParameters; ix++) { + if (ix > 0 && ix % 100 == 0) { + script = script + ";" + System.lineSeparator() + "x = x"; + } + script = script + " + x" + ix; + b.put("x" + ix, ix); + } + + assertEquals(0, engine.getLongRunCompilationCount()); + + engine.eval(script, b); + + assertEquals(1, engine.getLongRunCompilationCount()); + } +} http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/4bdeac4b/gremlin-groovy/src/test/java/org/apache/tinkerpop/gremlin/groovy/jsr223/GroovyCompilerGremlinPluginTest.java ---------------------------------------------------------------------- diff --git a/gremlin-groovy/src/test/java/org/apache/tinkerpop/gremlin/groovy/jsr223/GroovyCompilerGremlinPluginTest.java b/gremlin-groovy/src/test/java/org/apache/tinkerpop/gremlin/groovy/jsr223/GroovyCompilerGremlinPluginTest.java index f795fa7..bacfc90 100644 --- a/gremlin-groovy/src/test/java/org/apache/tinkerpop/gremlin/groovy/jsr223/GroovyCompilerGremlinPluginTest.java +++ b/gremlin-groovy/src/test/java/org/apache/tinkerpop/gremlin/groovy/jsr223/GroovyCompilerGremlinPluginTest.java @@ -18,12 +18,6 @@ */ package org.apache.tinkerpop.gremlin.groovy.jsr223; -import org.apache.tinkerpop.gremlin.groovy.jsr223.customizer.CompileStaticCustomizerProvider; -import org.apache.tinkerpop.gremlin.groovy.jsr223.customizer.ConfigurationCustomizerProvider; -import org.apache.tinkerpop.gremlin.groovy.jsr223.customizer.InterpreterModeCustomizerProvider; -import org.apache.tinkerpop.gremlin.groovy.jsr223.customizer.ThreadInterruptCustomizerProvider; -import org.apache.tinkerpop.gremlin.groovy.jsr223.customizer.TimedInterruptCustomizerProvider; -import org.apache.tinkerpop.gremlin.groovy.jsr223.customizer.TypeCheckedCustomizerProvider; import org.apache.tinkerpop.gremlin.jsr223.Customizer; import org.codehaus.groovy.control.CompilerConfiguration; import org.junit.Test; @@ -54,6 +48,7 @@ public class GroovyCompilerGremlinPluginTest { @Test public void shouldConfigureWithCompileStatic() { final GroovyCompilerGremlinPlugin plugin = GroovyCompilerGremlinPlugin.build(). + expectedCompilationTime(0). compilation(Compilation.COMPILE_STATIC).create(); final Optional<Customizer[]> customizers = plugin.getCustomizers("gremlin-groovy"); assertThat(customizers.isPresent(), is(true)); @@ -64,6 +59,7 @@ public class GroovyCompilerGremlinPluginTest { @Test public void shouldConfigureWithTypeChecked() { final GroovyCompilerGremlinPlugin plugin = GroovyCompilerGremlinPlugin.build(). + expectedCompilationTime(0). compilation(Compilation.TYPE_CHECKED).create(); final Optional<Customizer[]> customizers = plugin.getCustomizers("gremlin-groovy"); assertThat(customizers.isPresent(), is(true)); @@ -76,6 +72,7 @@ public class GroovyCompilerGremlinPluginTest { final Map<String,Object> conf = new HashMap<>(); conf.put("Debug", true); final GroovyCompilerGremlinPlugin plugin = GroovyCompilerGremlinPlugin.build(). + expectedCompilationTime(0). compilerConfigurationOptions(conf).create(); final Optional<Customizer[]> customizers = plugin.getCustomizers("gremlin-groovy"); assertThat(customizers.isPresent(), is(true)); @@ -94,6 +91,7 @@ public class GroovyCompilerGremlinPluginTest { @Test public void shouldConfigureWithInterpreterMode() { final GroovyCompilerGremlinPlugin plugin = GroovyCompilerGremlinPlugin.build(). + expectedCompilationTime(0). enableInterpreterMode(true).create(); final Optional<Customizer[]> customizers = plugin.getCustomizers("gremlin-groovy"); assertThat(customizers.isPresent(), is(true)); @@ -104,6 +102,7 @@ public class GroovyCompilerGremlinPluginTest { @Test public void shouldConfigureWithThreadInterrupt() { final GroovyCompilerGremlinPlugin plugin = GroovyCompilerGremlinPlugin.build(). + expectedCompilationTime(0). enableThreadInterrupt(true).create(); final Optional<Customizer[]> customizers = plugin.getCustomizers("gremlin-groovy"); assertThat(customizers.isPresent(), is(true)); @@ -114,6 +113,7 @@ public class GroovyCompilerGremlinPluginTest { @Test public void shouldConfigureWithTimedInterrupt() { final GroovyCompilerGremlinPlugin plugin = GroovyCompilerGremlinPlugin.build(). + expectedCompilationTime(0). timedInterrupt(60000).create(); final Optional<Customizer[]> customizers = plugin.getCustomizers("gremlin-groovy"); assertThat(customizers.isPresent(), is(true)); @@ -121,8 +121,18 @@ public class GroovyCompilerGremlinPluginTest { assertThat(customizers.get()[0], instanceOf(TimedInterruptGroovyCustomizer.class)); } + @Test + public void shouldConfigureWithCompilationOptions() { + final GroovyCompilerGremlinPlugin plugin = GroovyCompilerGremlinPlugin.build(). + expectedCompilationTime(30000).create(); + final Optional<Customizer[]> customizers = plugin.getCustomizers("gremlin-groovy"); + assertThat(customizers.isPresent(), is(true)); + assertEquals(1, customizers.get().length); + assertThat(customizers.get()[0], instanceOf(CompilationOptionsCustomizer.class)); + } + @Test(expected = IllegalStateException.class) public void shouldNotConfigureIfNoSettingsAreSupplied() { - GroovyCompilerGremlinPlugin.build().create(); + GroovyCompilerGremlinPlugin.build().expectedCompilationTime(0).create(); } }
