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");
If I understand, this will just test with -XX:+UseG1GC and
-XX:+UseConcMarkSweepGC
but will not test with default GC which is also G1.
Thanks.
joe
>
> 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
>