RFR: 8351594: JFR: Rate-limited sampling of Java events [v3]

[Alan Bateman]

On Tue, 3 Jun 2025 12:50:49 GMT, Erik Gahlin <egahlin at 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 one additional commit since the last revision:
> 
>   Fix adjust boundary

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

> 968:             long end = SocketReadEvent.timestamp();
> 969:             long duration = end - start;
> 970:             if (SocketReadEvent.shouldThrottleCommit(duration, end)) {

The use sites want to ask if an event should be committed. Does the word "Throttle" need to be in name as I don't think the use cases need to care about this.

Also,  just to point out that the shouldXXX method is called with the the end timestamp and duration whereas the offset/emit/commit methods are called with the start timestamp and duration. So two long timestamps and a long measure of time, easy to get it wrong somewhere. Maybe not this PR but I think would be clearer at the use sites to use start or end consistently and reduce potential for mistakes.

src/jdk.jfr/share/classes/jdk/jfr/Throttle.java line 77:

> 75:      * Specifying {@code "off"} (case-sensitive) results in all events being emitted.
> 76:      *
> 77:      * @return the throttle value, default {@code "off"} not {@code null}

I think it would be clear to drop "not null" from the return description.

src/jdk.jfr/share/conf/jfr/default.jfc line 762:

> 760:       <setting name="enabled">true</setting>
> 761:       <setting name="stackTrace">true</setting>
> 762:       <setting name="threshold">20 ms</setting>

I agree FileForce isn't a good candidate to throttle.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/25559#discussion_r2126765474
PR Review Comment: https://git.openjdk.org/jdk/pull/25559#discussion_r2126722810
PR Review Comment: https://git.openjdk.org/jdk/pull/25559#discussion_r2126719134