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

[Vladimir Kozlov]

Harold,

First, we are missing check in arguments.cpp that ClassMetaspaceSize < 
KlassPtrEncodingHeapMax. We can switch off compressed class pointers 
even with compressed oops because it worked before Roland pushed his fix 
for compressed class pointers.

Second, the right way to fix your problem, I think, is set 
Universe::_narrow_klass._base separately (it requires changing logic for 
loading base into register and other checks that the register has base). 
It is a lot more changes then this but right one.

You can hardcode KlassPtrEncodingHeapMax value in globalDefinitions.hpp 
since LogKlassAlignmentInBytes = 3 always:

   const int KlassAlignment           = KlassAlignmentInBytes / 
HeapWordSize;
+ const int KlassPtrEncodingHeapMax = (uint64_t(max_juint) + 1) << 
LogKlassAlignmentInBytes;

We don't use file static variables, pass aligned_metaspace_size to 
preferred_heap_base() as argument or make it Universe's field.

Instead of aligned_metaspace_size, I would call it class_metaspace_size.

The next check is incorrect:

class_metaspace_size + HeapBaseMinAddress <= KlassPtrEncodingHeapMax

should be (and add parenthesis):

((OopEncodingHeapMax - heap_size + class_metaspace_size) <= 
KlassPtrEncodingHeapMax)

because base = (OopEncodingHeapMax - heap_size)

Printing (verbose output) KlassPtrEncodingHeapMax is useless since it is 
always the same value. I would print klass metaspace start address instead.

Thanks,
Vladimir

On 1/11/13 11:13 AM, harold seigel wrote:
> Hi,
>
> Please review the following change to fix bug 8000968.
>
> Summary:
> The cause of this problem is that the compression mode for Oops and
> KlassPointers compression is determined using OopEncodingHeapMax, which
> is based on the alignment and shifting of CompressedOops.  When
> ObjectAlignmentInBytes=32, Oops pointers can be shifted by 5 bits.
> However, KlassPointers are still 8 byte aligned and can only be shifted
> by 3 bits.  Hence, a common compression mode that is calculated based on
> CompressedOop's 5 bit shift does not work for CompressedKlassPointers.
>
> This fix adds a new variable, KlassPtrEncodingHeapMax, which is based on
> the alignment and shifting of CompressedKlassPointers.  It then uses
> KlassPtrEncodingHeapMax, along with OopEncodingHeapMax, to determine
> which compression mode to use.  This means that a compression mode is
> selected only if it works for both Oops and KlassPointers.  Previously,
> only Oops were looked at.
>
> This was tested with JPRT, JCK vm and lang tests,  ute vm.quck.testlist,
> and hand testing.
>
> Openwebrev at http://cr.openjdk.java.net/~hseigel/bug_8000968/
> <http://cr.openjdk.java.net/%7Ehseigel/bug_8000968/>
>
> Bug link at http://bugs.sun.com/view_bug.do?bug_id=8000968
>
> Thanks!  Harold