June 2018 - core-libs-dev

RFR(M): 8203826: Chain class initialization exceptions into later NoClassDefFoundErrors
by Volker Simonis 09 Jul '18

09 Jul '18

Hi, can I please have a review for the following change which saves ExceptionInInitializerError thrown during class initialization and chains them as cause into potential NoClassDefFoundErrors for the same class. We are using this features since years in our commercial SAP JVM and it proved extremely useful for detecting and fixing errors especially in big deployments. This is a follow-up on a discussion previously started by Goetz [1]. His first proposal (which is close to our current, internal implementation) inserted an additional field into java.lang.Class objects to save potential ExceptionInInitializerErrors. This was considered to much overhead in the initial discussion [1]. http://cr.openjdk.java.net/~simonis/webrevs/2018/8203826.v2/ https://bugs.openjdk.java.net/browse/JDK-8203826 So in this change, I've completely re-implemented the feature by using a java.lang.Hashtable which is attached to the ClassLoaderData object. The Hashtable is lazily created when the first ExceptionInInitializerError is thrown and maps the Class which triggered the ExceptionInInitializerError during the execution of its static initializer to the corresponding ExceptionInInitializerError. If the same class will be accessed once again, this will directly lead to a plain NoClassDefFoundError (as per the JVMS, 5.5 Initialization) because the static initializer won't be executed a second time. Until now, this NoClassDefFoundError wasn't linked in any way to the root cause of the problem (i.e. the first ExceptionInInitializerError together with the chained exception that happened during the execution of the static initializer). With this change, the NoClassDefFoundError will now chain the initial ExceptionInInitializerError as cause, making it much easier to detect the problem which lead to the NoClassDefFoundError. Following is an example from the new JTreg tests which comes which this change to demonstrate the feature. Until know, a typical stack trace from a NoClassDefFoundError looked as follows: java.lang.NoClassDefFoundError: Could not initialize class NoClassDefFound$ClassWithFailedInitializer at java.base/java.lang.Class.forName0(Native Method) at java.base/java.lang.Class.forName(Class.java:291) at NoClassDefFound.main(NoClassDefFound.java:38) With this change, the same stack trace now looks as follows: java.lang.NoClassDefFoundError: Could not initialize class NoClassDefFound$ClassWithFailedInitializer at java.base/java.lang.Class.forName0(Native Method) at java.base/java.lang.Class.forName(Class.java:315) at NoClassDefFound.main(NoClassDefFound.java:38) Caused by: java.lang.ExceptionInInitializerError at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method) at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62) at java.base/jdk.internal.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45) at java.base/java.lang.reflect.Constructor.newInstance(Constructor.java:490) at java.base/java.lang.Class.newInstance(Class.java:584) at NoClassDefFound$ClassWithFailedInitializer.<clinit>(NoClassDefFound.java:20) at java.base/java.lang.Class.forName0(Native Method) at java.base/java.lang.Class.forName(Class.java:315) at NoClassDefFound.main(NoClassDefFound.java:30) Caused by: java.lang.ArrayIndexOutOfBoundsException: Index 2 out of bounds for length 1 at NoClassDefFound$A.<clinit>(NoClassDefFound.java:9) ... 9 more As you can see, the reason for the NoClassDefFoundError when accessing the class 'NoClassDefFound$ClassWithFailedInitializer' is actually not even in the class or its static initializer itself, but in the class 'NoClassDefFound$A' which is a base class of 'NoClassDefFound$ClassWithFailedInitializer'. This is not easily detectible from the old, plain NoClassDefFoundError. As I wrote, the only overhead we have with the new implementation is an additional OopHandle field per ClassLoaderData which I think is acceptable. The Hashtable object itself is only created lazily, after the first occurrence of an ExceptionInInitializerError in the corresponding class loader. The whole Hashtable creation and storing/quering of ExceptionInInitializerErrors in ClassLoaderData::record_init_exception()/ClassLoaderData::query_init_exception() is optional in the sense that any errors/exceptions occurring during the execution of these functions are ignored and cleared. Finally, we also take care to recursively convert all native backtraces in the stored ExceptionInInitializerErrors (and their suppressed and chained exceptions) into symbolic stack traces in order to avoid holding references to classes and prevent their unloading. This is implemented in the new private, static method java.lang.Throwable::removeNativeBacktrace() which is called for each ExceptionInInitializerError before it is stored in the Hashtable. Thank you and best regards, Volker [1] http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2018-June/028310…

4 10

RFR(L): 8202035: Archive the set of ModuleDescriptor and ModuleReference objects for system modules
by Jiangli Zhou 06 Jul '18

06 Jul '18

