RFR(S): 8010738: G1: Output for full GCs with +PrintGCDetails should contain perm gen size/meta data change info

[Bengt Rutisson]

Hi Coleen,

On 5/8/13 1:44 PM, Coleen Phillimore wrote:
> On 5/7/2013 11:40 PM, Bengt Rutisson wrote:
>>
>> Hi John,
>>
>> This looks good!
>>
>> One question. Why do we only print the metaspace information for full 
>> GCs? I see that this is consistent with the other GCs, but the 
>> metaspace information changes over time, so wouldn't it be 
>> interesting to print it for every GC?
>
> The metaspace will only grow between young collections as new classes 
> are loaded.  It will only shrink during full GC's or any GC that does 
> class unloading.

Right, but I thought it would actually be interesting to see how the 
metaspace grows.

> It seems expensive to print it out for all GC's.

Is it really that expensive?

Anyway, I'm fine with the way John did it now. It is consistent with the 
other GCs.

John, I just noticed one thing that I find a bit odd. The 
G1CollectorPolicy::print_detailed_heap_transition() method is only 
called from G1CollectedHeap::do_collection(), so it really doesn't have 
to take the full parameter. It could always print the metaspace 
information, right?

Thanks,
Bengt

>
> Coleen
>
>>
>> Also a heads up, as you may know there are a couple of suggested 
>> changes to how the capacity and used methods for metaspace should 
>> work. This is probably fine with regards to your change since it is 
>> similar to the other GCs, but it may mean that the actual output will 
>> change.
>>
>> A couple of minor comments:
>>
>> The JTreg test does not have an @run tag. What does that mean? How is 
>> it executed?
>>
>> What do you think about renaming _heap_bytes_before_gc to 
>> _heap_used_before_gc? Seems more consistent with the other variable 
>> names such as _heap_capacity_before_gc.
>>
>> Similarly, what do you think about renaming the local varialbles 
>> *_byte_* to *_used_* in 
>> G1CollectorPolicy::print_detailed_heap_transition()?
>>
>> Thanks,
>> Bengt
>>
>>
>>
>> On 5/7/13 12:23 AM, John Cuthbertson wrote:
>>> Hi Everyone,
>>>
>>> Can I get a couple of volunteers review these fairly straight 
>>> forward changes? The webrev can be found at: 
>>> http://cr.openjdk.java.net/~johnc/8010738/webrev.0/.
>>>
>>> Summary:
>>> At sustaining engineering's request I've added the change 
>>> information about the metadata space to the detailed heap output for 
>>> full GCs:
>>>
>>>> 4.896: [Full GC (Allocation Failure) 512M->343M(512M), 4.3640117 secs]
>>>>    [Eden: 0.0B(25.0M)->0.0B(97.0M) Survivors: 0.0B->0.0B Heap: 
>>>> 512.0M(512.0M)->343.3M(512.0M)], [Metaspace: 1611K->2433K(6448K)]
>>>>  [Times: user=5.13 sys=0.15, real=4.36 secs]
>>>
>>> The information that gets printed mirrors that of the other 
>>> collectors. Here's parallel GC:
>>>
>>>> 33.245: [Full GC (Ergonomics) [PSYoungGen: 133632K->3423K(144640K)] 
>>>> [ParOldGen: 348158K->348158K(348160K)] 481790K->351581K(492800K), 
>>>> [Metaspace: 1612K->2433K(6448K)], 2.5788089 secs] [Times: user=9.96 
>>>> sys=0.00, real=2.58 secs]
>>>
>>> The information that's printed is the amount of metaspace used 
>>> before the GC, the capacity, and the amount of reserved space.
>>>
>>> The changes should be straight forward when this is backported to 
>>> hs24 - namely replacing the call to 
>>> MetaspaceAux::print_metaspace_change() to 
>>> perm_gen()->print_heap_change().
>>>
>>> Most of the changes are the result of renaming some of the fields in 
>>> G1CollectorPolicy and the local variables in 
>>> G1CollectorPolicy::print_detailed_heap_transition().
>>>
>>> The included regression test just spawns a child java process to run 
>>> a very simple system GC test program with G1 and +PrintGCDetails. 
>>> The output of the spawned process is checked for "Metaspace".
>>>
>>> Thanks,
>>>
>>> JohnC
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20130508/90a91ae9/attachment.htm>