7127687: MethodType leaks memory due to interning

Thu Mar 29 06:47:20 PDT 2012

If you have instances of a MethodType A and B, that have the exact same return type and arguments, then they are equivalent and it is necessary to only keep one (let's say A.)  It's also desirable to only keep one copy because of the equals test, not just java code, but also internally in the Hotspot C++ code.   Thus, if A and B are constructed independently, we need to go thru an interning process to guarantee uniqueness (a singleton for each unique MethodType.)  Using a set S only gets you half the way thru the process.  We can add A to S and we can then find B in S, but we need a function F(S, B) -> A, so we can discard B and use A in it's stead.  Sets do not have such a function,  maps do, but then we get into the hard reference issue.  WeakInternSet is effectively a set with function F added.

On 2012-03-29, at 10:25 AM, Vitaly Davidovich wrote:

> 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
> 
> _______________________________________________
> 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/mlvm-dev/attachments/20120329/45b7d100/attachment-0001.html 