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

[Kim Barrett]

> On Jan 22, 2016, at 10:49 AM, Mikael Gerdin <mikael.gerdin at oracle.com> 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.
> -> 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.
> 
> * Update all locations to refer to Klass::vtable_{length,start}_offset instead of InstanceKlass.
> 
> * Modify SA to look for _vtable_len in Klass.
> 
> * 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.
> 
> 
> 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!

Looks good.

A couple of surprises noted.

------------------------------------------------------------------------------
src/share/vm/oops/arrayKlass.cpp 
  85 ArrayKlass::ArrayKlass(Symbol* name) :
src/share/vm/oops/instanceKlass.cpp 
 211 InstanceKlass::InstanceKlass(const ClassFileParser& parser, unsigned kind) :

[Pre-existing]

I was surprised by the indentation of the bodies of these
constructors. I would have placed the opening brace on its own line
(to separate the init-list from the body) and indented the body
normally.  I'm guessing there are more like this, so perhaps I should
get over my surprise.

------------------------------------------------------------------------------ 
src/share/vm/oops/arrayKlass.cpp 
  91     set_vtable_length(Universe::base_vtable_size());
src/share/vm/oops/instanceKlass.cpp 
 216     set_vtable_length(parser.vtable_size());

[Sort of pre-existing]
I was surprised that the Klass constructor left the new _vtable_len
field uninitialized, with assignment done in subclasses. I was
expecting the Klass constructor to be called with arguments that would
be used to initialize various fields, with no need for the setter
functions. But what's in the webrev seems to be of the general style
used in this vicinity.  Perhaps a followup cleanup is called for?

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

I agree with Chris that something has gone awry with

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

But I don't have a suggestion for improvement right now.

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