RFR: JDK-8232069: Enable CDS even when UseCompressedClassPointers and/or UseCompressedOops are false

[Yumin Qi]

HI, Ioi

   Thanks, will remove the ArrayList.


Yumin

On 2/6/20 4:09 PM, Ioi Lam wrote:
> Hi Yumin,
>
> Looks good! Just one small nit:
>
>  190                 ArrayList<String> testArgs = new 
> ArrayList<String>();
>  191                 testArgs
>  192                     .add("-cp");
>  193                 testArgs
>  194                     .add(helloJar);
>  195                 testArgs
>  196                     .add("-Xlog:cds");
>  197                 testArgs
>  198 .add(getCompressedOopsArg(c.useCompressedOops));
>  199                 testArgs
>  200 .add(getCompressedClassPointersArg(c.useCompressedClassPointers));
>  201                 testArgs
>  202                     .add("Hello");
>  203                 out = TestCommon.exec(helloJar, 
> testArgs.toArray(new String[testArgs.size()]));
>
>
> This can be simplified to
>
> out = TestCommon.exec(helloJar, "-cp", helloJar, "-Xlog:cds",
>                       getCompressedOopsArg(c.useCompressedOops),
> getCompressedClassPointersArg(c.useCompressedClassPointers),
>                       "Hello");
>
> (No need for new webrev).
>
> Thanks
> - Ioi
>
>
> On 2/6/20 2:07 PM, Yumin Qi wrote:
>> Hi, Ioi and all
>>
>>
>>   Forget to mention tests: hs-tier1 - 4 passed.
>>
>>
>> Thanks
>>
>> Yumin
>>
>> On 2/6/20 2:04 PM, Yumin Qi wrote:
>>> Hi, Ioi
>>>
>>>
>>>   Thanks for the review. See answers embedded.
>>>
>>>    New URL: http://cr.openjdk.java.net/~minqi/8232069/webrev-01/
>>>
>>> On 2/5/20 10:34 AM, Ioi Lam wrote:
>>>> Hi Yumin,
>>>>
>>>>
>>>> Looks good. Some nitpicks on the tests:
>>>>
>>>> CommandLineFlagComboNegative.java: the following line is not needed.
>>>>
>>>> import jdk.test.lib.cds.CDSTestUtils.Result;
>>>>
>>> Removed.
>>>>
>>>> TestCombinedCompressedFlags.java: DUMP_NORMAL_MSG doesn't seem to 
>>>> be used by the test.
>>>>
>>> Removed.
>>>
>>> Also changed to use TestCommon.dump/TestCommon.exec which better to 
>>> check dum/run results.
>>>
>>>> DifferentHeapSizes.java: after your change, 
>>>> CDSTestUtils.MSG_COMPRESSION_MUST_BE_USED is no longer used 
>>>> anywhere in the tests. Also, This message is no longer printed by 
>>>> the VM. I think the CDSTestUtils.MSG_COMPRESSION_MUST_BE_USED field 
>>>> should be removed.
>>>>
>>>>
>>> Removed.
>>>> IncompatibleOptions.java: COOPS_DUMP_WARNING can be removed.
>>>>
>>> Removed.
>>>> The check_shared parameter for testExec() should be removed. It's 
>>>> currently used only when expectedToFail == false. However, in all 
>>>> the cases where expectedToFail == false, check_shared is always true.
>>>>
>>>>
>>> Removed.
>>>
>>>> Also, in filemap.cpp, before checking "compressed_oops() != 
>>>> UseCompressedOops ...", I think it will be good to add a log to 
>>>> print out the expected compressed mode. That way if the user gets 
>>>> an error, they can run with -Xlog:cds to find out exactly what the 
>>>> reason is. E.g.
>>>>
>>>>     log_info(cds)("Archive was created with UseCompressedOops = %d, 
>>>> UseCompressedClassPointers = %d",
>>>>                           compressed_oops(), 
>>>> compressed_class_pointers());
>>>>     if (compressed_oops() != UseCompressedOops ...
>>>>
>>> Added.
>>>
>>>
>>> Thanks
>>>
>>> Yumin
>>>
>>>>
>>>> On 2/4/20 6:01 PM, Yumin Qi wrote:
>>>>> Hi,
>>>>>
>>>>>    Please review fix:
>>>>>
>>>>>    Bug: https://bugs.openjdk.java.net/browse/JDK-8232069
>>>>>
>>>>> Webrev:http://cr.openjdk.java.net/~minqi/8232069/webrev/
>>>>>
>>>>>     Description: Currently CDS only works when using compressed 
>>>>> Oops turned on, -XX:+UseCompressedOops (default), the fix is 
>>>>> allowing CDS works with uncompressed oops and uncompressed class 
>>>>> pointers. The contents of InstanceKlass::_fields are depending on 
>>>>> UseCompressedOops and UseCompressedClassPointers on/off. There are 
>>>>> three suggestions as described in bug description:
>>>>>
>>>>> 1. Remember the compression mode at CDS dump time. At runtime, 
>>>>> disable CDS if the compression mode is changed.
>>>>> 2. Same as 1, but save more than one CDS archives in the JDK 
>>>>> image. Choose the appropriate one at start-up
>>>>> 3. Recompute re-compute InstanceKlass::_fields at runtime.
>>>>>
>>>>> The fix takes the first one, storing UseCompressedOops and 
>>>>> UseCompressedClassPointers in shared archive. CDS is only allowed 
>>>>> at runtime if the flags are consistent with the stored versions in 
>>>>> archive.
>>>>>
>>>>>  Testing: hs-tier1-2
>>>>>
>>>>>
>>>>>   Thanks
>>>>>
>>>>>   Yumin
>>>>>
>>>>
>