Moving the _vtable_len into Klass

[Chris Plummer]

Hi Mikael,

On 12/16/15 3:54 AM, Mikael Gerdin wrote:
> Hi all,
>
> In order to reduce the number of virtual calls in the GC hot path I'd 
> like to propose that the field _vtable_len be moved to class Klass.
> The reason for requiring access to the vtable length in the GC is that 
> the "nonstatic oop maps" are packed after the v and itables after the 
> actual C++ InstanceKlass.
>
> In its current form, the vtable_length() accesor is a pure virtual on 
> Klass, and is implemented in ArrayKlass and InstanceKlass by returning 
> a simple member variable _vtable_len which is present in both 
> ArrayKlass and InstanceKlass.
> My suggestion is to move the _vtable_len member to Klass and move the 
> accessor for it to Klass as well.
>
> This also brings up the issue of 
> InstanceKlass::vtable_length_offset(), which is a very special beast. 
> It returns the offset of the _vtable_len field, in words. This means 
> that the int field _vtable_len must be on a word aligned address in 
> order to work, otherwise we crash at startup in interesting ways.
Well that seems like fragile programming. The only reason it works right 
now is because _vtable_len comes right after a pointer type. Insert 
another int field before it and it will crash.
> I should note that all callers of vtable_length_offset rectify the 
> problem of it being a word offset by multiplying the value by wordSize 
> in order to convert it to a byte offset.
Yep. I just grep'd and saw the same.
>
> Does anyone know if there is a reason for vtable_length_offset to be a 
> word offset?
Sorry, I don't have any historical perspective here, other than saying 
that specifying size, lengths, and offsets for InstanceKlass related 
stuff seems to all be scaled to HeapWordSize. Just started looking in 
this area recently for the first time myself. BTW, I think it should be 
using wordSize instead of HeapWordSize, although they do both evaluate 
to the same thing.
>
> If the offset of the _vtable_len field could be accessed as a byte 
> offset then one would not need to be so careful about the Klass field 
> layout when attempting to fit _vtable_len while simultaneously not 
> increasing the total size of the subclasses of Klass where the field 
> is moved from.
I'm assuming you've prototyped moving it to just after 
_accumulated_modified_oops, taking advantage of the wasted bytes in the 
word set aside for the jshort _shared_class_path_index field that follows.
>
> Regardless of the vtable_length_offset mess, are there any other 
> concerns about making the vtable_length accessor nonvirtual?
>
> In order to keep the code consistent and reduce duplication the 
> vtable() accessor could also be moved to Klass, together with 
> start_of_vtable().
> Unfortunately, there is a circular dependency with InstanceKlass for 
> calculating the vtable start address because the calculation depends 
> on the size of InstanceKlass. However I still think it might be workth 
> consolidating the vtable related methods in Klass since they are in 
> fact part of all subclasses of class.
So how will you get around the circular dependency?

Chris
>
> Questions/concerns about this suggested change are of course welcome!
>
> /Mikael