This is a follow-up RFE of JDK-8201650 (Move iteration order randomization of unmodifiable Set and Map to iterators), which was resolved to allow Set/Map objects being archived at CDS dump time (thanks Claes and Stuart Marks). In the current RFE, it archives the set of system ModuleReference and ModuleDescriptor objects (including their referenced objects) in 'open' archive heap region at CDS dump time. It allows reusing of the objects and bypassing the process of creating the system ModuleDescriptors and ModuleReferences at runtime for startup improvement. My preliminary measurements on linux-x64 showed ~5% startup improvement when running HelloWorld from -cp using archived module objects at runtime (without extra tuning). The library changes in the following webrev are contributed by Alan Bateman. Thanks Alan and Mandy for discussions and help. Thanks Karen, Lois and Ioi for discussion and suggestions on initialization ordering. The majority of the module object archiving code are in heapShared.hpp and heapShared.cpp. Thanks Coleen for pre-review and Eric Caspole for helping performance tests. webrev: http://cr.openjdk.java.net/~jiangli/8202035/webrev.00/ RFE: https://bugs.openjdk.java.net/browse/JDK-8202035?filter=14921 Tested using tier1 - tier6 via mach5 including all new test cases added in the webrev. Following are the details of system module archiving, which are duplicated in above bug report. --------------------------------------------------------------------------------------------------------------------------- Support archiving system module graph when the initial module is unnamed module from -cp currently. Support G1 GC, 64-bit (non-Windows). Requires UseCompressedOops and UseCompressedClassPointers. Dump time system module object archiving ================================= At dump time, the following fields in ArchivedModuleGraph are set to record the system module information created by ModuleBootstrap for archiving. private static SystemModules archivedSystemModules; private static ModuleFinder archivedSystemModuleFinder; private static String archivedMainModule; The archiving process starts from a given static field in ArchivedModuleGraph class instance (java mirror object). The process archives the complete network of java heap objects that are reachable directly or indirectly from the starting object by following references. 1. Starts from a given static field within the Class instance (java mirror). If the static field is a refererence field and points to a non-null java object, proceed to the next step. The static field and it's value is recorded and stored outside the archived mirror. 2. Archives the referenced java object. If an archived copy of the current object already exists, updates the pointer in the archived copy of the referencing object to point to the current archived object. Otherwise, proceed to the next step. 3. Follows all references within the current java object and recursively archive the sub-graph of objects starting from each reference encountered within the object. 4. Updates the pointer in the archived copy of referecing object to point to the current archived object. 5. The Klass of the current java object is added to a list of Klasses for loading and initializing before any object in the archived graph can be accessed at runtime. Runtime initialization from archived system module objects ============================================ VM.initializeFromArchive(<class>) is called from ArchivedModuleGraph's static initializer to initialize from the archived module information. Klasses in the recorded list are loaded, linked and initialized. The static fields in ArchivedModuleGraph class instance are initialized using the archived field values. After initialization, the archived system module objects can be used directly. If the archived java heap data is not successfully mapped at runtime, or there is an error during VM.initializeFromArchive(), then all static fields in ArchivedModuleGraph are not initialized. In that case, system ModuleDescriptor and ModuleReference objects are created as normal. In non-CDS mode, VM.initializeFromArchive(<class>) returns immediately with minimum added overhead for normal execution. Thanks, Jiangli

5 10

RFR 8206123 : ArrayDeque created with default constructor can only hold 15 elements
by Ivan Gerasimov 05 Jul '18

05 Jul '18

Hello! The spec states that an ArrayDeque created with the default constructor should be able to hold 16 elements. https://docs.oracle.com/javase/10/docs/api/java/util/ArrayDeque.html#%3Cini… """ Constructs an empty array deque with an initial capacity sufficient to hold 16 elements. """ In the constructor an array of size 16 is created: elements = new Object[16]; However, one element must always be null: /** * The array in which the elements of the deque are stored. * All array cells not holding deque elements are always null. * The array always has at least one null slot (at tail). */ transient Object[] elements; which leaves us space for only 15 elements, contrarily to what the spec promises. Would you please help review a trivial fix? BUGURL: https://bugs.openjdk.java.net/browse/JDK-8206123 WEBREV: http://cr.openjdk.java.net/~igerasim/8206123/00/webrev/ Thanks in advance! -- With kind regards, Ivan Gerasimov

4 9

[JDK 11] RFR 8187069: The case auto failed with the "java.lang.ClassNotFoundException: IPv6NameserverPlatformParsingTest" exception
by Chris Yin 04 Jul '18

04 Jul '18

