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

[Alan Bateman]

On Thu, 5 Jun 2025 09:31:39 GMT, Erik Gahlin <egahlin at openjdk.org> wrote:

>> I think we (from both the java.base and jdk.jfr perspectives) need to keep an eye on the complexity of the use sites. The new throttling stuff requires a new local variable. By itself this isn't a big deal, but there are 12 events being updated here. In addition, each requires a start, end, and duration, and clearly duration = end - start. These are all long values, and the different calls require different long values, so sooner or later somebody is going to get this wrong.
>> 
>> To a certain extent we can do more cleanup on the java.base side, by using things like SocketReadEvent's offer() method instead of its emit() method. Unfortunately only one site uses offer() -- NIO SocketChannelImpl -- and note that it didn't need to be updated! The other events should have something like the offer() method, which groups together the testing of shouldCommit/shouldThrottleCommit with the committing of the event. (This also should avoid recalculating the duration, but really, this is only a subtraction of two longs, so it should take only one cycle.)
>> 
>> But note what we're doing here is constructing an internal API within java.base, between the use sites (like java.net.Socket) and the java.base-side JFR stuff (jdk.internal.event.SocketReadEvent). Maybe after things are cleaned up and consolidated here we can see if the API between jdk.internal.event and jdk.jfr can be improved.
>
> I updated FileRead and FileWrite to use an offer method like SocketRead and SocketWrite, reducing boilerplate.

Thank you for this update, it looks much better now. I chatted briefly with Stuart yesterday and mostly agreed there would need to be some follow-up cleanup of the use-sites. Doing in now is good as it addresses our grumblings.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/25559#discussion_r2128410969