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

[Erik Helin]

Hi David,

thanks for the review!

On 2013-07-09, David Holmes wrote:
> Hi Erik,
> 
> One minor nit - can you fix the spelling of anonymous in the agent code:
> 
> MISC_IS_ANOYMOUS

I will fix it before I push.

Thanks,
Erik

> 
> Thanks.
> 
> Reviewed.
> 
> David
> 
> On 5/07/2013 7:09 PM, 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?   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
> >>