7127687: MethodType leaks memory due to interning

[Vitaly Davidovich]

I don't see anything precluding == equality check.  MethodType is the key,
it implements equals() via ==.  In order to look it up in the data
structure you need to have a reference to one of these instances already,
correct? So if you have a reference to it, call containsKey() using it and
get true, then you already have a handle to the same instance.  I must be
missing something though ...

Sent from my phone
On Mar 29, 2012 9:03 AM, "Jim Laskey" <jlaskey at me.com> wrote:

> That's probably a legitimate argument.  I only chose Set because it was
> based on the existing code in WeakHashMap/WeakHashSet.  Maybe WeakInterner
> may be more appropriate.
>
> "although possibly different instance"  is THE thing.  We only want one
> instance of each MethodType in the environment so that MethodType.equals is
> a simple == test.
>
> On 2012-03-29, at 9:55 AM, Vitaly Davidovich wrote:
>
> No set that I know of allows you to get the key - you can check for its
> presence or remove it.  Since containsKey() requires that you pass it the
> key to find, there's nothing to return since you already have an equivalent
> key (although possibly different instance).  If that's not the intent of
> WeakInternSet then it shouldn't be called a set :).
>
> Sent from my phone
> On Mar 29, 2012 5:46 AM, "Jim Laskey" <jlaskey at me.com> wrote:
>
>> What we are trying to do is intern the MethodType. Maps are designed to
>> provide the key -> value relationship (one way.) If we only used the key
>> and a dummy value in the Map, it would be equivalent to a write only
>> collection, since there is no method to return a found key.  We would have
>> to set the value to sometime meaningful like the existing MethodType. Doing
>> so creates the hard reference to the MethodType and transitively to the
>> unloadable Classes.
>>
>> We encountered this problem in Nashorn where we generate object classes
>> at runtime. MethodTypes that referred to these classes "hung on" to them
>> indefinitely.  When running JavaScript test suites, we soon exhausted the
>> perm gen.
>>
>> I tried using WeakHashMap initially but then realized the implications.
>> WeakHashSet is useless, since it uses WeakHashMap, making it not weak at
>> all.
>>
>> This fix has been well tested, as it is integrated in our test system (4
>> times a day on several different platforms.)
>>
>>
>> Sent from my iPhone
>>
>> On 2012-03-28, at 11:51 PM, Vitaly Davidovich <vitalyd at gmail.com> wrote:
>>
>> We only care about keys here anyway, right? WeakHashMap with a dummy
>> value object (some static reference) should achieve the same thing since
>> its Entry will have the key referenced weakly and the value strongly (but
>> we don't care since the value is a dummy static ref).  Maybe I
>> misunderstood the intent though ...
>>
>> Sent from my phone
>> On Mar 28, 2012 9:02 PM, "Jim Laskey" <jlaskey at me.com> wrote:
>>
>>> The WeakHashMap leads to a non-weak reference to the class, since only
>>> the key is weak. Same is true for public versions of WeakHashSet. The
>>> collection used here is truly weak.
>>>
>>> Sent from my iPhone 4
>>>
>>> On 2012-03-28, at 9:42 PM, Vitaly Davidovich <vitalyd at gmail.com> wrote:
>>>
>>> Hi John,
>>>
>>> I think you can use diamond generic inference when declaring the weak
>>> intern set.
>>>
>>> Also any reason you didn't use WeakHashMap directly with dummy value to
>>> simulate the set? Or wrap the WeakHashMap and synchronize the accessors to
>>> it?
>>>
>>> Sent from my phone
>>> On Mar 28, 2012 7:52 PM, "John Rose" <john.r.rose at oracle.com> wrote:
>>>
>>>> http://cr.openjdk.java.net/~jrose/7127687/webrev.00/
>>>>
>>>> 7127687: MethodType leaks memory due to interning
>>>> Summary: Replace internTable with a weak-reference version.
>>>>
>>>> This is a point fix for JDK 8, and will (pending approval) also be
>>>> back-ported to JDK 7u.
>>>>
>>>> — John
>>>>
>>>> Notes on process:  This code is part of JSR 292.  Therefore the review
>>>> comments will be collected in mlvm-dev, and changes will be integrated via
>>>> hsx/hotspot-comp.
>>>>
>>>> At least one reviewer must be an official Reviewer the JDK 8 Project
>>>> [1], but other reviewers are most welcome.
>>>>
>>>> [1] http://openjdk.java.net/census#jdk8
>>>>
>>>> _______________________________________________
>>>> mlvm-dev mailing list
>>>> mlvm-dev at openjdk.java.net
>>>> http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
>>>>
>>> _______________________________________________
>>> mlvm-dev mailing list
>>> mlvm-dev at openjdk.java.net
>>> http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
>>>
>>>
>>> _______________________________________________
>>> mlvm-dev mailing list
>>> mlvm-dev at openjdk.java.net
>>> http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
>>>
>>> _______________________________________________
>> mlvm-dev mailing list
>> mlvm-dev at openjdk.java.net
>> http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
>>
>>
>> _______________________________________________
>> mlvm-dev mailing list
>> mlvm-dev at openjdk.java.net
>> http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
>>
>> _______________________________________________
> mlvm-dev mailing list
> mlvm-dev at openjdk.java.net
> http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
>
>
>
> _______________________________________________
> mlvm-dev mailing list
> mlvm-dev at openjdk.java.net
> http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20120329/911402fb/attachment-0001.html