February 2017 - core-libs-dev

Review Request: JDK-8173712: Rename JAVA_OPTIONS environment variable to JDK_JAVA_OPTIONS
by Mandy Chung 10 Feb '17

10 Feb '17

Henry, Kumar, Can you please review this patch: http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8173712/webrev.00/ JAVA_OPTIONS was introduced by JDK-8170832 but it turns out that environment variable name conflicts with existing uses. It might not be uncommon for scripts to set JAVA_OPTIONS. To mitigate the compatibility concern, we decide to rename JAVA_OPTIONS to JDK_JAVA_OPTIONS. With multi-word name and `JDK_` prefix, that should reduce the chance of name clash. thanks Mandy

4 4

RFR: 8029019: (ann) Optimize annotation handling in core reflection
by Peter Levart 10 Feb '17

10 Feb '17

Hi, This is a preview of a patch that addresses the following issue: https://bugs.openjdk.java.net/browse/JDK-8029019 But it is also an attempt to clean-up failure reporting for invalid annotation types. Here's the 1st webrev: http://cr.openjdk.java.net/~plevart/jdk9-dev/8029019_Annotations.optimizati… With this patch applied, running the following benchmark: http://cr.openjdk.java.net/~plevart/jdk9-dev/8029019_Annotations.optimizati… Produces the following results: Original: Benchmark Mode Cnt Score Error Units AnnotationsBench.a1_PrimitiveMember avgt 10 20.790 ± 1.317 ns/op AnnotationsBench.a2_ObjectMember avgt 10 21.550 ± 0.580 ns/op AnnotationsBench.a3_PrimitiveArrayMember avgt 10 28.314 ± 1.477 ns/op AnnotationsBench.a4_ObjectArrayMember avgt 10 27.094 ± 0.574 ns/op AnnotationsBench.a5_AnnotationType avgt 10 13.047 ± 0.983 ns/op AnnotationsBench.a6_HashCode avgt 10 158.001 ± 9.347 ns/op AnnotationsBench.a7_Equals avgt 10 663.921 ± 27.256 ns/op AnnotationsBench.a8_ToString avgt 10 1772.901 ± 107.355 ns/op Patched: Benchmark Mode Cnt Score Error Units AnnotationsBench.a1_PrimitiveMember avgt 10 8.547 ± 0.100 ns/op AnnotationsBench.a2_ObjectMember avgt 10 8.352 ± 0.340 ns/op AnnotationsBench.a3_PrimitiveArrayMember avgt 10 17.526 ± 0.696 ns/op AnnotationsBench.a4_ObjectArrayMember avgt 10 15.639 ± 0.116 ns/op AnnotationsBench.a5_AnnotationType avgt 10 4.692 ± 0.154 ns/op AnnotationsBench.a6_HashCode avgt 10 142.954 ± 7.460 ns/op AnnotationsBench.a7_Equals avgt 10 184.535 ± 0.917 ns/op AnnotationsBench.a8_ToString avgt 10 1806.685 ± 96.370 ns/op Allocation rates are also reduced: Original: AnnotationsBench.a1_PrimitiveMember:·gc.alloc.rate.norm avgt 10 16.000 ± 0.001 B/op AnnotationsBench.a2_ObjectMember:·gc.alloc.rate.norm avgt 10 16.000 ± 0.001 B/op AnnotationsBench.a3_PrimitiveArrayMember:·gc.alloc.rate.norm avgt 10 64.000 ± 0.001 B/op AnnotationsBench.a4_ObjectArrayMember:·gc.alloc.rate.norm avgt 10 48.000 ± 0.001 B/op AnnotationsBench.a5_AnnotationType:·gc.alloc.rate.norm avgt 10 16.000 ± 0.001 B/op AnnotationsBench.a6_HashCode:·gc.alloc.rate.norm avgt 10 128.000 ± 0.001 B/op AnnotationsBench.a7_Equals:·gc.alloc.rate.norm avgt 10 120.001 ± 0.001 B/op AnnotationsBench.a8_ToString:·gc.alloc.rate.norm avgt 10 5200.002 ± 0.001 B/op Patched: AnnotationsBench.a1_PrimitiveMember:·gc.alloc.rate.norm avgt 10 ≈ 10⁻⁵ B/op AnnotationsBench.a2_ObjectMember:·gc.alloc.rate.norm avgt 10 ≈ 10⁻⁵ B/op AnnotationsBench.a3_PrimitiveArrayMember:·gc.alloc.rate.norm avgt 10 48.000 ± 0.001 B/op AnnotationsBench.a4_ObjectArrayMember:·gc.alloc.rate.norm avgt 10 32.000 ± 0.001 B/op AnnotationsBench.a5_AnnotationType:·gc.alloc.rate.norm avgt 10 ≈ 10⁻⁵ B/op AnnotationsBench.a6_HashCode:·gc.alloc.rate.norm avgt 10 64.000 ± 0.001 B/op AnnotationsBench.a7_Equals:·gc.alloc.rate.norm avgt 10 24.000 ± 0.001 B/op AnnotationsBench.a8_ToString:·gc.alloc.rate.norm avgt 10 5136.002 ± 0.001 B/op As for the failure reporting: requesting an annotation for invalid annotation type now always produces AnnotationFormatError. Previously, some invalid annotations were simply skipped, some produced AnnotationFormatError and for some of them, AnnotationFormatError was raised only when evaluating Annotation's equals() method. I would like to discuss this failure behavior more in-depth. Regards, Peter

