Request for review:7133260:Remove -Xaprof and the usage of Klass:_alloc_count and ArrayKlass::_alloc_size

David Holmes david.holmes at oracle.com
Wed Jun 5 17:24:17 PDT 2013


Hi Jiangli,

Yes you must wait for CCC approval before submitting the changeset.

Do you have a utility to report the actual sizes of these objects? 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.

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