Request for review: 8000968: NPG UseCompressedKlassPointers asserts withObjectAlignmentInBytes for > 32G Compressed Oops

[harold seigel]

Hi Vladimir,
Thanks for the explanation.

I added the HeapBaseMinAddress check condition to the 'if' statement in 
this new webrev: http://cr.openjdk.java.net/~hseigel/bug_8000968_5/ 
<http://cr.openjdk.java.net/%7Ehseigel/bug_8000968_5/>

Could you take one more look when you have a chance?

Thanks, Harold

On 1/22/2013 5:56 PM, Vladimir Kozlov wrote:
> On 1/22/13 2:38 PM, harold seigel wrote:
>> Hi Vladimir,
>>
>> Can you explain why the HeapBaseMinAddress check is needed, if the heap
>> is going to be based at (KlassEncodingMetaspaceMax -
>> Universe::class_metaspace_size()) ?
>>
>> Is it because the CDS region is based at HeapBaseMinAddress?
>
> HeapBaseMinAddress is usually set by default to the address below 
> which you can't reserve memory. For example, on sparc 64bit VM can't 
> reserve below 4Gb address and HeapBaseMinAddress=4g there.
>
> We have this check in the first condition:
>
> (total_size <= OopEncodingHeapMax) which is (heap_size + 
> HeapBaseMinAddress <= OopEncodingHeapMax)
>
> We need similar check if we want the base to be in 
> KlassEncodingMetaspaceMax.
>
> Vladimir
>
>>
>> Thanks, Harold
>>
>> On 1/22/2013 3:04 PM, Vladimir Kozlov wrote:
>>> Harold,
>>>
>>> New condition is not accurate since it does not take into account
>>> HeapBaseMinAddress. You need additional condition:
>>>
>>> +     } else if (UseCompressedKlassPointers && (mode !=
>>> HeapBasedNarrowOop) &&
>>> +         (Universe::class_metaspace_size() + HeapBaseMinAddress <=
>>> KlassEncodingMetaspaceMax) &&
>>> +         (KlassEncodingMetaspaceMax + heap_size -
>>> Universe::class_metaspace_size() <= OopEncodingHeapMax)) {
>>>
>>> Vladimir
>>>
>>> On 1/22/13 10:22 AM, harold seigel wrote:
>>>> HI Roland,
>>>>
>>>> Thanks again for your comments.  I incorporated them in this new 
>>>> webrev:
>>>> http://cr.openjdk.java.net/~hseigel/bug_8000968_4/
>>>> <http://cr.openjdk.java.net/%7Ehseigel/bug_8000968_4/>
>>>>
>>>> The only changes from the previous webrev are to module universe.cpp.
>>>>
>>>> Could you take another look when you have a chance?
>>>>
>>>> Thanks, Harold
>>>>
>>>> On 1/18/2013 6:44 AM, Roland Westrelin wrote:
>>>>> Hi Harold,
>>>>>
>>>>>> I updated the webrev at
>>>>>> http://cr.openjdk.java.net/~hseigel/bug_8000968_3/<http://cr.openjdk.java.net/%7Ehseigel/bug_8000968_3/> 
>>>>>>
>>>>>>
>>>>>> with this change.
>>>>>   696     } else if ((total_size<= OopEncodingHeapMax)&&  (mode !=
>>>>> HeapBasedNarrowOop)&&
>>>>>   697         (!UseCompressedKlassPointers ||
>>>>>   698           (((OopEncodingHeapMax - heap_size) +
>>>>> Universe::class_metaspace_size())<= KlassEncodingMetaspaceMax))) {
>>>>>
>>>>> heap_size<  OopEncodingHeapMax - KlassEncodingMetaspaceMax is
>>>>> possible, right? Then compressed klass pointers are off with this
>>>>> code. So wouldn't you also want to check for:
>>>>>
>>>>> KlassEncodingMetaspaceMax + heap_size -
>>>>> Universe::class_metaspace_size()<= OopEncodingHeapMax
>>>>>
>>>>> ?
>>>>>
>>>>> and then use KlassEncodingMetaspaceMax -
>>>>> Universe::class_metaspace_size() as base.
>>>>>
>>>>> Roland.