Please review below one line change for manual test case com/sun/jndi/dns/Test6991580.java to build test class automatically which will be used in manual steps, thanks bug: https://bugs.openjdk.java.net/browse/JDK-8187069 <https://bugs.openjdk.java.net/browse/JDK-8187069> Review change as below: diff -r 2d3e99a72541 test/jdk/com/sun/jndi/dns/Test6991580.java --- a/test/jdk/com/sun/jndi/dns/Test6991580.java Wed Jun 27 17:02:41 2018 -0700 +++ b/test/jdk/com/sun/jndi/dns/Test6991580.java Thu Jun 28 09:50:37 2018 +0800 @@ -37,6 +37,7 @@ * @summary IPv6 Nameservers in resolv.conf throws NumberFormatException * @modules java.desktop * jdk.naming.dns/com.sun.jndi.dns + * @build IPv6NameserverPlatformParsingTest * @run main/manual Test6991580 */ Regards, Chris

3 6

ArrayDeque and List
by Jonas Konrad 30 Jun '18

30 Jun '18

Hey, The introduction of ArrayDeque in Java 1.6 has basically replaced LinkedList as the "default" implementation of java.util.Queue, which is probably for the best. However, from time to time, I still see use cases where a class that implements both Deque and List is wanted for ease of use. LinkedList is currently the only class that supports this. One example would be sorting the queue in-place. Has there been any discussion on having ArrayDeque implement List? There is no *technical* reason against doing so - the internal structure of ArrayDeque is fully capable of supporting an implementation of List, and there would be no additional fields needed that would change current memory behavior. The existing Deque interface would also remain unaffected. I see two possible reasons against doing this: - Added maintenance effort, a lot of it duplicated from ArrayList. This *could* be done gradually: ArrayDeque only extends AbstractCollection right now so implementations for e.g. subList could be inherited from AbstractList. Performance tweaking would be nice in many places though. - Design - conceptually, a Deque is something very different from a List. LinkedList implementing both could be seen as a mistake from that point of view. When coding to interfaces, which is good practice anyway, this change isn't visible though. Thoughts? - Jonas Konrad

2 1

ImmutableCollections.MapN optimisation ?
by Remi Forax 30 Jun '18

30 Jun '18

Playing with the value types, i've remarked that ImmutableCollections.MapN is missing an override for getOrDefault which making get() 2 times faster than getOrDefault on my test. The other problem is more subtle, get() uses probe() and probe() uses an infinite loop and not a counted loop, the result is that c2 generates lot of assembly codes for probe() as a consequence it doesn't inline probe() in get() making get() really slow compared to HashMap.get() which is fully inlined. There are several wys to fix that: - you can try to transform the while(true) loop in probe into 2 subsequents loops from idx to length + from 0 to idx. - you can inline the code of probe() into get() and even peel the first loop body evaluation as HashMap.get() does. - you can do both. - your idea here :) Also, i wonder if it's not better to make Map1 to also take cares of the zero case instead of making MapN to manage that case, because it's trading a size check with a null check and usually nullcheck can be removed by the VM. regards, Rémi

3 3

RFR(JDK12/JAXP/java.xml) 8190835: Subtraction with two javax.xml.datatype.Duration gives incorrect result
by Joe Wang 29 Jun '18

29 Jun '18

Hi, Please review a quick fix for the first of JDK12/JAXP. JBS: https://bugs.openjdk.java.net/browse/JDK-8190835 webrevs: http://cr.openjdk.java.net/~joehw/jdk12/8190835/webrev/ Thanks, Joe

2 2

RFR: 8177275: IllegalArgumentException when MH would have too many parameters is not specified for several methods
by Vivek Theeyarath 28 Jun '18

28 Jun '18

Hi All, Please review fix for https://bugs.openjdk.java.net/browse/JDK-8177275 . The jtreg test runs fine with the changes. http://cr.openjdk.java.net/~vtheeyarath/8177275/webrev.02/ CSR : https://bugs.openjdk.java.net/browse/JDK-8205917 Regards Vivek

2 1

RE: RFR: 8179887 - Build failure with glibc >= 2.24: error: 'int readdir_r(DIR*, dirent*, dirent**)' is deprecated
by Pengfei Li 28 Jun '18

28 Jun '18

