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

[David Holmes]

On 5/12/2014 4: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".

Precisely what needed to be verified!

Looks okay to me now.

Thanks,
David

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