RFR(XS): 8240691: serviceability/sa/ClhsdbCDSJstackPrintAll.java and serviceability/sa/ClhsdbCDSCore.java not work with ZGC

[Yumin Qi]

Hi, Ioi
   Thanks for clearing the whole things.
   Since it is a trivial fix, I will change the synopsis of the bug at 
push using your updated version: ClhsdbCDSJstackPrintAll incorrectly 
thinks CDS is in use

    Let me know if it is OK for push.

Thanks
Yumin

On 3/10/20 11:11 AM, Ioi Lam wrote:
>
>
> On 3/10/20 10:26 AM, Chris Plummer wrote:
>> Ok, that makes sense. I committed a change recently that issues an 
>> "echo true" command so all commands that follow are echoed, which is 
>> why you now see "verbose true" in the output. So this resulted in it 
>> always thinking that shared spaces was enabled, even when not. But 
>> this did not cause any difference in behavior I think because of 
>> JDK-8240698 (I'm still not real clear on that).
>>
>> And after fixing this issue and JDK-8240698 we will then start to see 
>> issues because ZGC does not support SA, and therefore need a 3rd bug 
>> to fix that?
>>
>
> Hi Chris,
>
>
> I think we need to fix things in the following order. The goal is to 
> increase test coverage for CDS.
>
>
>
> [1] 8240691 - (I renamed the bug to) "ClhsdbCDSJstackPrintAll 
> incorrectly thinks CDS is in use")
>
> After the above is fixed, we still won't run ClhsdbCDSJstackPrintAll 
> for jtreg -vmoption:-XX:+UseZGC yet, because this test has "@requires 
> vm.cds", which reports false when ZGC is selected.
>
>
>
> [2] 8240563 - WB_IsCDSIncludedInVmBuild should support uncompressed 
> oops/klasses
>
> After the above is fixed, we can run a lot of *other* CDS tests with 
> CDS+ZGC.
>
> However, for ClhsdbCDSJstackPrintAll, when jtreg -vmoption:-XX:+UseZGC 
> is used
>
> + the test dumps a CDS archive with ZGC enabled (use compressed oops = 
> false)
>
> + the test tries to use the CDS archive without enabling ZGC (due to 
> 8240698), so compressed oops = true. As a result, the VM cannot load 
> this CDS archive because the oop compression mode doesn't match.
>
>
>
> [3] 8240698 - LingeredApp did not pass getTestJavaOpts() to the 
> children process if vmArguments is already specified
>
> After the above is fixed, ClhsdbCDSJstackPrintAll will dump and run 
> with the archive with -XX+UseZGC, so we can test the behavior of SA 
> when ZGC is enabled.
>
>
>
> But as Chris said, SA doesn't support ZGC yet (JDK-8220624), so we 
> probably don't need to fix 8240698 until SA supports ZGC. As a 
> precaution, we should disable this test in ProblemList-zgc.txt as a 
> part of 8240563.
>
> Also, I think the behavior of 8240698 may be intentional -- in many 
> cases, especially when with "jtreg -vmoption:-Xcomp", you probably 
> don't want the LingerApp to run with -Xcomp. So we should have an API 
> in LingerApp.startApp() to explicitly allow the passing of 
> getTestJavaOpts() to the child process.
>
> Thanks
> - Ioi
>
>
>> Chris
>>
>> On 3/10/20 9:38 AM, Ioi Lam wrote:
>>> Hi Chris,
>>>
>>>
>>> The test uses this to check if CDS is on:
>>>
>>>     List<String> cmds = List.of("flags UseSharedSpaces");
>>>     String useSharedSpacesOutput = test.run(theApp.getPid(), cmds,
>>>                                                     null, null);
>>>     ....
>>>     if (!useSharedSpacesOutput.contains("true")) {
>>>         // CDS archive is not mapped. Skip the rest of the test.
>>>
>>> The output looks like this:
>>>
>>>     hsdb> hsdb> + verbose true
>>>     hsdb> + flags UseSharedSpaces
>>>     UseSharedSpaces = false command line
>>>     hsdb> + quit
>>>
>>> I think at one point the "if" worked, but the "+ verbose true" part 
>>> of the log must have been added recently to cause the "if" to 
>>> determine the wrong result.
>>>
>>> Thanks
>>> - Ioi
>>>
>>>
>>>
>>> On 3/9/20 9:33 PM, Chris Plummer wrote:
>>>> Hi Yumin,
>>>>
>>>> While your fix is a more bullet proof way of checking if CDS is on, 
>>>> what was broken with the original version (was there a stray 
>>>> "false" in the output"), and what does this have to do with ZGC?
>>>>
>>>> thanks,
>>>>
>>>> Chris
>>>>
>>>> On 3/9/20 8:06 PM, Yumin Qi wrote:
>>>>>
>>>>> Hi,
>>>>>
>>>>>     I made a change after discussed with Ioi, the test should 
>>>>> check against UseSharedSpaces instead, it passed with/without 
>>>>> change of 8240563 (tested in local). Also passed tier1,2,3.
>>>>>
>>>>>      URL: http://cr.openjdk.java.net/~minqi/8240691/webrev/
>>>>>
>>>>>
>>>>>     Thanks
>>>>>
>>>>>      Yumin
>>>>>
>>>>>
>>>>> On 3/6/20 2:26 PM, Yumin Qi wrote:
>>>>>> HI,
>>>>>>
>>>>>>   Can I have your reviews on
>>>>>>
>>>>>>   Bug: https://bugs.openjdk.java.net/browse/JDK-8240691
>>>>>>
>>>>>>   webrev: http://cr.openjdk.java.net/~minqi/8240691/webrev/
>>>>>>
>>>>>>   Summary: With JDK-8232069 
>>>>>> (https://bugs.openjdk.java.net/browse/JDK-8232069), CDS works 
>>>>>> with UseCompressedOop and UseCompressedClassPointers off. The 
>>>>>> code for detecting CDS will report false with ZGC since ZGC 
>>>>>> turned off those two flags. The detection code will be changed in 
>>>>>> JDK-8240563(https://bugs.openjdk.java.net/browse/JDK-8240563), 
>>>>>> this will cause those two tests fail.
>>>>>>
>>>>>>
>>>>>>    Tests: jtreg to show the tests not selected with ZGC.
>>>>>>
>>>>>>    Thanks
>>>>>>
>>>>>>    Yumin
>>>>>>
>>>>
>>>
>>
>