March 2013 - core-libs-dev

JDK 8 code review request for JDK-8004979 java.lang.reflect.Modifier.toString should include "default"
by Joe Darcy 29 Mar '13

29 Mar '13

Hello, Please review these changes to add support for the "default" modifier to the output of Method.to[Generic]String: 8004979 java.lang.reflect.Modifier.toString should include "default" http://cr.openjdk.java.net/~darcy/8004979.0/ Patch also included below. Thanks, -Joe diff -r d254a5f9b93f src/share/classes/java/lang/reflect/Constructor.java --- a/src/share/classes/java/lang/reflect/Constructor.java Wed Mar 27 13:40:26 2013 -0400 +++ b/src/share/classes/java/lang/reflect/Constructor.java Thu Mar 28 00:02:06 2013 -0700 @@ -284,9 +284,13 @@ * modifiers {@code public}, {@code protected} or * {@code private}. Only one of these may appear, or none if the * constructor has default (package) access. + * + * @return a string describing this {@code Constructor} + * @jls 8.8.3. Constructor Modifiers */ public String toString() { return sharedToString(Modifier.constructorModifiers(), + false, parameterTypes, exceptionTypes); } @@ -328,10 +332,11 @@ * include type parameters * * @since 1.5 + * @jls 8.8.3. Constructor Modifiers */ @Override public String toGenericString() { - return sharedToGenericString(Modifier.constructorModifiers()); + return sharedToGenericString(Modifier.constructorModifiers(), false); } @Override diff -r d254a5f9b93f src/share/classes/java/lang/reflect/Executable.java --- a/src/share/classes/java/lang/reflect/Executable.java Wed Mar 27 13:40:26 2013 -0400 +++ b/src/share/classes/java/lang/reflect/Executable.java Thu Mar 28 00:02:06 2013 -0700 @@ -89,20 +89,24 @@ } - void printModifiersIfNonzero(StringBuilder sb, int mask) { + void printModifiersIfNonzero(StringBuilder sb, int mask, boolean isDefault) { int mod = getModifiers() & mask; if (mod != 0) { sb.append(Modifier.toString(mod)).append(' '); } + if (isDefault) { + sb.append("default "); + } } String sharedToString(int modifierMask, + boolean isDefault, Class<?>[] parameterTypes, Class<?>[] exceptionTypes) { try { StringBuilder sb = new StringBuilder(); - printModifiersIfNonzero(sb, modifierMask); + printModifiersIfNonzero(sb, modifierMask, isDefault); specificToStringHeader(sb); sb.append('('); @@ -124,11 +128,11 @@ */ abstract void specificToStringHeader(StringBuilder sb); - String sharedToGenericString(int modifierMask) { + String sharedToGenericString(int modifierMask, boolean isDefault) { try { StringBuilder sb = new StringBuilder(); - printModifiersIfNonzero(sb, modifierMask); + printModifiersIfNonzero(sb, modifierMask, isDefault); TypeVariable<?>[] typeparms = getTypeParameters(); if (typeparms.length > 0) { diff -r d254a5f9b93f src/share/classes/java/lang/reflect/Field.java --- a/src/share/classes/java/lang/reflect/Field.java Wed Mar 27 13:40:26 2013 -0400 +++ b/src/share/classes/java/lang/reflect/Field.java Thu Mar 28 00:02:06 2013 -0700 @@ -288,6 +288,9 @@ * {@code protected} or {@code private} first, and then other * modifiers in the following order: {@code static}, {@code final}, * {@code transient}, {@code volatile}. + * + * @return a string describing this {@code Field} + * @jls 8.3.1 Field Modifiers */ public String toString() { int mod = getModifiers(); @@ -315,6 +318,7 @@ * its generic type * * @since 1.5 + * @jls 8.3.1 Field Modifiers */ public String toGenericString() { int mod = getModifiers(); diff -r d254a5f9b93f src/share/classes/java/lang/reflect/Method.java --- a/src/share/classes/java/lang/reflect/Method.java Wed Mar 27 13:40:26 2013 -0400 +++ b/src/share/classes/java/lang/reflect/Method.java Thu Mar 28 00:02:06 2013 -0700 @@ -343,10 +343,16 @@ * {@code public}, {@code protected} or {@code private} first, * and then other modifiers in the following order: * {@code abstract}, {@code static}, {@code final}, - * {@code synchronized}, {@code native}, {@code strictfp}. + * {@code synchronized}, {@code native}, {@code strictfp}, + * {@code default}. + * + * @return a string describing this {@code Method} + * + * @jls 8.4.3 Method Modifiers */ public String toString() { return sharedToString(Modifier.methodModifiers(), + isDefault(), parameterTypes, exceptionTypes); } @@ -389,16 +395,19 @@ * {@code public}, {@code protected} or {@code private} first, * and then other modifiers in the following order: * {@code abstract}, {@code static}, {@code final}, - * {@code synchronized}, {@code native}, {@code strictfp}. + * {@code synchronized}, {@code native}, {@code strictfp}, + * {@code default}. * * @return a string describing this {@code Method}, * include type parameters * * @since 1.5 + * + * @jls 8.4.3 Method Modifiers */ @Override public String toGenericString() { - return sharedToGenericString(Modifier.methodModifiers()); + return sharedToGenericString(Modifier.methodModifiers(), isDefault()); } @Override diff -r d254a5f9b93f test/java/lang/reflect/Method/GenericStringTest.java --- a/test/java/lang/reflect/Method/GenericStringTest.java Wed Mar 27 13:40:26 2013 -0400 +++ b/test/java/lang/reflect/Method/GenericStringTest.java Thu Mar 28 00:02:06 2013 -0700 @@ -23,7 +23,7 @@ /* * @test - * @bug 5033583 6316717 6470106 + * @bug 5033583 6316717 6470106 8004979 * @summary Check toGenericString() and toString() methods * @author Joseph D. Darcy */ @@ -39,6 +39,7 @@ classList.add(TestClass1.class); classList.add(TestClass2.class); classList.add(Roebling.class); + classList.add(TestInterface1.class); for(Class<?> clazz: classList) @@ -129,6 +130,20 @@ void varArg(Object ... arg) {} } +interface TestInterface1 { + @ExpectedGenericString( + "public default void TestInterface1.foo()") + @ExpectedString( + "public default void TestInterface1.foo()") + public default void foo(){;} + + @ExpectedString( + "public default java.lang.Object TestInterface1.bar()") + @ExpectedGenericString( + "public default <A> A TestInterface1.bar()") + default <A> A bar(){return null;} +} + @Retention(RetentionPolicy.RUNTIME) @interface ExpectedGenericString { String value();

