CR 8005604: HPROF should report the actual instance size

[Krystal Mok]

Hi Aleksey,

It's good to see that this problem is being addressed. I wonder what's
the history of making HPROF calculate the instance size on its own...

A nitpick: even though the proposed patch should work just right, but
I think it's better to access the size_helper through
InstanceKlass::oop_size() instead of through
InstanceKlass::size_helper() directly. That way you'd only need a
klassHandle kh, and call through kh->oop_size().

Thanks,
Kris

On Sat, Dec 29, 2012 at 5:14 PM, Aleksey Shipilev
<aleksey.shipilev at oracle.com> wrote:
> Hi guys,
>
> This issue popped out (again?) in the course of @Contended work, which
> does the padding around fields in some corner cases. HPROF just iterates
> the fields to deduce the instance size, which can give a terribly wrong
> instance size estimate.
>
> I have filed CR 8005604 to track this.
>
> FWIW, the fix appears to be very simple, we can reuse the instance size
> data already available in instanceKlass, like this:
>
> --- a/src/share/vm/services/heapDumper.cpp      Mon Dec 24 11:46:38 2012 -0800
> +++ b/src/share/vm/services/heapDumper.cpp      Sat Dec 29 13:08:51 2012 +0400
> @@ -772,33 +772,7 @@
>  u4 DumperSupport::instance_size(Klass* k) {
>    HandleMark hm;
>    instanceKlassHandle ikh = instanceKlassHandle(Thread::current(), k);
> -
> -  int size = 0;
> -
> -  for (FieldStream fld(ikh, false, false); !fld.eos(); fld.next()) {
> -    if (!fld.access_flags().is_static()) {
> -      Symbol* sig = fld.signature();
> -      switch (sig->byte_at(0)) {
> -        case JVM_SIGNATURE_CLASS   :
> -        case JVM_SIGNATURE_ARRAY   : size += oopSize; break;
> -
> -        case JVM_SIGNATURE_BYTE    :
> -        case JVM_SIGNATURE_BOOLEAN : size += 1; break;
> -
> -        case JVM_SIGNATURE_CHAR    :
> -        case JVM_SIGNATURE_SHORT   : size += 2; break;
> -
> -        case JVM_SIGNATURE_INT     :
> -        case JVM_SIGNATURE_FLOAT   : size += 4; break;
> -
> -        case JVM_SIGNATURE_LONG    :
> -        case JVM_SIGNATURE_DOUBLE  : size += 8; break;
> -
> -        default : ShouldNotReachHere();
> -      }
> -    }
> -  }
> -  return (u4)size;
> +  return (u4)ikh->size_helper();
>  }
>
>  // dumps static fields of the given class
>
>
> Please let me know if you have troubles with pushing this. If not, I can
> get the proper due diligence to get this fixed.
>
> Thanks,
> Aleksey.