On Thu, 5 Jun 2025 08:40:44 GMT, Erik Gahlin <egah...@openjdk.org> wrote:
>> Could I have review of an enhancement that adds rate-limited sampling to
>> Java events, 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 with a new target base due to a
> merge or a rebase. The incremental webrev excludes the unrelated changes
> brought in by the merge/rebase. The pull request contains nine additional
> commits since the last revision:
>
> - Merge remote-tracking branch 'upstream/master' into throttle-mania-3
> - Merge remote-tracking branch 'upstream/master' into throttle-mania-3
> - Merge remote-tracking branch 'upstream/master' into throttle-mania-3
> - Fix adjust boundary
> - Some reviewer feedback
> - Consistent annotation
> - Fix typos
> - Fix whitespace
> - Initial
src/java.base/share/classes/java/io/RandomAccessFile.java line 594:
> 592: } finally {
> 593: long end = FileWriteEvent.timestamp();
> 594: long duration = end - start;
Suggestion:
long duration = end - start;
src/jdk.jfr/share/classes/jdk/jfr/internal/EventInstrumentation.java line 65:
> 63: public final class EventInstrumentation {
> 64: public static final long MASK_THROTTLE = 1 << 62;
> 65: public static final long MASK_THROTTLE_CHECK = 1 << 63;
Let's align
Suggestion:
public static final long MASK_THROTTLE = 1 << 62;
public static final long MASK_THROTTLE_CHECK = 1 << 63;
src/jdk.jfr/share/classes/jdk/jfr/internal/EventInstrumentation.java line 593:
> 591: if (throttled) {
> 592: codeBuilder.aload(0);
> 593: getfield(codeBuilder, eventClassDesc,
> ImplicitFields.FIELD_DURATION);
Suggestion:
getfield(codeBuilder, eventClassDesc,
ImplicitFields.FIELD_DURATION);
test/jdk/jdk/jfr/api/recording/settings/TestSettingsAvailability.java line 93:
> 91: testSetting(EventNames.JVMInformation, "enabled", "period");
> 92: testSetting(EventNames.FileRead, "enabled", "threshold",
> "stackTrace", "throttle");
> 93: testSetting(EventNames.FileWrite, "enabled",
> "threshold","stackTrace", "throttle");
Suggestion:
testSetting(EventNames.FileWrite, "enabled", "threshold", "stackTrace",
"throttle");
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/25559#discussion_r2128322876
PR Review Comment: https://git.openjdk.org/jdk/pull/25559#discussion_r2128321445
PR Review Comment: https://git.openjdk.org/jdk/pull/25559#discussion_r2128322474
PR Review Comment: https://git.openjdk.org/jdk/pull/25559#discussion_r2128323708
