On Thu, 22 Jun 2023 10:21:46 GMT, Alan Bateman <al...@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/sun/nio/ch/SocketChannelImpl.java line 408:
> 
>> 406:     @Override
>> 407:     public int read(ByteBuffer buf) throws IOException {
>> 408:         if (!SocketReadEvent.enabled()) {
> 
> The read/write with sun.nio.ch.SocketInputStream and SocketOutputStream does 
> not go through SC.read/write so I think SocketAdaptor read/write will need 
> attention, maybe a future PR as there are other code paths that aren't 
> covered in this PR.

I've created https://bugs.openjdk.org/browse/JDK-8310978 to drive the future PR 
to support the missing code paths

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

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

Reply via email to