RFR: 8351594: JFR: Rate-limited sampling of Java events

[Markus Grönlund]

On Fri, 30 May 2025 22:30:25 GMT, Erik Gahlin <egahlin at 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

src/jdk.jfr/share/classes/jdk/jfr/internal/EventInstrumentation.java line 261:

> 259:             getfield(codeBuilder, eventClassDesc, ImplicitFields.FIELD_DURATION);
> 260:             Bytecode.invokevirtual(codeBuilder, TYPE_EVENT_CONFIGURATION, METHOD_THROTTLE);
> 261:             int result = codeBuilder.allocateLocal(TypeKind.LONG);

Do we really need to store the result in a local? Can't we just dup it on the expression stack and store it directly into the field after another aload, or dup? Perhaps dup twice to then issue the mask operation?

src/jdk.jfr/share/classes/jdk/jfr/internal/EventInstrumentation.java line 590:

> 588:     private void setDuration(CodeBuilder codeBuilder, Consumer<CodeBuilder> expression) {
> 589:         codeBuilder.aload(0);
> 590:         expression.accept(codeBuilder);

dont know what expression.accept() does, but does it really consume the this pointer? I see its pushed again (aload(0)) if its throttled below?

src/jdk.jfr/share/classes/jdk/jfr/internal/event/EventConfiguration.java line 59:

> 57:     // static boolean shouldThrottleCommit(long timestamp)
> 58:     public boolean shouldThrottleCommit(long timestamp) {
> 59:         return throttler.sample(timestamp);

Can we assert on isEnabled?

src/jdk.jfr/share/classes/jdk/jfr/internal/event/EventConfiguration.java line 77:

> 75:             return rawDuration;
> 76:         }
> 77:         long endTime = startTime + rawDuration;

I see now that you are using endTime as well, nice.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/25559#discussion_r2118272073
PR Review Comment: https://git.openjdk.org/jdk/pull/25559#discussion_r2118272829
PR Review Comment: https://git.openjdk.org/jdk/pull/25559#discussion_r2118277498
PR Review Comment: https://git.openjdk.org/jdk/pull/25559#discussion_r2118279057