RFR(M) 8147461: Use byte offsets for vtable start and vtable length offsets
mikael.gerdin at oracle.com
Fri Jan 22 09:46:32 UTC 2016
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
Will do, would you mind pinging me when you've integrated 8143608?
> 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)
>> On 2016-01-15 18:04, Mikael Gerdin wrote:
>>> Hi all,
>>> As per the previous discussion in mid-December about moving the
>>> _vtable_length field to class Klass, here's the first RFR and webrev,
>>> according to my suggested plan:
>>>> 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.
More information about the hotspot-compiler-dev