You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@groovy.apache.org by Alain Stalder <as...@span.ch> on 2016/05/08 05:22:19 UTC

To ClassValue or not to ClassValue: That is the question!

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

"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?

Any news on that which could already be communicated here?

Best wishes,
Alain

Re: Re: To ClassValue or not to ClassValue: That is the question!

Posted by Alain Stalder <as...@span.ch>.
Also copied:

PS: The source I just posted does not really work, the inner value can 
be garbage collected, so just posted as a "general idea" - not wanting 
to really dive fully into this I am better getting out now... ;)

Alain

On 11.05.16 06:35, Alain Stalder wrote:
> Copied from my comment at 
> https://issues.apache.org/jira/browse/GROOVY-7591 (partially in 
> response to what Craig added there yesterday):
>
> To me the ideal solution would be to drop the "groovy.use.classvalue" 
> system property in Groovy 2.4.7, to always use ClassValue (with Java 7 
> and later), but to give the Java VM a ClassValue that it can garbage 
> collect already without a JVM fix.
>
> Why drop the system property: The matter is complex enough, 
> maintaining more variants than absolutely necessary should be avoided.
>
> Why not wait for a fix in the Java VM: Because even if/when it comes 
> to Java 9/8/7, it will still require people to upgrade their VMs which 
> is not a given at all.
>
> Here is a first attempt to resolve this as simply as possible in 
> principle: A modified version of the test sources that were submitted 
> with the Java VM bug report.
>
> I am simply wrapping the MyClassValue class with another instance of 
> ClassValue that only has weak references to the MyClassValue instance.
>
> This worked on Java 7 and 8. With Java 7 I saw used PermGen rise close 
> to the configured maximum and then it was collected. With Java 8, 
> Metaspace did not even rise, which is important because by default 
> there is no maxium set for Metaspace.
>
> -- See https://issues.apache.org/jira/browse/GROOVY-7591 for Java 
> sources --
>
> I wrap the classes resulting from compiling MyClassValue and 
> MyClassValueWrapper into t/t.jar and then run the test with "java -ea 
> -XX:MaxMetaspaceSize=64m -cp . CVTest" resp. "java -ea 
> -XX:MaxPermGenSize=64m -cp . CVTest".
>
> I know that there is already much more specific work going on with 
> regard to fixing these issues and there might be more issues with 
> garbage collecting classes in the Groovy code - I just wanted to say 
> that it might be good to strive for a solution with minimal complexity.
>
> PS: I hope it is OK to copy this, when I post here, code gets broken 
> up with line breaks, making it possibly harder to read/try out, on the 
> other hand that issue seems like maybe not the ideal place to discuss 
> the issue?
>
> Alain
>


Re: To ClassValue or not to ClassValue: That is the question!

Posted by Alain Stalder <as...@span.ch>.
Copied from my comment at 
https://issues.apache.org/jira/browse/GROOVY-7591 (partially in response 
to what Craig added there yesterday):

To me the ideal solution would be to drop the "groovy.use.classvalue" 
system property in Groovy 2.4.7, to always use ClassValue (with Java 7 
and later), but to give the Java VM a ClassValue that it can garbage 
collect already without a JVM fix.

Why drop the system property: The matter is complex enough, maintaining 
more variants than absolutely necessary should be avoided.

Why not wait for a fix in the Java VM: Because even if/when it comes to 
Java 9/8/7, it will still require people to upgrade their VMs which is 
not a given at all.

Here is a first attempt to resolve this as simply as possible in 
principle: A modified version of the test sources that were submitted 
with the Java VM bug report.

I am simply wrapping the MyClassValue class with another instance of 
ClassValue that only has weak references to the MyClassValue instance.

This worked on Java 7 and 8. With Java 7 I saw used PermGen rise close 
to the configured maximum and then it was collected. With Java 8, 
Metaspace did not even rise, which is important because by default there 
is no maxium set for Metaspace.

-- See https://issues.apache.org/jira/browse/GROOVY-7591 for Java sources --

I wrap the classes resulting from compiling MyClassValue and 
MyClassValueWrapper into t/t.jar and then run the test with "java -ea 
-XX:MaxMetaspaceSize=64m -cp . CVTest" resp. "java -ea 
-XX:MaxPermGenSize=64m -cp . CVTest".

I know that there is already much more specific work going on with 
regard to fixing these issues and there might be more issues with 
garbage collecting classes in the Groovy code - I just wanted to say 
that it might be good to strive for a solution with minimal complexity.

PS: I hope it is OK to copy this, when I post here, code gets broken up 
with line breaks, making it possibly harder to read/try out, on the 
other hand that issue seems like maybe not the ideal place to discuss 
the issue?

Alain

Re: To ClassValue or not to ClassValue: That is the question!

Posted by Alain Stalder <as...@span.ch>.
For completeness the opposite case which fails with 
groovy.use.classvalue=false and works with groovy.use.class.value=true 
(from my comment at GROOVY-7591):

"This issue puzzles me, because I observe exactly the opposite when I 
run a script like
    while(true) new GroovyShell().parse('42')
or
    while(true) new ConfigSlurper().parse('72')
and limit PermGen resp. Metaspace, say, to 64m, this crashes various VMs 
after about a minute with an "OutOfMemoryError: PermGen space|Metaspace" 
in the following cases:
- Groovy 2.4.6, groovy.use.shareclasses=false (or not set) => crash
- Groovy 2.4.6, groovy.use.shareclasses=true => works, PermGen/Metaspace 
is garbage collected when it nears the configured maximum (resp. already 
earlier with JDK 9)"
https://issues.apache.org/jira/browse/GROOVY-7591

Alain

On 10.05.16 00:23, Alain Stalder wrote:
> 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
>>>
>>
>> .
>>
>
> .
>


Re: To ClassValue or not to ClassValue: That is the question!

Posted by Alain Stalder <as...@span.ch>.
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
>>
>
> .
>


Re: To ClassValue or not to ClassValue: That is the question!

Posted by Alain Stalder <as...@span.ch>.
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
>


Re: To ClassValue or not to ClassValue: That is the question!

Posted by Jochen Theodorou <bl...@gmx.org>.
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