On Thu, 5 Jun 2025 09:31:39 GMT, Erik Gahlin <egah...@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