4 6

RFR : JDK-8001642 : Add Optional<T>, OptionalDouble, OptionalInt, OptionalLong
by Mike Duigou 28 Mar '13

28 Mar '13

Hello All; This patch introduces Optional container objects to be used by the lambda streams libraries for returning results. The reference Optional type, as defined, intentionally does not allow null values. null may be used with the Optional.orElse() method. All of the Optional types define hashCode() and equals implementations. Use of Optional types in collections should be generally discouraged but having useful equals() and hashCode() is ever so convenient. http://cr.openjdk.java.net/~mduigou/JDK-8001642/0/webrev/ Mike

8 10

hg: jdk8/tl/langtools: 8010511: Tests are creating files in /tmp
by jonathan.gibbons＠oracle.com 28 Mar '13

28 Mar '13

Changeset: d3648557391b Author: jjg Date: 2013-03-28 10:58 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/langtools/rev/d3648557391b 8010511: Tests are creating files in /tmp Reviewed-by: darcy ! test/tools/javac/T6558476.java ! test/tools/javac/T6900149.java ! test/tools/javac/diags/CheckExamples.java ! test/tools/javac/diags/RunExamples.java

1 0

hg: jdk8/tl/langtools: 8006346: doclint should make allowance for headers generated by standard doclet
by jonathan.gibbons＠oracle.com 28 Mar '13

28 Mar '13

