RFR: 8017588: SA: jstack -l throws UnalignedAddressException while attaching to core file for java that was started with CMS GC

[Erik Helin]

On 2013-07-05, Erik Helin wrote:
> Hi Coleen,
> 
> thanks for reviewing!
> 
> On 2013-07-03, Coleen Phillimore wrote:
> > 
> > Eric,
> > 
> > I like the cleanup of instanceKlass.cpp a lot.
> > 
> > In the serviceability agent,  does getHeap().getOopSize() return 32
> > for compressed oops?

Sorry, I forgot to answer this question. With hotspot 24 and compressed
oops, I do not think that oops in C++ objects such as methodOop in
vtableEntry gets compressed.

methodOop gets "typedeffed" at compile-time in oops/oopsHierarchy.hpp to
methodOopDesc*, so it should always have sizeof(char*), i.e. oopSize, as
size.

Am I correct?

Thanks,
Erik

> > The vtables and itables are Method*, ie
> > metadata.   Is there a better size for these?
> 
> This change is for hotspot 24, so we have a methodOop in vtableEntry :)
> 
> So, ObjectHeap.getOopSize() will return oopSize in globalDefinitions.hpp
> which is sizeof(char*). VM.getAddressSize in SA is dependent on
> platform, for x86-64 it will return
> MachineDescriptionAMD64.getAddressSize(), which is defined as 8.
> 
> Therefore, it does not matter if the code in
> InstanceKlass.getObjectSize() uses ObjectHeap.getOopSize() or
> VM.getAddressSize(), the result will be the same.
> 
> In order to keep the SA code and the hotspot code as similar as
> possible, I think it is better to use ObjectHeap.getOopSize(), since the
> hotspot code does sizeof(klassOop).
> 
> Please see a new webrev at:
> http://cr.openjdk.java.net/~ehelin/8017588/webrev.01/
> 
> I've also done some further testing on, the following testlists were
> successfully run:
> - vm.quick.testlist
> - vm.tmtools.testlist
> - nsk.sajdi.testlist
> 
> in addition to the previous testing:
> - JPRT
> - Running jstack -l -F successfully on a java process using -XX:+UseConcMarkSweepGC
> 
> Thanks,
> Erik
> 
> > Another set of tests that I think we run with vm.quick.testlist is
> > the nsk.sajdi.testlist and the vm.tmtools.testlist in ute.
> > 
> > Thanks,
> > Coleen
> > 
> > On 7/3/2013 4:33 AM, Erik Helin wrote:
> > >Hi all,
> > >
> > >this change fixes an issue where we could not run jstack -l -F on a
> > >java process running with -XX:+UseConcMarkSweepGC.
> > >
> > >The problem originated from the following change in hotspot:
> > >  changeset:   3707:8bafad97cd26
> > >  parent:      3693:973046802b6f
> > >  7158552: The instanceKlsss::_host_klass is only needed for anonymous class for JSR 292 support.
> > >
> > >  diff --git a/src/share/vm/oops/instanceKlass.hpp b/src/share/vm/oops/instanceKlass.hpp
> > >  --- a/src/share/vm/oops/instanceKlass.hpp
> > >  +++ b/src/share/vm/oops/instanceKlass.hpp
> > >  @@ -717,9 +763,11 @@
> > >     {
> > >       return object_size(align_object_offset(vtable_length()) +
> > >                          align_object_offset(itable_length()) +
> > >  -                       (is_interface() ?
> > >  -                        (align_object_offset(nonstatic_oop_map_size()) + (int)sizeof(klassOop)/HeapWordSize) :
> > >  -                        nonstatic_oop_map_size()));
> > >  +                       ((is_interface() || is_anonymous()) ?
> > >  +                         align_object_offset(nonstatic_oop_map_size()) :
> > >  +                         nonstatic_oop_map_size()) +
> > >  +                       (is_interface() ? (int)sizeof(klassOop)/HeapWordSize : 0) +
> > >  +                       (is_anonymous() ? (int)sizeof(klassOop)/HeapWordSize : 0));
> > >     }
> > >
> > >The corresponding code in the serviceability agent was not updated:
> > >
> > >  agent/src/share/classes/sun/jvm/hotspot/oops/InstanceKlass.java:
> > >   public long getObjectSize() {
> > >   long bodySize =    alignObjectOffset(getVtableLen() * getHeap().getOopSize())
> > >                    + alignObjectOffset(getItableLen() * getHeap().getOopSize())
> > >                    + (getNonstaticOopMapSize()) * getHeap().getOopSize();
> > >   return alignObjectSize(headerSize + bodySize);
> > >   }
> > >
> > >This fix updates the SA code to be like the hotspot code. I've also
> > >introduced variables in the hotspot code and in the SA code to make it
> > >easier to compare the two versions.
> > >
> > >Webrev: http://cr.openjdk.java.net/~ehelin/8017588/webrev.00/
> > >
> > >Testing:
> > >- JPRT
> > >- Running jstack -l -F successfully on a java process using -XX:+UseConcMarkSweepGC
> > >
> > >Thanks,
> > >Erik
> >