[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-hotspot-compiler-dev
Subject:    Re: RFR(M) 8147461: Use byte offsets for vtable start and vtable length offsets
From:       Christian Thalinger <christian.thalinger () oracle ! com>
Date:       2016-01-28 9:41:58
Message-ID: FCE6B7E1-9965-40F1-83EA-AD99B5D46746 () oracle ! com
[Download RAW message or body]


> On Jan 22, 2016, at 10:46 AM, Mikael Gerdin <mikael.gerdin@oracle.com> wrote:
> 
> 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.

For the record, the JVMCI changes look good.

> 
> > 
> > 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
> > > 
> > 
> 


[prev in list] [next in list] [prev in thread] [next in thread]