4 11

RFR of JDK-8174698: Fix @since in module-info.java in dev/corba repo
by Hamlin Li 10 Feb '17

10 Feb '17

Would you please review the below patch? bug: https://bugs.openjdk.java.net/browse/JDK-8174698 webrev: http://cr.openjdk.java.net/~mli/8174698/webrev.00/ Thank you -Hamlin

2 1

RFR of JDK-8174697: Fix @since in module-info.java in dev/jaxws repo
by Hamlin Li 10 Feb '17

10 Feb '17

Would you please review the below patch? bug: https://bugs.openjdk.java.net/browse/JDK-8174697 webrev: http://cr.openjdk.java.net/~mli/8174697/webrev.00/ Thank you -Hamlin

1 0

RFR of JDK-8174696: Fix @since in module-info.java in dev/jaxp repo
by Hamlin Li 10 Feb '17

10 Feb '17

Would you please review the below patch? bug: https://bugs.openjdk.java.net/browse/JDK-8174696 webrev: http://cr.openjdk.java.net/~mli/8174696/webrev.00/ Thank you -Hamlin

1 0

RFR of JDK-8173957: Fix @since in module-info.java in dev/jdk repo
by Hamlin Li 10 Feb '17

10 Feb '17

Would you please review the below patch? bug: https://bugs.openjdk.java.net/browse/JDK-8173957 webrev: http://cr.openjdk.java.net/~mli/8173957/webrev.00/ Thank you -Hamlin

3 3

[9] RFR: 8169715: jimage fails with IAE when attempts to inspect an empty file
by Denis Kononenko 09 Feb '17

09 Feb '17

Hi, Could please someone review the fix for JDK-8169715. The problem occurs when jimage tool is attempting to read an empty or incomplete image. The current implementation works with ByteBuffer and always relies on that there's enough data to inspect the image. This results in IAE inside of BasicImageReader.slice method when the buffer is smaller then it's expected. I added a few checks to avoid IAE and throw IOException with a more descriptive text. BUGURL: https://bugs.openjdk.java.net/browse/JDK-8169715 WEBREV: http://cr.openjdk.java.net/~dkononenko/8169715/webrev.01 Thank you, Denis.

1 0

Re: [9] RFR 8170113: jimage extract to readonly directory causes MissingResourceException
by Denis Kononenko 09 Feb '17

09 Feb '17

