On Thu, 5 Jun 2025 09:37:45 GMT, Erik Gahlin <egah...@openjdk.org> wrote:
>> Could I have review of an enhancement that adds rate-limited sampling to
>> Java events, including five events in the JDK (SocketRead, SocketWrite,
>> FileRead, FileWrite, and JavaExceptionThrow).
>>
>> Testing: test/jdk/jdk/jfr
>>
>> Thanks
>> Erik
>
> Erik Gahlin has updated the pull request incrementally with two additional
> commits since the last revision:
>
> - Remove the mistakenly added file.
> - Fix whitespace
src/java.base/share/classes/java/io/FileInputStream.java line 219:
> 217: }
> 218: } finally {
> 219: FileReadEvent.offer(start, path, bytesRead);
Thanks for this update, this looks much better. It think would be better again
if `start = FileReadEvent.timestamp()` is moved to before the try.
src/java.base/share/classes/sun/nio/ch/FileChannelImpl.java line 328:
> 326: } finally {
> 327: long end = FileReadEvent.timestamp();
> 328: FileReadEvent.offer(start, path, bytesRead);
I don't think "end" is needed now.
src/java.base/share/classes/sun/nio/ch/FileChannelImpl.java line 382:
> 380: FileWriteEvent.offer(start, path, bytes);
> 381: }
> 382: return bytes;
The method returns the number of bytes written and traceImplRead uses bytesRead
for the number of bytes read. So I think better to leave it as bytesWritten.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/25559#discussion_r2128400058
PR Review Comment: https://git.openjdk.org/jdk/pull/25559#discussion_r2128401565
PR Review Comment: https://git.openjdk.org/jdk/pull/25559#discussion_r2128406420
