Bug in FieldsAllocationStyle=2 logic

[David Dabbs]

> -----Original Message-----
> From: Vladimir Kozlov [mailto:vladimir.kozlov at oracle.com]
> Sent: Wednesday, June 22, 2011 3:56 PM
> To: Krystal Mok
> Cc: David Dabbs; hotspot-dev
> Subject: Re: Bug in FieldsAllocationStyle=2 logic
> 
> I have nothing to add to Kris's nice explanation.
> I just want to say that you should check if it really can help your
> application.
> It may help GC but it may also harm execution performance since
> different layout
> of fields may increase data cache misses.
> 
> Vladimir
> 

I knew there would be some 'fine print.'
Would one need a tool like Intel VTune to measure cache miss impact 
before/after tweaking the option?

David


> Krystal Mok wrote:
> > Hi David,
> >
> > I'll let Vladimir, the original author, give the definitive answer.
> :-)
> >
> > Anyway, here's my guess:
> > Packing oop fields together makes iterating oops in an object faster.
> > For example, the marking phase of GC would benefit from this.
> >
> > Supposed we had a few classes as follows:
> >
> > class Base {
> >   Object o1;
> >   int i;
> > }
> >
> > class Derived extends Base {
> >   Object o2;
> >   float f;
> > }
> >
> > An instance of Derived would contain a Base subobject within itself.
> > It's straightforward to keep the Base's field layout in the subobject
> in
> > an Derived instance; in other words, Base::o1 and Base::i would be at
> > the same offset in a Base instance and in a Derived instance.
> >
> > So, to pack oops in Base and Derived together while keeping Base's
> > layout intact in Derived, a reasonable solution is to alternate
> between
> > FieldsAllocationStyle=1 and FieldsAllocationStyle=0 in an inheritance
> > chain. That's exactly what FieldsAllocationStyle is doing.
> >
> > But if we push hard to pack all oop fields in an object together,
> we'd
> > probably end up using a bidirectional object layout, like the one
> used
> > in SableVM (http://www.sablevm.org/features.html). It'd be really fun
> to
> > see this implemented in HotSpot VM sometime in the future.
> >
> > This paper is worth reading if you're interested in object layouts:
> > A Comprehensive Evaluation of Object Scanning
> > Techniques, http://users.cecs.anu.edu.au/~steveb/downloads/pdf/scan-
> ismm-2011.pdf
> >
> > Regards,
> > Kris Mok
> >
> > On Thu, Jun 23, 2011 at 2:29 AM, David Dabbs <dmdabbs at gmail.com
> > <mailto:dmdabbs at gmail.com>> wrote:
> >
> >
> >
> >      > -----Original Message-----
> >      > From: hotspot-dev-bounces at openjdk.java.net
> >     <mailto:hotspot-dev-bounces at openjdk.java.net> [mailto:hotspot-
> dev-
> >     <mailto:hotspot-dev->
> >      > bounces at openjdk.java.net <mailto:bounces at openjdk.java.net>] On
> >     Behalf Of Vladimir Kozlov
> >      > Sent: Wednesday, June 22, 2011 12:39 PM
> >      > To: Krystal Mok
> >      > Cc: hotspot-dev
> >      > Subject: Re: Bug in FieldsAllocationStyle=2 logic
> >      >
> >      > Thank you for such detail analysis and suggested fix.
> >      > I filed bug and I will fix it in 7u2.
> >      >
> >      > 7058036: FieldsAllocationStyle=2 does not work in 32-bit VM
> >      >
> >      > Thanks,
> >      > Vladimir
> >      >
> >      > PS: if possible, please, report problems to hotspot-
> >      > dev at openjdk.java.net <mailto:dev at openjdk.java.net> alias
> >      > so all Hotspot developers can see it. I did original changes
> but I am
> >      > in
> >      > Compiler (JIT) group.
> >      >
> >      > Krystal Mok wrote:
> >      > > Hi all,
> >      > >
> >      > > I think I've found a bug in
> ClassFileParser::parseClassFile() that
> >      > deals
> >      > > with FieldsAllocationStyle=2, which was introduced about a
> year
> >      > > ago:
> http://hg.openjdk.java.net/jdk6/jdk6/hotspot/rev/b9d85fcdf743
> >      > >
> >      > > int map_size = super_klass->nonstatic_oop_map_size();
> >      > > OopMapBlock* first_map =
> >     super_klass->start_of_nonstatic_oop_maps();
> >      > > OopMapBlock* last_map = first_map + map_size - 1;
> >      > >
> >      > > This code accidentally works on LP64 systems because it
> takes 1
> >     word
> >      > per
> >      > > OopMapBlock, so nonstatic_oop_map_size() and
> >      > nonstatic_oop_map_count()
> >      > > would actually return the same value.
> >      > >
> >      > > But on 32-bit systems, an OopMapBlock takes 2 words, which
> >      > > makes nonstatic_oop_map_size() == nonstatic_oop_map_count()
> *
> >     2, and
> >      > > breaks the code above.
> >      > >
> >      > > The code should really be:
> >      > >
> >      > > int map_count = super_klass->nonstatic_oop_map_count();
> >      > > OopMapBlock* first_map =
> >     super_klass->start_of_nonstatic_oop_maps();
> >      > > OopMapBlock* last_map = first_map + map_count - 1;
> >      > >
> >      > > I found this because FieldsAllocationStyle=2 doesn't work
> for me on
> >      > > 32-bit Windows (JDK6u25 and 6u26), but works on 64-bit
> Ubuntu
> >      > JDK6u25.
> >      > > Here's a min repro of my test:
> https://gist.github.com/1037866
> >      > >
> >      > > Could anybody please verify this?
> >      > > Just checked the current tip of hsx/hotspot-rt, and it still
> >     has this
> >      > > behavior:
> >      > > http://hg.openjdk.java.net/hsx/hotspot-
> >      >
> >
> rt/hotspot/file/1744e37e032b/src/share/vm/classfile/classFileParser.cpp
> >      > > line 3290
> >      > >
> >      > > Regards,
> >      > > Kris Mok
> >
> >
> >     What would be the use case/work load for choosing this option?
> >
> >
> >     Thanks,
> >
> >     David
> >
> >
> >