7127687: MethodType leaks memory due to interning

[Bengt Rutisson]

Hi John,

This is really not a part of the code that I know well. I don't think I 
can fully review this change, but I do have some questions.

First, I have the same concern as David. This change means that we will 
produce more WeakReference objects. Since I don't know the code that 
well I can't estimate if it will be many or few. But it seems to me that 
if you get a PermGen OOME now you are probably creating quite a few 
instances of the MethodType that you will add to the WeakInternSet, 
right? I think we can expect some performance issues in the GC from that.

Second, I still don't really understand how WeakInternSet is different 
from what Vitaly suggested. To just use WeakHashMap with a dummy object 
as value. Can you explain the difference in more detail?

The CR mentions another way to solve the issue: "We could intern 
MethodTypes involving only bootstrap types. For other MethodTypes, we 
could always use .equals to compare rather than identity comparison."

Again, I really don't know what I am talking about here, but would that 
solution avoid the WeakReferences? If yes, would it be possible to pick 
that solution instead?


Two minor things as well:

* Don't think you need "import java.util.HashMap;" anymore.
* Copyright year 2012 ;-)

Bengt


On 2012-03-29 04:50, David Holmes wrote:
> Hi John,
>
> On 29/03/2012 9:51 AM, John Rose 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.
>
> The change looks okay in principle but I can't validate the new data 
> structure semantics.
>
> One query I have is whether, given its intended usage, this change 
> might add pressure to the reference processing subsystem?
>
> David
> -----
>
>> [1] http://openjdk.java.net/census#jdk8
>>