RFR: 8308995: Update Network IO JFR events to be static mirror events

Alan Bateman alanb at openjdk.org
Thu Jun 22 10:25:11 UTC 2023


On Wed, 21 Jun 2023 15:48:30 GMT, Alan Bateman <alanb at openjdk.org> wrote:

>> The socket read/write JFR events currently use instrumentation of java.base code using templates in the jdk.jfr modules. This results in some java.base code residing in the jdk.jfr module which is undesirable.
>> 
>> JDK19 added static support for event classes. The old instrumentor classes should be replaced with mirror events using the static support.
>> 
>> In the java.base module:
>> Added two new events, jdk.internal.event.SocketReadEvent and jdk.internal.event.SocketWriteEvent.
>> java.net.Socket and sun.nio.ch.SocketChannelImpl were changed to make use of the new events.
>> 
>> In the jdk.jfr module:
>> jdk.jfr.events.SocketReadEvent and jdk.jfr.events.SocketWriteEvent were changed to be mirror events.
>> In the package jdk.jfr.internal.instrument, the classes SocketChannelImplInstrumentor, SocketInputStreamInstrumentor, and SocketOutputStreamInstrumentor were removed. The JDKEvents class was updated to reflect all of those changes.
>> 
>> The existing tests in test/jdk/jdk/jfr/event/io continue to pass with the new implementation:
>> Passed: jdk/jfr/event/io/TestSocketChannelEvents.java
>> Passed: jdk/jfr/event/io/TestSocketEvents.java
>> 
>> I added a micro benchmark which measures the overhead of handling the jfr socket events.
>> test/micro/org/openjdk/bench/java/net/SocketEventOverhead.java.
>> It needs access the jdk.internal.event package, which is done at runtime with annotations that add the extra arguments.
>> At compile time the build arguments had to be augmented in make/test/BuildMicrobenchmark.gmk
>
> src/java.base/share/classes/java/net/Socket.java line 1109:
> 
>> 1107:                 nbytes = read0(b, off, len);
>> 1108:             } finally {
>> 1109:                 SocketReadEvent.checkForCommit(start, nbytes, parent.getRemoteSocketAddress(), parent.getSoTimeout());
> 
> So if read throws, this will commit a jdk.SocketReadEvent with size 0, maybe this will change later to include the exception?

The other issue to think about here is where the Socket is asynchronously closed. In t hat case, implRead will throw but we'll end up with a confusing suppressed exception due to the call to getSoTimeout. I think this will have to be replaced with a call to a helper method that returns the timeout or 0.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/14342#discussion_r1238315000



More information about the security-dev mailing list