Changeset: 991f11e13598 Author: jjg Date: 2013-03-28 10:49 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/langtools/rev/991f11e13598 8006346: doclint should make allowance for headers generated by standard doclet Reviewed-by: mcimadamore ! src/share/classes/com/sun/tools/doclint/Checker.java ! src/share/classes/com/sun/tools/doclint/DocLint.java ! src/share/classes/com/sun/tools/doclint/Env.java ! src/share/classes/com/sun/tools/javac/main/Main.java ! src/share/classes/com/sun/tools/javadoc/DocEnv.java + test/tools/javac/doclint/ImplicitHeadersTest.java + test/tools/javadoc/doclint/ImplicitHeadersTest.java

1 0

hg: jdk8/tl/jdk: 8001642: Add Optional<T>, OptionalDouble, OptionalInt, OptionalLong
by mike.duigou＠oracle.com 28 Mar '13

28 Mar '13

Changeset: 2241a2d34085 Author: mduigou Date: 2013-03-19 16:05 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/2241a2d34085 8001642: Add Optional<T>, OptionalDouble, OptionalInt, OptionalLong Reviewed-by: mduigou, darcy, alanb, jjb Contributed-by: Brian Goetz <brian.goetz(a)oracle.com> + src/share/classes/java/util/Optional.java + src/share/classes/java/util/OptionalDouble.java + src/share/classes/java/util/OptionalInt.java + src/share/classes/java/util/OptionalLong.java + test/java/util/Optional/Basic.java + test/java/util/Optional/BasicDouble.java + test/java/util/Optional/BasicInt.java + test/java/util/Optional/BasicLong.java

4 3

8010309 : PlatformLogger: isLoggable performance / waste due to HashMap<Integer, Level> leads to Integer allocations (boxing)
by Laurent Bourgès 28 Mar '13

28 Mar '13

