Request for review: 8003424 - Enable Class Data Sharing for CompressedOops

[Vladimir Kozlov]

On 7/18/13 3:03 PM, Vladimir Kozlov wrote:
> Nice clean up, Harold
>
> Could you add the size of class metaspace in output with
> PrintCompressedOopsMode (and with verbose). I don't want to remember an
> other very long flag name: TraceMetavirtualspaceAllocation.
>
> Also it would be nice to print these info line (and compressed oops info
> line) in hs_err files.
>
> About "effect(KILL cr);   /// ???? is this KILL needed?" in x86_64.ad.
> encode_klass_not_null() and decode_klass_not_null() use arithmetic
> instructions which modify flags: subq, addq, shrq, xorptr. Also note
> that code in .ad file does not check the encoding mode for narrow
> klass/oop so it should be conservative and assume that arithmetic
> instructions are used.
>
> If Universe::narrow_klass_shift() == 0 you have klass base below 4Gb so
> the value is 32 bit. You can use it directly in narrow klass
> encoding/decoding without destroying R12.

Correction. You can do it only if base < 2Gb max positive int (sign bit 
is extended so you will get wrong result with address > 2gb).

You definitely should not use R12 in decode_klass_not_null(Register dst, 
Register src) method where you have free 'dst' register.

About CompressedKlassPointersBase 0x800000000 (32*G). Actually it should 
depend on ObjectAlignmentInBytes. For =16 it should be 64Gb.
The idea was to avoid using R12 by using shifted base:

decode:
if (Universe::narrow_klass_base_shifted() != 0) {
   if (Universe::set_narrow_klass_shift() == 0) {
     shrq(r, LogKlassAlignmentInBytes);
   }
   addq(r, Universe::narrow_klass_base_shifted());
   shlq(r, LogKlassAlignmentInBytes);
}

Universe::narrow_klass_base_shifted() is set only if 
(Universe::narrow_klass_base >> LogKlassAlignmentInBytes) <= maxint (max 
positive int).

And you have general memory reservation problem. At least on Solaris, as 
I remember, OS will always try to keep protected 4Kb pages between 
different requested memory regions. That is why in jdk7 we first reserve 
one huge space and then cut it on java heap, perm gen and protection 
page below heap to catch implicit NULL exceptions with compressed oops.

So, please, verify that you are getting 'cds_address + cds_total' 
address or CompressedKlassPointersBase without CDS and with compressed 
oops and with Xmx20Gb.

>
> instr_size_for_decode_klass_not_null() is ugly but unfortunately we
> can't do other way. I assume you use max size because depending on
> registers used in instructions you will or will not get prefix byte
> (r8-r15).
>
> I will look on changes in more details may be late.

What 'vsm' stands for in using_class_vsm()? Don't use abbreviations 
which are not obvious.


>
> Thanks,
> Vladimir
>
> On 7/18/13 10:54 AM, harold seigel wrote:
>> Hi,
>>
>> Please review this fix for bug 8003424.
>>
>> Open webrev at http://cr.openjdk.java.net/~hseigel/bug_8003424/
>> <http://cr.openjdk.java.net/%7Ehseigel/bug_8003424/>
>> Open bug links at:
>>
>>     http://bugs.sun.com/view_bug.do?bug_id=8003424
>>     http://bugs.sun.com/view_bug.do?bug_id=8016729
>>     http://bugs.sun.com/view_bug.do?bug_id=8011610
>>
>> JBS Bug links at
>>
>>     https://jbs.oracle.com/bugs/browse/JDK-8003424
>>     https://jbs.oracle.com/bugs/browse/JDK-8016729
>>     https://jbs.oracle.com/bugs/browse/JDK-8011610
>>
>>
>> This fix provides support for using compressed klass pointers with CDS.
>> It also changes the class metaspace allocation on 64-bit platforms when
>> UseCompressedKlassPointers is true.  Instead of allocating the class
>> metaspace as part of the Java Heap allocation and locating it at the
>> base of that allocation, the metaspace will now be allocated separately,
>> above the Java heap.  This will enable future expansion of the metaspace
>> because it won't be backed up against the Java heap.  If CDS is being
>> used, then the CDS region will be allocated between the Java heap and
>> the metaspace.
>>
>> The new class metaspace allocation code tries to allocate memory at 32G,
>> or above the CDS region, if it is present.  Because of this, encoding
>> and decoding of compressed klass pointers will always require use of a
>> base register.  So, encoding and decoding of compressed klass pointers
>> will differ from compressed oops.
>>
>> There are no class metaspace allocation changes if
>> UseCompressedKlassPointers is turned off or if running on a 32-bit
>> platform.
>>
>> The code changes also include some cleanup and will fix bugs 8016729,
>> 8011610, and 8003424.
>>
>>
>> Many of the modules in this webrev contain a lot of changes. Modules
>> macroAssembler_sparc.cpp and macroAssembler_x86.cpp were changed to
>> support the new encoding and decoding of compressed klass pointers.
>>
>> Module metaspace.cpp was changed significantly to support the new
>> allocation for metaspace and the CDS region and to determine compressed
>> klass pointer encoding base and shifting values.
>>
>> Most of the changes to module universe.cpp were to remove code related
>> to allocating metaspace and remove code that considered compressed klass
>> pointers when determining the compressed oops compression mechanism.
>>
>> Modules klass.inline.hpp and oop.inline.hpp were changed as part of a
>> cleanup requested by Coleen.
>>
>>
>> These changes were tested with JCK Lang and VM tests, JTReg tests, JPRT,
>> GCBasher, refworkload, ute vm.quick.testlist and vm.mlvm.testlist
>> tests.  Most of the test were run with -Xshare:on and -Xshare:off (with
>> and without CDS), and were run on Solaris Sparc, 32-Bit Linux and 64-Bit
>> Linux.  Jtreg tests were run on Windows 7 and Mac OS.  JCK tests were
>> run on Solaris x86.
>>
>> The performance impact of these changes is TBD.
>>
>> Thanks, Harold
>>
>>