Hi I have tried the patch ( http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-May/052991.html ) on Ubuntu 18.04 machines (x86_64 & aarch64) with glibc=2.26.1 and build is OK. There's a small issue within the following code in src/java.base/unix/native/libnio/fs/UnixNativeDispatcher.c Unlike readdir_r(), readdir() does not return int value. The value of res is always 0 before #ifdef. 727 /* EINTR not listed as a possible error */ 728 errno = 0; 729 ptr = readdir64(dirp); 730 res = errno; 731 732 #ifdef _AIX 733 /* On AIX, readdir() returns EBADF (i.e. '9') and sets 'result' to NULL for the */ 734 /* directory stream end. Otherwise, 'errno' will contain the error code. */ 735 if (res != 0) { 736 res = (ptr == NULL && res == EBADF) ? 0 : errno; 737 } 738 #endif May I know that if this core-libs change going to be merged recently, or more platforms needs to be explored? -- Thanks, Pengfei > On May 7, 2018, at 11:20 AM, B. Blaser <bsrbnd(a)gmail.com> wrote: > > On 7 May 2018 at 14:19, B. Blaser <bsrbnd(a)gmail.com> wrote: >> On 6 May 2018 at 18:35, B. Blaser <bsrbnd(a)gmail.com> wrote: >>> On 5 May 2018 at 22:26, Kim Barrett <kim.barrett(a)oracle.com> wrote: >>>>> On May 5, 2018, at 8:03 AM, B. Blaser <bsrbnd(a)gmail.com> wrote: >>>>> >>>>> On 4 May 2018 at 17:42, Alan Bateman <Alan.Bateman(a)oracle.com> wrote: >>>>>> On 04/05/2018 16:31, B. Blaser wrote: >>>>>>> >>>>>>> : >>>>>>> >>>>>>> I'll create a new JBS issue (unless you want to) since the fix >>>>>>> is mostly located in core-libs and only intended to make the >>>>>>> build complete. >>>>>>> But I'll still need someone to review and push the fix, would >>>>>>> you be interested in doing this? >>>>>>> >>>>>> I think someone needs to explore the behavior on other platforms >>>>>> too as the #ifndef __linux__ patches are ugly. >>>>> >>>>> Yes sure but I cannot test it elsewhere myself... and we can >>>>> easily remove them once POSIX officially deprecates 'readdir_r' or >>>>> if someone validates the fix on other systems. >>>> >>>> I'm not sure anyone has the ability to test on all supported. That >>>> doesn't (and really can't) prevent making changes across platforms >>>> to platform-specific code. It just means one needs to get help >>>> with testing. Make the change across platforms, test on those >>>> platforms one has access to, and ask here for help testing on others. >>> >>> OK, I'll provide a cross-platform fix to be evaluated on other systems too. >> >> Here it is, tier1 is successful on Linux with glibc=2.26. >> Any feedback on other systems would be very helpful. > > Some more cleanup…

3 4

RFR(XXS): 8205916: [test] Fix jdk/tools/launcher/RunpathTest to handle both, RPATH and RUNPATH
by Volker Simonis 28 Jun '18

28 Jun '18

Hi, can I please have a review for the following tiny test fix (I'm actually not sure which would be the appropriate mailing list for this fix so please redirect if necessary): http://cr.openjdk.java.net/~simonis/webrevs/2018/8205916/ https://bugs.openjdk.java.net/browse/JDK-8205916 The test currently only checks for RPATH in the dynamic section of the launcher, but some linkers / Linux distributions (notably SLES) use RUNPATH instead. Following are the gory details: The test jdk/tools/launcher/RunpathTest.java checks that the java launcher on Linux and Solaris has the correct RPATH path baked into the executable. Unfortunately, the situation with RPATH is a little weird: - in order to bake a runtime path into a dynamically linked library or executable one has to use the "-rpath <PATH>" linker option (from the C/C++ compiler this is usually available as "-Wl,-rpath,<path>"). - depending on the dynamic linker version and Linux distribution, the "-rpath" linker option adds either a "RPATH" entry (Ubuntu 16.04, RHEL 7.4) or a "RUNPATH" entry (SLES 12.1, SLES 15) or both entries together (SLES 11.3) to the dynamic section of the shared library/executable. - the semantics of "RPATH" and "RUNPATH" are slightly different: "RPATH" is evaluated at runtime before LD_LIBRARY_PATH (if "RUNPATH" isn't present) while "RUNPATH" is evaluated after LD_LIBRARY_PATH (i.e. RUNPATH can be overridden at runtime by setting LD_LIBRARY_PATH). - "RPATH" is considered obsolete and should be replaced by "RUNPATH" according to the man-page of "ld.so (8)". - the linker option "--enable-new-dtags"/"--disable-new-dtags" (or the corresponding compiler flags "-Wl,--enable-new-dtags"/"-Wl,--disable-new-dtags") can be used to enforce the generation of "RUNPATH"/"RPATH" respectively (except for systems like SLES 11.3 where "--enable-new-dtags" generates both "RPATH" and "RUNPATH" while "--disable-new-dtags" only generates "RPATH"). But this issue is not about fixing the build so to cut a long story short - the test RunpathTest.java should be able to handle both runtime path variants equally well. This can be easily achieved by extending the match pattern from ".*RPATH.*\\$ORIGIN/../lib.*" to ".*R(UN)?PATH.*\\$ORIGIN/../lib.*" Thank you and best regards, Volker

3 3