RFR(M) 8147461: Use byte offsets for vtable start and vtable length offsets

[Mikael Gerdin]

Hi Chris,

On 2016-01-21 04:17, Chris Plummer wrote:
> Hi Mikael,
>
> The changes look good except I think you should get someone from the
> compiler team to make sure the change in
> HotSpotResolvedJavaMethodImpl.java and HotSpotVMConfig.java are ok. I'm
> not sure why you chose to remove instanceKlassVtableStartOffset() rather
> than just fix it.

I'm cc:ing hotspot-compiler-dev and graal-dev to see if I can get 
someone to ok the JVMCI parts.

The reason for removing the method is that the only reason for it being 
a method was to apply the wordSize scaling on the value and since I 
changed the offset to be a byte offset it does not need scaling and can 
be treated similar to the other constants in HotSpotVMConfig which are 
accessed without any accessor method.

>
> I think some of your changes may conflict with my changes for
> JDK-8143608. Coleen is pushing JDK-8143608 for me once hs-rt opens up.
> I'd appreciate it if you could wait until after then before doing your
> push.

Will do, would you mind pinging me when you've integrated 8143608?

/Mikael

>
> thanks,
>
> Chris
>
> On 1/20/16 4:31 AM, Mikael Gerdin wrote:
>> Hi again,
>>
>> I've rebased the on hs-rt and had to include some additional changes
>> for JVMCI.
>> I've also updated the copyright years.
>> Unfortunately I can't generate an incremental webrev since i rebased
>> the patch and there's no good way that I know of to make that work
>> with webrev.
>>
>> New webrev at: http://cr.openjdk.java.net/~mgerdin/8147461/webrev.1/
>>
>> Testing: JPRT again (which includes the JVMCI jtreg tests)
>>
>> /Mikael
>>
>> On 2016-01-15 18:04, Mikael Gerdin wrote:
>>> Hi all,
>>>
>>> As per the previous discussion in mid-December[0] about moving the
>>> _vtable_length field to class Klass, here's the first RFR and webrev,
>>> according to my suggested plan[1]:
>>>
>>>> My current plan is to first modify the vtable_length_offset accessor to
>>>> return a byte offset (which is what it's translated to by all callers).
>>>>
>>>> Then I'll tackle moving the _vtable_len field to Klass.
>>>>
>>>> Finally I'll try to consolidate the vtable related methods to Klass,
>>>> where they belong.
>>>
>>> This change actually consists of three changes:
>>> * modifying InstanceKlass::vtable_length_offset to become a byte offset
>>> and use the ByteSize type to communicate the scaling.
>>> * modifying InstanceKlass::vtable_start_offset to become a byte offset
>>> and use the ByteSize type, for symmetry reasons mainly.
>>> * adding a vtableEntry::size_in_bytes() since in many places the vtable
>>> entry size is used in combination with the vtable start to compute a
>>> byte offset for vtable lookups.
>>>
>>> I don't foresee any issues with the fact that the byte offset is
>>> represented as an int, for two reasons:
>>> 1) If the offset of any of these grows to over 2 gigabytes then we have
>>> a huge footprint problem with InstanceKlass
>>> 2) The offsets are converted to byte offsets and stored in ints already
>>> in the cpu specific code I've modified.
>>>
>>> Bug link: https://bugs.openjdk.java.net/browse/JDK-8147461
>>> Webrev: http://cr.openjdk.java.net/~mgerdin/8147461/webrev.0/
>>>
>>> Testing: JPRT on Oracle supported platforms, testing on AARCH64 and
>>> PPC64 would be much appreciated, appropriate mailing lists have been
>>> CC:ed to notify them of the request.
>>>
>>>
>>> [0]
>>> http://mail.openjdk.java.net/pipermail/hotspot-dev/2015-December/021152.html
>>>
>>>
>>> [1]
>>> http://mail.openjdk.java.net/pipermail/hotspot-dev/2015-December/021224.html
>>>
>>>
>>>
>>> Thanks!
>>> /Mikael
>>
>