RFR: 8365611: Use lookup table for JfrEventThrottler [v2]

[Robert Toyonaga]

On Mon, 18 Aug 2025 13:41:51 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>> This patch prepares the way to throttle any JFR event, not only the three events for which we hard-code throttlers. It also cleans up the throttler code and replaces the switch construct with a lookup table.
>> 
>> The patch does not yet enable throttling for any other event, since that may be a contentious move, and I would like for this patch to go in without too much fuss. So I did not make any new events "throttleble".
>> 
>> If it turns out to be non-contentious, I am fine with just enabling throttling capability for all events.
>
> Thomas Stuefe has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Markus Feedback

I think that the reasoning behind this change makes sense (to avoid the risk of high overhead for a Unsafe.allocateMemory event). I agree there is also immediate benefit in generalizing the throttler code rather than hardcoding for specific event types.

I've left a couple minor comments below:

src/hotspot/share/jfr/recorder/service/jfrEventThrottler.cpp line 103:

> 101: JfrEventThrottler* JfrEventThrottler::for_event(JfrEventId event_id) {
> 102:   JfrEventThrottler* const throttler = _throttler_table.at(event_id);
> 103:   assert(throttler != nullptr, "Event type %d has an unconfigured throttler", (int)event_id);

The call to configure throttlers originates from up in the Java level JFR code.  Would it be possible to attempt sampling when a throttler is created, but not yet configured? 

If this is possible, then it was possible before this PR too (not introduced here).

src/hotspot/share/jfr/recorder/service/jfrEventThrottler.cpp line 109:

> 107: void JfrEventThrottler::configure(JfrEventId event_id, int64_t sample_size, int64_t period_ms) {
> 108:   JfrEventThrottler* const throttler = _throttler_table.at(event_id);
> 109:   assert(throttler != nullptr, "Event type %d has an unconfigured throttler", (int)event_id);

Small nitpick: it's actually just checking that the throttler exists. We configure it on the next line.
If it doesn't make sense to make this distinction, feel free to ignore this.

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

Marked as reviewed by roberttoyonaga at github.com (no known OpenJDK username).

PR Review: https://git.openjdk.org/jdk/pull/26801#pullrequestreview-3137462451
PR Review Comment: https://git.openjdk.org/jdk/pull/26801#discussion_r2288759661
PR Review Comment: https://git.openjdk.org/jdk/pull/26801#discussion_r2288747936