February 2019 - core-libs-dev

RFR: JDK-8217751: jpackage messages and failures
by Andy Herrick 01 Feb '19

01 Feb '19

Please review the jpackage fix for bug [1] at [2]. This is a fix for the JDK-8200758-branch branch of the open sandbox repository (jpackage). RFR: JDK-8217751: jpackage messages and failures This is a fix to Error and Warning processing and Error message content. [1] JDK-8217751 <https://bugs.openjdk.java.net/browse/JDK-8217751> [2] http://cr.openjdk.java.net/~herrick/8217751/webrev.01/

2 1

RFR 8218195: [testlibrary] Diagnostic output for test library ProcessTools
by Roger Riggs 01 Feb '19

01 Feb '19

Please review the addition of diagnostic output to the test library ProcessTools.executeProcess to aid in diagnosing several unexplained timeouts. It adds a second check for process being aliev using ProcessHandle and prints information about the current process and its descendents and all processes. Webrev: http://cr.openjdk.java.net/~rriggs/webrev-pt-alive-patch-1/ Thanks, Roger

3 3

Re: [RFR]: Per thread IO statistics in JFR
by Thomas Stüfe 01 Feb '19

01 Feb '19

Hi Gunter, (I remove svc but add corelibs and hs-runtime for the jdk/hs parts, respectively. Hope this is not too spammy). This looks all very good. I am not a JFR reviewer (is there such a thing?) but only a JDK reviewer, so I am not sure if I am allowed to review the JFR parts. That said, here it goes: General remarks: A) we may reduce the number of sent events vastly by only sending updates for threads which actually did IO. I know that would require expanding the ThreadStatisticalInfo to keep JFR-specific counters. But I think the memory overhead of the increased size of ThreadStatisticalInfo would be more than offset by the space saved for noop events. What to you think? (Side note, it would be nice to have generic support for this in the JFR layer. If the JFR layer could remember the last values for an event and omit sending it if nothing changed.) B) You could save a bit of work by folding multiple values into one event. Other events seem to do that too, eg. MetaspaceSummary. E.g: instead of having ThreadNetworkWriteStatistics and ThreadNetworkReadStatistics, you could have ThreadNetworkStatistics with "read" and "write" fields, and thus combine the polling functions for those two. Maybe even combine all four values into a single "ThreadIOStatistics" event. Review: Hook functions: - This is bike shedding, but I would like something like "JVM_On<xxx>()" more, sounding more "hookish", since the functions do not "xxx" but inform the hotspot of "xxx" having occurred - Also, I wonder whether it would make sense to make the len input 64bit (jlong instead of jint) in case we want to use them to instrument 64bit reads too - If you wanted to minimize the number of new JVM_.. hook functions, you could combine network and file io hooks into one: JVM_OnIORead(.. bool is_socket) JVM_OnIOWrite(.. bool is_socket) or even JVM_OnIO(.., bool is_read, bool is_socket). This would also make the dispatching in src/java.base/unix/native/libnio/ch/FileDispatcherImpl.c easier. But this is pure aesthetics, I leave it up to you. --- src/hotspot/share/jfr/metadata/metadata.xml All event definitions have one value, called and labeled "Thread". If you keep them separate, does this field name/label make sense? (I am not fluid with JMC so I do not know where exactly this label text would appear). --- src/hotspot/share/jfr/periodic/jfrPeriodic.cpp GrowableArray<jlong> written(initial_size); GrowableArray<traceid> thread_ids(initial_size); You could probably save one GrowableArray and some append calls by something like this: struct data { jlong num; trace_id tid; } GrowableArray<data> - Question, I assume you go thru the trouble of storing the values temporarily since you do not want to call JfrEvent::commit inside the Threads_lock? --- src/hotspot/share/runtime/thread.cpp Nice addition to the extended thread dump. --- src/java.base/unix/native/libnio/ch/FileDispatcherImpl.c - There may be a number of unnecessary casts: e.g. Java_sun_nio_ch_FileDispatcherImpl_read0 JVM_callNetworkReadBytes(env, (jint) readBytes); readBytes is already jint. - You could reduce the code by factoring out that frequent if-else statements: 87 if (readBytes > 0) { 88 if (socket) { 89 JVM_callNetworkReadBytes(env, (jint) readBytes); 90 } else { 91 JVM_callFileReadBytes(env, (jint)readBytes); 92 } 93 } like this static void on_io_read(env, jint nread, jboolean socket) { if (socket) { JVM_callNetworkReadBytes(env, nread); } else { JVM_callFileReadBytes(env, nread); } } you could even add the n>0 condition to the function. - Java_sun_nio_ch_FileDispatcherImpl_pwrite0: you truncate the return value of pread64 to 32bit on 64bit platforms. --- src/java.base/windows/native/libnet/DualStackPlainDatagramSocketImpl.c Probably philosophical but I would leave peek out of the condition. A read is a read. -- src/java.base/unix/native/libnet/SocketOutputStream.c You could move the call to JVM_callNetworkWriteBytes out of the loop (you did this in other places too). -- src/java.base/windows/native/libnio/ch/DatagramDispatcher.c src/java.base/windows/native/libnio/ch/FileDispatcherImpl.c at various places: 52 DWORD read = 0; 83 if (read > 0) { 84 JVM_callNetworkReadBytes(env, read); 85 } DWORD is unsigned, may overflow jint arg? Moot point if you make it 64bit. ---- src/java.base/windows/native/libnet/DualStackPlainDatagramSocketImpl.c Java_java_net_DualStackPlainDatagramSocketImpl_socketSend 462 if (rv > 0) { 463 JVM_callFileWriteBytes(env, rv); 464 } Should this be NetworkWriteBytes? --- src/java.base/windows/native/libnet/SocketOutputStream.c Same here. --- src/java.base/windows/native/libnet/TwoStacksPlainDatagramSocketImpl.c - if (sendto(fd, fullPacket, packetBufferLen, 0, addrp, + if (ret = sendto(fd, fullPacket, packetBufferLen, 0, addrp, addrlen) == SOCKET_ERROR) { NET_ThrowCurrent(env, "Datagram send failed"); } + if (ret > 0) { + JVM_callFileWriteBytes(env, ret); + } + Same here. --- http://cr.openjdk.java.net/~ghaug/webrevs/8216981/src/java.base/windows/nat… Java_sun_nio_ch_DatagramChannelImpl_send0 Smae here. --- src/java.base/windows/native/libnio/ch/DatagramDispatcher.c src/java.base/windows/native/libnio/ch/SocketDispatcher.c Same here? --- src/java.base/windows/native/libnio/ch/WindowsSelectorImpl.c + written = send(scoutFd, &byte, 1, 0); + + if (written > 0) { + JVM_callFileWriteBytes(env, written); + } ditto :) Thanks for your work! Cheers, Thomas On Mon, Jan 14, 2019 at 4:59 PM Haug, Gunter <gunter.haug(a)sap.com> wrote: > Hi All, > > Could I please have a review and possibly some opinions on the following > enhancement to JFR and the JDK? > > At SAP we have a per thread IO statistic among our supportability > enhancements which proved to be very helpful for our support engineers. It > might be beneficial for JFR as well and would certainly help us to drive > adoption of OpenJDK. > > The basic idea is simple, we have added fields to the thread class where > the number of bytes read and written from/to file and network are counted > in. The newly created JFR events are written periodically as for example > the ThreadAllocationStatistics event already is. > > In order to collect the data, we have added hooks to the JDK C coding > calling back into the VM. > > I have opened a bug here: > https://bugs.openjdk.java.net/browse/JDK-8216981 > > Here is a webrev: > http://cr.openjdk.java.net/~ghaug/webrevs/8216981 > > There are no tests yet and the code be a bit nicer in places. We will work > on this if/when this feature is deemed acceptable. > > Finally, we have an API in our SAP version of the JDK to access this data > from a Java application. This is used by many SAP applications and I think > we could add an MXBean in a second step, to provide similar functionality. > > Thanks in advance, > Gunter > > > > >

7 16

PreparedStatement.setBinaryStream Javadoc
by Bernd Eckenfels 01 Feb '19

01 Feb '19

Hello, The JavaDoc for some of the setBinary/ASCII... methods of PS say something about „more practical“ but it is not clear if it means this method is more practical than another method, or if this method should not be used in favor of another. When a very large binary value is input to a LONGVARBINARY parameter, it may be more practical to send it via a java.io.InputStream object. Also, does this not apply to BLOB or VARBINARY? I would clarify this to something like (if I understood the meaning correctly): For large content the use of an InputStream/Reader can avoid keeping the data in memory, It however depends on the driver how the data is internally processed or buffered. BTW those functions do not mention if the stream is closed or not. After the method returns the stream is not closed. Some more clarifications could also be given (not sure if all of them are correct): The stream is positioned after the specified length. The reading blocks till the end of stream is signaled (will not return when abailable()==0) EOFException is thrown If stream is shorter than specified length. Gruss Bernd -- http://bernd.eckenfels.net

1 0

[PATCH] test/jdk/java/rmi/transport/runtimeThreadInheritanceLeak/RuntimeThreadInheritanceLeak.java failed in JITed code
by Jie Fu 01 Feb '19

01 Feb '19

Hi all, My name is Jie Fu, a JVM developer at Loongson working on the MIPS port of OpenJDK. I hope this is the right place to discuss the issue. test/jdk/java/rmi/transport/runtimeThreadInheritanceLeak/RuntimeThreadInheritanceLeak.java failed with "java -Xcomp RuntimeThreadInheritanceLeak". The failure was caused by the lost of an OopMap item for "loaderRef" which was optimized out by the JIT with the liveness analysis optimization. For more details, see [1]. It seems that RuntimeThreadInheritanceLeak.java is only suitable for testing interpreters which is not adapted to JITs at all. Different behaviors performed by the interpreter and JITs with the same test case really confused us a lot. And I think it's worth making the test also suitable for JITs. A tiny change can fix this issue --------------------------------- diff -r 02e648ae46c3 test/jdk/java/rmi/transport/runtimeThreadInheritanceLeak/RuntimeThreadInheritanceLeak.java --- a/test/jdk/java/rmi/transport/runtimeThreadInheritanceLeak/RuntimeThreadInheritanceLeak.java Wed Jan 09 01:06:19 2019 +0100 +++ b/test/jdk/java/rmi/transport/runtimeThreadInheritanceLeak/RuntimeThreadInheritanceLeak.java Thu Jan 10 20:49:22 2019 +0800 @@ -106,7 +106,10 @@ * context class loader-- by giving it a chance to pass away. */ Thread.sleep(2000); - System.gc(); + while (loaderRef.get() != null) { + System.gc(); + Thread.sleep(100); + } System.err.println( "waiting to be notified of loader being weakly reachable..."); --------------------------------- Could you please review it and tell me if it's OK to file a bug on JBS. Thanks. [1] https://github.com/DamonFool/MyBlog/blob/master/JVM/RuntimeThreadInheritanc… Best Regards, Jie

5 26