Repository: tinkerpop Updated Branches: refs/heads/master 77de2950f -> 65e8edb38
TINKERPOP-1560 Used ManagedConcurrentValueMap in GremlinGroovyClassLoader By configuring the ManagedConcurrentValueMap to have weak references, the GremlinGroovyClassLoader, and therefore the GremlinGroovyScriptEngine, are now able to "forget" classes that are no longer used. It was determined that the cache of these classes would grow indefinitely for each script passed to the GremlinGroovyScriptEngine, thus allowing the metaspace to continue to grow. It isn't really possible to write tests to verify that this change works, but I did test manually by watching memory usage in a profiler and could see that metaspace memory stayed stable and that classes were unloading from the classloader over time. Project: http://git-wip-us.apache.org/repos/asf/tinkerpop/repo Commit: http://git-wip-us.apache.org/repos/asf/tinkerpop/commit/97265772 Tree: http://git-wip-us.apache.org/repos/asf/tinkerpop/tree/97265772 Diff: http://git-wip-us.apache.org/repos/asf/tinkerpop/diff/97265772 Branch: refs/heads/master Commit: 97265772db79618cd5de83bb45d22f3e2bb6be9e Parents: f84f454 Author: Stephen Mallette <sp...@genoprime.com> Authored: Fri Jan 20 11:54:45 2017 -0500 Committer: Stephen Mallette <sp...@genoprime.com> Committed: Mon Jan 23 07:45:26 2017 -0500 ---------------------------------------------------------------------- CHANGELOG.asciidoc | 1 + .../groovy/jsr223/GremlinGroovyClassLoader.java | 24 ++++++++- .../jsr223/ManagedConcurrentValueMap.java | 4 ++ .../GremlinGroovyScriptEngineIntegrateTest.java | 55 ++++++++++++++++++++ 4 files changed, 82 insertions(+), 2 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/97265772/CHANGELOG.asciidoc ---------------------------------------------------------------------- diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index fb0f8da..5e06a1c 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -34,6 +34,7 @@ TinkerPop 3.2.4 (Release Date: NOT OFFICIALLY RELEASED YET) * SASL negotiation supports both a byte array and Base64 encoded bytes as a string for authentication to Gremlin Server. * Deprecated `TinkerIoRegistry` replacing it with the more consistently named `TinkerIoRegistryV1d0`. * Made error messaging more consistent during result iteration timeouts in Gremlin Server. +* Fixed a memory leak in the classloader for the `GremlinGroovyScriptEngine` where classes in the loader were not releasing from memory as a strong reference was always maintained. * `PathRetractionStrategy` does not add a `NoOpBarrierStep` to the end of local children as its wasted computation in 99% of traversals. * Fixed a bug in `AddVertexStartStep` where if a side-effect was being used in the parametrization, an NPE occurred. * Fixed a bug in `LazyBarrierStrategy` where `profile()` was deactivating it accidentally. http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/97265772/gremlin-groovy/src/main/java/org/apache/tinkerpop/gremlin/groovy/jsr223/GremlinGroovyClassLoader.java ---------------------------------------------------------------------- diff --git a/gremlin-groovy/src/main/java/org/apache/tinkerpop/gremlin/groovy/jsr223/GremlinGroovyClassLoader.java b/gremlin-groovy/src/main/java/org/apache/tinkerpop/gremlin/groovy/jsr223/GremlinGroovyClassLoader.java index e987ce0..2bc122c 100644 --- a/gremlin-groovy/src/main/java/org/apache/tinkerpop/gremlin/groovy/jsr223/GremlinGroovyClassLoader.java +++ b/gremlin-groovy/src/main/java/org/apache/tinkerpop/gremlin/groovy/jsr223/GremlinGroovyClassLoader.java @@ -20,19 +20,39 @@ package org.apache.tinkerpop.gremlin.groovy.jsr223; import groovy.lang.GroovyClassLoader; import org.codehaus.groovy.control.CompilerConfiguration; +import org.codehaus.groovy.util.ReferenceBundle; + +import java.util.Map; +import java.util.concurrent.ConcurrentMap; /** - * A {@code GroovyClassLoader} extension that provides access to the {@code removeClassCacheEntry(String)} method. + * A {@code GroovyClassLoader} extension that uses a {@link ManagedConcurrentValueMap} configured with soft references. + * In this way, the classloader will "forget" scripts allowing them to be garbage collected and thus prevent memory + * issues in JVM metaspace. * * @author Stephen Mallette (http://stephen.genoprime.com) */ class GremlinGroovyClassLoader extends GroovyClassLoader { + private final ManagedConcurrentValueMap<String, Class> classSoftCache; + public GremlinGroovyClassLoader(final ClassLoader parent, final CompilerConfiguration conf) { super(parent, conf); + classSoftCache = new ManagedConcurrentValueMap<>(ReferenceBundle.getSoftBundle()); } @Override protected void removeClassCacheEntry(final String name) { - super.removeClassCacheEntry(name); + this.classSoftCache.remove(name); + } + + @Override + protected void setClassCacheEntry(final Class cls) { + this.classSoftCache.put(cls.getName(), cls); + } + + @Override + protected Class getClassCacheEntry(final String name) { + if(null == name) return null; + return this.classSoftCache.get(name); } } http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/97265772/gremlin-groovy/src/main/java/org/apache/tinkerpop/gremlin/groovy/jsr223/ManagedConcurrentValueMap.java ---------------------------------------------------------------------- diff --git a/gremlin-groovy/src/main/java/org/apache/tinkerpop/gremlin/groovy/jsr223/ManagedConcurrentValueMap.java b/gremlin-groovy/src/main/java/org/apache/tinkerpop/gremlin/groovy/jsr223/ManagedConcurrentValueMap.java index dce056b..f1ee00d 100644 --- a/gremlin-groovy/src/main/java/org/apache/tinkerpop/gremlin/groovy/jsr223/ManagedConcurrentValueMap.java +++ b/gremlin-groovy/src/main/java/org/apache/tinkerpop/gremlin/groovy/jsr223/ManagedConcurrentValueMap.java @@ -76,6 +76,10 @@ class ManagedConcurrentValueMap<K, V> { internalMap.put(key, ref); } + public void remove(final K key) { + internalMap.remove(key); + } + /** * Clear the map. */ http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/97265772/gremlin-groovy/src/test/java/org/apache/tinkerpop/gremlin/jsr223/GremlinGroovyScriptEngineIntegrateTest.java ---------------------------------------------------------------------- diff --git a/gremlin-groovy/src/test/java/org/apache/tinkerpop/gremlin/jsr223/GremlinGroovyScriptEngineIntegrateTest.java b/gremlin-groovy/src/test/java/org/apache/tinkerpop/gremlin/jsr223/GremlinGroovyScriptEngineIntegrateTest.java new file mode 100644 index 0000000..5242d3b --- /dev/null +++ b/gremlin-groovy/src/test/java/org/apache/tinkerpop/gremlin/jsr223/GremlinGroovyScriptEngineIntegrateTest.java @@ -0,0 +1,55 @@ +/* + * 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 org.apache.tinkerpop.gremlin.groovy.jsr223.GremlinGroovyScriptEngine; +import org.javatuples.Pair; +import org.junit.Ignore; +import org.junit.Test; + +import java.util.ArrayList; +import java.util.List; +import java.util.Random; + +import static org.junit.Assert.assertEquals; + +/** + * @author Stephen Mallette (http://stephen.genoprime.com) + */ +public class GremlinGroovyScriptEngineIntegrateTest { + @Test + @Ignore("This is not a test that needs to run on build - it's more for profiling the GremlinGroovyScriptEngine") + public void shouldTest() throws Exception { + final Random r = new Random(); + final List<Pair<String, Integer>> scripts = new ArrayList<>(); + final GremlinGroovyScriptEngine engine = new GremlinGroovyScriptEngine(); + for (int ix = 0; ix < 1000000; ix++) { + final String script = "1 + " + ix; + final int output = (int) engine.eval(script); + assertEquals(1 + ix, output); + + if (ix % 1000 == 0) scripts.add(Pair.with(script, output)); + + if (ix % 25 == 0) { + final Pair<String,Integer> p = scripts.get(r.nextInt(scripts.size())); + assertEquals(p.getValue1().intValue(), (int) engine.eval(p.getValue0())); + } + } + } +}
