RFR: 8170297: runtime/SharedArchiveFile/LargeSharedSpace.java didn't run out of memory

[Jiangli Zhou]

Hi David and Ioi,

Here is the updated test:

  http://cr.openjdk.java.net/~jiangli/8170297/webrev.01/ <http://cr.openjdk.java.net/~jiangli/8170297/webrev.01/>

I’ve split the test cases and reworked the comments. The original bug (8169870) was found on 32-bit platform, so it is worth to test -XX:SharedMiscCodeSize=1600386047 on 32-bit. Tested on linux-64, linux-32 and aarch64. Also running RBT.

Thanks,
Jiangli

> On Nov 28, 2016, at 2:57 PM, Jiangli Zhou <jiangli.zhou at Oracle.COM> wrote:
> 
> Hi David and Ioi,
> 
> Thank you so much for the comments!
> 
> For the compressed klass limit, it is determined by "shared space size + compressed klass space size". I agree, the test case and comments are not well defined for that. I’ll update the test to incorporate your suggestions.
> 
> Thanks,
> Jiangli
> 
>> On Nov 28, 2016, at 2:47 PM, Ioi Lam <ioi.lam at oracle.com> wrote:
>> 
>> 
>> 
>> On 11/28/16 2:16 PM, David Holmes wrote:
>>> Hi Jiangli,
>>> 
>>> On 29/11/2016 7:52 AM, Jiangli Zhou wrote:
>>>> Please review the following fix for test bug JDK-8170297 <https://bugs.openjdk.java.net/browse/JDK-8170297>.
>>>> 
>>>> http://cr.openjdk.java.net/~jiangli/8170297/webrev.00/ <http://cr.openjdk.java.net/~jiangli/8170297/webrev.00/>
>>>> 
>>>> On some test platforms, the archive might be dumped successfully with the specified large size. Added an extra level of checking in the test. Tested with linux-64, linux-32 and aarch64.
>>> 
>>> I have a problem with this test. :) The test says "the size is too large, exceeding the limit for compressed klass support" - so how can it pass? Is there a limit or not? If this size is too small to fail should we be using a bigger size? Does it need to be dynamic based on the host machine? Do we need to calculate a size based on available memory?
>>> 
>> 
>> Maybe the comments should spell out the exact same size of the compress klass
>> size limit, in both hex and decimal forms? Otherwise it will be hard to know
>> what 1066924031 would actually lead to.
>> 
>> I would also suggest splitting the test into two different portions, instead
>> of using the loop. It's hard to understand how the 3 output message would related
>> to the 2 input numbers.I think a test case should be clear about exactly what it
>> intends to test, and not just be "here's bunch of input parameters and it
>> should just work".
>> 
>> 
>> E.g, for the second case (1600386047):
>> 
>>   "Loading classes to share" should not happen on (64-bit platforms && compressed
>>   pointers.)
>> 
>>   In this case (1600386047), since the intention is to test the effect on
>>   the compress klass size limit, you should explicitly pass -XX:+UseCompressedOops
>>   -XX:+UseCompressedClassPointersto the end of your command-line, so that
>>   they would override any VM options specified via JTREG.
>> 
>> Also, LargeSharedSpace.java can be run on 32-bit platforms, but the
>> message " larger than compressed klass limit" only happens in LP64 code.
>> So I think the second test (1600386047) should be excluded from 32-bit platforms
>> by using Platform.is32bit().
>> 
>> #ifdef _LP64
>>   if (cds_total + compressed_class_space_size() > UnscaledClassSpaceMax) {
>>     vm_exit_during_initialization("Unable to dump shared archive.",
>>         err_msg("Size of archive (" SIZE_FORMAT ") + compressed class space ("
>>                 SIZE_FORMAT ") == total (" SIZE_FORMAT ") is larger than compressed "
>>                 "klass limit: " UINT64_FORMAT, cds_total, compressed_class_space_size(),
>>                 cds_total + compressed_class_space_size(), UnscaledClassSpaceMax));
>>   }
>> ...
>> 
>> Thanks
>> - Ioi
>> 
>> 
>>> Style nit: you added a few // comments but this test is using /* ... */ comments. The mix looks odd to me especially when comments run into each other.
>>> 
>>> Thanks,
>>> David
>>> 
>>>> Thanks,
>>>> Jiangli
>>>> 
>> 
>