Request for review: 8003424 - Enable Class Data Sharing for CompressedOops
harold seigel
harold.seigel at oracle.com
Fri Aug 2 13:31:55 PDT 2013
Hi,
Please review this updated webrev for bug 8003424:
http://cr.openjdk.java.net/~hseigel/bug_8003424.2/
<http://cr.openjdk.java.net/%7Ehseigel/bug_8003424.2/>
The updated webrev has a lot of changes from the previous webrev,
including the following:
1. Class metaspace information is now output when
-XX:+PrintCompressedOopsMode is specified.
2. decode_klass_not_null(Register dst, Register src) no longer
clobbers R12.
3. Method using_class_vsm() was renamed to using_class_space().
4. Type narrowKlass was added and replaces narrowOop, where appropriate.
5. The Metaspace:: qualifier was removed, where it was unnecessary.
6. Metaspace::initialize_class_space() was made private.
7. Method max_heap_for_compressed_oops(), in arguments.cpp, was updated.
Performance tests were run by Jenny using specjvm2008, specjbb2005, and
specjbb2013. The results showed no significant performance difference
in terms of scores and gc behavior.
Additional testing was done on Solaris Sparc and Linux x86 using
SpecJBB2005 with -Xmx34g and -XX:ObjectAlignmentInBytes=16 & 32.
Please let me know what additional tests should be run.
Thanks!
Harold
On 7/24/2013 4:36 PM, Vladimir Kozlov wrote:
> Hi Harold,
>
> > Usually the narrow_klass_base will be 32G and the
> > LogKlassAlignmentInBytes will be 3 (8 bytes alignment) which means that
> > narrow_klass_base >> 3 will exceed maxint. So, we will need R12, unless
> > we do multiple addq(r, narrow_klass_base_shifted()). Does that make
> sense?
>
> My bad, I miscalculated. But we have "perfect value" 0x800000000 and I
> am tempted to use may be bts (bit set) to avoid R12 usage (assuming or
> with check that class metaspace size < 32Gb).
>
> > Is there one prefix byte per instruction, or just for certain
> instructions?
>
> "Not all instructions require a REX prefix in 64-bit mode. A prefix is
> necessary only if an instruction references one of the extended
> registers or uses a 64-bit operand."
>
> I want you also run tests with -XX:ObjectAlignmentInBytes=16 and =32
> and big java heap. Recently we got several bugs which indicated that
> we did not separate cleanly oops and klass pointers en/decoding.
>
> Thanks,
> Vladimir
>
> On 7/24/13 11:35 AM, harold seigel wrote:
>> Hi Vladimir,
>>
>> Thank you for the review! Please see inline comments.
>>
>> Thanks, Harold
>>
>> On 7/19/2013 7:56 PM, Vladimir Kozlov wrote:
>>> 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.
>> Will be done in next webrev.
>>>>
>>>> Also it would be nice to print these info line (and compressed oops
>>>> info
>>>> line) in hs_err files.
>> I filed an enhancement bug for this: 8021264
>> <https://jbs.oracle.com/bugs/browse/JDK-8021264>.
>>>>
>>>> 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.
>> OK. Thanks.
>>>>
>>>> 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).
>> But the base will normally be 32Gb. So this case won't happen very
>> often.
>>>
>>> You definitely should not use R12 in decode_klass_not_null(Register
>>> dst, Register src) method where you have free 'dst' register.
>> Will be fixed in next webrev.
>>>
>>> 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).
>> Usually the narrow_klass_base will be 32G and the
>> LogKlassAlignmentInBytes will be 3 (8 bytes alignment) which means that
>> narrow_klass_base >> 3 will exceed maxint. So, we will need R12, unless
>> we do multiple addq(r, narrow_klass_base_shifted()). Does that make
>> sense?
>>>
>>> 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.
>> I verifed that we get either cds_address+cds_total as the address, or
>> CompressedKlassPointersBase as the address without CDS on Linux, Windows
>> 7, Mac OS, and Solaris 5.11. This is true with default heap sizes and
>> -Xmx32G.
>>
>> On Solaris 5.10, I don't get CompressedKlassPointersBase or the desired
>> CDS address for class metaspace regardless of heap size.
>>>
>>>>
>>>> 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).
>> Is there one prefix byte per instruction, or just for certain
>> instructions?
>>>>
>>>> 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.
>> I changed using_class_vsm() to using_class_space().
>>>
>>>
>>>>
>>>> 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
>>>>>
>>>>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/attachments/20130802/3e2153e6/attachment-0001.html
More information about the hotspot-runtime-dev
mailing list