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

[Calvin Cheung]

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

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