On Fri, 29 Apr 2022 20:57:01 GMT, Erik Gahlin <egah...@openjdk.org> 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

Reply via email to