groovy-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Cédric Champeau <cedric.champ...@gmail.com>
Subject Re: Rolling back change to use ClassValue
Date Tue, 08 Sep 2015 16:04:38 GMT
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 <glaforge@gmail.com>:

> 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 <cedric.champeau@gmail.com
> > 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 <glaforge@gmail.com>:
>>
>>> 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 <blackdrag@gmx.org>
>>> 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>
>

Mime
View raw message