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

[Vladimir Kozlov]

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.