Yes. Plus env var to enable for folks wanting to try it out? On Tue, Sep 8, 2015 at 6:04 PM, Cédric Champeau <[email protected]> wrote:
> Basically that flag already exists: > https://github.com/apache/incubator-groovy/blob/master/src/main/org/codehaus/groovy/reflection/GroovyClassValueFactory.java#L30-L36 > > If ClassValue is not present, we fall back to the old behavior. So it > would just be about always using the old behavior in any case. > > 2015-09-08 18:01 GMT+02:00 Guillaume Laforge <[email protected]>: > >> When I set adding a flag, that's a flag to re-enable the ClassValue based >> approach, but by default, it's disabled, back as it was before. >> So you wouldn't have to set any Groovy flag on the Gradle side. >> >> On Tue, Sep 8, 2015 at 9:58 AM, Cédric Champeau < >> [email protected]> wrote: >> >>> There's a problem with an environment variable or flag in general: if >>> people are not aware of the problem, they just can't know. Also in my case, >>> there were lots of places to "fix" in Gradle: the Ant builder, a build >>> script, the ApiGroovyCompiler, ... all because they use Groovy at some >>> point, which will spread its ClassValue everywhere. I just managed - I hope >>> - this morning to get all those leaks fixed, but it's nevertheless in some >>> situations beyond our control: it depends on the version of Groovy that the >>> user chooses, and the version of the VM that they use. Gradle wouldn't be >>> able to set the flag for them, especially in forked environments. >>> >>> I would prefer to temporary disable it, until we know at least one Java >>> 7 and one Java 8 VM works properly. >>> >>> 2015-09-08 9:52 GMT+02:00 Guillaume Laforge <[email protected]>: >>> >>>> Or perhaps a kind of feature toggle, with an environment variable? >>>> We use the former mechanism by default, unless this env var is set to >>>> true? >>>> That way we can still easily check if the VM bug / behavior is fixed / >>>> changed? >>>> >>>> On Tue, Sep 8, 2015 at 9:35 AM, Jochen Theodorou <[email protected]> >>>> wrote: >>>> >>>>> Am 08.09.2015 09:07, schrieb Cédric Champeau: >>>>> >>>>>> Hi guys, >>>>>> >>>>>> As some of you may know, I've been investigating a memory leak which >>>>>> involves all versions of Groovy starting from 2.4. The leak comes >>>>>> from a >>>>>> bug in the VM regarding how it handles ClassValue, that Groovy 2.4 >>>>>> started using. All VMs are affected (7, 8 and 9) and it's still >>>>>> unclear >>>>>> when this will be fixed. So I would like to suggest to rollback this >>>>>> change for the next release. >>>>>> >>>>>> Basically, this commit: >>>>>> >>>>>> https://github.com/apache/incubator-groovy/commit/97d78e9e52deb52c8e66db501ef208f30384d014 >>>>>> >>>>>> It greatly affects Gradle, so I would suggest to make the change ASAP >>>>>> (2.4.5) if everyone agrees. >>>>>> >>>>> >>>>> -1 >>>>> >>>>> We can disable it by default till we find a better solution. But we >>>>> don't need to roll it back completely. I am afraid of the fix not being >>>>> applicable later on anymore >>>>> >>>>> bye blackdrag >>>>> >>>>> -- >>>>> Jochen "blackdrag" Theodorou >>>>> blog: http://blackdragsview.blogspot.com/ >>>>> >>>>> >>>> >>>> >>>> -- >>>> Guillaume Laforge >>>> Apache Groovy committer & PMC member >>>> Product Ninja & Advocate at Restlet <http://restlet.com> >>>> >>>> Blog: http://glaforge.appspot.com/ >>>> Social: @glaforge <http://twitter.com/glaforge> / Google+ >>>> <https://plus.google.com/u/0/114130972232398734985/posts> >>>> >>> >>> >> >> >> -- >> Guillaume Laforge >> Apache Groovy committer & PMC member >> Product Ninja & Advocate at Restlet <http://restlet.com> >> >> Blog: http://glaforge.appspot.com/ >> Social: @glaforge <http://twitter.com/glaforge> / Google+ >> <https://plus.google.com/u/0/114130972232398734985/posts> >> > > -- Guillaume Laforge Apache Groovy committer & PMC member Product Ninja & Advocate at Restlet <http://restlet.com> Blog: http://glaforge.appspot.com/ Social: @glaforge <http://twitter.com/glaforge> / Google+ <https://plus.google.com/u/0/114130972232398734985/posts>
