RFR: 8310978: JFR events SocketReadEvent/SocketWriteEvent for Socket adaptor ops [v2]

[Erik Gahlin]

On Tue, 24 Oct 2023 06:50:22 GMT, Alan Bateman <alanb at openjdk.org> wrote:

>> Tim Prinzing has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   requested change to location of socket adaptor event logic
>>   
>>   bring back event offer semantics where it was fine.
>
> src/java.base/share/classes/sun/nio/ch/SocketInputStream.java line 82:
> 
>> 80:         long start = SocketReadEvent.timestamp();
>> 81:         int n = implRead(b, off, len);
>> 82:         SocketReadEvent.offer(start, n, sc.remoteAddress(), timeoutSupplier.getAsInt());
> 
> One of the things left over from the original refactoring was to dig into why the event needs the timeout, it would be nice if that could go away.
> 
> For now, I'd prefer if the timeout could be captured once as otherwise there is no guarantee that the event will be reported with the same timeout used for the socket read. In other words, add a timeout parameter to implRead so that the method calls implRead with the timeout, avoids calling timeoutSuppler::getAsInt twice.

I think SocketRead.emit() have several advantages:

- No need execute remoteAddress() when the threshold don't exceed 20 ms
- Avoid the synchronized block in SocketChannelImpl::remoteAddress().
- The end time is taken more close to start time.
- One less frame for JFR to walk

The disadvantage is some code repetition, but it's not that much.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16251#discussion_r1369960907