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

[Erik Gahlin]

On Mon, 2 Jun 2025 08:59:57 GMT, Volkan Yazici <vyazici at openjdk.org> wrote:

>> Erik Gahlin has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Some reviewer feedback
>
> src/jdk.jfr/share/classes/jdk/jfr/internal/settings/Throttler.java line 30:
> 
>> 28: import java.util.concurrent.locks.ReentrantLock;
>> 29: import jdk.jfr.internal.PlatformEventType;
>> 30: public final class Throttler {
> 
> **Disclaimer:** I am a passer-by and I don't possess a deep JFR experience.
> 
> Throttling (aka. rate limiting) is a pretty much non-trivial functionality. Shall we
> 
> - Revamp the JavaDoc and document the used algorithm?
> - Have a dedicated test for `Throttler`? (I see that the newly added `TestThrottle` tests `@Throttle`, hence it also covers `Throttler`. Though there I could not see any verification based on the time[stamp] of events.)
> 
> ### References
> 
> - Resilience4j:
>   - [AtomicRateLimiter](https://github.com/resilience4j/resilience4j/blob/master/resilience4j-ratelimiter/src/main/java/io/github/resilience4j/ratelimiter/internal/AtomicRateLimiter.java)
>   - [AtomicRateLimiterTest](https://github.com/resilience4j/resilience4j/blob/master/resilience4j-ratelimiter/src/test/java/io/github/resilience4j/ratelimiter/internal/AtomicRateLimiterTest.java)
> - Guava:
>   - [RateLimiter](https://github.com/google/guava/blob/master/guava/src/com/google/common/util/concurrent/RateLimiter.java)
>   - [RateLimiterTest](https://github.com/google/guava/blob/master/guava-tests/test/com/google/common/util/concurrent/RateLimiterTest.java)

I don't think we want to write the algorithm in the specification. It would make it harder for us to make adjustments in the future. The contract is x events per time unit without details on how JFR adapts the sampling. The algorithm is the same one used by the jdk.ObjectAllocationSample event, which has been in use since JDK 16, but implemented in native code and with a gtest:
https://github.com/openjdk/jdk/blob/master/src/hotspot/share/jfr/support/jfrAdaptiveSampler.cpp
 
The code in the PR is a method per method port of that code. It's hard to write stable tests that include timestamps. We have tried that in the past for other settings/events, but it only resulted in false positives.

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

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