On Fri, 29 Apr 2022 20:57:01 GMT, Erik Gahlin <[email protected]> wrote:
>> test/jdk/jdk/jfr/api/consumer/TestManyRecordings.java line 57:
>>
>>> 55: int classLoaderCount = Integer.parseInt(args[0]);
>>> 56: int classCount = Integer.parseInt(args[1]);
>>> 57: for (int i = 0; i <classCount; i++) {
>>
>> Did you mean classLoaderCount here instead of classCount? Also, please make
>> sure there is a space between < and classLoaderCount.
>
> The JFR "tests" look strange. I would expect a test called TestManyRecording
> to start recordings, not create a lot of classes, similar to TestManyClasses.
> How is this related to Loom? Could this be a merge gone bad?
>
> Also, in TestActiveSettingEvent.java
>
> +settingValues.put(EventNames.VirtualThreadPinned + "#threshold", "20 ms");
>
> The reason to exclude a setting (threshold or stackTrace) from a .jfc file is
> if it doesn't make sense to configure. For example, if the event is always
> instantaneous (so duration is always 0) or periodic (so the stack trace only
> show JFR internals) then "threshold" and "stackTrace" can be removed from the
> configuration file, but needs to be added to test to avoid false positive.
>
> The value "20 ms" seems like something a user might want to configure. If the
> event is instant, then the value should be "0 ms".
It seems that test/jdk/jdk/jfr/api/consumer/TestManyClasses.java,
TestManyRecordings.java, and TestParse.java were added for another JFR event
(nothing to do with VirtualThreadPinned). @egahlin has contributed some cleanup
and these files are removed.
-------------
PR: https://git.openjdk.java.net/jdk/pull/8166
