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

[Stefan Karlsson]

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,
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
>>>
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/attachments/20130606/3b82ba2c/attachment.html