Hi, > > On 06/02/2017 10:27, Denis Kononenko wrote: > > Hi, > > > > Could someone please review this very small fix. > > > > The class JImageTask > (src/jdk.jlink/share/classes/jdk/tools/jimage/JImageTask.java) has > references to an info message by 'err.cannot.create.dir' key. > Unfortunately the corresponding resource file > (src/jdk.jlink/share/classes/jdk/tools/jimage/resources/jimage.properties) > doesn't contain this key. This results in long not user friendly > stacktraces in output when jimage cannot create a directory. The fix > is very small, I added the missing definition into the resource file. > > > > BUGURL: https://bugs.openjdk.java.net/browse/JDK-8170113 > > WEBREV: http://cr.openjdk.java.net/~dkononenko/8170113/webrev.00 > > > This looks okay as a short term fix but I think that extract need to > be-implemented to use the new file system API so that the reasons why > > the directory can be printed too. Also there is an anomaly in errors > related to the directory result in an error message whereas any errors > > extracting to the directory lead to an exception. > > -Alan I think we might have to defer such big change to the next release. If there's no objections I'd propose to deliver this fix AS IS and open a new enhancement request to re-implement 'extract' method. Thank you, Denis.

2 1

RE: jdk10 : simplify jexec build settings
by Baesken, Matthias 09 Feb '17

09 Feb '17

Hello Erik, thanks for the comments . >>Then there is handling for macosx left , but the build is not enabled for macosx, does it still make sense to include the macosx handling (there is even a separate jexec.c for macosx) : > >This is indeed weird. I don't think we ever built jexec for macosx so the source file should likely be removed. That is however a question for core-libs I think. I include core-libs-dev , can someone comment on jexec on macosx ? Currently jexec.c is not compiled for macosx (jdk9/10), see jdk/make/launcher/Launcher-java.base.gmk . It is just compiled on linux . So I wonder - is it ok to remove the macosx handling in the mentioned makefile jdk/make/launcher/Launcher-java.base.gmk for jexec ? >This will automatically look in all the directories where source files are supposed to be and pick the most specific one if there are any with the same name. >Since the share launcher dir contains more src files, you will need to leave the INCLUDE_FILES := jexec.c this time. >From what I observe in the linux build log, just jexec.c is compiled into jexec.o and then this single object is used to create the executable ("program") jexec. Other src files are not used in the compilation of jexec, so just using $(JDK_TOPDIR)/src/$(MODULE)/unix/native/launcher would be fine (as long as macosx can be dropped). Best regards, Matthias From: Erik Joelsson [mailto:erik.joelsson@oracle.com] Sent: Donnerstag, 9. Februar 2017 13:42 To: Baesken, Matthias <matthias.baesken(a)sap.com>; build-dev(a)openjdk.java.net Cc: Langer, Christoph <christoph.langer(a)sap.com> Subject: Re: jdk10 : simplify jexec build settings Hello, On 2017-02-09 12:36, Baesken, Matthias wrote: Hello , while adjusting the jspawnhelper build settings with 8174086, it has been noticed that the jexec build settings need some simplification as well. The bug https://bugs.openjdk.java.net/browse/JDK-8174242 has been created for this. When looking into it, I had some questions : http://hg.openjdk.java.net/jdk10/jdk10/jdk/file/ae7afa9abe67/make/launcher/… The makefile (make/launcher/Launcher-java.base.gmk ) handles Solaris 32bit, but is this really supported in jdk 9 or 10 ( I think it was removed in 9 and only 64bit Solaris support remains ) ? We do not support 32bit Solaris. The issue with jexec for Solaris was raised a while back and the conclusion was to scrap support, so no need to build it for Solaris at all. ifeq ($(OPENJDK_TARGET_OS), solaris) ifeq ($(OPENJDK_TARGET_CPU_BITS), 32) BUILD_JEXEC := 1 endif endif ifeq ($(OPENJDK_TARGET_OS), linux) BUILD_JEXEC := 1 endif # OPENJDK_TARGET_OS # # jdk/make/java/jexec/Makefile # ifeq ($(BUILD_JEXEC), 1) Then there is handling for macosx left , but the build is not enabled for macosx, does it still make sense to include the macosx handling (there is even a separate jexec.c for macosx) : This is indeed weird. I don't think we ever built jexec for macosx so the source file should likely be removed. That is however a question for core-libs I think. The makefile logic shouldn't need to be that specific though. Our modern pattern for this is something like this: SRC := $(call uniq, $(wildcard \ $(JDK_TOPDIR)/src/$(MODULE)/$(OPENJDK_TARGET_OS)/native/launcher \ $(JDK_TOPDIR)/src/$(MODULE)/$(OPENJDK_TARGET_OS_TYPE)/native/launcher \ $(JDK_TOPDIR)/src/$(MODULE)/share/native/launcher)) This will automatically look in all the directories where source files are supposed to be and pick the most specific one if there are any with the same name. Since the share launcher dir contains more src files, you will need to leave the INCLUDE_FILES := jexec.c this time. For libraries, we have abstracted this construct in the macro FindSrcDirForLib (LibCommon.gmk) but we haven't done the same for launchers yet. /Erik ifeq ($(OPENJDK_TARGET_OS), windows) else ifeq ($(OPENJDK_TARGET_OS), macosx) BUILD_JEXEC_SRC := $(JDK_TOPDIR)/src/java.base/macosx/native/launcher else BUILD_JEXEC_SRC := $(JDK_TOPDIR)/src/java.base/unix/native/launcher endif Should I remove the solaris 32bit / macosx handling for jexec from make/launcher/Launcher-java.base.gmk ? Regards, Matthias

