CR 8005604: HPROF should report the actual instance size
Aleksey Shipilev
aleksey.shipilev at oracle.com
Sat Dec 29 02:10:31 PST 2012
Alan,
Good catch. This is the excerpt from the HPROF binary format in
heapDumper.cpp:
HPROF_GC_CLASS_DUMP
(----snip-----)
u4 instance size (in bytes)
(----snip-----)
u2 number of static fields
[id, static field name,
ty, type,
vl]* and value
(----snip-----)
u2 number of inst. fields (not inc. super)
[id, instance field name,
ty]* type
HPROF_GC_INSTANCE_DUMP
u4 number of bytes that follow
[vl]* instance field values (class, followed
by super, super's super ...)
Seems like that instance_size() method is used to populate both GC_CLASS
and GC_INSTANCE. Would it be OK to push the VM-reported size to GC_CLASS
only, and leave GC_INSTANCE intact?
-Aleksey.
On 12/29/2012 02:02 PM, Alan Bateman wrote:
> Aleksey,
>
> I'm a bit out of touch with this code but just to say that the HPROF
> format is VM independent and the length that follows the class ID in the
> INSTANCE record is the size for the fields that follow.
>
> -Alan
>
> On 29/12/2012 09:14, Aleksey Shipilev 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.
>
More information about the serviceability-dev
mailing list