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