RFR(XS): 8065050: vm crashes during CDS dump when very small SharedMiscDataSize is specified

[Jiangli Zhou]

Hi Calvin,

On 12/04/2014 10:55 PM, Calvin Cheung wrote:
> On 12/4/2014 8:08 PM, David Holmes wrote:
>> On 5/12/2014 12:40 PM, Calvin Cheung wrote:
>>> Hi David,
>>>
>>> On 12/4/2014 3:14 PM, David Holmes wrote:
>>>> On 4/12/2014 11:40 AM, Calvin Cheung wrote:
>>>>> Hi Jiangli,
>>>>>
>>>>> I've updated the webrev at the same location:
>>>>>      http://cr.openjdk.java.net/~ccheung/8065050/webrev/
>>>>
>>>> Seems okay. Do we have a test that sets the flags to these allowed
>>>> minimum values and dumps and then uses the archive?
>>>
>>> I've added more test scenarios to the testcase.
>>> Updated webrev is at the same location.
>>
>> These values:
>>
>> !                     value = Platform.is64bit() ? "10M" : "8M";
>> !                     break;
>> !                 case RW:
>> !                     value = Platform.is64bit() ? "13M" : "7M";
>>
>> should match these:
>>
>>      min_ro_size = NOT_LP64(7*M) LP64_ONLY(9*M),
>>      min_rw_size = NOT_LP64(6*M) LP64_ONLY(12*M)
>
> For 64-bit, I've changed the testcase to match with the definitions.
> For 32-bit, I've changed the definitions to match with the testcase.
> Otherwise, the test fails with "not enough space".
> It may have something to do with the following calculation:
>       SharedReadOnlySize  = align_size_up(SharedReadOnlySize, 
> max_alignment);
>       SharedReadWriteSize = align_size_up(SharedReadWriteSize, 
> max_alignment);

That's strange. Why would the test fail when using 7M/6M? The minimum 
aligned sizes should be less than those value, if the status output 
reports the correct values. Is there any bug that causes the sizes of 
the reported memory usage wrong?

Thanks,
Jiangli

