On Sun, 1 Jun 2025 20:56:50 GMT, Erik Gahlin <egah...@openjdk.org> wrote:
>> Could I have review of an enhancement that adds rate-limited sampling to
>> Java event, 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:
>
> 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)
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/25559#discussion_r2120496703
