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

[Ioi Lam]

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
>>>>
>>>