Request for review:7133260:Remove -Xaprof and the usage of Klass:_alloc_count and ArrayKlass::_alloc_size
Jiangli Zhou
jiangli.zhou at oracle.com
Wed Jun 5 20:02:06 PDT 2013
Hi David,
On 06/05/2013 05:24 PM, David Holmes wrote:
> Hi Jiangli,
>
> Yes you must wait for CCC approval before submitting the changeset.
Ok. Thanks for letting me know.
>
> Do you have a utility to report the actual sizes of these objects?
Yes. :) Using 'sizeof' shows 4bytes reduce for Klass size and 8 bytes
reduce for ArrayKlass size. jcmd <pid> GC.class_stats reports real
saving in meta data. From the before and after comparison below, class
size (KlassBytes, 2nd column) is reduced by 8bytes. The extra 4 bytes
saving is due to alignment. Array class has the same saving as regular
class. That's due to the change in Klass (not ArrayKlass).
Before:
Index Super InstBytes KlassBytes annotations CpAll MethodCount
Bytecodes MethodAll ROAll RWAll Total ClassName
1 -1 80560 288 0 0 0 0
0 12 376 388 [C
2 42 43488 368 0 9748 127
4682 18816 13716 16376 30092 java.lang.Class
3 -1 24976 288 0 0 0 0
0 12 376 388 [B
...
7 284 6696 416 0 3076 43
1667 6648 5016 5608 10624 java.lang.reflect.Field
8 251 4096 336 0 2364 54
3221 9008 6632 5624 12256 java.lang.Integer
After:
Index Super InstBytes KlassBytes annotations CpAll MethodCount
Bytecodes MethodAll ROAll RWAll Total ClassName
1 -1 80528 280 0 0 0 0
0 12 368 380 [C
2 42 43488 360 0 9748 127
4682 17800 13716 15352 29068 java.lang.Class
3 -1 24976 280 0 0 0 0
0 12 368 380 [B
...
7 284 6696 408 0 3076 43
1667 6304 5016 5256 10272 java.lang.reflect.Field
8 251 4096 328 0 2364 54
3221 8576 6632 5184 11816 java.lang.Integer
> I must admit I'm still scratching my head over the static_size issue,
> and a reporting utility would have made it obvious that the savings
> were not as expected.
The ArrayKlass::static_size() calculates the size when allocating an
array class. It use InstanceKlass size instead of ArrayKlass size when
computing the array size. That's why any reduction in ArrayKlass does
not help, but reduction in InstanceKlass and Klass helps. Hope that
clears some of the confusion.
Thanks,
Jiangli
>
> David
>
> On 6/06/2013 9:33 AM, Jiangli Zhou wrote:
>> Hi Stefan,
>>
>> On 06/05/2013 03:05 PM, Stefan Karlsson wrote:
>>> On 6/5/13 10:29 PM, Jiangli Zhou wrote:
>>>> Hi Stefan,
>>>>
>>>> Here is updated webrev with object_iterate_since_last_GC and
>>>> with_array_klasses_do removed.
>>>>
>>>> http://cr.openjdk.java.net/~jiangli/7133260/webrev.01/
>>>
>>> Thanks!
>>>
>>> Looks good.
>>>
>>> The GC team should probably verify that _last_gc and
>>> object_iterate_from are still needed:
>>> -void
>>> OneContigSpaceCardGeneration::object_iterate_since_last_GC(ObjectClosure*
>>> blk) {
>>> - // Deal with delayed initialization of _the_space,
>>> - // and lack of initialization of _last_gc.
>>> - if (_last_gc.space() == NULL) {
>>> - assert(the_space() != NULL, "shouldn't be NULL");
>>> - _last_gc = the_space()->bottom_mark();
>>> - }
>>> - the_space()->object_iterate_from(_last_gc, blk);
>>> -}
>>>
>>>
>>> but we can do that in a separate changeset if you want to get this
>>> pushed.
>>
>> Thanks for taking another look. Should I wait until CCC is approved to
>> push the changeset? If so, I can wait for GC team's confirmation on
>> _last_gc and object_iterate_from.
>>
>> Thanks!
>> Jiangli
>>
>>>
>>> thanks,
>>> StefanK
>>>
>>>>
>>>> Thanks,
>>>> Jiangli
>>>>
>>>> On 06/05/2013 09:44 AM, Jiangli Zhou wrote:
>>>>> Hi Stefan,
>>>>>
>>>>> On 06/05/2013 01:48 AM, Stefan Karlsson wrote:
>>>>>> On 06/05/2013 04:15 AM, Jiangli Zhou wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> Please review following change for 7133260.
>>>>>>>
>>>>>>> http://ccc.us.oracle.com/7133260
>>>>>>
>>>>>> I looked at:
>>>>>> http://cr.openjdk.java.net/~jiangli/7133260/webrev.00
>>>>>
>>>>> Thanks for doing that! Sorry for the wrong link.
>>>>>
>>>>>>
>>>>>> Looks good. Thanks for doing this cleanup.
>>>>>>
>>>>>> You remove the last usage of:
>>>>>> object_iterate_since_last_GC
>>>>>> with_array_klasses_do
>>>>>>
>>>>>> so there's even more cleanups that can be done here.
>>>>>
>>>>> That does appear to be the case. Thanks for noticing that. I'll try
>>>>> to remove them as well.
>>>>>
>>>>>>
>>>>>>>
>>>>>>> In JDK8, -Xaprof support is outdated and broken. A new feature,
>>>>>>> GC.class_stats command has been added to jcmd in JDK8. It provides
>>>>>>> detailed class metadata memory statistics information. -Xaprof
>>>>>>> uses Klass:_alloc_count and ArrayKlass::_alloc_size. Removing
>>>>>>> -Xaprof and related fields would save 4bytes per class and 8bytes
>>>>>>> per array class.
>>>>>>
>>>>>> FTR, I don't think you save 8 bytes for the array classes. The size
>>>>>> of ArrayKlass increased so that the vtable offset matches the
>>>>>> vtable offset of the InstanceKlasses. See ArrayKlass::static_size.
>>>>>
>>>>> Thanks for pointing out the ArrayKlass::static_size. You are right,
>>>>> the array class size is increased to be the same as InstanceKlass
>>>>> size in ArrayKlass::static_size. I will fix CCC. So removing fields
>>>>> from ArrayKlass does not yield real saving. :( It also means any
>>>>> size reduction in InstanceKlass would help array class. :) Any
>>>>> reason why ArrayKlass and InstanceKlass vtable offset need to be
>>>>> matched?
>>>>>
>>>>> Thanks for the review!
>>>>>
>>>>> Jiangli
>>>>>
>>>>>>
>>>>>> thanks
>>>>>> StefanK
>>>>>>
>>>>>>> Please see more information in the CCC request:
>>>>>>> http://ccc.us.oracle.com/7133260.
>>>>>>>
>>>>>>> Tested with JPTR and vm.quick.testlist.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Jiangli
>>>>>>
>>>>>
>>>>
>>>
>>
More information about the hotspot-runtime-dev
mailing list