RFR: 8253278: Refactor/cleanup oopDesc::*_klass_addr

Aleksey Shipilev shade at openjdk.java.net
Thu Sep 17 10:08:24 UTC 2020


On Thu, 17 Sep 2020 08:37:26 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

> Right now this one has the comment mentioning CMS:
> 
> Klass** oopDesc::klass_addr(HeapWord* mem) {
>   // Only used internally and with CMS and will not work with
>   // UseCompressedOops
>   assert(!UseCompressedClassPointers, "only supported with uncompressed klass pointers");
>   ByteSize offset = byte_offset_of(oopDesc, _metadata._klass);
>   return (Klass**) (((char*)mem) + in_bytes(offset));
> }
> 
> But that is not true, it is used on the generic paths.
> 
> The rest are the snowballing changes:
>  - in `load_klass_raw` and `set_klass`: `compressed_klass_addr` and `klass_addr` are used where the direct access to
>    `_metadata` can be done, avoiding these methods
>  - the leftover use is in `set_klass_release`, where we can simplify things by inlining `_addr` methods directly
>  - `CHECK_SET_KLASS` macro seems too much ceremony for just two asserts
> 
> Note that new code uses the universal `klass_offset_in_bytes`, that is technically `offset_of(..., _metadata._klass)`.
> This is different from the previous code, but it matches what other code is doing -- it trusts that both
> `_metadata._klass` and `_metadata._compressed_klass` are on the same offsets. I thought to add a very paranoid assert
> there, but decided not to.  Testing: Linux x86_64 fastdebug tier1

objdumped `MemAllocator::finish` that uses `oop::set_klass_release`, and it has no differences. Looks like this
before/after:

0000000000000000 <_ZNK12MemAllocator6finishEPP12HeapWordImpl>:
       0:       f3 0f 1e fa             endbr64
       4:       48 8b 0d 00 00 00 00    mov    0x0(%rip),%rcx        # b <_ZNK12MemAllocator6finishEPP12HeapWordImpl+0xb>
       b:       48 89 f0                mov    %rsi,%rax
       e:       ba 01 00 00 00          mov    $0x1,%edx
      13:       80 39 00                cmpb   $0x0,(%rcx)
      16:       74 0b                   je     23 <_ZNK12MemAllocator6finishEPP12HeapWordImpl+0x23>
      18:       48 8b 57 10             mov    0x10(%rdi),%rdx
      1c:       48 8b 92 b8 00 00 00    mov    0xb8(%rdx),%rdx
      23:       48 8b 0d 00 00 00 00    mov    0x0(%rip),%rcx        # 2a <_ZNK12MemAllocator6finishEPP12HeapWordImpl+0x2a>
      2a:       48 89 10                mov    %rdx,(%rax)
      2d:       48 8b 57 10             mov    0x10(%rdi),%rdx
      31:       80 39 00                cmpb   $0x0,(%rcx)
      34:       74 1a                   je     50 <_ZNK12MemAllocator6finishEPP12HeapWordImpl+0x50>
      36:       48 8b 0d 00 00 00 00    mov    0x0(%rip),%rcx        # 3d <_ZNK12MemAllocator6finishEPP12HeapWordImpl+0x3d>
      3d:       48 2b 11                sub    (%rcx),%rdx
      40:       8b 49 08                mov    0x8(%rcx),%ecx
      43:       48 d3 ea                shr    %cl,%rdx
      46:       89 50 08                mov    %edx,0x8(%rax)
      49:       c3                      retq
      4a:       66 0f 1f 44 00 00       nopw   0x0(%rax,%rax,1)
      50:       48 89 50 08             mov    %rdx,0x8(%rax)
      54:       c3                      retq
      55:       90                      nop
      56:       66 2e 0f 1f 84 00 00    nopw   %cs:0x0(%rax,%rax,1)
      5d:       00 00 00

-------------

PR: https://git.openjdk.java.net/jdk/pull/221


More information about the hotspot-dev mailing list