RFR: 8214484: ZGC: Exclude SA tests ClhsdbJhisto and TestHeapDumpFor*

[Per Liden]

On 2018-11-29 17:40, Aleksey Shipilev wrote:
> On 11/29/18 5:26 PM, Per Liden wrote:
>> Hi,
>>
>> On 11/29/18 4:48 PM, Aleksey Shipilev wrote:
>>> On 11/29/18 4:34 PM, Per Liden wrote:
>>>> On 11/29/18 4:17 PM, Aleksey Shipilev wrote:
>>>>> On 11/29/18 4:07 PM, Per Liden wrote:
>>>>>> There's no support for SA heap walking when using ZGC. Hence the tests for this should not execute
>>>>>> when ZGC is enabled.
>>>>>>
>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8214484
>>>>>> Webrev: http://cr.openjdk.java.net/~pliden/8214484/webrev.0
>>>>>
>>>>> Looks good.
>>>>>
>>>>> Stylistic: why not "!vm.gc.Z"?
>>>>
>>>> That has a slightly different meaning. vm.gc.Z will be true if -XX:+UseZGC is an "acceptable"
>>>> option. For example, ZGC is supported by the build and no other GC was explicitly chosen. vm.gc will
>>>> be "Z" if the test was explicitly executed with -XX:+UseZGC.
>>>>
>>>> So if we used !vm.gc.Z on a build with ZGC support, then the test would never execute, unless you
>>>> also explicitly specified what GC to use.
>>>>
>>>> Yeah, I it's messy... I hope I got the details right there... ;)
>>>
>>> Mmm. That's not what we see with TestFullGCCount test, for example, which has pre-existing check for
>>> CMS, and our new check for Shenandoah:
>>>
>>>    * @requires !(vm.gc.ConcMarkSweep & vm.opt.ExplicitGCInvokesConcurrent == true)
>>>    * @requires !(vm.gc.Shenandoah & vm.opt.ExplicitGCInvokesConcurrent == true)
>>
>> Those @requires tags look wrong to me. With the above, the test will not be executed if you _only_
>> supply -XX:+ExplicitGCInvokesConcurrent (and use the default GC). It will work as intended if you
>> instead use:
>>
>>   @requires !(vm.gc == "ConcMarkSweep" & vm.opt.ExplicitGCInvokesConcurrent)
>>   @requires !(vm.gc == "Shenandoah" & vm.opt.ExplicitGCInvokesConcurrent)
>>
>> Right?
> 
> Damn. Right. What a mess! Your current patch looks good then.

Thanks for reviewing!

Btw, I don't think anyone would object if someone proposed a better and 
less confusing scheme to deal with the vm.gc stuff.

/Per