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" <jlas...@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 <vita...@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" <jlas...@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 <vita...@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.r...@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@openjdk.java.net >>> http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev >>> _______________________________________________ >>> mlvm-dev mailing list >>> mlvm-dev@openjdk.java.net >>> http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev >> >> _______________________________________________ >> mlvm-dev mailing list >> mlvm-dev@openjdk.java.net >> http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev >> >> _______________________________________________ >> mlvm-dev mailing list >> mlvm-dev@openjdk.java.net >> http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev > > _______________________________________________ > mlvm-dev mailing list > mlvm-dev@openjdk.java.net > http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev > > _______________________________________________ > mlvm-dev mailing list > mlvm-dev@openjdk.java.net > http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
_______________________________________________ mlvm-dev mailing list mlvm-dev@openjdk.java.net http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
