groovy-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Alain Stalder <astal...@span.ch>
Subject Re: To ClassValue or not to ClassValue: That is the question!
Date Mon, 09 May 2016 22:23:27 GMT
I wrote:
 > *Can anyone provide some Groovy test code that shows a class leak 
with groovy 2.4.6 and groovy.use.classvalue=true?*

Ah, I found a test that leaks with groovy.use.classvalue=true (and 
apparently not with groovy.use.classvalue=false resp. if not explicitly 
set).

Compile this (with javac):

--- CVTest.java ---

import java.io.File;
import java.net.URLClassLoader;
import java.net.URL;

public class CVTest {

     public static void main(String[] args) throws Throwable {
         Thread.sleep(10000);
         for (long i = 0; i<10000000; i++) {
             File dir1 = new File("t/t.jar");
             File dir2 = new File("t/groovy-2.4.6.jar");
             URLClassLoader classLoader = new URLClassLoader(new 
URL[]{dir1.toURI().toURL(),dir2.toURI().toURL()});
             Object me = classLoader.loadClass("MyClass").newInstance();
             assert me.toString().equals("hello");
             classLoader.close();
         }

     }
}

Compile this (with groovyc):

--- MyClass.groovy ---

class MyClass {
     String toString() {
         return "hello"
     }
}

Then put CVTest.class into ".", put MyClass.class into a "t/t.jar" and 
put groovy-2.4.6.jar also into the "t" directory.

Then run this:

java -XX:MaxMetaspaceSize=64m -Dgroovy.use.classvalue=true -cp . CVTest

This crashes the VM within seconds with an OutOfMemoryError (due to 
Metaspace filling up).

If setting -Dgroovy.use.classvalue=false it appears to run without any 
issues, classes are repeatedly garbage collected in Metaspace.

I guess this test resembles more the situation in Gradle, which was the 
cause for GROOVY-7591 and the corresponding JSR issue.

So, I would naively say that if you load Groovy 2.4.6 and then parse/run 
some scripts from script texts or files etc., you are usually be better 
off if you set groovy.use.classvalue=true and in a scenario more like 
the one just presented, where both Groovy and compiled scripts are 
loaded by the same class loader, you are usually better off with the 
default (groovy.use.classvalue=false)?

I am sure things are more convoluted in detail, but would that do as 
crude high-level description?

Alain


