7127687: MethodType leaks memory due to interning
bengt.rutisson at oracle.com
Wed Mar 28 23:45:48 PDT 2012
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 ;-)
On 2012-03-29 04:50, David Holmes wrote:
> Hi John,
> On 29/03/2012 9:51 AM, John Rose wrote:
>> 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
>> , 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?
>>  http://openjdk.java.net/census#jdk8
More information about the hotspot-compiler-dev