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

Ioi Lam ioi.lam at oracle.com
Tue Nov 29 04:26:44 UTC 2016


Hi Jiangli,

The new changes look good. Thanks for taking my suggestion :-)

- Ioi


On 11/28/16 7:00 PM, Jiangli Zhou wrote:
> Hi David and Ioi,
>
> Here is the updated test:
>
> http://cr.openjdk.java.net/~jiangli/8170297/webrev.01/ 
> <http://cr.openjdk.java.net/%7Ejiangli/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 
>> <mailto: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 
>>> <mailto: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/%7Ejiangli/8170297/webrev.00/> 
>>>>> <http://cr.openjdk.java.net/~jiangli/8170297/webrev.00/ 
>>>>> <http://cr.openjdk.java.net/%7Ejiangli/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
>>>>>
>>>
>>
>



More information about the hotspot-runtime-dev mailing list