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

[Mikael Gerdin]

Hi Coleen,

On 2016-01-20 15:24, Coleen Phillimore wrote:
>
> http://cr.openjdk.java.net/~mgerdin/8147461/webrev.1/src/share/vm/oops/klassVtable.hpp.udiff.html
>
>
> I thought this would return ByteSize?

I thought about it but there wasn't really any precedent for using 
ByteSize, instead several methods had the _in_bytes suffix and in that 
case I thought that forcing all users to do
in_bytes(klassVtable::size_in_bytes()) would be a bit too much.

In the case of the InstanceKlass variants some architectures have 
Address objects in their assembler implementations which allow me to 
pass a ByteSize, allowing me to skip the in_bytes(..) in at least some 
cases.


>
> This looks really good.  Although I thought you were going to move
> _vtable_len to Klass ?

As Kim said, that's coming up next.

Thanks for the review.
/Mikael

>
> Coleen
>
> On 1/20/16 7: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
>>
>