March 2012 - core-libs-dev

Re: 7127687: MethodType leaks memory due to interning
by John Rose 29 Mar '12

29 Mar '12

Jim's and Remi's comments on the problem are correct. In the current design, MH.type values must be interned. Comments on this design decision are interesting and useful, but this is not new ground; we went over it during the JDK 7 engineering and spec. work. It is likely we will revisit this decision at some point, but not today. To risk stating the obvious: I am not asking at this point for a review of the use of interned method types. I am asking for a review of a change to the mechanism *by which they are interned*. — John P.S. I will also point out that the "intern" design pattern, although very old in Java (String.intern), is poorly supported by the collections API. I think a lot of the comments people have are based on the assumption, "surely there must be a way to do this without so much code copying". One would hope so, but in this case one would be wrong. What's needed is a Set with the following properties: 1. Assuming set.contains(x), a way to return a reference to the equal value x' already in the set. 2. Assuming !set.contains(x), a way to insert x. 3. Thread safety. 4. Weak references, so that unreferenced elements x' are quietly dropped. 1,2 would be covered by something like this hypothetical API: interface InterningSet<T> extends Set<T> { E intern(E x); // ensures in set; returns x or pre-existing x' E find(E x); // returns null if not in set, else pre-existing x' } Every JVM has some mechanism like this for String.intern, somewhere.

1 0

Re: 7127687: MethodType leaks memory due to interning
by Vitaly Davidovich 29 Mar '12

29 Mar '12

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(a)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(a)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(a)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(a)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(a)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(a)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(a)openjdk.java.net >>>> http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev >>>> >>> _______________________________________________ >>> mlvm-dev mailing list >>> mlvm-dev(a)openjdk.java.net >>> http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev >>> >>> >>> _______________________________________________ >>> mlvm-dev mailing list >>> mlvm-dev(a)openjdk.java.net >>> http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev >>> >>> _______________________________________________ >> mlvm-dev mailing list >> mlvm-dev(a)openjdk.java.net >> http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev >> >> >> _______________________________________________ >> mlvm-dev mailing list >> mlvm-dev(a)openjdk.java.net >> http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev >> >> _______________________________________________ > mlvm-dev mailing list > mlvm-dev(a)openjdk.java.net > http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev > > > > _______________________________________________ > mlvm-dev mailing list > mlvm-dev(a)openjdk.java.net > http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev > >

1 0

Re: 7127687: MethodType leaks memory due to interning
by Vitaly Davidovich 29 Mar '12

29 Mar '12

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(a)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(a)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(a)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(a)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(a)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(a)openjdk.java.net >>> http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev >>> >> _______________________________________________ >> mlvm-dev mailing list >> mlvm-dev(a)openjdk.java.net >> http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev >> >> >> _______________________________________________ >> mlvm-dev mailing list >> mlvm-dev(a)openjdk.java.net >> http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev >> >> _______________________________________________ > mlvm-dev mailing list > mlvm-dev(a)openjdk.java.net > http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev > > > _______________________________________________ > mlvm-dev mailing list > mlvm-dev(a)openjdk.java.net > http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev > >

1 0

Re: 7127687: MethodType leaks memory due to interning
by John Rose 29 Mar '12

29 Mar '12

Thanks, Jim. -- John (on my iPhone T-1000) On Mar 28, 2012, at 6:01 PM, Jim Laskey <jlaskey(a)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(a)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(a)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(a)openjdk.java.net >> http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev >> _______________________________________________ >> mlvm-dev mailing list >> mlvm-dev(a)openjdk.java.net >> http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev > _______________________________________________ > mlvm-dev mailing list > mlvm-dev(a)openjdk.java.net > http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

2 1

hg: jdk8/tl/jdk: 7070436: Support Unicode 6.1.0 in JDK 8
by yuka.kamiya＠oracle.com 29 Mar '12

29 Mar '12

Changeset: b2687ac9cffb Author: peytoia Date: 2012-03-29 18:02 +0900 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/b2687ac9cffb 7070436: Support Unicode 6.1.0 in JDK 8 Reviewed-by: okutsu ! make/tools/GenerateCharacter/CharacterData00.java.template ! make/tools/UnicodeData/PropList.txt ! make/tools/UnicodeData/Scripts.txt ! make/tools/UnicodeData/SpecialCasing.txt ! make/tools/UnicodeData/UnicodeData.txt ! make/tools/UnicodeData/VERSION ! src/share/classes/java/awt/font/NumericShaper.java ! src/share/classes/java/lang/Character.java ! src/share/classes/sun/text/resources/ubidi.icu ! src/share/classes/sun/text/resources/unorm.icu ! src/share/classes/sun/text/resources/uprops.icu ! test/java/lang/Character/CheckProp.java ! test/java/lang/Character/CheckScript.java ! test/java/lang/Character/PropList.txt ! test/java/lang/Character/PropertyValueAliases.txt ! test/java/lang/Character/Scripts.txt

1 0

Re: 7127687: MethodType leaks memory due to interning
by Vitaly Davidovich 29 Mar '12

29 Mar '12

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(a)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(a)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(a)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(a)openjdk.java.net >> http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev >> > _______________________________________________ > mlvm-dev mailing list > mlvm-dev(a)openjdk.java.net > http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev > > > _______________________________________________ > mlvm-dev mailing list > mlvm-dev(a)openjdk.java.net > http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev > >

1 0

7087658: MethodHandles.Lookup.findVirtual is confused by interface methods that are multiply inherited
by John Rose 28 Mar '12

28 Mar '12

http://cr.openjdk.java.net/~jrose/7087658/webrev.00/ 7087658: MethodHandles.Lookup.findVirtual is confused by interface methods that are multiply inherited 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

1 0

Please review 7156976: improve java tools testing
by Kumar Srinivasan 28 Mar '12

28 Mar '12

Hi, We are adding new test to test the tool functionality of the launcher: 1. verify the options intended for a tool does gets to it intact, Steve Sides from SQE has contributed this test, which applies various test vectors to a simulated javac launcher. 2. enhanced the version checking to ensure a tool can be launched 3. The version checker now has inverted logic to select the tool selection, it will execute all possible tools in the bin directory. Thus any new tools added will be tested. Therefore, if a new tool is not compliant, the onus is on the tool developer to: a: make the tool compliant. b: if a is impossible, then add it to the excluded list. http://cr.openjdk.java.net/~ksrini/7156976/webrev/ http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7156976 Thanks Kumar & Steve

2 1

hg: jdk8/tl/jdk: 7156831: The jcmd man page is not included in generated bundles
by frederic.parain＠oracle.com 28 Mar '12

28 Mar '12

Changeset: 62228dc7a4ac Author: fparain Date: 2012-03-28 02:20 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/62228dc7a4ac 7156831: The jcmd man page is not included in generated bundles Reviewed-by: dholmes, sla, dsamersoff ! make/common/Release.gmk

1 0

hg: jdk8/tl/jdk: 7134701: [macosx] Support legacy native library names
by michael.x.mcmahon＠oracle.com 27 Mar '12

27 Mar '12

Changeset: 396533b75ea0 Author: michaelm Date: 2012-03-27 21:08 +0100 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/396533b75ea0 7134701: [macosx] Support legacy native library names Reviewed-by: alanb, dcubed ! make/java/java/FILES_java.gmk + src/macosx/classes/java/lang/ClassLoaderHelper.java ! src/share/classes/java/lang/ClassLoader.java + src/solaris/classes/java/lang/ClassLoaderHelper.java + src/windows/classes/java/lang/ClassLoaderHelper.java

1 0