Here is an improved patch tested on JDK7u13 and JDK8 internal build on my machine linux x64: http://jmmc.fr/~bourgesl/share/webrev-8010309/ FYI, I removed completely the Map<Integer, Object> levelObjects and used two arrays to perform the PlatformLogger's level (int) to j.u.l.Level mapping: I decided to keep it simple as possible (no enum ...) and used a switch case based on current level occurences: 510 /** 511 * Return the corresponding j.u.l.Level instance 512 * @param level PlatformLogger level as integer 513 * @return Object (j.u.l.Level instance) or null if no matching level 514 */ 515 private static Object getLevel(final int level) { 516 if (levelObjects == null) { 517 return null; 518 } 519 // higher occurences first (finest, fine, finer, info) 520 // based on isLoggable(level) calls (03/20/2013) 521 // in jdk project only (including generated sources) 522 switch (level) { 523 case FINEST : return levelObjects[IDX_FINEST]; // 116 + 2257 matches in generated files 524 case FINE : return levelObjects[IDX_FINE]; // 270 525 case FINER : return levelObjects[IDX_FINER]; // 157 526 case INFO : return levelObjects[IDX_INFO]; // 39 527 case WARNING : return levelObjects[IDX_WARNING]; // 12 528 case CONFIG : return levelObjects[IDX_CONFIG]; // 6 529 case SEVERE : return levelObjects[IDX_SEVERE]; // 1 530 case OFF : return levelObjects[IDX_OFF]; // 0 531 case ALL : return levelObjects[IDX_ALL]; // 0 532 default : return null; 533 } 534 } I enhanced the PlatformLoggerTest class also and figured out that TLAB optimized Integer allocations but I think the patch is still useful. Can you have a look to the patch ? Should I write a jtreg test (performance / GC issue) ? Cheers, Laurent 2013/3/20 Mandy Chung <mandy.chung(a)oracle.com> > Hi Laurent, > > Thank you for signing the OCA. Your contribution is very welcome. You > can submit a patch for this bug (see [1]) to Core libraries group which > owns logging. Jim Gish and I will sponsor it. > > Thanks > Mandy > [1] http://openjdk.java.net/contribute/ > > > On 3/20/2013 5:45 AM, Laurent Bourgès wrote: > > Hi mandy, > > Do you want me to propose an improved patch to remove the former Map and > fix the getLevel() method ? or you prefer fix on your own ? > > Is it better to discuss the fix on the bug database (still not visible) ? > > Laurent > > 2013/3/19 Mandy Chung <mandy.chung(a)oracle.com> > >> Hi Laurent, >> >> Thanks for the contribution. I agree that the map can be replaced with a >> direct mapping from a int value to Level object and avoid the autoboxing >> conversion. >> >> I have filed a bug to track this and target this for JDK8: >> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id= 8010309 >> >> Thanks >> Mandy >> >> >> On 3/19/13 5:19 AM, Laurent Bourgès wrote: >> >> Dear all, >> >> I run recently netbeans profiler on my swing application (Aspro2:http://www.jmmc.fr/aspro >> ) under linux x64 platform and figured out a >> performance and waste issue related to PlatformLogger. >> >> Actually, the JavaLogger implementation uses a Map<Integer, Object> >> levelObjects to store mapping between PlatformLogger's levels (int) and JUL >> Level instances. >> >> However, the isLoggable(int level) method is highly used by awt project and >> other JDK projects and it leads to many Integer allocations as autoboxing >> converts the level as int to an Integer instance used by the Map.get() call. >> >> /** >> * JavaLogger forwards all the calls to its corresponding >> * java.util.logging.Logger object. >> */ >> static class JavaLogger extends LoggerProxy { >> private static final* Map<Integer, Object>* levelObjects = new >> HashMap<>(); >> ... >> public boolean isLoggable(*int level*) { >> return LoggingSupport.isLoggable(javaLogger, * >> levelObjects.get(level)*); >> } >> } >> >> I wrote a simple test to illustrate that performance / waste problem: >> PlatformLoggerTest that simply performs 1 million DISABLED log statements: >> if (log.isLoggable(PlatformLogger.FINEST)) { >> log.finest("test PlatformLogger.FINEST"); >> } >> >> As you can see in screenshots: >> - 5 million Integer instances are allocated >> - Integer.valueOf(int) is called 5 million times (autoboxing) >> - HashMap.get() represents 30% of the test time >> - patched PlatformLogger is 3x times faster >> [jvm options: -Xms8m -Xmx8m -verbose:gc] >> >> I suggest you to use an alternative way to map PlatformLogger's levels >> (int) and JUL Level instances to fix that performance / memory issue: I >> added the getLevel(int level) method that performs a switch case to return >> the corresponding Level object (quick and dirty solution). >> >> I advocate this is not a very clean solution but I prefer efficiency here: >> any better solution may be appropriate to avoid at least Integer allocation >> and maybe enhance performance. >> >> Best regards, >> Laurent Bourgès >> >> PS: here is the patch as text: >> >> # This patch file was generated by NetBeans IDE >> # It uses platform neutral UTF-8 encoding and \n newlines. >> --- PlatformLogger.java (6767) >> +++ PlatformLogger.java (6768) >> @@ -468,31 +468,13 @@ >> * java.util.logging.Logger object. >> */ >> static class JavaLogger extends LoggerProxy { >> - /** Note: using Integer keys leads to a lot of new Integer >> instances !! */ >> - private static final Map<Integer, Object> levelObjects = new >> HashMap<>(); >> - /** fastest mapping to Level instances from PlatformLogger level >> as integer */ >> - private static final Object[] fastLevelObjects; >> - >> - >> + private static final Map<Integer, Object> levelObjects = >> + new HashMap<>(); >> + >> static { >> if (LoggingSupport.isAvailable()) { >> // initialize the map to Level objects >> getLevelObjects(); >> - >> - // initialize the fastLevelObjects: >> - fastLevelObjects = new Object[] { >> - LoggingSupport.parseLevel(getLevelName(OFF)), // >> 0 >> - LoggingSupport.parseLevel(getLevelName(SEVERE)), // >> 1 >> - LoggingSupport.parseLevel(getLevelName(WARNING)), // >> 2 >> - LoggingSupport.parseLevel(getLevelName(INFO)), // >> 3 >> - LoggingSupport.parseLevel(getLevelName(CONFIG)), // >> 4 >> - LoggingSupport.parseLevel(getLevelName(FINE)), // >> 5 >> - LoggingSupport.parseLevel(getLevelName(FINER)), // >> 6 >> - LoggingSupport.parseLevel(getLevelName(FINEST)), // >> 7 >> - LoggingSupport.parseLevel(getLevelName(ALL)) // >> 8 >> - }; >> - } else { >> - fastLevelObjects = null; // check null >> } >> } >> >> @@ -515,7 +497,7 @@ >> this.javaLogger = LoggingSupport.getLogger(name); >> if (level != 0) { >> // level has been updated and so set the Logger's level >> - LoggingSupport.setLevel(javaLogger, getLevel(level)); >> + LoggingSupport.setLevel(javaLogger, >> levelObjects.get(level)); >> } >> } >> >> @@ -526,11 +508,11 @@ >> * not be updated. >> */ >> void doLog(int level, String msg) { >> - LoggingSupport.log(javaLogger, getLevel(level), msg); >> + LoggingSupport.log(javaLogger, levelObjects.get(level), msg); >> } >> >> void doLog(int level, String msg, Throwable t) { >> - LoggingSupport.log(javaLogger, getLevel(level), msg, t); >> + LoggingSupport.log(javaLogger, levelObjects.get(level), msg, >> t); >> } >> >> void doLog(int level, String msg, Object... params) { >> @@ -544,12 +526,12 @@ >> for (int i = 0; i < len; i++) { >> sparams [i] = String.valueOf(params[i]); >> } >> - LoggingSupport.log(javaLogger, getLevel(level), msg, sparams); >> + LoggingSupport.log(javaLogger, levelObjects.get(level), msg, >> sparams); >> } >> >> boolean isEnabled() { >> Object level = LoggingSupport.getLevel(javaLogger); >> - return level == null || level.equals(getLevel(OFF)) == false; >> + return level == null || level.equals(levelObjects.get(OFF)) == >> false; >> } >> >> int getLevel() { >> @@ -566,34 +548,12 @@ >> >> void setLevel(int newLevel) { >> levelValue = newLevel; >> - LoggingSupport.setLevel(javaLogger, getLevel(newLevel)); >> + LoggingSupport.setLevel(javaLogger, >> levelObjects.get(newLevel)); >> } >> >> public boolean isLoggable(int level) { >> - return LoggingSupport.isLoggable(javaLogger, getLevel(level)); >> + return LoggingSupport.isLoggable(javaLogger, >> levelObjects.get(level)); >> } >> - >> - /** >> - * Return the corresponding level object (fastest implementation) >> - * @param level PlatformLogger level as primitive integer >> - * @return Object (JUL Level instance) >> - */ >> - private static Object getLevel(final int level) { >> - // higher occurences first (finest, fine, finer, info): >> - switch (level) { >> - case FINEST : return fastLevelObjects[7]; >> - case FINE : return fastLevelObjects[5]; >> - case FINER : return fastLevelObjects[6]; >> - case INFO : return fastLevelObjects[3]; >> - case CONFIG : return fastLevelObjects[4]; >> - case WARNING : return fastLevelObjects[2]; >> - case SEVERE : return fastLevelObjects[1]; >> - case ALL : return fastLevelObjects[8]; >> - case OFF : return fastLevelObjects[0]; >> - default : return null; >> - } >> - } >> - >> } >> >> private static String getLevelName(int level) { >> >> >> > >