On 09.05.16 23:17, Alain Stalder wrote:
> Thanks a lot for the info :)
>
> I am still trying to figure out an example of something that leaks 
> classes on Groovy 2.4.6 with groovy.use.classvalue=true.
>
> Looking at the corresponding JSR issue, 
> https://bugs.openjdk.java.net/browse/JDK-8136353 I see the following 
> two attached Java files:
>
> --- CVTest.java ---
>
> import java.lang.ClassValue;
> import java.io.File;
> import java.net.URLClassLoader;
> import java.net.URL;
>
> public class CVTest {
>
>     public static void main(String[] args) throws Throwable {
>         for (long i = 0; i<10000000; i++) {
>             File dir = new File("t/t.jar");
>             URLClassLoader classLoader = new URLClassLoader(new 
> URL[]{dir.toURI().toURL()});
>             ClassValue cv = (ClassValue) 
> classLoader.loadClass("MyClassValue").newInstance();
>             Object value = cv.get(Integer.TYPE);
>             assert value !=null;
>             assert value.getClass().getClassLoader() == classLoader;
>             classLoader.close();
>         }
>
>     }
> }
>
> --- MyClassValue.java ---
>
> import java.lang.ClassValue;
> import java.lang.Class;
>
> public class MyClassValue extends ClassValue {
>     static class Dummy {
>       static Object o;
>     }
>     protected Object computeValue(Class type) {
>         Dummy ret = new Dummy();
>         Dummy.o = this;
>         return ret;
>     }
> }
>
> I compiled both and put the classes resulting from the latter into 
> "t/t.jar", CVTest.class into "." and then ran the test program with
>
>    java -XX:MaxMetaspaceSize=64m -cp . CVTest
>
> and within a few seconds this filled up Metaspace and crashed with an 
> "OutOfMemoryError: Metaspace".
>
> Next I compared the test code with how things are implemented in 
> Groovy and found a difference that might be significant:
>
> --- ClassInfo.java (excerpt) ---
>
>     private static final GroovyClassValue<ClassInfo> globalClassValue 
> = GroovyClassValueFactory.createGroovyClassValue(new 
> ComputeValue<ClassInfo>(){
>         @Override
>         public ClassInfo computeValue(Class<?> type) {
>             ClassInfo ret = new ClassInfo(type);
>             globalClassSet.add(ret);
>             return ret;
>         }
>     });
>
>     private static final GlobalClassSet globalClassSet = new 
> GlobalClassSet();
>
> Again the class for the variable returned by computeValue contains a 
> static field which contains a reference to the object.
>
> *But there is a crucial difference:* In the example from the JSR, the 
> class Dummy is loaded by the same class loader as MyClassValue, 
> whereas in groovy, ClassInfo is typically loaded only once, at least 
> not per compiled script class.
>
> So I refactored the example from the JSR to be closer to the situation 
> in Groovy:
>
> --- Dummy.java ---
>
> public class Dummy {
>     public static Object o;
> }
>
> --- MyClassValue.java ---
>
> import java.lang.ClassValue;
> import java.lang.Class;
>
> public class MyClassValue extends ClassValue {
>     protected Object computeValue(Class type) {
>         Dummy ret = new Dummy();
>         Dummy.o = this;
>         return ret;
>     }
> }
>
> And I compiled both, this time adding only MyClassValue.class to 
> "t/t.jar" and putting Dummy.class and CVTest.class in ".", and ran the 
> same command as above again.
>
> No leak this time, classes are garbage collected.
>
> *Can anyone provide some Groovy test code that shows a class leak with 
> groovy 2.4.6 and groovy.use.classvalue=true?*
>
> It would be important to know under which situations this can happen.
>
> And let me rephrase slightly: To ClassValue or not to ClassValue with 
> Groovy 2.4.6: That is the question! (as long as there is no 2.4.7)
>
> Alain
>
>
> On 09.05.16 10:10, Jochen Theodorou wrote:
>>
>> On 08.05.2016 07:22, Alain Stalder wrote:
>>> GROOVY-7591 "Use of ClassValue causes major memory leak",
>>> https://issues.apache.org/jira/browse/GROOVY-7591
>>>
>>> introduced a new System Property groovy.use.shareclasses in
>>> Groovy 2.4.5 and 2.4.6 which is "false" by default.
>>>
>>> But this also caused follow-up issues with garbage collection
>>> of "Groovy" classes, which go away if setting
>>> groovy.use.shareclasses=true, which was also my experience.
>>>
>>> GROOVY-7683 Memory leak when using Groovy as JSR-223 scripting language
>>> GROOVY-7646 Classes generated by Eval() never collected from
>>> Permgen/Metaspace
>>
>> I think 2.4.7 will have an improved version here, that fixes the 
>> memory problems for ClassValue and for not ClassValue. Because both 
>> versions are supposed to be working, unless you fall over a JVM bug.
>>
>>> "Not to ClassValue" (default):
>>>
>>> Don't do this if you parse many Groovy scripts or only load
>>> many classes compiled from Groovy scripts - this will fill up
>>> PermGen/Metaspace and blow up with an "OutOfMemoryError" and
>>> you will see lots of MetaMethodIndex$Entry in heap dumps. (Right?)
>>>
>>> "To ClassValue":
>>>
>>> Personally, I have not observed any issues with this setting,
>>> with Groovy 2.4.6 - under which circumstances would I have a
>>> leak with groovy.use.shareclasses=true?
>>>
>>> Can this be explained in a few sentences?
>>>
>>> There has been some very recent conversation at GROOVY-7683 by
>>> John Wagenleitner and Jochen Theodorou, so maybe there is a fix
>>> for an upcoming version in preparation?
>>
>> what did happen was, that the implementation for ClassValue caused 
>> some refactorings on the old code, which produced a difficult to 
>> diagnose memory leak in the old code, as well as the new code not 
>> always working. The problem is that the code for ClassValue itself is 
>> still in flux.. just last week there had been for example discussions 
>> about replacing the whole map used in ClassValue... with like 4 
>> different versions to choose from. Anyway... as there are still 
>> things in flux we got an implementation that worked sometimes and 
>> under certain circumstances, but not always. But I am positive those 
>> problems are fixed then in 2.4.7... it is also not that all Groovy 
>> versions are hit by this problem. Any version before the ClassValue 
>> change for example is unaffected. A general use classvalue or not, 
>> cannot be adviced really... it depends on the version.
>>
>>> Any news on that which could already be communicated here?
>>
>> I think we should work on getting a proper 2.4.7 out and then 
>> communicate that people should upgrade
>>
>> bye Jochen
>>
>
> .
>


Mime
View raw message