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

Alan Bateman alanb at openjdk.org
Tue Aug 22 07:41:38 UTC 2023


On Wed, 28 Jun 2023 06:09:14 GMT, Alan Bateman <alanb at openjdk.org> wrote:

>> Tim Prinzing has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains ten commits:
>> 
>>  - remove unused SOCKET_READ and SOCKET_WRITE configurations.
>>  - Merge branch 'master' into JDK-8308995
>>    
>>    # Conflicts:
>>    #	src/jdk.jfr/share/classes/jdk/jfr/events/EventConfigurations.java
>>  - Avoid exceptions getting address/timeout for jfr event. Remove unused
>>    EventConiguration fields SOCKET_READ and SOCKET_WRITE.  Remove spurious
>>    whitespace.
>>  - some changes from review.
>>    
>>    read0() to implRead()
>>    write0() to implWrite()
>>    trailing whitespace
>>  - fix copyright date
>>  - Added micro benchmark to measure socket event overhead.
>>  - Some changes from review.
>>    
>>    Append a 0 to method names being wrapped.  Use getHostString to avoid
>>    a reverse lookup when fetching the hostname of the remote address.
>>  - remove unnecessary cast
>>  - 8308995: Update Network IO JFR events to be static mirror events
>
> src/java.base/share/classes/java/net/Socket.java line 1133:
> 
>> 1131:                 return parent.getSoTimeout();
>> 1132:             } catch (Throwable t) {
>> 1133:                 // ignored - avoiding exceptions in jfr event data gathering
> 
> This should be SocketException, not Throwable. That said, I think it would be useful to know why the SocketReadEvent includes the timeout. Is this used to see If read durations are close to the timeout? I assume once this code is fixed to deal with the exceptional case that the need to include the timeout for the success case will mostly go away.

Were you able to find out what the timeout is used for?

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

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


More information about the nio-dev mailing list