RFR(M) 8148047: Move the vtable length field to Klass

Chris Plummer chris.plummer at oracle.com
Fri Jan 22 19:21:07 UTC 2016


Hi Mikael,

On 1/22/16 7:49 AM, Mikael Gerdin wrote:
> Hi all,
>
> Here's the second part of the set of changes to move most of the 
> vtable related code to Klass.
>
> This change consists of the following parts:
> * Move the field _vtable_len to Klass, making its accessor nonvirtual.
>  -> Ensure that this does not result in any footprint regression by 
> moving TRACE_DEFINE_KLASS_TRACE_ID in Klass and _itable_len in 
> InstanceKlass to fill out alignment gaps.
I also double checked this, including with minimal VM. The sizeof(Klass) 
remains unchanged.

And your relocation of InstanceKlass::_itable_len should result in 
InstanceKlass being 8 bytes smaller on 64-bit, although I did not double 
check that.
>  -> Move vtable_length_offset to Klass. Move vtable_start_offset to 
> Klass to keep the code consistent. vtable_start_offset depends on the 
> size of InstanceKlass and must therefore be defined outside of klass.hpp.
The abstraction has kind of gone awry:

  662 ByteSize Klass::vtable_start_offset() {
  663   return in_ByteSize(InstanceKlass::header_size() * wordSize);
  664 }

Note the reference to InstanceKlass. I understand why you are doing 
this. All Klass subclasses have their sizes padded out to the size of 
InstanceKlass so the vtable is always in the same place. I was just 
wondering if there is a cleaner way. I guess the abstraction was broken 
before your changes, but in a different place.  We used to call 
InstanceKlass::vtable_start_offset() even for array types. Now you 
instead call Klass::vtable_start_offset(), and it does the dirty deed of 
referencing InstanceKlass. I guess I'm ok with it. Just wanted to point 
it out in case you had more thoughts.

>
> * Update all locations to refer to Klass::vtable_{length,start}_offset 
> instead of InstanceKlass.
Looks good.
>
> * Modify SA to look for _vtable_len in Klass.
Looks good.
>
> * Rename CompilerToVM::Data::InstanceKlass_vtable_{length,start}_offset
> and 
> jdk.vm.ci.hotspot.HotSpotVMConfig.instanceKlassVtable{Length,Start}Offset 
> to properly represent where the offsets are coming from.
Looks good.
>
>
> Webrev: http://cr.openjdk.java.net/~mgerdin/8148047/webrev.0/
> Bug: https://bugs.openjdk.java.net/browse/JDK-8148047
>
> Testing: JPRT on Oracle supported platforms.
>
> As in the previous change I've updated the PPC64 and AARCH64 ports but 
> I have not tested the changes. Build and test feedback from porters is 
> most welcome!

Cheers,

Chris
>
> Thanks
> /Mikael



More information about the hotspot-dev mailing list