1 0

JDK 9 RFR for JDK-8174241: ProblemList update for TestWsImport, JdbMethodExitTest and jimage tests
by Amy Lu 09 Feb '17

09 Feb '17

Please review the ProblemList.txt cleanup on updating bugid for several tests. javax/xml/ws/clientjar/TestWsImport.java 8170370 generic-all This test is in ProblemList, related bug JDK-8170370 has been resolved. Test is going to be revisited in JDK-8173317 tools/jimage/JImageExtractTest.java 8169713 generic-all tools/jimage/JImageListTest.java 8169713 generic-all tools/jimage/JImageVerifyTest.java 8169713 generic-all Mentioned bug JDK-8169713 has been closed, while test is going to be fixed in JDK-8170120 com/sun/jdi/JdbMethodExitTest.sh 6902121 generic-all Mentioned bug JDK-6902121 has been closed, test is going to be revisited in JDK-8171483 Thanks, Amy --- old/test/ProblemList.txt 2017-02-09 17:28:52.000000000 +0800 +++ new/test/ProblemList.txt 2017-02-09 17:28:51.000000000 +0800 @@ -253,9 +253,9 @@ tools/launcher/FXLauncherTest.java 8068049 linux-all,macosx-all -tools/jimage/JImageExtractTest.java 8169713 generic-all -tools/jimage/JImageListTest.java 8169713 generic-all -tools/jimage/JImageVerifyTest.java 8169713 generic-all +tools/jimage/JImageExtractTest.java 8170120 generic-all +tools/jimage/JImageListTest.java 8170120 generic-all +tools/jimage/JImageVerifyTest.java 8170120 generic-all tools/jlink/multireleasejar/JLinkMultiReleaseJarTest.java 8169971 windows-x64 @@ -266,7 +266,7 @@ com/sun/jdi/RedefineImplementor.sh 8004127 generic-all -com/sun/jdi/JdbMethodExitTest.sh 6902121 generic-all +com/sun/jdi/JdbMethodExitTest.sh 8171483 generic-all com/sun/jdi/RepStep.java 8043571 generic-all @@ -307,6 +307,6 @@ javax/rmi/PortableRemoteObject/8146975/RmiIiopReturnValueTest.java 8169737 linux-all -javax/xml/ws/clientjar/TestWsImport.java 8170370 generic-all +javax/xml/ws/clientjar/TestWsImport.java 8173317 generic-all ############################################################################

2 1