RFR (XS) JDK-8153582 Logging of ConcGCThreads is done too early

[Dmitry Fazunenko]

On 18.05.2016 17:24, Joseph Provino wrote:
> Latest webrev with Dima's suggestions:
>
> http://cr.openjdk.java.net/~jprovino/8153582/webrev.02

Looks good!

Thanks
Dima


>
> joe
>
> On 5/18/2016 7:33 AM, Dmitry Fazunenko wrote:
>> Hi Joe,
>>
>> > @requires vm.gc=="ConcMarkSweep" | vm.gc=="G1" | vm.gc=="null"
>>
>> Will not help unfortunately :(
>>
>> This test will fail on minimal VM where serial is always selected
>> You need:
>>    @requires vm.flavor != "minimal"
>>
>>
>> The way you spawn a new VM ignores external flags:
>>
>>     pb = ProcessTools.createJavaProcessBuilder("-XX:+" + gcFlag, 
>> "-Xlog:gc=debug", "-version");
>>
>> It's recommended to NOT ignore external flags:
>>     pb = ProcessTools.createJavaProcessBuilder(*true, *"-XX:+" + 
>> gcFlag, "-Xlog:gc=debug", "-version");
>>
>> But in this case the test will not work if either G1 or CMS is set 
>> explicitly:
>>    verifyNumberOfConcurrentAndParallelGCThreads(null);
>>    verifyNumberOfConcurrentAndParallelGCThreads("UseG1GC");
>> verifyNumberOfConcurrentAndParallelGCThreads("UseConcMarkSweepGC");
>> either second or third case will fail because of collector conflict.
>> To avoid collector conflicts you need:
>>    @requires vm.gc=="null"
>> But in this case,
>>    verifyNumberOfConcurrentAndParallelGCThreads(null);
>> will fail if ergonomics will select Serial or Parallel...
>>
>>
>> What I suggest
>>
>> add:
>>    @requires vm.flavor != "minimal"
>>    @requires vm.gc=="null"
>>
>> get rid of the null case
>>    40 verifyNumberOfConcurrentAndParallelGCThreads(null);
>>    41
>>    42 verifyNumberOfConcurrentAndParallelGCThreads("UseG1GC");
>>    43
>>    44 
>> verifyNumberOfConcurrentAndParallelGCThreads("UseConcMarkSweepGC");
>>
>> ->
>> verifyNumberOfConcurrentAndParallelGCThreads("UseG1GC");
>> verifyNumberOfConcurrentAndParallelGCThreads("UseConcMarkSweepGC");
>>
>>
>> don't ignore external flags:
>>    50         if (gcFlag != null) {
>>    51             pb = ProcessTools.createJavaProcessBuilder("-XX:+" 
>> + gcFlag, "-Xlog:gc=debug", "-version");
>>    52         } else {
>>    53             pb = 
>> ProcessTools.createJavaProcessBuilder("-Xlog:gc=debug", "-version");
>>    54         }
>> -->
>>               pb = ProcessTools.createJavaProcessBuilder(*true*, 
>> "-XX:+" + gcFlag, "-Xlog:gc=debug", "-version");
>>   Thanks,
>> Dima
>>
>>
>> On 18.05.2016 11:12, Thomas Schatzl wrote:
>>> Hi again,
>>>
>>> On Wed, 2016-05-18 at 10:09 +0200, Thomas Schatzl wrote:
>>>> Hi Joe,
>>>>
>>>> On Tue, 2016-05-17 at 15:17 -0400, Joseph Provino wrote:
>>>>> I changed the log level to debug and added created a test.
>>>>>
>>>>> Webrev:http://cr.openjdk.java.net/~jprovino/8153582/webrev.01
>>>>>
>>>>    in the test, please limit the allowable GCs to CMS and G1. I do not
>>>> think it makes sense to call the test with other GCs.
>>>>
>>>> Like
>>>>
>>>> @requires vm.gc=="ConcMarkSweep" | vm.gc=="G1" | vm.gc=="null"
>>> Also the copyright date should be 2016 only.
>>>
>>>> I do not need a re-review for this change.
>>> Still applies.
>>>
>>> Thanks,
>>>    Thomas
>>
>