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

[Alan Bateman]

On Wed, 28 Jun 2023 18:53:12 GMT, Tim Prinzing <tprinzing 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
>
> Tim Prinzing has updated the pull request incrementally with one additional commit since the last revision:
> 
>   less exception filtering when fetching socket read timeout

Overall, it's good to move away from the "faraway instrumentation". Looks forward to the next steps to get to a more complete solution.

src/java.base/share/classes/java/net/Socket.java line 44:

> 42: import java.util.Collections;
> 43: 
> 44: 

Minor nit, I assume this additional blank line is left over from a previous iteration.

src/java.base/share/classes/sun/nio/ch/SocketChannelImpl.java line 465:

> 463:     public long read(ByteBuffer[] dsts, int offset, int length)
> 464:         throws IOException
> 465:     {

The supporting methods for read (beginRead, endRead, throwConnectionReset, ...) are declared before the read methods. It's probably best to put the implRead before the read too so that all the supporting methods are together. Same thing with the write methods.

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

Marked as reviewed by alanb (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/14342#pullrequestreview-1588627786
PR Review Comment: https://git.openjdk.org/jdk/pull/14342#discussion_r1301134829
PR Review Comment: https://git.openjdk.org/jdk/pull/14342#discussion_r1301144638