5 45

Re: RFR JDK-8010267 & JDK-8010268 : Makefile maintenance for test targets
by Mike Duigou 28 Mar '13

28 Mar '13

I still need a review for both of these changes. Mike On Mar 18 2013, at 22:48 , Mike Duigou wrote: > A two small changes to review: > > If approved I will commit to TL (or someone else can commit to build for me) > > Mike > > JDK-8010267 : Add test-clean for cleaning of testoutput directory from output directory. > > diff --git a/common/makefiles/Main.gmk b/common/makefiles/Main.gmk > --- a/common/makefiles/Main.gmk > +++ b/common/makefiles/Main.gmk > @@ -191,7 +191,7 @@ source-tips: $(OUTPUT_ROOT)/source_tips > > > # Remove everything, except the output from configure. > -clean: clean-langtools clean-corba clean-jaxp clean-jaxws clean-hotspot clean-jdk clean-images clean-overlay-images clean-bootcycle-build clean-docs > +clean: clean-langtools clean-corba clean-jaxp clean-jaxws clean-hotspot clean-jdk clean-images clean-overlay-images clean-bootcycle-build clean-docs clean-test > @($(CD) $(OUTPUT_ROOT) && $(RM) -r tmp source_tips build.log* build-trace*.log*) > @$(ECHO) Cleaned all build artifacts. > > @@ -230,6 +230,8 @@ clean-bootcycle-build: > clean-docs: > $(call CleanComponent,docs) > $(call CleanComponent,docstemp) > +clean-test: > + $(call CleanComponent,testoutput) > > .PHONY: langtools corba jaxp jaxws hotspot jdk images overlay-images install > .PHONY: langtools-only corba-only jaxp-only jaxws-only hotspot-only jdk-only images-only overlay-images-only install-only > > > JDK-8010268 : Remove dependence upon clean target from jdk/test/Makefile prep target > > None of the current users seem to depend upon the clean behaviour of "prep" > > diff --git a/test/Makefile b/test/Makefile > --- a/test/Makefile > +++ b/test/Makefile > @@ -336,7 +336,7 @@ all: jdk_default > @$(ECHO) "Testing completed successfully" > > # Prep for output > -prep: clean > +prep: > @$(MKDIR) -p $(ABS_TEST_OUTPUT_DIR) > @$(MKDIR) -p `$(DIRNAME) $(ARCHIVE_BUNDLE)` > >

