Moving the _vtable_len into Klass
mikael.gerdin at oracle.com
Wed Dec 16 19:35:02 UTC 2015
On 2015-12-16 18:26, Chris Plummer wrote:
> 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.
Yep, I'd say it's a bug that it's so fragile.
>> 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.
Yep, I noticed that too.
>> 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.
In my current variant I've experimented with moving some things around:
diff --git a/src/share/vm/oops/klass.hpp b/src/share/vm/oops/klass.hpp
@@ -130,18 +130,18 @@
jint _modifier_flags; // Processed access flags, for use by
AccessFlags _access_flags; // Access flags. The class/interface
distinction is stored here.
// Biased locking implementation and statistics
// (the 64-bit chunk goes first, to avoid some fragmentation)
markOop _prototype_header; // Used when biased locking is both
enabled and disabled for this type
- jint _biased_lock_revocation_count;
// vtable length
+ jint _biased_lock_revocation_count;
// Remembered sets support for the oops in the klasses.
jbyte _modified_oops; // Card Table Equivalent (YC/CMS
jbyte _accumulated_modified_oops; // Mod Union Equivalent (CMS support)
>> 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
>> 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?
klass.cpp already includes instanceKlass.hpp (for better or worse) so I
can simply make the vtable start getter a non-inline member function in
Klass and it will work out. But I feel a little dirty adding another
dependency from Klass to InstanceKlass.
>> Questions/concerns about this suggested change are of course welcome!
More information about the hotspot-dev