Mailing-List: contact dev-help@groovy.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@groovy.apache.org
X-IMP-AUTH_USERNAME: <astalder@span.ch>
X-Original-IP: 194.191.233.117
Subject: Re: To ClassValue or not to ClassValue: That is the question!
To: dev@groovy.apache.org
References: <572ECD0B.1070808@span.ch> <5730460C.608@gmx.org>
From: Alain Stalder <astalder@span.ch>
Message-ID: <5730FE62.6050807@span.ch>
Date: Mon, 9 May 2016 23:17:22 +0200
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:38.0)
 Gecko/20100101 Thunderbird/38.5.1
MIME-Version: 1.0
In-Reply-To: <5730460C.608@gmx.org>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 7bit
archived-at: Mon, 09 May 2016 21:17:36 -0000

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
>