3 2

hg: jdk8/tl/jdk: 8010125: keytool -importkeystore could create a pkcs12 keystore with different storepass and keypass
by weijun.wang＠oracle.com 28 Mar '13

28 Mar '13

Changeset: a87fac00915e Author: weijun Date: 2013-03-28 20:27 +0800 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/a87fac00915e 8010125: keytool -importkeystore could create a pkcs12 keystore with different storepass and keypass Reviewed-by: vinnie ! src/share/classes/sun/security/tools/keytool/Main.java ! src/share/classes/sun/security/tools/keytool/Resources.java + test/sun/security/tools/keytool/p12importks.sh

1 0

hg: jdk8/tl/langtools: 2 new changesets
by maurizio.cimadamore＠oracle.com 28 Mar '13

28 Mar '13

Changeset: 7bebe17ff323 Author: mcimadamore Date: 2013-03-28 11:38 +0000 URL: http://hg.openjdk.java.net/jdk8/tl/langtools/rev/7bebe17ff323 8010469: Bad assertion in LambdaToMethod Summary: Add assertion in LambdaToMethod.serializedLambdaName Reviewed-by: jjg ! src/share/classes/com/sun/tools/javac/comp/LambdaToMethod.java Changeset: a200d8ccfe47 Author: mcimadamore Date: 2013-03-28 11:39 +0000 URL: http://hg.openjdk.java.net/jdk8/tl/langtools/rev/a200d8ccfe47 8010490: FindBugs: double assignments in LambdaToMethod.visitIdent Summary: Remove dead code from LambdaToMethod Reviewed-by: jjg ! src/share/classes/com/sun/tools/javac/comp/LambdaToMethod.java

1 0

hg: jdk8/tl/jdk: 8010991: Enable test/javax/script/GetInterfaceTest.java again
by sundararajan.athijegannathan＠oracle.com 28 Mar '13

28 Mar '13

Changeset: 811c771acf65 Author: sundar Date: 2013-03-28 14:36 +0530 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/811c771acf65 8010991: Enable test/javax/script/GetInterfaceTest.java again Reviewed-by: lagergren, hannesw ! test/javax/script/GetInterfaceTest.java

1 0