>
> I've updated the webrev at the same location.
>
> thanks,
> Calvin
>>
>> David
>> -----
>>
>>> thanks,
>>> Calvin
>>>
>>>
>>>>
>>>> Thanks,
>>>> David
>>>>
>>>>> Previous version is saved at:
>>>>> http://cr.openjdk.java.net/~ccheung/8065050/webrev.00/
>>>>>
>>>>> thanks,
>>>>> Calvin
>>>>>
>>>>> On 12/3/2014 3:53 PM, Jiangli Zhou wrote:
>>>>>> Hi Calvin,
>>>>>>
>>>>>> On 12/03/2014 03:51 PM, Calvin Cheung wrote:
>>>>>>> On 12/3/2014 2:55 PM, Jiangli Zhou wrote:
>>>>>>>> Hi Calvin,
>>>>>>>>
>>>>>>>> It's better to define 12M and 16M as enums in metaspaceShared.hpp
>>>>>>>> now they are referenced in more than one place.
>>>>>>> So global.hpp will need to include metaspaceShared.hpp.
>>>>>>>>
>>>>>>>> I also have some questions. The 12M/16M are not introduced by this
>>>>>>>> change, do you know why those values were chosen as the default RO
>>>>>>>> and RW sizes?
>>>>>>> Sorry. I don't know the reasons why those values were chosen.
>>>>>>>> Now we require both spaces have to be at lease 12M on 32-bit
>>>>>>>> machines and 16M on 64-bit machine, is it a reasonable 
>>>>>>>> requirement?
>>>>>>>> What's the minimum size requirement for the RO and RW spaces with
>>>>>>>> the default classlist?
>>>>>>>
>>>>>>> Below are the numbers for the RO and RW spaces with the default
>>>>>>> classlist for various platforms:
>>>>>>> =====
>>>>>>> linux
>>>>>>> =====
>>>>>>> 64-bit
>>>>>>> ro space:   8433480 [ 37.8% of total] out of  16777216 bytes [50.3%
>>>>>>> used] at 0x0000000800000000
>>>>>>> rw space:  11418608 [ 51.1% of total] out of  16777216 bytes [68.1%
>>>>>>> used] at 0x0000000801000000
>>>>>>>
>>>>>>> 32-bit
>>>>>>> ro space:   6316488 [ 48.9% of total] out of  12582912 bytes [50.2%
>>>>>>> used] at 0x80000000
>>>>>>> rw space:   5794312 [ 44.9% of total] out of  12582912 bytes [46.0%
>>>>>>> used] at 0x80c00000
>>>>>>>
>>>>>>> ========
>>>>>>> windows
>>>>>>> ========
>>>>>>> 64-bit
>>>>>>> ro space:   7888680 [ 37.7% of total] out of  16777216 bytes [47.0%
>>>>>>> used] at 0x0000000800000000
>>>>>>> rw space:  10704496 [ 51.1% of total] out of  16777216 bytes [63.8%
>>>>>>> used] at 0x0000000801000000
>>>>>>>
>>>>>>> 32-bit
>>>>>>> ro space:   6030640 [ 49.3% of total] out of  12582912 bytes [47.9%
>>>>>>> used] at 0x14690000
>>>>>>> rw space:   5440904 [ 44.5% of total] out of  12582912 bytes [43.2%
>>>>>>> used] at 0x15290000
>>>>>>>
>>>>>>> ====
>>>>>>> mac
>>>>>>> ====
>>>>>>> 64-bit
>>>>>>> ro space:   6798968 [ 37.0% of total] out of  16777216 bytes [40.5%
>>>>>>> used] at 0x0000000800000000
>>>>>>> rw space:   9446240 [ 51.4% of total] out of  16777216 bytes [56.3%
>>>>>>> used] at 0x0000000801000000
>>>>>>>
>>>>>>> ====
>>>>>>>
>>>>>>> So maybe we can define some enums as follows and leave the default
>>>>>>> values in globals.hpp alone?
>>>>>>>
>>>>>>> min_ro_size  NOT_LP64(7*M) LP64_ONLY(9*M)
>>>>>>> min_rw_size  NOT_LP64(6*M) LP64_ONLY(12*M)
>>>>>>
>>>>>> Sounds good to me.
>>>>>>
>>>>>> Thanks,
>>>>>> Jiangli
>>>>>>
>>>>>>>
>>>>>>> thanks,
>>>>>>> Calvin
>>>>>>>
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Jiangli
>>>>>>>>
>>>>>>>> On 12/01/2014 12:10 PM, Calvin Cheung wrote:
>>>>>>>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8065050
>>>>>>>>>
>>>>>>>>> Adding more checks on the SharedMiscDataSize, ShareReadOnlySize,
>>>>>>>>> and SharedReadWriteSize.
>>>>>>>>>
>>>>>>>>> For the SharedMiscDataSize, it is based on
>>>>>>>>> MetaspaceShared::generate_vtable_methods(). Similar to what was
>>>>>>>>> done for the SharedMiscCodeSize.
>>>>>>>>>
>>>>>>>>> For the ShareReadOnlySize and SharedReadWriteSize, I'm 
>>>>>>>>> checking if
>>>>>>>>> they are at least the default size.
>>>>>>>>> I think it's reasonable to enforce the ro and rw sizes to be at
>>>>>>>>> least the default size. A default dump of CDS archive requires 
>>>>>>>>> >8M
>>>>>>>>> of ro space and >11M of rw space.
>>>>>>>>>
>>>>>>>>> webrev:
>>>>>>>>> http://cr.openjdk.java.net/~ccheung/8065050/webrev/
>>>>>>>>>
>>>>>>>>> tests:
>>>>>>>>>     ran the testcase via jtreg on linux_x64 and windows_x64
>>>>>>>>>     JPRT
>>>>>>>>>
>>>>>>>>> thanks,
>>>>>>>>> Calvin
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>
>